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
