[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAHXqBFL5m2J=XRBY1jEO3wWPyy-y1AKwuVTd_dVrtFNbtJ76JQ@mail.gmail.com>
Date: Thu, 17 Nov 2011 21:22:12 +0100
From: Michał Mirosław <mirqus@...il.com>
To: Ian Campbell <Ian.Campbell@...rix.com>
Cc: Eric Dumazet <eric.dumazet@...il.com>,
David Miller <davem@...emloft.net>,
Jesse Brandeburg <jesse.brandeburg@...el.com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH 0/4] skb paged fragment destructors
2011/11/17 Ian Campbell <Ian.Campbell@...rix.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
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists