| Message ID | 20220602055633.849545-1-dzm91@hust.edu.cn (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fs: nfsd: make destory function more elegant | expand |
> On Jun 2, 2022, at 1:56 AM, Dongliang Mu <dzm91@hust.edu.cn> wrote: > > From: Dongliang Mu <mudongliangabcd@gmail.com> > > In init_nfsd, the undo operation of create_proc_exports_entry is: > > remove_proc_entry("fs/nfs/exports", NULL); > remove_proc_entry("fs/nfs", NULL); > > This may lead to maintaince issue. Declare the undo function "maintenance" > destroy_proc_exports_entry based on CONFIG_PROC_FS IIUC, the issue is that if CONFIG_PROC_FS is not set, fs/nfsd/nfsctl.c fails to compile? > Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com> > --- > fs/nfsd/nfsctl.c | 16 ++++++++++++---- > 1 file changed, 12 insertions(+), 4 deletions(-) > > diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c > index 0621c2faf242..83b34ccbef5a 100644 > --- a/fs/nfsd/nfsctl.c > +++ b/fs/nfsd/nfsctl.c > @@ -1450,11 +1450,21 @@ static int create_proc_exports_entry(void) > } > return 0; > } > + > +static void destroy_proc_exports_entry(void) > +{ > + remove_proc_entry("fs/nfs/exports", NULL); > + remove_proc_entry("fs/nfs", NULL); > +} > #else /* CONFIG_PROC_FS */ > static int create_proc_exports_entry(void) > { > return 0; > } > + > +static void destroy_proc_exports_entry(void) > +{ > +} > #endif > > unsigned int nfsd_net_id; > @@ -1555,8 +1565,7 @@ static int __init init_nfsd(void) > out_free_subsys: > unregister_pernet_subsys(&nfsd_net_ops); > out_free_exports: > - remove_proc_entry("fs/nfs/exports", NULL); > - remove_proc_entry("fs/nfs", NULL); > + destroy_proc_exports_entry(); > out_free_lockd: > nfsd_lockd_shutdown(); > nfsd_drc_slab_free(); > @@ -1576,8 +1585,7 @@ static void __exit exit_nfsd(void) > unregister_cld_notifier(); > unregister_pernet_subsys(&nfsd_net_ops); > nfsd_drc_slab_free(); > - remove_proc_entry("fs/nfs/exports", NULL); > - remove_proc_entry("fs/nfs", NULL); > + destroy_proc_exports_entry(); > nfsd_stat_shutdown(); > nfsd_lockd_shutdown(); > nfsd4_free_slabs(); > -- > 2.25.1 > -- Chuck Lever
On Thu, Jun 2, 2022 at 10:30 PM Chuck Lever III <chuck.lever@oracle.com> wrote: > > > > > On Jun 2, 2022, at 1:56 AM, Dongliang Mu <dzm91@hust.edu.cn> wrote: > > > > From: Dongliang Mu <mudongliangabcd@gmail.com> > > > > In init_nfsd, the undo operation of create_proc_exports_entry is: > > > > remove_proc_entry("fs/nfs/exports", NULL); > > remove_proc_entry("fs/nfs", NULL); > > > > This may lead to maintaince issue. Declare the undo function > > "maintenance" Sorry for the typo. > > > > destroy_proc_exports_entry based on CONFIG_PROC_FS > > IIUC, the issue is that if CONFIG_PROC_FS is not set, > fs/nfsd/nfsctl.c fails to compile? The answer is no. There is no bug in the code. At first, I thought this might cause the miscompilation or bug. But actually the code is fine. Because remove_proc_entry is defined as an empty function if CONFIG_PROC_FS is not set, > > > > Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com> > > --- > > fs/nfsd/nfsctl.c | 16 ++++++++++++---- > > 1 file changed, 12 insertions(+), 4 deletions(-) > > > > diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c > > index 0621c2faf242..83b34ccbef5a 100644 > > --- a/fs/nfsd/nfsctl.c > > +++ b/fs/nfsd/nfsctl.c > > @@ -1450,11 +1450,21 @@ static int create_proc_exports_entry(void) > > } > > return 0; > > } > > + > > +static void destroy_proc_exports_entry(void) > > +{ > > + remove_proc_entry("fs/nfs/exports", NULL); > > + remove_proc_entry("fs/nfs", NULL); > > +} > > #else /* CONFIG_PROC_FS */ > > static int create_proc_exports_entry(void) > > { > > return 0; > > } > > + > > +static void destroy_proc_exports_entry(void) > > +{ > > +} > > #endif > > > > unsigned int nfsd_net_id; > > @@ -1555,8 +1565,7 @@ static int __init init_nfsd(void) > > out_free_subsys: > > unregister_pernet_subsys(&nfsd_net_ops); > > out_free_exports: > > - remove_proc_entry("fs/nfs/exports", NULL); > > - remove_proc_entry("fs/nfs", NULL); > > + destroy_proc_exports_entry(); > > out_free_lockd: > > nfsd_lockd_shutdown(); > > nfsd_drc_slab_free(); > > @@ -1576,8 +1585,7 @@ static void __exit exit_nfsd(void) > > unregister_cld_notifier(); > > unregister_pernet_subsys(&nfsd_net_ops); > > nfsd_drc_slab_free(); > > - remove_proc_entry("fs/nfs/exports", NULL); > > - remove_proc_entry("fs/nfs", NULL); > > + destroy_proc_exports_entry(); > > nfsd_stat_shutdown(); > > nfsd_lockd_shutdown(); > > nfsd4_free_slabs(); > > -- > > 2.25.1 > > > > -- > Chuck Lever > > >
> On Jun 3, 2022, at 9:50 AM, Dongliang Mu <mudongliangabcd@gmail.com> wrote: > > On Thu, Jun 2, 2022 at 10:30 PM Chuck Lever III <chuck.lever@oracle.com> wrote: >> >> >> >>> On Jun 2, 2022, at 1:56 AM, Dongliang Mu <dzm91@hust.edu.cn> wrote: >>> >>> From: Dongliang Mu <mudongliangabcd@gmail.com> >>> >>> In init_nfsd, the undo operation of create_proc_exports_entry is: >>> >>> remove_proc_entry("fs/nfs/exports", NULL); >>> remove_proc_entry("fs/nfs", NULL); >>> >>> This may lead to maintaince issue. Declare the undo function >> >> "maintenance" > > Sorry for the typo. > >> >> >>> destroy_proc_exports_entry based on CONFIG_PROC_FS >> >> IIUC, the issue is that if CONFIG_PROC_FS is not set, >> fs/nfsd/nfsctl.c fails to compile? > > The answer is no. There is no bug in the code. > > At first, I thought this might cause the miscompilation or bug. But > actually the code is fine. > > Because remove_proc_entry is defined as an empty function if > CONFIG_PROC_FS is not set, Code inspection does suggest there could be a problem, but I'm glad there isn't one. I can drop this one. >>> Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com> >>> --- >>> fs/nfsd/nfsctl.c | 16 ++++++++++++---- >>> 1 file changed, 12 insertions(+), 4 deletions(-) >>> >>> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c >>> index 0621c2faf242..83b34ccbef5a 100644 >>> --- a/fs/nfsd/nfsctl.c >>> +++ b/fs/nfsd/nfsctl.c >>> @@ -1450,11 +1450,21 @@ static int create_proc_exports_entry(void) >>> } >>> return 0; >>> } >>> + >>> +static void destroy_proc_exports_entry(void) >>> +{ >>> + remove_proc_entry("fs/nfs/exports", NULL); >>> + remove_proc_entry("fs/nfs", NULL); >>> +} >>> #else /* CONFIG_PROC_FS */ >>> static int create_proc_exports_entry(void) >>> { >>> return 0; >>> } >>> + >>> +static void destroy_proc_exports_entry(void) >>> +{ >>> +} >>> #endif >>> >>> unsigned int nfsd_net_id; >>> @@ -1555,8 +1565,7 @@ static int __init init_nfsd(void) >>> out_free_subsys: >>> unregister_pernet_subsys(&nfsd_net_ops); >>> out_free_exports: >>> - remove_proc_entry("fs/nfs/exports", NULL); >>> - remove_proc_entry("fs/nfs", NULL); >>> + destroy_proc_exports_entry(); >>> out_free_lockd: >>> nfsd_lockd_shutdown(); >>> nfsd_drc_slab_free(); >>> @@ -1576,8 +1585,7 @@ static void __exit exit_nfsd(void) >>> unregister_cld_notifier(); >>> unregister_pernet_subsys(&nfsd_net_ops); >>> nfsd_drc_slab_free(); >>> - remove_proc_entry("fs/nfs/exports", NULL); >>> - remove_proc_entry("fs/nfs", NULL); >>> + destroy_proc_exports_entry(); >>> nfsd_stat_shutdown(); >>> nfsd_lockd_shutdown(); >>> nfsd4_free_slabs(); >>> -- >>> 2.25.1 >>> >> >> -- >> Chuck Lever -- Chuck Lever
diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c index 0621c2faf242..83b34ccbef5a 100644 --- a/fs/nfsd/nfsctl.c +++ b/fs/nfsd/nfsctl.c @@ -1450,11 +1450,21 @@ static int create_proc_exports_entry(void) } return 0; } + +static void destroy_proc_exports_entry(void) +{ + remove_proc_entry("fs/nfs/exports", NULL); + remove_proc_entry("fs/nfs", NULL); +} #else /* CONFIG_PROC_FS */ static int create_proc_exports_entry(void) { return 0; } + +static void destroy_proc_exports_entry(void) +{ +} #endif unsigned int nfsd_net_id; @@ -1555,8 +1565,7 @@ static int __init init_nfsd(void) out_free_subsys: unregister_pernet_subsys(&nfsd_net_ops); out_free_exports: - remove_proc_entry("fs/nfs/exports", NULL); - remove_proc_entry("fs/nfs", NULL); + destroy_proc_exports_entry(); out_free_lockd: nfsd_lockd_shutdown(); nfsd_drc_slab_free(); @@ -1576,8 +1585,7 @@ static void __exit exit_nfsd(void) unregister_cld_notifier(); unregister_pernet_subsys(&nfsd_net_ops); nfsd_drc_slab_free(); - remove_proc_entry("fs/nfs/exports", NULL); - remove_proc_entry("fs/nfs", NULL); + destroy_proc_exports_entry(); nfsd_stat_shutdown(); nfsd_lockd_shutdown(); nfsd4_free_slabs();