netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ben Hutchings <bhutchings@solarflare.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: netdev@vger.kernel.org, linux-net-drivers@solarflare.com
Subject: Re: New driver "sfc" for Solarstorm SFC4000 controller.
Date: Wed, 7 May 2008 18:55:19 +0100	[thread overview]
Message-ID: <20080507175518.GY2217@solarflare.com> (raw)
In-Reply-To: <20080506125008.GA2217@solarflare.com>

Ben Hutchings wrote:
> Andrew Morton wrote:
> > On Fri, 2 May 2008 17:05:35 +0100 Ben Hutchings <bhutchings@solarflare.com> wrote:
<snip>
> > > > > +		rx_buf->page = alloc_pages(__GFP_COLD | __GFP_COMP | GFP_ATOMIC,
> > > > > +					   efx->rx_buffer_order);
> > > > 
> > > > I don't think we should be using the open-coded __GFP_COMP here.  That's
> > > > more an mm-internal thing.
> > > 
> > > What's the alternative?
> > 
> > Just remove the __GFP_COMP, I expect.  __GFP_COMP will ask the page
> > allocator to add extra book-keeping info to the pageframe (via
> > prep_compound_page()).  I doubt if the driver uses that information.
> 
> It looks like this flag was used by mistake, based on past experience with
> a driver that needed buffers to be mapped into user space.  The original
> author of the RX page-allocation code is away at the moment, so I will
> have to wait to ask why he used it.

This is not a mistake.   We need multi-page buffers to be treated as
compound pages because once they are attached to an skbuff they will
be freed by skb_release_data() calling put_page(), not __free_pages().

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.

  parent reply	other threads:[~2008-05-07 17:55 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <200804301925.m3UJPc72001651@hera.kernel.org>
2008-05-01 19:08 ` New driver "sfc" for Solarstorm SFC4000 controller Andrew Morton
2008-05-02 16:05   ` Ben Hutchings
2008-05-02 18:09     ` Andrew Morton
2008-05-06 12:50       ` Ben Hutchings
2008-05-06 14:24         ` Andrew Morton
2008-05-06 15:04           ` Ben Hutchings
2008-05-06 15:12             ` Andrew Morton
2008-05-06 15:40               ` Ben Hutchings
2008-05-07 17:55         ` Ben Hutchings [this message]
2008-05-02 19:10   ` Michael Brown
2008-05-03 20:01     ` Ben Hutchings

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080507175518.GY2217@solarflare.com \
    --to=bhutchings@solarflare.com \
    --cc=akpm@linux-foundation.org \
    --cc=linux-net-drivers@solarflare.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).