-
Notifications
You must be signed in to change notification settings - Fork 585
Fix incorrect conditional usage of .owner #349
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
ab1e29c
to
5fba871
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for taking a look.
Hmm. So it looks like the new kernel even removed the .owner field from struct class? I’ll double-check. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No issues found across 4 files
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
5fba871
to
e413729
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No issues found across 4 files
The examples code wrapped .owner = THIS_MODULE inside version checks, suggesting that it is no longer needed after v6.4. This is only true for driver types where the driver core sets .owner automatically (e.g. platform and i2c drivers). For structures such as struct file_operations, .owner must still be set explicitly by the user. Without it, module reference counting will be broken, allowing a module to be unloaded while still in use, which can lead to kernel panics. struct class is a special case: its .owner field was removed in upstream Linux commit 6e30a66433af ("driver core: class: remove struct module owner out of struct class"). Explicitly setting .owner for struct class is safe on older kernels but must be conditionally compiled out on newer kernels. Remove the unnecessary version guards and unconditionally sets .owner = THIS_MODULE in the affected example code. Closes: sysprog21#348
e413729
to
f798488
Compare
Thank @visitorckw for contributing! |
The examples code wrapped .owner = THIS_MODULE inside version checks, suggesting that it is no longer needed after v6.4. This is only true for driver types where the driver core sets .owner automatically (e.g. platform and i2c drivers).
For structures such as struct file_operations, .owner must still be set explicitly by the user. Without it, module reference counting will be broken, allowing a module to be unloaded while still in use, which can lead to kernel panics.
struct class is a special case: its .owner field was removed in upstream Linux commit 6e30a66433af ("driver core: class: remove struct module owner out of struct class"). Explicitly setting .owner for struct class is safe on older kernels but must be conditionally compiled out on newer kernels.
Remove the unnecessary version guards and unconditionally sets .owner = THIS_MODULE in the affected example code.
Closes: #348
Summary by cubic
Remove version guards and always set .owner = THIS_MODULE in example drivers to fix module refcounting and avoid unload panics. This also makes the examples clearer.