* 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