From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail191.messagelabs.com (mail191.messagelabs.com [216.82.242.19]) by kanga.kvack.org (Postfix) with SMTP id 8BB929000C1 for ; Tue, 26 Apr 2011 10:10:52 -0400 (EDT) Date: Tue, 26 Apr 2011 15:10:48 +0100 From: Mel Gorman Subject: Re: [PATCH 09/13] netvm: Set PF_MEMALLOC as appropriate during SKB processing Message-ID: <20110426141048.GG4658@suse.de> References: <1303803414-5937-1-git-send-email-mgorman@suse.de> <1303803414-5937-10-git-send-email-mgorman@suse.de> <20110426222157.33a461f8@notabene.brown> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-15 Content-Disposition: inline In-Reply-To: <20110426222157.33a461f8@notabene.brown> Sender: owner-linux-mm@kvack.org List-ID: To: NeilBrown Cc: Linux-MM , Linux-Netdev , LKML , David Miller , Peter Zijlstra On Tue, Apr 26, 2011 at 10:21:57PM +1000, NeilBrown wrote: > On Tue, 26 Apr 2011 08:36:50 +0100 Mel Gorman wrote: > > > diff --git a/net/core/dev.c b/net/core/dev.c > > index 3871bf6..2d79a20 100644 > > --- a/net/core/dev.c > > +++ b/net/core/dev.c > > @@ -3095,6 +3095,27 @@ static void vlan_on_bond_hook(struct sk_buff *skb) > > } > > } > > > > +/* > > + * Limit which protocols can use the PFMEMALLOC reserves to those that are > > + * expected to be used for communication with swap. > > + */ > > +static bool skb_pfmemalloc_protocol(struct sk_buff *skb) > > +{ > > + if (skb_pfmemalloc(skb)) > > + switch (skb->protocol) { > > + case __constant_htons(ETH_P_ARP): > > + case __constant_htons(ETH_P_IP): > > + case __constant_htons(ETH_P_IPV6): > > + case __constant_htons(ETH_P_8021Q): > > + break; > > + > > + default: > > + return false; > > + } > > + > > + return true; > > +} > > This sort of thing really bugs me :-) > Neither the comment nor the function name actually describe what the function > is doing. The function is checking *2* things. > is_pfmemalloc_skb_or_pfmemalloc_protocol() > might be a more correct name, but is too verbose. > > I would prefer the skb_pfmemalloc test were removed from here and .... > > > + if (!skb_pfmemalloc_protocol(skb)) > > + goto drop; > > + > > ...added here so this becomes: > > if (!skb_pfmemalloc(skb) && !skb_pfmemalloc_protocol(skb)) > goto drop; > > which actually makes sense. > Moving the check is neater but that check should be if (skb_pfmemalloc(skb) && !skb_pfmemalloc_protocol(skb)) ? It's only if the skb was allocated from emergency reserves that we need to consider dropping it to make way for other packets to be received. -- Mel Gorman SUSE Labs -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: email@kvack.org