From mboxrd@z Thu Jan 1 00:00:00 1970 Date: Fri, 14 Sep 2007 17:07:33 -0700 From: Andrew Morton Subject: Re: [PATCH 1/6] cpuset write dirty map Message-Id: <20070914170733.dbe89493.akpm@linux-foundation.org> In-Reply-To: References: <469D3342.3080405@google.com> <46E741B1.4030100@google.com> <46E742A2.9040006@google.com> <20070914161536.3ec5c533.akpm@linux-foundation.org> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: owner-linux-mm@kvack.org Return-Path: To: Satyam Sharma Cc: Ethan Solomita , linux-mm@kvack.org, LKML , Christoph Lameter List-ID: On Sat, 15 Sep 2007 05:17:48 +0530 "Satyam Sharma" wrote: > > It's unobvious why the break point is at MAX_NUMNODES = BITS_PER_LONG and > > we might want to tweak that in the future. Yet another argument for > > centralising this comparison. > > Looks like just an optimization to me ... Ethan wants to economize and not bloat > struct address_space too much. > > So, if sizeof(nodemask_t) == sizeof(long), i.e. when: > MAX_NUMNODES <= BITS_PER_LONG, then we'll be adding only sizeof(long) > extra bytes to the struct (by plonking the object itself into it). > > But even when MAX_NUMNODES > BITS_PER_LONG, because we're storing > a pointer, and because sizeof(void *) == sizeof(long), so again the maximum > bloat addition to struct address_space would only be sizeof(long) bytes. yup. Note that "It's unobvious" != "It's unobvious to me". I review code for understandability-by-others, not for understandability-by-me. > I didn't see the original mail, but if the #ifdeffery for this > conditional is too much > as a result of this optimization, Ethan should probably just do away > with all of it > entirely, and simply put a full nodemask_t object (irrespective of MAX_NUMNODES) > into the struct. After all, struct task_struct does the same unconditionally ... > but admittedly, there are several times more address_space struct's resident in > memory at any given time than there are task_struct's, so this optimization does > make sense too ... I think the optimisation is (probably) desirable, but it would be best to describe the tradeoff in the changelog and to add some suitable code-commentary for those who read the code in a year's time and to avoid sprinkling the logic all over the tree. -- 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