Hello!
On Fri, Apr 25, 2014 at 12:47:28PM -0700, John Watson wrote:
> Hi Maxim,
>
> Any chance you can fix up this patch and get it into 1.7?
It's still in my queue, I hope I can spend some time on it in the
near future. (It's relatively low priority though, as nothing in
nginx itself uses shared memory allocations larger than page
size.)
>
> This patch is not compatible with 1.6.
>
> Best,
>
> John
>
> On Wed, Jan 22, 2014 at 8:51 AM, Maxim Dounin <mdou...@mdounin.ru> wrote:
> > Hello!
> >
> > On Wed, Jan 22, 2014 at 01:39:54AM -0200, Wandenberg Peixoto wrote:
> >
> >> Hello Maxim,
> >>
> >> did you have opportunity to take a look on this last patch?
> >
> > It looks more or less correct, though I don't happy with the
> > checks done, and there are various style issues. I'm planning to
> > look into it and build a better version as time permits.
> >
> >>
> >>
> >> Regards,
> >> Wandenberg
> >>
> >>
> >> On Thu, Dec 26, 2013 at 10:12 PM, Wandenberg Peixoto
> >> <wandenb...@gmail.com>wrote:
> >>
> >> > Hello Maxim,
> >> >
> >> > I changed the patch to check only the p->next pointer.
> >> > And checking if the page is in an address less than the (pool->pages +
> >> > pages).
> >> >
> >> > + ngx_slab_page_t *prev, *p;
> >> > + ngx_uint_t pages;
> >> > + size_t size;
> >> > +
> >> > + size = pool->end - (u_char *) pool - sizeof(ngx_slab_pool_t);
> >> > + pages = (ngx_uint_t) (size / (ngx_pagesize +
> >> > sizeof(ngx_slab_page_t)));
> >> > +
> >> > + p = &page[page->slab];
> >> > +
> >> > + if ((p < pool->pages + pages) &&
> >> > + (p->next != NULL) &&
> >> > + ((p->prev & NGX_SLAB_PAGE_MASK) == NGX_SLAB_PAGE)) {
> >> >
> >> >
> >> > I hope that now I did the right checks.
> >> >
> >> > Regards,
> >> > Wandenberg
> >> >
> >> >
> >> > On Fri, Dec 20, 2013 at 2:49 PM, Maxim Dounin <mdou...@mdounin.ru> wrote:
> >> >
> >> >> Hello!
> >> >>
> >> >> On Tue, Dec 17, 2013 at 09:05:16PM -0200, Wandenberg Peixoto wrote:
> >> >>
> >> >> > Hi Maxim,
> >> >> >
> >> >> > sorry for the long delay. I hope you remember my issue.
> >> >> > In attach is the new patch with the changes you suggest.
> >> >> > Can you check it again? I hope it can be applied to nginx code now.
> >> >> >
> >> >> > About this point "2. There is probably no need to check both prev and
> >> >> > next.",
> >> >> > I check both pointers to avoid a segmentation fault, since in some
> >> >> > situations the next can be NULL and the prev may point to pool->free.
> >> >> > As far as I could follow the code seems to me that could happen one of
> >> >> this
> >> >> > pointers, next or prev, point to a NULL.
> >> >> > I just made a double check to protect.
> >> >>
> >> >> As far as I see, all pages in the pool->free list are expected to
> >> >> have both p->prev and p->next set. And all pages with type
> >> >> NGX_SLAB_PAGE will be either on the pool->free list, or will have
> >> >> p->next set to NULL.
> >> >>
> >> >> [...]
> >> >>
> >> >> > > > +{
> >> >> > > > + ngx_slab_page_t *neighbour = &page[page->slab];
> >> >> > >
> >> >> > > Here "neighbour" may point outside of the allocated page
> >> >> > > structures if we are freeing the last page in the pool.
> >> >>
> >> >> It looks like you've tried to address this problem with the
> >> >> following check:
> >> >>
> >> >> > +static ngx_int_t
> >> >> > +ngx_slab_merge_pages(ngx_slab_pool_t *pool, ngx_slab_page_t *page)
> >> >> > +{
> >> >> > + ngx_slab_page_t *prev, *p;
> >> >> > +
> >> >> > + p = &page[page->slab];
> >> >> > + if ((u_char *) p >= pool->end) {
> >> >> > + return NGX_DECLINED;
> >> >> > + }
> >> >>
> >> >> This looks wrong, as pool->end points to the end of the pool, not
> >> >> the end of allocated page structures.
> >> >>
> >> >> --
> >> >> Maxim Dounin
> >> >> http://nginx.org/
> >> >>
> >> >> _______________________________________________
> >> >> nginx-devel mailing list
> >> >> nginx-devel@nginx.org
> >> >> http://mailman.nginx.org/mailman/listinfo/nginx-devel
> >> >>
> >> >
> >> >
> >
> >> _______________________________________________
> >> nginx-devel mailing list
> >> nginx-devel@nginx.org
> >> http://mailman.nginx.org/mailman/listinfo/nginx-devel
> >
> >
> > --
> > Maxim Dounin
> > http://nginx.org/
> >
> > _______________________________________________
> > nginx-devel mailing list
> > nginx-devel@nginx.org
> > http://mailman.nginx.org/mailman/listinfo/nginx-devel
>
> _______________________________________________
> nginx-devel mailing list
> nginx-devel@nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx-devel
--
Maxim Dounin
http://nginx.org/
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel
