Messages in this thread | | | Subject | Re: [PATCH v0] kernfs: Skip kernfs_put of parent from child node | From | Gaurav Kohli <> | Date | Fri, 5 Apr 2019 17:13:00 +0530 |
| |
On 4/5/2019 5:03 PM, Greg KH wrote: > On Fri, Apr 05, 2019 at 04:51:07PM +0530, Gaurav Kohli wrote: >> While adding kernfs node for child to the parent kernfs >> node and when child node founds that parent kn count is >> zero, then below comes like: >> >> WARNING: fs/kernfs/dir.c:494 kernfs_get+0x64/0x88 >> >> This indicates that parent is in kernfs_put path/ or already >> freed, and if the child node keeps continue to >> make new kernfs node, then there is chance of >> below race for parent node: >> >> CPU0 CPU1 >> //Parent node //child node >> kernfs_put >> atomic_dec_and_test(&kn->count) >> //count is 0, so continue >> kernfs_new_node(child) >> kernfs_get(parent); >> //increment parent count to 1 >> //warning come as parent count is 0 >> /* link in */ >> kernfs_add_one(kn); >> // this should fail as parent is >> //in free path. >> kernfs_put(child) >> kmem_cache_free(parent) >> kmem_cache_free(child) >> kn = parent >> atomic_dec_and_test(&kn->count)) >> //this is 0 now, so release will >> continue for parent. >> kmem_cache_free(parent) >> >> To prevent this race, child simply has to decrement count of parent >> kernfs node and keep continue the free path for itself. >> >> Signed-off-by: Gaurav Kohli <gkohli@codeaurora.org> >> Signed-off-by: Mukesh Ojha <mojha@codeaurora.org> >> >> diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c >> index b84d635..d5a36e8 100644 >> --- a/fs/kernfs/dir.c >> +++ b/fs/kernfs/dir.c >> @@ -515,7 +515,6 @@ void kernfs_put(struct kernfs_node *kn) >> if (!kn || !atomic_dec_and_test(&kn->count)) >> return; >> root = kernfs_root(kn); >> - repeat: >> /* >> * Moving/renaming is always done while holding reference. >> * kn->parent won't change beneath us. >> @@ -545,8 +544,8 @@ void kernfs_put(struct kernfs_node *kn) >> >> kn = parent; >> if (kn) { >> - if (atomic_dec_and_test(&kn->count)) >> - goto repeat; >> + /* Parent may be on free path, so simply decrement the count */ > That's the wrong indentation :( > > And how are you hitting this issue? What user of kernfs is causing > this?
Hi Greg,
Thanks, will fix comment indentation, seen during sys-executor running:
We have only one instance , In logs below warning also came:
WARNING: CPU: 4 kernel/msm-4.14/fs/kernfs/dir.c:494 kernfs_get+0x64/0x88
which indicated parent is in put path.
[ 160.125151] Disabling lock debugging due to kernel taint [ 160.130626] INFO: Allocated in __kernfs_new_node+0x8c/0x3c0 age=11 cpu=2 pid=7098 [ 160.138314] kmem_cache_alloc+0x358/0x388 [ 160.142445] __kernfs_new_node+0x8c/0x3c0 [ 160.146590] kernfs_new_node+0x80/0xc8 [ 160.150462] kernfs_create_dir_ns+0x44/0xfc [ 160.154777] sysfs_create_dir_ns+0xa8/0x130 [ 160.158416] CPU5: update max cpu_capacity 1024 [ 160.159085] kobject_add_internal+0x278/0x650 [ 160.163567] kobject_add_varg+0xe0/0x130 [ 160.167606] kobject_add+0x15c/0x1d0 [ 160.168452] CPU5: update max cpu_capacity 780 [ 160.171287] get_device_parent+0x2d0/0x34c [ 160.175510] device_add+0x240/0xde0 [ 160.178371] CPU6: update max cpu_capacity 916 [ 160.179108] input_register_device+0x5f4/0xa0c [ 160.183686] uinput_ioctl_handler+0x1184/0x2198 [ 160.202436] INFO: Freed in kernfs_put+0x2c8/0x434 age=14 cpu=0 pid=7096 [ 160.209230] kernfs_put+0x2c8/0x434 [ 160.212825] kobject_del+0x50/0xcc [ 160.216332] cleanup_glue_dir+0x124/0x16c [ 160.220456] device_del+0x55c/0x5c8 [ 160.224047] __input_unregister_device+0x274/0x2a8 [ 160.228974] input_unregister_device+0x90/0xd0 [ 160.233553] uinput_destroy_device+0x15c/0x1dc [ 160.238131] uinput_release+0x44/0x5c [ 160.241898] __fput+0x1f4/0x4e4 [ 160.245127] ____fput+0x20/0x2c
during code review, I have found race between kernfs parent put call and child get call.
Regards
Gaurav
> > thanks, > > greg k-h
-- Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
| |