Re: [PATCH 0/4] skb paged fragment destructors

["Michał Mirosław" <mirqus@gmail.com>]

2011/11/17 Ian Campbell <Ian.Campbell@citrix.com>:
> On Wed, 2011-11-09 at 17:49 +0000, Eric Dumazet wrote:
>> struct skb_frag_page_desc {
>>       struct page *p;
>>       atomic_t ref;
>>       int (*destroy)(void *data);
>> /*    void *data; */ /* no need, see container_of() */
>
> It turns out that container_of is not so useful here as the users
> typically has a list of pages not a single page and hence has a list of
> destructors too. What you actually need is the container of the pointer
> to that list, IYSWIM, which you can't get at given only a pointer to an
> element of the list. So you end up doing
>
>        struct subsys_page_desc {
>                struct subsys_container *container;
>                struct sbk_frag_page_desc;
>        }
>
> *container here is basically the same as the void * so you might as well
> include it in the base datastructure.

If you reverse the order fields in subsys_page_desc then
container_of() compiles to no-op, and you don't have to impose this
extra field for all users, even if they don't need it.

If you see it useful, then there could be base skb_frag_page_desc and
then, skb_frag_page_desc_with_data (or othre) extending it. Core code
only ever needs the base structure.

BTW, destroy() prototype should be:

void destroy(struct skb_frag_page_desc *desc);

Because: 1. you know the parameter's type, 2. whatever would be
returned is not going to be useful.

Best Regards,
Michał Mirosław