Help with shared memory usage

[Wandenberg Peixoto]

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 <mdounin at 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 at nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.nginx.org/pipermail/nginx-devel/attachments/20131226/7ec07155/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: unfrag_slab_memory4.patch
Type: text/x-patch
Size: 1825 bytes
Desc: not available
URL: <http://mailman.nginx.org/pipermail/nginx-devel/attachments/20131226/7ec07155/attachment.bin>