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

[Oswald Buddenhagen <oswald.buddenhagen@gmx.de>]

On Wed, Jun 14, 2023 at 08:36:47AM +0200, Takashi Iwai wrote:
>On Tue, 13 Jun 2023 19:14:18 +0200,
>Oswald Buddenhagen wrote:
>> On Tue, Jun 13, 2023 at 05:43:58PM +0200, Takashi Iwai wrote:
>> > Crashing an existing application is the worst-case scenario.
>> > 
>> a new driver (which this effectively is) crashing a broken application
>> is perfectly legitimate, as it doesn't affect any existing users.
>
>No, you can't ignore it.
>
you're allowing _hypothetical_ crappy 3rd party code to dictate what you 
can and cannot do. that's a completely unreasonable and 
counterproductive attitude, akin to letting hostage-takers set the 
rules.

>> > Oh well, that's really not a change to be advertised for creating /
>> > deleting kctls from the put callback at all.
>> > 
>> and? it's done, and it's basically impossible to revert. so we may
>> reap its full benefits just as well, as i did in that previous commit.
>
>Well, I can revert your commit, too...
>
sure, but my point was that there are likely many more such commits, 
some of them not explicitly marked as such. it would be a very costly 
and risky exercise to actually do that revert at this point.

>Basically the content protection shouldn't be covered by this rwsem.
>It's rather a misuse.
>
yes, sort of.
otoh, the commit message is rather convincing, and you clearly saw it 
that way as well.

>> > Sorry, but my answer is same: NO.  I see no reason why kctl 
>> > deletion
>> > and creation _must_ be implemented _inevitably_ in that way.
>> > 
>> being the most straight-forward way to implement it certainly
>> qualifies as a good reason for doing it that way.
>> and i still see no convincing reason why it shouldn't.
>
>I still see no convincing reason why it must be done so, either.
>
the very convincing reason is that it was already done that way, and 
you'd have to bring forward a very convincing justification for further 
investment into a much more complex solution, esp. considering what 
driver we're talking about.

>The way you're trying to implement is an anti-pattern,
>
that's something you keep repeating in various ways, but i see no 
evidence that there is an _actual_ problem.

>not seen in other drivers that have been developed over decades.
>
that alone doesn't mean anything.

what are SNDRV_CTL_EVENT_MASK_{ADD,REMOVE} for, if not exactly that? if 
you're afraid to use it because it makes some poorly written apps crash, 
then you may delete the API just as well.

and as i've argued in a previous mail, doing it synchronously from the 
put() callback would cause an additional risk only for poorly coded 
multi-threaded apps, another purely hypothetical problem. it certainly 
does not appear to create problems in the kernel, given current locking 
realities (though it would not hurt to have a few more eyes confirm 
that).

regards,
ossi