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
