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
