FYI, this fix uncovered other issues, fixed by the following master branch commits:
c1be8fb Fix: urcu-bp segfault in glibc pthread_kill() 95b9424 Fix urcu-bp: don't move registry 5c02e37 Fix: compat futex duplicated lock and completion 976720e Fix: i386 compat code duplicated mutex instances 9d8612b Fix: urcu-bp: Bulletproof RCU arena resize bug f61dfd9 Fix: test_mutex.c uninitialized mutex Comments are welcome, Thanks, Mathieu ----- Original Message ----- > From: "Mathieu Desnoyers" <[email protected]> > To: "Milosz Tanski" <[email protected]> > Cc: [email protected] > Sent: Monday, September 30, 2013 12:09:59 PM > Subject: Re: [lttng-dev] Bulletproof RCU arena resize bug > > > > ----- Original Message ----- > > From: "Mathieu Desnoyers" <[email protected]> > > To: "Milosz Tanski" <[email protected]> > > Cc: [email protected] > > Sent: Monday, September 30, 2013 11:49:11 AM > > Subject: Re: [lttng-dev] Bulletproof RCU arena resize bug > > > > ----- Original Message ----- > > > From: "Milosz Tanski" <[email protected]> > > > To: [email protected] > > > Sent: Monday, September 30, 2013 11:16:37 AM > > > Subject: [lttng-dev] Bulletproof RCU arena resize bug > > > > > > Hi, > > > > > > While trying to use the BP flavor of RCU I ran into random crashes. I > > > tracked it down to issues with resizing of the BP RCU memory pool. > > > > > > The problem is in the urcu-bp.c file in the resize_arena() function. > > > On successful allocation / remapping the len member of the > > > registry_arena struct is never set anywhere function. On the second > > > resize of the arena the code in resize_arena() still thinks the > > > previous size is equal to the original mapping size. I've fixed this > > > issue locally by just adding the following code at the bottom of > > > resize_arena(). > > > > > > I hope this helps, > > > > Good catch !! > > > > However, I think your fix misses one case: if we happen to re-use the same > > region, we want to update the length right ? I propose: > > Sorry, my previous patch was wrong: I updated arena->len before it was used > (expecting old value). > > Here is an updated fix. It includes a bit of cleanup so we can make some > sense of the variables there. > > diff --git a/urcu-bp.c b/urcu-bp.c > index 34bd14e..93d781f 100644 > --- a/urcu-bp.c > +++ b/urcu-bp.c > @@ -305,27 +305,37 @@ int rcu_read_ongoing(void) > */ > static void resize_arena(struct registry_arena *arena, size_t len) > { > - void *new_arena; > + void *new_p; > + size_t old_len; > + > + old_len = arena->len; > > if (!arena->p) > - new_arena = mmap(arena->p, len, > - PROT_READ | PROT_WRITE, > - MAP_ANONYMOUS | MAP_PRIVATE, > - -1, 0); > + new_p = mmap(arena->p, len, > + PROT_READ | PROT_WRITE, > + MAP_ANONYMOUS | MAP_PRIVATE, > + -1, 0); > else > - new_arena = mremap_wrapper(arena->p, arena->len, > - len, MREMAP_MAYMOVE); > - assert(new_arena != MAP_FAILED); > - arena->len = len; > + new_p = mremap_wrapper(arena->p, old_len, > + len, MREMAP_MAYMOVE); > + assert(new_p != MAP_FAILED); > + > + /* > + * Zero the newly allocated memory. Since mmap() does not > + * clearly specify if memory is zeroed or not (although it is > + * very likely that it is), be extra careful by not expecting > + * the new range to be zeroed by mremap. > + */ > + bzero(new_p + old_len, len - old_len); > > /* > - * re-used the same region ? > + * If we did not re-use the same region, we need to update the > + * arena pointer. > */ > - if (new_arena == arena->p) > - return; > + if (new_p != arena->p) > + arena->p = new_p; > > - bzero(new_arena + arena->len, len - arena->len); > - arena->p = new_arena; > + arena->len = len; > } > > /* Called with signals off and mutex locked */ > > Thoughts ? > > Thanks, > > Mathieu > > -- > Mathieu Desnoyers > EfficiOS Inc. > http://www.efficios.com > > _______________________________________________ > lttng-dev mailing list > [email protected] > http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev > -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com _______________________________________________ lttng-dev mailing list [email protected] http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
