| Message ID | 20210201075041.1201-1-abel.w@icloud.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | PM: hibernate: add sanity check on power_kobj | expand |
On Mon 2021-02-01 02:50:41, Abel Wu wrote: > The @power_kobj is initialized in pm_init() which is the same > initcall level as pm_disk_init(). Although this dependency is > guaranteed based on the current initcall serial execution model, > it would still be better do a cost-less sanity check to avoid > oops once the dependency is broken. I don't believe this is good idea. If the dependency is ever broken, this will make failure more subtle and harder to debug. > Signed-off-by: Abel Wu <abel.w@icloud.com> > --- > kernel/power/hibernate.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c > index da0b41914177..060089cc261d 100644 > --- a/kernel/power/hibernate.c > +++ b/kernel/power/hibernate.c > @@ -1262,6 +1262,9 @@ static const struct attribute_group attr_group = { > > static int __init pm_disk_init(void) > { > + if (!power_kobj) > + return -EINVAL; > + > return sysfs_create_group(power_kobj, &attr_group); > } > > -- > 2.27.0
> On Feb 1, 2021, at 6:52 PM, Pavel Machek <pavel@ucw.cz> wrote: > > On Mon 2021-02-01 02:50:41, Abel Wu wrote: >> The @power_kobj is initialized in pm_init() which is the same >> initcall level as pm_disk_init(). Although this dependency is >> guaranteed based on the current initcall serial execution model, >> it would still be better do a cost-less sanity check to avoid >> oops once the dependency is broken. > > I don't believe this is good idea. If the dependency is ever broken, > this will make failure more subtle and harder to debug. Thanks for reviewing. I think the cmdline parameter initcall_debug will help in this case. Actually we are trying to make initcalls being called asynchronously to reduce boot time which is crucial to our cloud-native business. And we resolve this kind of dependencies by retrying failed initcalls. Best regards, Abel > >> Signed-off-by: Abel Wu <abel.w@icloud.com> >> --- >> kernel/power/hibernate.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c >> index da0b41914177..060089cc261d 100644 >> --- a/kernel/power/hibernate.c >> +++ b/kernel/power/hibernate.c >> @@ -1262,6 +1262,9 @@ static const struct attribute_group attr_group = { >> >> static int __init pm_disk_init(void) >> { >> + if (!power_kobj) >> + return -EINVAL; >> + >> return sysfs_create_group(power_kobj, &attr_group); >> } >> >> -- >> 2.27.0 > > -- > http://www.livejournal.com/~pavelmachek
On Tue 2021-02-02 09:59:11, Abel Wu wrote: > > > > On Feb 1, 2021, at 6:52 PM, Pavel Machek <pavel@ucw.cz> wrote: > > > > On Mon 2021-02-01 02:50:41, Abel Wu wrote: > >> The @power_kobj is initialized in pm_init() which is the same > >> initcall level as pm_disk_init(). Although this dependency is > >> guaranteed based on the current initcall serial execution model, > >> it would still be better do a cost-less sanity check to avoid > >> oops once the dependency is broken. > > > > I don't believe this is good idea. If the dependency is ever broken, > > this will make failure more subtle and harder to debug. > > Thanks for reviewing. I think the cmdline parameter initcall_debug will > help in this case. > Actually we are trying to make initcalls being called asynchronously to > reduce boot time which is crucial to our cloud-native business. And we > resolve this kind of dependencies by retrying failed initcalls. And this patch is okay if that gets mainlined, but not before. Best regards, Pavel
diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c index da0b41914177..060089cc261d 100644 --- a/kernel/power/hibernate.c +++ b/kernel/power/hibernate.c @@ -1262,6 +1262,9 @@ static const struct attribute_group attr_group = { static int __init pm_disk_init(void) { + if (!power_kobj) + return -EINVAL; + return sysfs_create_group(power_kobj, &attr_group); }
The @power_kobj is initialized in pm_init() which is the same initcall level as pm_disk_init(). Although this dependency is guaranteed based on the current initcall serial execution model, it would still be better do a cost-less sanity check to avoid oops once the dependency is broken. Signed-off-by: Abel Wu <abel.w@icloud.com> --- kernel/power/hibernate.c | 3 +++ 1 file changed, 3 insertions(+)