Re: [PATCH 05/26] cxl/core: Simplify cxl_dpa_set_mode()

From: Dan Williams
Date: Fri May 03 2024 - 21:19:58 EST


Ira Weiny wrote:
> Alison Schofield wrote:
> > On Sun, Mar 24, 2024 at 04:18:08PM -0700, Ira Weiny wrote:
> > > cxl_dpa_set_mode() checks the mode for validity two times, once outside
> > > of the DPA RW semaphore and again within.
> >
> > Not true.
>
> Sorry for not being clear. It does check the mode 2x but not for
> validity. I'll clarify.
>
> > It only checks mode once before the lock. It checks for
> > capacity after the lock. If it didn't check mode before the lock,
> > then unsupported modes would fall through.
>
> But we can check the mode 1 time and either check the size or fail.
>
> >
> > > The function is not in a critical path.
> >
> > Implying what here? OK to check twice (even though it wasn't)
> > or OK to expand scope of locking.
>
> Implying that checking the mode outside the lock is not required.

The @mode check outside the lock is there to taking the lock when not
necessary because the passed in mode is already bogus.

The lock is about making sure the write of cxled->mode relative to the
state of the dpa partitions is an atomic check-and-set.

So this makes the function unconditionally take the lock when it might
be bogus to do so. The value of reorganizing this is questionable.