Re: [RFC PATCH v3 1/4] watchdog: core: add mechanism to prevent removing if NOWAYOUT
> > Is the intent of the proposed changes to prevent the unload of > > a module if it was loaded with the "nowayout" parameter? > > > > If so, I thought the WD "nowayout" semantic was only supposed to be in > > effect > > after /dev/watchdog was opened. The proposed change looks like it makes the > > "nowayout" semantic take effect before /dev/watchdog is opened. > > Thanks, that's IMHO a very valid point. I agree, thanks for this input. signature.asc Description: PGP signature
Re: [RFC PATCH v3 1/4] watchdog: core: add mechanism to prevent removing if NOWAYOUT
Hi Jerry,
On Wed, Aug 29, 2018 at 10:55 PM Jerry Hoemann wrote:
> On Wed, Aug 29, 2018 at 09:42:38AM +0200, Wolfram Sang wrote:
> > To prevent removing if NOWAYOUT, we invalidate the .remove function and
> > suppress the bind/unbind attributes in sysfs. These are driver
> > capabilities, so we need to set it up at runtime during init. To avoid
> > boilerplate, introduce module_watchdog_driver() similar to
> > module_driver(). On top of that, we then build
> > module_watchdog_platform_driver(). Others may follow, if needed.
> >
> > Signed-off-by: Wolfram Sang
> > ---
> > include/linux/watchdog.h | 17 +
> > 1 file changed, 17 insertions(+)
> >
> > diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
> > index 44985c4a1e86..c8ecbc53c807 100644
> > --- a/include/linux/watchdog.h
> > +++ b/include/linux/watchdog.h
> > @@ -216,4 +216,21 @@ extern void watchdog_unregister_device(struct
> > watchdog_device *);
> > /* devres register variant */
> > int devm_watchdog_register_device(struct device *dev, struct
> > watchdog_device *);
> >
> > +#define module_watchdog_driver(__driver, __register, __unregister,
> > __nowayout, ...) \
> > +static int __init __driver##_init(void) \
> > +{ \
> > + __driver.driver.suppress_bind_attrs = !!(__nowayout); \
> > + return __register(&(__driver) ##__VA_ARGS__); \
> > +} \
>
> Sorry, I'm not familiar w/ this level of detail of the driver interface.
>
> Is the intent of the proposed changes to prevent the unload of
> a module if it was loaded with the "nowayout" parameter?
>
> If so, I thought the WD "nowayout" semantic was only supposed to be in effect
> after /dev/watchdog was opened. The proposed change looks like it makes the
> "nowayout" semantic take effect before /dev/watchdog is opened.
Thanks, that's IMHO a very valid point.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [RFC PATCH v3 1/4] watchdog: core: add mechanism to prevent removing if NOWAYOUT
On Wed, Aug 29, 2018 at 09:42:38AM +0200, Wolfram Sang wrote:
> To prevent removing if NOWAYOUT, we invalidate the .remove function and
> suppress the bind/unbind attributes in sysfs. These are driver
> capabilities, so we need to set it up at runtime during init. To avoid
> boilerplate, introduce module_watchdog_driver() similar to
> module_driver(). On top of that, we then build
> module_watchdog_platform_driver(). Others may follow, if needed.
>
> Signed-off-by: Wolfram Sang
> ---
> include/linux/watchdog.h | 17 +
> 1 file changed, 17 insertions(+)
>
> diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
> index 44985c4a1e86..c8ecbc53c807 100644
> --- a/include/linux/watchdog.h
> +++ b/include/linux/watchdog.h
> @@ -216,4 +216,21 @@ extern void watchdog_unregister_device(struct
> watchdog_device *);
> /* devres register variant */
> int devm_watchdog_register_device(struct device *dev, struct watchdog_device
> *);
>
> +#define module_watchdog_driver(__driver, __register, __unregister,
> __nowayout, ...) \
> +static int __init __driver##_init(void) \
> +{ \
> + __driver.driver.suppress_bind_attrs = !!(__nowayout); \
> + return __register(&(__driver) ##__VA_ARGS__); \
> +} \
Sorry, I'm not familiar w/ this level of detail of the driver interface.
Is the intent of the proposed changes to prevent the unload of
a module if it was loaded with the "nowayout" parameter?
If so, I thought the WD "nowayout" semantic was only supposed to be in effect
after /dev/watchdog was opened. The proposed change looks like it makes the
"nowayout" semantic take effect before /dev/watchdog is opened.
Thanks
> +module_init(__driver##_init); \
> +static void __exit __driver##_exit(void) \
> +{ \
> + __unregister(&(__driver), ##__VA_ARGS__); \
> +} \
> +module_exit(__driver##_exit)
> +
> +#define module_watchdog_platform_driver(__platform_driver, __nowayout) \
> + module_watchdog_driver(__platform_driver, platform_driver_register, \
> + platform_driver_unregister, __nowayout)
> +
> #endif /* ifndef _LINUX_WATCHDOG_H */
> --
> 2.11.0
--
-
Jerry Hoemann Software Engineer Hewlett Packard Enterprise
-
Re: [RFC PATCH v3 1/4] watchdog: core: add mechanism to prevent removing if NOWAYOUT
On Wed, Aug 29, 2018 at 09:42:38AM +0200, Wolfram Sang wrote:
> To prevent removing if NOWAYOUT, we invalidate the .remove function and
> suppress the bind/unbind attributes in sysfs. These are driver
> capabilities, so we need to set it up at runtime during init. To avoid
> boilerplate, introduce module_watchdog_driver() similar to
> module_driver(). On top of that, we then build
> module_watchdog_platform_driver(). Others may follow, if needed.
>
> Signed-off-by: Wolfram Sang
> ---
> include/linux/watchdog.h | 17 +
> 1 file changed, 17 insertions(+)
>
> diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
> index 44985c4a1e86..c8ecbc53c807 100644
> --- a/include/linux/watchdog.h
> +++ b/include/linux/watchdog.h
> @@ -216,4 +216,21 @@ extern void watchdog_unregister_device(struct
> watchdog_device *);
> /* devres register variant */
> int devm_watchdog_register_device(struct device *dev, struct watchdog_device
> *);
>
> +#define module_watchdog_driver(__driver, __register, __unregister,
> __nowayout, ...) \
Another option might be to add another argument to this macro. Something like:
#define module_watchdog_driver(__wdriver, __driver, __register, __unregister,
__nowayout, ...) \
static int __init __wdriver##_init(void) \
{ \
__driver->suppress_bind_attrs = !!(__nowayout); \
return __register(&(__wdriver) ##__VA_ARGS__); \
} \
module_init(__wdriver##_init); \
static void __exit __wdriver##_exit(void) \
{ \
__unregister(&(__wdriver), ##__VA_ARGS__); \
} \
module_exit(__wdriver##_exit)
#define module_watchdog_platform_driver(__platform_driver, __nowayout) \
module_watchdog_driver(__platform_driver, &__platform_driver->driver, \
platform_driver_register, \
platform_driver_unregister, __nowayout)
This would avoid the dependency of having a ".driver" structure in each
supported bus driver structure.
Would that make sense ?
Guenter
[RFC PATCH v3 1/4] watchdog: core: add mechanism to prevent removing if NOWAYOUT
To prevent removing if NOWAYOUT, we invalidate the .remove function and
suppress the bind/unbind attributes in sysfs. These are driver
capabilities, so we need to set it up at runtime during init. To avoid
boilerplate, introduce module_watchdog_driver() similar to
module_driver(). On top of that, we then build
module_watchdog_platform_driver(). Others may follow, if needed.
Signed-off-by: Wolfram Sang
---
include/linux/watchdog.h | 17 +
1 file changed, 17 insertions(+)
diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
index 44985c4a1e86..c8ecbc53c807 100644
--- a/include/linux/watchdog.h
+++ b/include/linux/watchdog.h
@@ -216,4 +216,21 @@ extern void watchdog_unregister_device(struct
watchdog_device *);
/* devres register variant */
int devm_watchdog_register_device(struct device *dev, struct watchdog_device
*);
+#define module_watchdog_driver(__driver, __register, __unregister, __nowayout,
...) \
+static int __init __driver##_init(void) \
+{ \
+ __driver.driver.suppress_bind_attrs = !!(__nowayout); \
+ return __register(&(__driver) ##__VA_ARGS__); \
+} \
+module_init(__driver##_init); \
+static void __exit __driver##_exit(void) \
+{ \
+ __unregister(&(__driver), ##__VA_ARGS__); \
+} \
+module_exit(__driver##_exit)
+
+#define module_watchdog_platform_driver(__platform_driver, __nowayout) \
+ module_watchdog_driver(__platform_driver, platform_driver_register, \
+ platform_driver_unregister, __nowayout)
+
#endif /* ifndef _LINUX_WATCHDOG_H */
--
2.11.0
