On Sat, 15 Sep 2007 05:17:48 +0530 "Satyam Sharma" <[EMAIL PROTECTED]> 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 from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
