Re: [bug report] cxl/port: Use scoped_guard()/guard() to drop device_lock() for cxl_port

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Thu, 17 Oct 2024 09:32:23 -0700
Dan Williams <dan.j.williams@intel.com> wrote:

> Jonathan Cameron wrote:
> [..]
> > > In general for CXL I want to say that no function should be converted to
> > > use cleanup helpers unless all gotos are removed at once, and if the
> > > conversion needs to reach for scoped_guard() reconsider even attempting
> > > the conversion. I.e. scoped_guard() is a leading indicator for needing
> > > code refactoring.  
> > 
> > I don't think it's a bug and ultimately Dan C didn't say it was.
> > It's ugly but a simpler path to resolve it logically is to
> > stop using the variable port for two purposes.  
> [..]
> > 			/* retry find to pick up the new dport information */
> > 			port = find_cxl_port_at(parent_port, dport_dev, &dport);
> > 			if (!port)
> > 				return -ENXIO;
> > 		}
> > 	}
> > 
> > Whilst I don't like the code, I'm not sure a revert is the best way out.  
> 
> The revert is for the scoped_guard() conversion which was, innocently,
> trying to preserve the subtlety of the existing code.
> 
> It is true that the subsequent find_cxl_port_at() saves this being an
> actual bug by elevating the new port's refcount, but it is subtle beyond
> reason. This whole function needs a re-think, not more band-aids. I will
> fix up the reverts to drop "Fixes:" since you are right there is no
> actual bug, and those can wait for 6.13, but intent is to say "let's not
> use scoped_guard() in CXL without an exceedingly good reason".
> 
That's fair.  Let's also rename that variable so there is less subtle
code involved.

Jonathan