LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Paul Jackson <pj@sgi.com>
To: David Rientjes <rientjes@google.com>
Cc: akpm@linux-foundation.org, clameter@sgi.com,
	Lee.Schermerhorn@hp.com, ak@suse.de,
	linux-kernel@vger.kernel.org
Subject: Re: [patch 3/6] mempolicy: add MPOL_F_STATIC_NODES flag
Date: Tue, 26 Feb 2008 15:39:44 -0600	[thread overview]
Message-ID: <20080226153944.025bf1cb.pj@sgi.com> (raw)
In-Reply-To: <alpine.DEB.1.00.0802261253510.10790@chino.kir.corp.google.com>

David wrote:
> No reader is going to understand immediately what 'mode', 'mode_flags', 
> and 'flags' are if you only provide a single line of the code like that. 

Duh - of course not.  I only presented that line to demonstrate that all
three words 'mode', 'mode_flags', and 'flags' appeared in the same context.

> It becomes rather obvious what they represent when you read the entire 
> sys_mbind() implementation,

How about picking less vacuous names than variations of mode and flag,
so that it's easier for the reader to distinguish without having to
read and understand an entire chunk of code first.

> This is a natural implementation detail to accomodate your insistance

My patch for this had explicit bit fields, which made the names clearer:

+       int mask_opts_rel:1;    /* c.original_mask is relative */
+       int mask_opts_abs:1;    /* c.original_mask is absolute */
+       /* if neither rel nor abs, then use c.latest_cpuset_mask */

You chose not to do that, which obfuscates what's going on.
Don't blame me that the result is obfuscated.

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@sgi.com> 1.940.382.4214

  parent reply	other threads:[~2008-02-26 21:39 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-02-25 15:35 [patch 1/6] mempolicy: convert MPOL constants to enum David Rientjes
2008-02-25 15:35 ` [patch 2/6] mempolicy: support optional mode flags David Rientjes
2008-02-25 15:35   ` [patch 3/6] mempolicy: add MPOL_F_STATIC_NODES flag David Rientjes
2008-02-25 15:35     ` [patch 4/6] mempolicy: add bitmap_onto() and bitmap_fold() operations David Rientjes
2008-02-25 15:35       ` [patch 5/6] mempolicy: add MPOL_F_RELATIVE_NODES flag David Rientjes
2008-02-25 15:35         ` [patch 6/6] mempolicy: update NUMA memory policy documentation David Rientjes
2008-02-26 17:34           ` Paul Jackson
2008-02-26 21:23             ` David Rientjes
2008-02-26  6:12         ` [patch 5/6] mempolicy: add MPOL_F_RELATIVE_NODES flag Paul Jackson
2008-02-26  6:45           ` David Rientjes
2008-02-26 17:44         ` Paul Jackson
2008-02-26 21:17           ` David Rientjes
2008-02-26 21:30             ` Paul Jackson
2008-02-26 21:27           ` Lee Schermerhorn
2008-02-27  1:17         ` David Rientjes
2008-02-27  1:31           ` David Rientjes
2008-02-27  2:30           ` Paul Jackson
2008-02-27 15:37           ` Lee Schermerhorn
2008-02-27 17:09             ` Paul Jackson
2008-02-28 21:08             ` David Rientjes
2008-02-26  5:46     ` [patch 3/6] mempolicy: add MPOL_F_STATIC_NODES flag Paul Jackson
2008-02-26  6:53       ` David Rientjes
2008-02-26 17:56     ` Paul Jackson
2008-02-26 21:02       ` David Rientjes
2008-02-26 21:32         ` Lee Schermerhorn
2008-02-26 21:54           ` David Rientjes
2008-02-26 22:08             ` Paul Jackson
2008-02-26 21:39         ` Paul Jackson [this message]
2008-02-26  3:20 ` [patch 1/6] mempolicy: convert MPOL constants to enum Paul Jackson
2008-02-26  3:35   ` David Rientjes
2008-02-26  4:02     ` Paul Jackson
2008-02-26  4:21       ` David Rientjes
2008-02-26  4:46         ` Paul Jackson
2008-02-27 19:35 ` Christoph Lameter
2008-02-27 19:59   ` David Rientjes
2008-03-01  0:44 David Rientjes
2008-03-01  0:45 ` [patch 2/6] mempolicy: support optional mode flags David Rientjes
2008-03-01  0:45   ` [patch 3/6] mempolicy: add MPOL_F_STATIC_NODES flag David Rientjes

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080226153944.025bf1cb.pj@sgi.com \
    --to=pj@sgi.com \
    --cc=Lee.Schermerhorn@hp.com \
    --cc=ak@suse.de \
    --cc=akpm@linux-foundation.org \
    --cc=clameter@sgi.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rientjes@google.com \
    --subject='Re: [patch 3/6] mempolicy: add MPOL_F_STATIC_NODES flag' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).