On Fri Oct 17, 2025 at 8:44 PM CEST, Jason Gunthorpe wrote: > On Fri, Oct 17, 2025 at 08:19:06PM +0200, Danilo Krummrich wrote: >> On 10/17/25 6:37 PM, Jason Gunthorpe wrote: >> > On Fri, Oct 17, 2025 at 06:29:10PM +0200, Danilo Krummrich wrote: >> > >> >> I'm not sure about MISC device though. Unless there's a good reason, >> >> I think MISC device should be "fenced" instead. >> > >> > misc is a very small wrapper around raw fops, and raw fops are >> > optimized for performance. Adding locking that many important things >> > like normal files don't need to all fops would not be agreed. >> > >> > The sketch in this series where we have a core helper to provide a >> > shim fops that adds on the lock is smart and I think could be an >> > agreeable way to make a synchronous misc and cdev unregister for >> > everyone to trivially use. >> >> Sure, for MISC devices without a parent for instance there are no device >> resources to access anyways. > > There are many situations with misc that can get people into trouble without > parent: > > misc_deregister(x); > timer_shutdown_sync(y); > kfree(z); > > For example. It is is buggy if the fops touch y or z. > > This is why a _sync version is such a nice clean idea because with 5 > letters the above can just be fixed. > > Wrapping everything in a revocable would be a huge PITA.
That's a bit of a different problem though. Revocable clearly isn't the solution. _sync() works, but doesn't account for the actual problem, which is that the file private has at least shared ownership of y and z. So, it's more of an ownership / lifetime problem. The file private data should either own y and z entirely or a corresponding reference count that is dropped in fops release(). Device resources are different though, since we can't just hold on to them with a reference count etc.; they're strictly gone once the bus device is unbound, hence revocable when there is no _sync().
