Buffer Reuse in Nginx

[Maxim Dounin]

Hello!

On Wed, Mar 07, 2012 at 02:41:55PM +0100, Soares Chen wrote:

[...]

> >> typedef void* ngx_buf_tag_t
> >> This mysterious tag seems to be the way for me to claim ownership to a
> >> buffer by assigning it a unique pointer value. However I could find
> >> almost no explanation on how to use this tag field properly. I'd like
> >> to know if setting this tag would guarantee that the buffers I created
> >> would never be shared ownership with other modules?
> >
> > It is used by ngx_chain_update_chains() to match buffers allocated
> > by your module.
> 
> Ok then this is my interpretation for the buffer tag: There is always
> exactly one ngx_buf_t object that points to a particular memory area,
> and it is owned by the module that set the tag field. The modules down
> the output chain would change the b->pos and b->last pointers and that
> change is used by the buffer owner to determine if the buffer is free
> to reuse again.

Mostly yes.

Though the "exactly one ngx_buf_t" isn't really correct, it's 
module responsibility to ensure that it's either true or handled 
somehow.  E.g. event pipe (src/event/ngx_event_pipe.c) uses 
multiple buffers with the same memory region (i.e. with identical 
b->start) and additionally handles this.

> >> void ngx_chain_update_chains(ngx_pool_t *p, ngx_chain_t **free,
> >> ngx_chain_t **busy, ngx_chain_t **out, ngx_buf_tag_t tag)
> >> I find that this function seems to be performing what I want, and it
> >> seems to be called in other modules that has similar buffer reuse
> >> mechanism. However I am really confused about the purpose of this
> >> function and what it does exactly. From the signature it seems to be
> >> determining which buffers are safe to reuse, and then reclaim the free
> >> buffers into the **free chain. However on close inspection I found
> >> that all it does is to move all tagged buffers at **busy and **out to
> >> free, while calling ngx_free_chain() on buffer chains that do not
> >> share the same tag. I don't know if the buffers freed by this function
> >> is guaranteed safe to be reused, and I don't know what happen with the
> >> buffers that have different tags.
> >
> > Buffers only moved to **free if they are indeed free, i.e. when
> > ngx_buf_size(cl->buf) == 0.  Buffers from **free will be then
> > reused either with ngx_chain_get_free_buf() or with your own code.
> 
> Ah now I realized I missed that line of code. So what you mean is that
> it is also possible to manually determine if a buffer is free for
> reuse by checking if ngx_buf_size(b) is 0?

Yes.

> To make sure I get the idea correct I am showing here my
> interpretation of this function:
> 
> void
> ngx_chain_update_chains(ngx_pool_t *p, ngx_chain_t **free, ngx_chain_t **busy,
>     ngx_chain_t **out, ngx_buf_tag_t tag)
> {
>     ngx_chain_t  *cl;
> 
>     // Append *out to the end of *busy chain, and empty the *out chain
>     if (*busy == NULL) {
>         *busy = *out;
> 
>     } else {
>         for (cl = *busy; cl->next; cl = cl->next) { /* void */ }
> 
>         cl->next = *out;
>     }
> 
>     *out = NULL;
> 
>     // For each buffer in the busy chain
>     while (*busy) {
>         cl = *busy;
> 
>         // If the size is not zero, then the buffer is still being used.
>         // We assume that the rest of buffers in the chain are still
>         // in use as well, so we break and return.
>         if (ngx_buf_size(cl->buf) != 0) {
>             break;
>         }
> 
>         // If the buffer is not owned by current module, proceed to
>         // the next buffer and free the chain link while ignoring the
>         // buffer pointer.
>         if (cl->buf->tag != tag) {
>             *busy = cl->next;
>             ngx_free_chain(p, cl);
>             continue;
>         }

Note well: all chain links for your out/free/busy chains are 
expected to be allocated by your module, either directly or via 
e.g. ngx_chain_add_copy().

> 
>         // else, the buffer is owned by the current module
>         // and the buffer is free for reuse.
>         // reset the pos and last pointer.
>         cl->buf->pos = cl->buf->start;
>         cl->buf->last = cl->buf->start;
> 
>         // Put the chain together with the buffer into the *free chain
>         // The buffer pointers at this *free chain link is guaranteed to
>         // be valid and it's memory region is safe for reuse.
>         *busy = cl->next;
>         cl->next = *free;
>         *free = cl;
>     }
> }

Looks correct.

> >> ngx_chain_t * ngx_chain_get_free_buf(ngx_pool_t *p, ngx_chain_t **free)
> >> I find that this function will return the buffers freed by
> >> ngx_chain_update_chains(). Most modules seem to do overwrite the data
> >> on the obtained buffer without any issue. That makes me wonder if
> >> ngx_chain_update_chains() really works.
> >
> > See above.
> 
> From what I understand ngx_chain_get_free_buf() will always return a
> ngx_chain_t object with valid pointer to a ngx_buf_t object. On the
> opposite, ngx_alloc_chain_link() only returns the ngx_chain_t object
> and the buf pointer must be reseted before any use.
> 
> Also, if the *free chain contains any chain link, that chain link will
> be returned and the buffer attached would have b->start and b->end
> pointing to memory region that is safe to write. But on the other hand
> of *free is empty, then ngx_chain_get_free_buf would only allocate new
> ngx_chain_t and ngx_buf_t objects but leave the b->start and b->end
> pointers as NULL. As a result the caller of ngx_chain_get_free_buf()
> must ensure that cl->buf->start points to some memory region, or the
> caller itself have to manually allocate that memory region and assign
> them to cl->buf.

I would rather suggest to interpret this a bit differently:

The ngx_alloc_chain_link() function returns ngx_chain_t structure 
with undefined contents.

The ngx_chain_get_free_buf() function returns ngx_chain_t 
structure and associated ngx_buf_t structure.  If returned chain + 
buffer pair is reused, it's guaranteed to have the same 
cl->buf->start as before; else it will have cl->buf->start set to 
NULL.

Net effect is the same though.

[...]

Maxim Dounin