From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail143.messagelabs.com (mail143.messagelabs.com [216.82.254.35]) by kanga.kvack.org (Postfix) with ESMTP id ECAA56B005A for ; Thu, 3 Sep 2009 14:34:34 -0400 (EDT) Received: from zps35.corp.google.com (zps35.corp.google.com [172.25.146.35]) by smtp-out.google.com with ESMTP id n83IYTLd013625 for ; Thu, 3 Sep 2009 19:34:30 +0100 Received: from pxi27 (pxi27.prod.google.com [10.243.27.27]) by zps35.corp.google.com with ESMTP id n83IYLdN015054 for ; Thu, 3 Sep 2009 11:34:27 -0700 Received: by pxi27 with SMTP id 27so116363pxi.15 for ; Thu, 03 Sep 2009 11:34:26 -0700 (PDT) Date: Thu, 3 Sep 2009 11:34:24 -0700 (PDT) From: David Rientjes Subject: Re: [PATCH 4/6] hugetlb: introduce alloc_nodemask_of_node In-Reply-To: <1251823334.4164.2.camel@useless.americas.hpqcorp.net> Message-ID: References: <20090828160314.11080.18541.sendpatchset@localhost.localdomain> <20090828160338.11080.51282.sendpatchset@localhost.localdomain> <20090901144932.GB7548@csn.ul.ie> <1251823334.4164.2.camel@useless.americas.hpqcorp.net> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-linux-mm@kvack.org To: Lee Schermerhorn Cc: Mel Gorman , linux-mm@kvack.org, akpm@linux-foundation.org, Nishanth Aravamudan , linux-numa@vger.kernel.org, Adam Litke , Andy Whitcroft , eric.whitney@hp.com List-ID: On Tue, 1 Sep 2009, Lee Schermerhorn wrote: > > > Index: linux-2.6.31-rc7-mmotm-090827-0057/include/linux/nodemask.h > > > =================================================================== > > > --- linux-2.6.31-rc7-mmotm-090827-0057.orig/include/linux/nodemask.h 2009-08-28 09:21:19.000000000 -0400 > > > +++ linux-2.6.31-rc7-mmotm-090827-0057/include/linux/nodemask.h 2009-08-28 09:21:29.000000000 -0400 > > > @@ -245,18 +245,34 @@ static inline int __next_node(int n, con > > > return min_t(int,MAX_NUMNODES,find_next_bit(srcp->bits, MAX_NUMNODES, n+1)); > > > } > > > > > > +#define init_nodemask_of_nodes(mask, node) \ > > > + nodes_clear(*(mask)); \ > > > + node_set((node), *(mask)); > > > + > > > > Is the done thing to either make this a static inline or else wrap it in > > a do { } while(0) ? The reasoning being that if this is used as part of an > > another statement (e.g. a for loop) that it'll actually compile instead of > > throw up weird error messages. > > Right. I'll fix this [and signoff/review orders] next time [maybe last > time?]. It occurs to me that I can also use this for > huge_mpol_nodes_allowed(), so I'll move it up in the series and fix that > [which you've already ack'd]. I'll wait a bit to hear from David before > I respin. > I think it should be an inline function just so there's typechecking on the first argument passed in (and so alloc_nodemask_of_node() below doesn't get a NULL pointer dereference on node_set() if nmp can't be allocated). I've seen the issue about the signed-off-by/reviewed-by/acked-by order come up before. I've always put my signed-off-by line last whenever proposing patches because it shows a clear order in who gathered those lines when submitting to -mm, for example. If I write Cc: Mel Gorman Signed-off-by: David Rientjes it is clear that I cc'd Mel on the initial proposal. If it is the other way around, for example, Signed-off-by: David Rientjes Cc: Mel Gorman Signed-off-by: Andrew Morton... then it indicates Andrew added the cc when merging into -mm. That's more relevant when such a line is acked-by or reviewed-by since it is now possible to determine who received such acknowledgement from the individual and is responsible for correctly relaying it in the patch submission. If it's done this way, it indicates that whoever is signing off the patch is responsible for everything above it. The type of line (signed-off-by, reviewed-by, acked-by) is enough of an indication about the development history of the patch, I believe, and it doesn't require specific ordering to communicate (and the first line having to be a signed-off-by line isn't really important, it doesn't replace the From: line). It also appears to be how both Linus merges his own patches with Cc's. > > > +/* > > > + * returns pointer to kmalloc()'d nodemask initialized to contain the > > > + * specified node. Caller must free with kfree(). > > > + */ > > > +#define alloc_nodemask_of_node(node) \ > > > +({ \ > > > + typeof(_unused_nodemask_arg_) *nmp; \ > > > + nmp = kmalloc(sizeof(*nmp), GFP_KERNEL); \ > > > + if (nmp) \ > > > + init_nodemask_of_nodes(nmp, (node)); \ > > > + nmp; \ > > > +}) > > > + > > -- 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/ . Don't email: email@kvack.org