Re: [PATCH] cxl: Convert pioson ops rwsem usages to scope based resource management

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

Dan Williams wrote:
> Alison Schofield wrote:
> > On Wed, Nov 15, 2023 at 04:55:11PM -0700, Dave Jiang wrote:
> > > 
> > > 
> > > On 11/15/23 16:32, Alison Schofield wrote:
> > > > On Tue, Nov 14, 2023 at 11:25:20AM -0700, Dave Jiang wrote:
> > > >> Cleanup the rwsem usages in the poison ops to reduce complexity and reduce
> > > >> code lines.
> > > >>
> > > >> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> > > >> ---
> > > >>
> > > >> Hi Alison, follow on patch to yours. Can't be backported, but nice clean
> > > >> up going forward.
> > > > 
> > > > Tell me more about your backport worry.  Are we expected to avoid using
> > > > the new guard any place where there is a backport possibility?  
> > > 
> > > Given that there's none of the cleanup.h support in stable kernels, I don't see how we can backport the guard() code automatically. Thus your original fix with a fixes tag plus a new cleanup patch follow on w/o backport issues seems necessary. Otherwise a separate backport patch would be needed no?
> > 
> > Sure, it would be needed. I guess I'm looking for why this backport
> > issue is so special. (not being sarcastic). Is there specific guidance
> > not to use the cleanup stuff if we think a patch might be backported?
> > I don't usually consider backportability when adding a Fixes tag to a
> > Patch. Have 'backport folks' asked us not to use it?
> > 
> > I'm imagining the slippery slope of not fixing something the best way
> > because we are worried that backport folks can't figure out how to
> > merge it.
> 
> Upstream should be fixing things "the best way" in the current kernel.
> If the best way requires some work when backporting, so be it.
> 
> The general rule for making backports easier is for when a fix
> identifies additional cleanups. In that scenario the fix should be made
> first and then the cleanups layered on top.
> 
> The cleanup.h helpers are an interesting case because they allow adding
> new locking calls and defining the scope of the lock at the same time. I
> submit that cleanup.h helpers are as easy / difficult to backport as
> open-coded lock / unlock calls.

The other concern here though is mixing a conversion to use cleanup.h
with a cleanup to use scope-based locking, and the fact that the
_interruptible version of the scope based locking is not available until
v6.8. So while I think it is ok to introduce new locking as a fix with
the cleanup.h headers. The old-style should be used when the fix overlaps
a conversion.