* Re: [PATCH 2/5] sky2: don't warn if page allocation fails
[not found] ` <20080515000432.989061538@vyatta.com>
@ 2008-05-22 9:57 ` Jeff Garzik
2008-05-27 18:47 ` Stephen Hemminger
0 siblings, 1 reply; 3+ messages in thread
From: Jeff Garzik @ 2008-05-22 9:57 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, LKML, Nick Piggin, Andrew Morton
>
> for (i = 0; i < sky2->rx_nfrags; i++) {
> - struct page *page = alloc_page(GFP_ATOMIC);
> + struct page *page = alloc_page(GFP_ATOMIC | __GFP_NOWARN);
>
> if (!page)
> goto free_partial;
IMO it's inappropriate to add these warnings to net drivers that
properly check all return values.
This approach is too maintenance intensive, and winds up fixing the same
problem over and over again -- a hint that the fix is in the wrong place.
Jeff
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 2/5] sky2: don't warn if page allocation fails
2008-05-22 9:57 ` [PATCH 2/5] sky2: don't warn if page allocation fails Jeff Garzik
@ 2008-05-27 18:47 ` Stephen Hemminger
2008-06-03 6:16 ` Nick Piggin
0 siblings, 1 reply; 3+ messages in thread
From: Stephen Hemminger @ 2008-05-27 18:47 UTC (permalink / raw)
To: Jeff Garzik; +Cc: Stephen Hemminger, netdev, LKML, Nick Piggin, Andrew Morton
On Thu, 22 May 2008 05:57:44 -0400
Jeff Garzik <jgarzik@pobox.com> wrote:
> >
> > for (i = 0; i < sky2->rx_nfrags; i++) {
> > - struct page *page = alloc_page(GFP_ATOMIC);
> > + struct page *page = alloc_page(GFP_ATOMIC | __GFP_NOWARN);
> >
> > if (!page)
> > goto free_partial;
>
>
> IMO it's inappropriate to add these warnings to net drivers that
> properly check all return values.
>
> This approach is too maintenance intensive, and winds up fixing the same
> problem over and over again -- a hint that the fix is in the wrong place.
>
> Jeff
>
>
So the __GFP_NOWARN should go away and get replaced by GFP_WARN?
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 2/5] sky2: don't warn if page allocation fails
2008-05-27 18:47 ` Stephen Hemminger
@ 2008-06-03 6:16 ` Nick Piggin
0 siblings, 0 replies; 3+ messages in thread
From: Nick Piggin @ 2008-06-03 6:16 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Jeff Garzik, Stephen Hemminger, netdev, LKML, Andrew Morton
On Wednesday 28 May 2008 04:47, Stephen Hemminger wrote:
> On Thu, 22 May 2008 05:57:44 -0400
>
> Jeff Garzik <jgarzik@pobox.com> wrote:
> > > for (i = 0; i < sky2->rx_nfrags; i++) {
> > > - struct page *page = alloc_page(GFP_ATOMIC);
> > > + struct page *page = alloc_page(GFP_ATOMIC | __GFP_NOWARN);
> > >
> > > if (!page)
> > > goto free_partial;
> >
> > IMO it's inappropriate to add these warnings to net drivers that
> > properly check all return values.
> >
> > This approach is too maintenance intensive, and winds up fixing the same
> > problem over and over again -- a hint that the fix is in the wrong place.
> >
> > Jeff
>
> So the __GFP_NOWARN should go away and get replaced by GFP_WARN?
We actually still want to see the messages, regardless of what is causing
them, because they can indicate problems in the VM.
The best way is not to add __GFP_NOWARN at all, and just improve the
throttling/thresholding/reporting of the page allocation failure warnings.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2008-06-03 6:16 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20080515000412.984337700@vyatta.com>
[not found] ` <20080515000432.989061538@vyatta.com>
2008-05-22 9:57 ` [PATCH 2/5] sky2: don't warn if page allocation fails Jeff Garzik
2008-05-27 18:47 ` Stephen Hemminger
2008-06-03 6:16 ` Nick Piggin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox