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

[David Rientjes]

On Tue, 12 Feb 2008, Paul Jackson wrote:

> Christoph wrote:
> > Good. And remove the enum.
> > 
> > It would be better to add some sort of flags field?
> 
> On the other hand, despite my brilliant (hah!) endorsement
> of bit field flags in my reply a few minutes ago, I'd settle
> for (1) removing the enum, and (2) using a flags field and
> more defines for the new stuff.  I will grant that Christoph
> is correct that that form is more common in the kernel, and
> there is something to be said for doing things in the most
> common manner.
> 

The enum has already been removed, as I've said a few times.

The 'flags' field would be wrong because you're ignoring that these flags 
are both passed by the user to the kernel and by the kernel to the user as 
part of the 'int *' parameter in either set_mempolicy() or mbind().

So they must have predefined shift in linux/mempolicy.h to separate that 
'int *' parameter into the policy mode and the optional mode flags.

If you introduced a separate flags member to struct mempolicy, you'd be 
wasting the lower MPOL_FLAG_SHIFT bits for no apparent purpose so your 
flag bits still work:

	#define MPOL_F_STATIC_NODES	(1 << MPOL_FLAG_SHIFT)

There's no way around that without shifting before storing and each time 
you read pol->flags.

So since those bits would have to be unused and can perfectly accomodate 
the mode bits (with the current definition of MPOL_FLAG_SHIFT at 8, we 
can accomodate up to 256 distinct modes), there is no reason why they 
cannot be combined.  That is why I implemented it that way.

		David
--
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/