Re: [MAINTAINER SUMMIT] Rethinking the acceptance policy for "trivial" patches

[Leon Romanovsky <leon@kernel.org>]

On Thu, Apr 22, 2021 at 03:33:49PM +0200, Mauro Carvalho Chehab wrote:
> Em Thu, 22 Apr 2021 15:35:11 +0300
> Leon Romanovsky <leon@kernel.org> escreveu:
> 
> > On Thu, Apr 22, 2021 at 01:26:04PM +0100, Mark Brown wrote:
> > > On Thu, Apr 22, 2021 at 03:01:39PM +0300, Leon Romanovsky wrote:  
> > > > On Thu, Apr 22, 2021 at 11:55:11AM +0200, Mauro Carvalho Chehab wrote:  
> > >   
> > > > > The reverted ones for media (29 patches) didn't contain any malicious code.
> > > > > One was useless (because the media core already fixes the pointed issue),
> > > > > but the other ones were valid patches.  
> > >   
> > > > And didn't you ask yourself after seeing same 29 patches that the
> > > > correct fix should be in another place? pm_runtime_get_sync?  
> > > 
> > > The runtime PM APIs are for legitimate reasons really fiddly to get
> > > right - there's a bunch of different ways to do things and disabling
> > > runtime PM in Kconfig can cause surprises.  It's been identified as an
> > > issue for years but it's really not trivial to address it at the API
> > > level.  
> > 
> > There is no need to fix all problems at once, but seeing same mistake
> > over and over like in commit 57cc666d36ad ("media: st-delta: Fix reference
> > count leak in delta_run_work") can be fixed very easily (+grep all source
> > code to remove extra put):
> > 
> > diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
> > index 18b82427d0cb..d73c967ddb80 100644
> > --- a/drivers/base/power/runtime.c
> > +++ b/drivers/base/power/runtime.c
> > @@ -1089,6 +1089,9 @@ int __pm_runtime_resume(struct device *dev, int rpmflags)
> >         retval = rpm_resume(dev, rpmflags);
> >         spin_unlock_irqrestore(&dev->power.lock, flags);
> > 
> > +       if (retval && rpmflags & RPM_GET_PUT)
> > +               atomic_dec(&dev->power.usage_count);
> > +
> >         return retval;
> >  }
> >  EXPORT_SYMBOL_GPL(__pm_runtime_resume);
> 
> This would break existing code that would try to do a _put_ themselves. 

I'll cite myself from the line above: "(+grep all source code to remove extra put)"

It doesn't matter.

Thanks