Re: [PATCH v3] cxl: Add cxl_region_rwsem around commit_end write during decoder enumeration

[Dan Williams <dan.j.williams@intel.com>]

Davidlohr Bueso wrote:
> On Wed, 22 Nov 2023, Dan Williams wrote:
> 
> >Dan Williams wrote:
> >> Davidlohr Bueso wrote:
> >> > On Fri, 17 Nov 2023, Dave Jiang wrote:
> >> >
> >> > >init_hdm_decoder() modifies port->commit_end without taking the
> >> > >cxl_region_rwsem. An assert splat emitted by cxl_num_decoders_committed().
> >> > >However looking at the code, it looks like the write version of the rwsem
> >> > >needs to be taken due to the modification of commit_end. Wrap the write
> >> > >version of the rwsem around reading and writing of commit_end.
> >> > >
> >> > >Fixes: 176baefb2eb5 ("cxl/hdm: Commit decoder state to hardware")
> >> > >Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> >> >
> >> > Uhmm but do we expect concurrency during the switch/port probing phase?
> >>
> >> I answered that that detail here in response to Fan:
> >>
> >> http://lore.kernel.org/r/654eb8ed72ced_46f0294c@dwillia2-mobl3.amr.corp.intel.com.notmuch
> >>
> >> The takeaway is:
> >>
> >> "I am in favor of adding the locking rather than depend on a subtle
> >> side-effect of how CXL operates, and to avoid adding an unlocked version
> >> of cxl_num_decoders_committed()"
> >>
> >> I will add a note to the changelog to that effect, and drop Fixes: since
> >> the lock is not strictly required for correctness in this path.
> >
> >I rewrote the changelog to this:
> 
> Thanks, this clarified my doubt.

Converted this ^^^ into an "Acked-by: Davidlohr Bueso
<dave@stgolabs.net>" if that's alright.