On Sun, 8 Nov 2020 at 04:05, Jakub Kicinski <kuba@xxxxxxxxxx> wrote: > Hi Jakub, Thank you for the review! > On Sat, 7 Nov 2020 17:21:31 +0000 Taehee Yoo wrote: > > When debugfs file is opened, its module should not be removed until > > it's closed. > > Because debugfs internally uses the module's data. > > So, it could access freed memory. > > > > In order to avoid panic, it just sets .owner to THIS_MODULE. > > So that all modules will be held when its debugfs file is opened. > > Hm, looks like some of the patches need to be revised because > .owner is already set in the ops, and a warning gets generated. Thanks, I found my mistake via patchwork. I will fix this problem. > > Also it'd be good to mention why Johannes's approach was abandoned. I'm sorry about skipping the explanation of the situation, Johannes sent RFC[1], which fixes this problem in the debugfs core logic. I tested it and it actually avoids this problem well. And I think there would be more discussion. So, I thought this series' approach is reasonable right now. I think setting .owner to THIS_MODULE is a common behavior and it doesn't hurt our logic even if Johannes's approach is merged. I'm expecting that both approaches of this series and Johannes are doing separately. [1] https://www.spinics.net/lists/linux-wireless/msg204171.html > > When you repost please separate out all the patches for > drivers/net/wireless/ and send that to Kalle's wireless drivers tree. > Patch 1 needs to be split in two. Patches 2 and 3 would go via Johannes. > The wimax patch needs to go to staging (wimax code has been moved). > The remaining patches can be posted individually, not as a series. Okay, I will do this. Thanks a lot! Taehee Yoo
