On Tue, Mar 24, 2020 at 08:27:12AM +0100, Christoph Hellwig wrote:

On Mon, Mar 23, 2020 at 10:14:50PM -0300, Jason Gunthorpe wrote:

...

+enum {

• HMM_NEED_FAULT = 1 << 0,
• HMM_NEED_WRITE_FAULT = HMM_NEED_FAULT | (1 << 1),
• HMM_NEED_ALL_BITS = HMM_NEED_FAULT | HMM_NEED_WRITE_FAULT,

I have to say I find the compound version of HMM_NEED_WRITE_FAULT way harder to understand than the logic in the previous version, and would refer keeping separate bits here.

Mostly beccause of statements like this:

...

• if ((required_fault & HMM_NEED_WRITE_FAULT) == HMM_NEED_WRITE_FAULT) {

which seems rather weird.

Okay, I checked it over, and there is one weird statement above but only one place that |'s them together, so it is overall simpler to split the enum.

I'll keep the HMM_NEED_ALL_BITS, I think that purpose is clear enough.

Thanks, Jason