From: Mr NeilBrown Date: Fri, 13 May 2022 02:16:12 +0000 (+1000) Subject: LU-15759 libcfs: debugfs file_operation should have an owner X-Git-Tag: 2.15.51~52 X-Git-Url: https://git.whamcloud.com/?a=commitdiff_plain;h=b2dfb4457f0f1e56f3df448cf67ac97e728f4417;p=fs%2Flustre-release.git LU-15759 libcfs: debugfs file_operation should have an owner If debugfs a file is open when unloading the libcfs/lnet module, it produces a kernel Oops (debugfs file_operations callbacks no longer exist). Crash generated with routerstat (/sys/kernel/debug/lnet/stats): [ 1449.750396] IP: [] SyS_lseek+0x83/0x100 [ 1449.750412] PGD 9fa14067 PUD 9fa16067 PMD d4e5d067 PTE 0 [ 1449.750428] Oops: 0000 [#1] SMP [ 1449.750883] [] system_call_fastpath+0x25/0x2a [ 1449.750897] [] ? system_call_after_swapgs+0xa2/0x13a This patch adds an owner to debugfs file_operation for libcfs and lnet_router entries (/sys/kernel/debug/lnet/*). The following behavior is expected: $ modprobe lustre $ routerstat 10 > /dev/null & $ lustre_rmmod rmmod: ERROR: Module lnet is in use Can't read statfile (ENODEV) [1]+ Exit 1 routerstat 10 > /dev/null $ lustre_rmmod Note that the allocated 'struct file_operations' cannot be freed until the module_exit() function is called, as files could still be open until then. Signed-off-by: Mr NeilBrown Change-Id: Ia0920313e0c2a4b6cdc875fed08221e174a12a73 Reviewed-on: https://review.whamcloud.com/47335 Reviewed-by: Etienne AUJAMES Tested-by: jenkins Tested-by: Maloo Reviewed-by: James Simmons Reviewed-by: Oleg Drokin --- diff --git a/libcfs/include/libcfs/libcfs.h b/libcfs/include/libcfs/libcfs.h index 601ebfe..50ad978 100644 --- a/libcfs/include/libcfs/libcfs.h +++ b/libcfs/include/libcfs/libcfs.h @@ -84,8 +84,10 @@ int libcfs_ioctl_data_adjust(struct libcfs_ioctl_data *data); extern struct workqueue_struct *cfs_rehash_wq; -void lnet_insert_debugfs(struct ctl_table *table); +void lnet_insert_debugfs(struct ctl_table *table, struct module *mod, + void **statep); void lnet_remove_debugfs(struct ctl_table *table); +void lnet_debugfs_fini(void **statep); /* helper for sysctl handlers */ int debugfs_doint(struct ctl_table *table, int write, diff --git a/libcfs/libcfs/module.c b/libcfs/libcfs/module.c index 82a75a7..3345203 100644 --- a/libcfs/libcfs/module.c +++ b/libcfs/libcfs/module.c @@ -768,19 +768,22 @@ static const struct file_operations lnet_debugfs_file_operations_wo = { .llseek = default_llseek, }; -static const struct file_operations *lnet_debugfs_fops_select(umode_t mode) +static const struct file_operations *lnet_debugfs_fops_select( + umode_t mode, const struct file_operations state[3]) { if (!(mode & S_IWUGO)) - return &lnet_debugfs_file_operations_ro; + return &state[0]; if (!(mode & S_IRUGO)) - return &lnet_debugfs_file_operations_wo; + return &state[1]; - return &lnet_debugfs_file_operations_rw; + return &state[2]; } -void lnet_insert_debugfs(struct ctl_table *table) +void lnet_insert_debugfs(struct ctl_table *table, struct module *mod, + void **statep) { + struct file_operations *state = *statep; if (!lnet_debugfs_root) lnet_debugfs_root = debugfs_create_dir("lnet", NULL); @@ -788,16 +791,37 @@ void lnet_insert_debugfs(struct ctl_table *table) if (IS_ERR_OR_NULL(lnet_debugfs_root)) return; + if (!state) { + state = kmalloc(3 * sizeof(*state), GFP_KERNEL); + if (!state) + return; + state[0] = lnet_debugfs_file_operations_ro; + state[0].owner = mod; + state[1] = lnet_debugfs_file_operations_wo; + state[1].owner = mod; + state[2] = lnet_debugfs_file_operations_rw; + state[2].owner = mod; + *statep = state; + } + /* We don't save the dentry returned in next two calls, because * we don't call debugfs_remove() but rather remove_recursive() */ for (; table && table->procname; table++) debugfs_create_file(table->procname, table->mode, lnet_debugfs_root, table, - lnet_debugfs_fops_select(table->mode)); + lnet_debugfs_fops_select(table->mode, + (const struct file_operations *)state)); } EXPORT_SYMBOL_GPL(lnet_insert_debugfs); +void lnet_debugfs_fini(void **state) +{ + kfree(*state); + *state = NULL; +} +EXPORT_SYMBOL_GPL(lnet_debugfs_fini); + static void lnet_insert_debugfs_links( const struct lnet_debugfs_symlink_def *symlinks) { @@ -819,6 +843,7 @@ void lnet_remove_debugfs(struct ctl_table *table) } EXPORT_SYMBOL_GPL(lnet_remove_debugfs); +static void *debugfs_state; static int __init libcfs_init(void) { int rc; @@ -865,7 +890,7 @@ static int __init libcfs_init(void) goto cleanup_wi; } - lnet_insert_debugfs(lnet_table); + lnet_insert_debugfs(lnet_table, THIS_MODULE, &debugfs_state); if (!IS_ERR_OR_NULL(lnet_debugfs_root)) lnet_insert_debugfs_links(lnet_debugfs_symlinks); @@ -898,6 +923,8 @@ static void __exit libcfs_exit(void) debugfs_remove_recursive(lnet_debugfs_root); lnet_debugfs_root = NULL; + lnet_debugfs_fini(&debugfs_state); + CDEBUG(D_MALLOC, "before Portals cleanup: kmem %lld\n", libcfs_kmem_read()); diff --git a/lnet/include/lnet/lib-lnet.h b/lnet/include/lnet/lib-lnet.h index e3b8428..aa3e10e 100644 --- a/lnet/include/lnet/lib-lnet.h +++ b/lnet/include/lnet/lib-lnet.h @@ -599,6 +599,7 @@ struct lnet_net *lnet_get_net_locked(__u32 net_id); int lnet_lib_init(void); void lnet_lib_exit(void); +void lnet_router_exit(void); extern unsigned int lnet_response_tracking; extern unsigned lnet_transaction_timeout; diff --git a/lnet/lnet/module.c b/lnet/lnet/module.c index e4fe3f8..fba972c 100644 --- a/lnet/lnet/module.c +++ b/lnet/lnet/module.c @@ -265,6 +265,7 @@ static void __exit lnet_exit(void) &lnet_ioctl_handler); LASSERT(rc == 0); + lnet_router_exit(); lnet_lib_exit(); } diff --git a/lnet/lnet/router_proc.c b/lnet/lnet/router_proc.c index 9268914..af40833 100644 --- a/lnet/lnet/router_proc.c +++ b/lnet/lnet/router_proc.c @@ -891,12 +891,19 @@ static struct ctl_table lnet_table[] = { { .procname = NULL } }; +static void *debugfs_state; + void lnet_router_debugfs_init(void) { - lnet_insert_debugfs(lnet_table); + lnet_insert_debugfs(lnet_table, THIS_MODULE, + &debugfs_state); } void lnet_router_debugfs_fini(void) { lnet_remove_debugfs(lnet_table); } +void lnet_router_exit(void) +{ + lnet_debugfs_fini(&debugfs_state); +}