On Sat, Feb 11, 2023 at 09:40:58PM -0500, Alan Stern wrote: > On Sat, Feb 11, 2023 at 06:24:42PM -0500, Kent Overstreet wrote: > > After scanning the rest of the thread: I don't think you want to create > > separate lockdep classes for each bus and device type, that's defeating > > how lockdep works. > > Not at all. In fact, exactly the opposite: lockdep works by creating a > class for each lock-inside-a-data-structure-type combination. A struct > device-bus_type/device_type combination is pretty much the same kind of > thing. > > > Maybe if it was only a small, _static_ number of new > > classes, > > The collection of bus_types and device_types _is_ static, in the sense > that each one is a structure defined in a driver source file. Whether > the number is "small" depends on your tolerance for large numbers; the > kernel has a lot of source files. :-) > > Mind you, I'm not saying that having lockdep classes for each bus_type > or device_type is always the right thing to do. There definitely are > cases where it wouldn't do what we want. But perhaps in some cases it > would work. > > > but the basic premesis of lockdep is that there are static > > human understandable lock ordering rules, so lockdep figures out what > > they are and checks them: if you create a bunch of dynamic classes, the > > classes are going to be different for everyone in practice and won't > > have any real bearing on the structure of the code > > As a rule, bus_type's and device_type's aren't dynamic. Maybe Greg KH > once published an example of such a thing; IIRC it was more like a > proof-of-principle rather than a serious recommendation on how to write > drivers. (Or else I'm misremembering and it was actually an example of > creating dynamic sysfs attributes.) > > Or maybe you're referring to what this patch does? It does indeed > create a bunch of dynamic classes -- one for each struct device. The > ordering rules derived by lockdep will be somewhat arbitrary, as you > say. But some of them certainly will be related to the structure of the > source code. I could be :) I haven't been able to find the patch in question - have a link? If you're talking about making lock_class_key dynamic, I think I stand by what I said though - OTOH, if all you're doing is lifting that to the caller of the device object init function, so it'll still be a static object in the driver, that would be totally fine. I probably should've found the patch before commenting :)
