On Wed, May 14, 2025 at 12:06:44PM -0700, Josh Poimboeuf wrote: > On Tue, May 13, 2025 at 11:40:05PM -0400, Kent Overstreet wrote: > > On Tue, May 13, 2025 at 08:38:57PM -0700, Josh Poimboeuf wrote: > > > On Tue, May 13, 2025 at 08:44:49PM -0400, Kent Overstreet wrote: > > > > On Tue, May 13, 2025 at 05:37:11PM -0700, Josh Poimboeuf wrote: > > > > > On Tue, May 13, 2025 at 07:34:59PM -0400, Kent Overstreet wrote: > > > > > > On Tue, May 13, 2025 at 02:24:46PM -0700, Josh Poimboeuf wrote: > > > > > > > > +++ b/include/linux/moduleparam.h > > > > > > > > @@ -122,6 +122,7 @@ struct kparam_array > > > > > > > > * charp: a character pointer > > > > > > > > * bool: a bool, values 0/1, y/n, Y/N. > > > > > > > > * invbool: the above, only sense-reversed (N = true). > > > > > > > > + * static_key_t: same as bool, but updates a 'struct > > > > > > > > static_key' > > > > > > > > > > > > > > The static_key_*() interfaces are deprecated because they're > > > > > > > really easy > > > > > > > to use incorrectly. I don't think we want to propagate that > > > > > > > misuse to > > > > > > > modules. > > > > > > > > > > > > > > It would be better to have type(s) based on static_key_false > > > > > > > and/or > > > > > > > static_key_true, depending on whatever the default is. > > > > > > > > > > > > Except those are just wrappers around struct static_key, so why does > > > > > > that matter here? > > > > > > > > > > Those struct wrappers are needed to work with static_branch_likely() > > > > > and > > > > > static_branch_unlikely(). > > > > > > > > Sure, but this has no bearing on that - unless I've missed them, there > > > > aren't separate methods for just setting and checking the value, which > > > > is all we're doing here. > > > > > > To make use of this feature, wouldn't the module need to use > > > static_key_false() or so to actually insert the static branch to check > > > the value? Otherwise what's the point of using static keys here? > > > > I'm not sure I follow? > > > > You just pass the inner static_key to the modparam, you still use > > static_key_true or static_key_false as normal. > > Ok, so the caller does something like so? > > DEFINE_STATIC_KEY_FALSE(foo); > module_param_named(foo, foo.key, static_key_t, 0); > > I guess that works, but 'foo.key' is awkward in that it's nonobvious and > breaks the abstraction. Driver people might end up allocating > static_key directly and using the deprecated interfaces again. > > My preference would be to stick to the supported interfaces, e.g.: > > DEFINE_STATIC_KEY_FALSE(foo); > module_param_named(foo, foo, static_key_false_t, 0);
Yeah, that looks reasonable
