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

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

Peter Zijlstra wrote:
> On Wed, Nov 15, 2023 at 02:01:14PM +0100, Peter Zijlstra wrote:
> > On Fri, Nov 10, 2023 at 12:21:56PM -0800, Dan Williams wrote:
> > 
> > >         /* decoders are enabled if committed */
> > > -       if (committed) {
> > > +       if (committed) scoped_guard(rwsem_write, &cxl_region_rwsem) {
> > >                 cxld->flags |= CXL_DECODER_F_ENABLE;
> > >                 if (ctrl & CXL_HDM_DECODER0_CTRL_LOCK)
> > >                         cxld->flags |= CXL_DECODER_F_LOCK;
> > > 
> > > 
> > > The alternative is a much more violent re-ident of the code to turn:
> > > 
> > >     if (...) scoped_guard(...) {
> > > 	...
> > >     } else {
> > >         ...
> > >     }
> > > 
> > > ...into:
> > > 
> > >     if (...) {
> > >         scoped_guard(...) {
> > > 	    ...
> > >         }
> > >     } else {
> > >         ...
> > >     }
> > > 
> > 
> > What's wrong with:
> > 
> > 	if (...) {
> > 		guard(rwsem_write, &cxl_region_rwsem);
> 
> Uh, that is:
> 		guard(rwsem_write)(&ctl_region_rwsem);

Oh, likely it is me not understanding when to use scoped_guard()...

/me re-reads the definition of guard(), and realizes that it indeed
arranges for the lock to be released at the end of the local compound
statement.

...and scoped_guard() is just a more convenient way to write:

    {
        guard(...);
    }