Re: [patch 1/4] mempolicy: convert MPOL constants to enum

From: Paul Jackson
Date: Tue Feb 12 2008 - 19:32:18 EST


Lee wrote:
> Why not leave it as an int. Will simplify this and subsequent patch.

I tend to agree with Lee on this one. If I recall correctly, Christoph
said something similar, regarding the change of the 'policy' field
of struct mempolicy from a short to an enum.

I'm inclined toward the original types for the 'policy' field.

Also, rather than trying to pack the new flag, MPOL_F_STATIC_NODES,
into the existing 'policy' field, I'd suggest instead adding a new
field to 'struct mempolicy' for this flag. Since 'policy' is only a
short, and since the next field in that struct, is a union that
includes a pointer that is aligned on most arch's to at least a 4 byte
boundary, therefore there is a hole of at least two bytes, following
the short policy field, in which another short or some flag bits can be
placed, with no increase in the size of struct mempolicy.

Specifically, I'd suggest adding the one line for 'mode_f_static_nodes'
as below, and leaving the code involving the encoding of the policy
field alone.

struct mempolicy {
atomic_t refcnt;
short policy; /* See MPOL_* above */
int mode_f_static_nodes:1; /* <== Added line <== */
union {
struct zonelist *zonelist; /* bind */
short preferred_node; /* preferred */
nodemask_t nodes; /* interleave */
/* undefined for default */
} v;
nodemask_t cpuset_mems_allowed; /* mempolicy relative to these nodes */
};

Single bit fields (The ":1" above) provide the simplest way to add
boolean flags to structs. Let the compiler do the work of packing
and unpacking the field.

Then, rather than having to code double-negative explicit masking
operations such as:

remap = !(mpol_flags(pol->policy) & MPOL_F_STATIC_NODES);
if (!remap)
blah blah ...

one can simply code:

if (pol->mode_f_static_nodes)
blah blah ...

--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <pj@xxxxxxx> 1.940.382.4214
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/