Re: psi_trigger_poll() is completely broken

[Tejun Heo <tj@kernel.org>]

Happy new year,

On Wed, Jan 05, 2022 at 11:13:30AM -0800, Linus Torvalds wrote:
> On Wed, Jan 5, 2022 at 11:07 AM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > Whoever came up with that stupid "replace existing trigger with a
> > write()" model should feel bad. It's garbage, and it's actively buggy
> > in multiple ways.
> 
> What are the users? Can we make the rule for -EBUSY simply be that you
> can _install_ a trigger, but you can't replace an existing one (except
> with NULL, when you close it).

I don't have enough context here and Johannes seems offline today. Let's
wait for him to chime in.

> That would fix the poll() lifetime issue, and would make the
> psi_trigger_replace() races fairly easy to fix - just use
> 
>         if (cmpxchg(trigger_ptr, NULL, new) != NULL) {
>                 ... free 'new', return -EBUSY ..
> 
> to install the new one, instead of
> 
>         rcu_assign_pointer(*trigger_ptr, new);
> 
> or something like that. No locking necessary.
> 
> But I assume people actually end up re-writing triggers, because
> people are perverse and have taken advantage of this completely broken
> API.

IIRC, the rationale for the shared trigger at the time was around the
complexities of preventing it from devolving into O(N) trigger checks on
every pressure update. If the overriding behavior is something that can be
changed, I'd prefer going for per-opener triggers even if that involves
adding complexities (maybe a rbtree w/ prev/next links for faster sweeps?).

Thanks.

-- 
tejun