Re: [PATCH 6/8] ALSA: emu10k1: add support for 2x/4x word clocks in E-MU D.A.S. mode

[Takashi Iwai <tiwai@suse.de>]

On Tue, 13 Jun 2023 16:00:34 +0200,
Oswald Buddenhagen wrote:
> 
> On Tue, Jun 13, 2023 at 01:08:55PM +0200, Takashi Iwai wrote:
> > On Tue, 13 Jun 2023 12:52:43 +0200,
> > Oswald Buddenhagen wrote:
> >> 
> >> On Tue, Jun 13, 2023 at 11:20:23AM +0200, Takashi Iwai wrote:
> >> > Creating and removing the controls from kctl put callback is no >
> >> good
> >> > idea.  In general, dynamic control creation/deletion already confuses
> >> > user-space.
> >> > i kind of expected that, but what i've tried so far worked
> >> remarkably
> >> well (ok, it was mostly alsamixer).
> >> 
> >> > On top of that, if it's done by a control element, it can
> >> > be even triggered endlessly by user.
> >> > it shouldn't, because there is no circularity between the
> >> controls. even if the app sets all controls as a response to new ones
> >> appearing, the second round will be a no-op for the multiplier
> >> control, and therefore causes no new creation/deletion notifications,
> >> and thus terminates the recursion.
> > 
> > Hmm I don't get it; if an application just toggles the kctl value
> > between two values in an infinite loop, it'll delete and recreate
> > kctls endlessly as well with your patch, no?
> > 
> yeah, but why should it toggle just so? it's not reasonable to do
> that. 

I'm arguing about a malicious or buggy applications.  Don't ask logics
or conscience behind it.

> >> also, i don't think that disabling would be fundamentally different
> >> from deleting: the particular code paths taken are somewhat different,
> >> but the high-level view is essentially the same. so we can't really
> >> make predictions which one would work better.
> > 
> > Creating and deleting needs a lot of different works and much heavier
> > tasks.
> > 
> it's entirely plausible that an application would tear down structures
> in response to controls being disabled, too.

But it's less dangerous.

> > And, above all, many user-space programs will be borked if an
> > element goes away, simply crashing.  Some (rather rare) nice ones will
> > still survive, though.  I've learned this from the past.
> > 
> yeah, but why should we care? it's not a regression when something new
> doesn't work with some crappy pre-existing code.

We can't break user-space.  That's a rule set in stone.

> >> > And, if we really have to create / delete a kctl element from some
> >> > kctl action, don't do it in the callback but process in another work.
> >> > would that really improve anything?
> > 
> > As a primary reason, I don't want to expose such a stuff.  If you need
> > such an unlocked version, you're already doing something very exotic,
> > and in 99% cases, it's something that needs more care.
> > 
> i don't see being "exotic" as something to avoid per se. and before
> putting in "more care" i want to see some evidence that there is
> actually a problem that needs to be addressed, in this
> place. esp. when the proposed much more complex alternative hasn't
> been shown to be actually better in relevant ways, even theoretically.

Well, then another, maybe foremost reason: you can't create / delete
kctls from the callback, simply because the callbacks are called in
the read lock.  Adding / deleting an element may crash the another
concurrent task that traverses the list.


Takashi