* [PATCH] ext4: don't work without procfs @ 2011-10-03 20:46 Yargil 2011-10-04 9:20 ` Lukas Czerner 0 siblings, 1 reply; 4+ messages in thread From: Yargil @ 2011-10-03 20:46 UTC (permalink / raw) To: linux-ext4; +Cc: Yargil Regression from commit dd68314ccf3fb918c1fb6471817edbc60ece4b52 --- fs/ext4/super.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-) diff --git a/fs/ext4/super.c b/fs/ext4/super.c index 44d0c8d..8e7298d 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -53,7 +53,9 @@ #define CREATE_TRACE_POINTS #include <trace/events/ext4.h> +#ifdef CONFIG_PROC_FS static struct proc_dir_entry *ext4_proc_root; +#endif static struct kset *ext4_kset; static struct ext4_lazy_init *ext4_li_info; static struct mutex ext4_li_mtx; @@ -812,9 +814,11 @@ static void ext4_put_super(struct super_block *sb) es->s_state = cpu_to_le16(sbi->s_mount_state); ext4_commit_super(sb, 1); } +#ifdef CONFIG_PROC_FS if (sbi->s_proc) { remove_proc_entry(sb->s_id, ext4_proc_root); } +#endif kobject_del(&sbi->s_kobj); for (i = 0; i < sbi->s_gdb_count; i++) @@ -4984,9 +4988,11 @@ static int __init ext4_init_fs(void) ext4_kset = kset_create_and_add("ext4", NULL, fs_kobj); if (!ext4_kset) goto out6; +#ifdef CONFIG_PROC_FS ext4_proc_root = proc_mkdir("fs/ext4", NULL); if (!ext4_proc_root) goto out5; +#endif err = ext4_init_feat_adverts(); if (err) @@ -5022,8 +5028,10 @@ out2: out3: ext4_exit_feat_adverts(); out4: +#ifdef CONFIG_PROC_FS remove_proc_entry("fs/ext4", NULL); out5: +#endif kset_unregister(ext4_kset); out6: ext4_exit_system_zone(); -- 1.7.2.5 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] ext4: don't work without procfs 2011-10-03 20:46 [PATCH] ext4: don't work without procfs Yargil @ 2011-10-04 9:20 ` Lukas Czerner 2011-10-04 11:03 ` yargil 0 siblings, 1 reply; 4+ messages in thread From: Lukas Czerner @ 2011-10-04 9:20 UTC (permalink / raw) To: Yargil; +Cc: linux-ext4 On Mon, 3 Oct 2011, Yargil wrote: > Regression from commit dd68314ccf3fb918c1fb6471817edbc60ece4b52 Hi, the commit you've mentioned does not have anything to do with procfs. Maybe you meant commit c9de560ded61faa5b754137b7753da252391c55a ext4: Add multi block allocator for ext4 which adds multi block allocator for ext4 (mballoc.c) and has been added in Jan 29 2008. > --- > fs/ext4/super.c | 8 ++++++++ > 1 files changed, 8 insertions(+), 0 deletions(-) > > diff --git a/fs/ext4/super.c b/fs/ext4/super.c > index 44d0c8d..8e7298d 100644 > --- a/fs/ext4/super.c > +++ b/fs/ext4/super.c > @@ -53,7 +53,9 @@ > #define CREATE_TRACE_POINTS > #include <trace/events/ext4.h> > > +#ifdef CONFIG_PROC_FS > static struct proc_dir_entry *ext4_proc_root; > +#endif This is not needed I think, since proc_dir_entry is defined in linux/proc_fs.h even if CONFIG_PROC_FS is not defined. > static struct kset *ext4_kset; > static struct ext4_lazy_init *ext4_li_info; > static struct mutex ext4_li_mtx; > @@ -812,9 +814,11 @@ static void ext4_put_super(struct super_block *sb) > es->s_state = cpu_to_le16(sbi->s_mount_state); > ext4_commit_super(sb, 1); > } > +#ifdef CONFIG_PROC_FS > if (sbi->s_proc) { > remove_proc_entry(sb->s_id, ext4_proc_root); This function exists even if CONFIG_PROC_FS is not defined. #define remove_proc_entry(name, parent) do {} while (0) > } > +#endif > kobject_del(&sbi->s_kobj); > > for (i = 0; i < sbi->s_gdb_count; i++) > @@ -4984,9 +4988,11 @@ static int __init ext4_init_fs(void) > ext4_kset = kset_create_and_add("ext4", NULL, fs_kobj); > if (!ext4_kset) > goto out6; > +#ifdef CONFIG_PROC_FS > ext4_proc_root = proc_mkdir("fs/ext4", NULL); The same here static inline struct proc_dir_entry *proc_mkdir(const char *name, struct proc_dir_entry *parent) {return NULL;} > if (!ext4_proc_root) > goto out5; However this is wrong, because in case that procfs is not compiled in ext4_proc_root will be NULL, but there is no reason to error out in this case. The question is whether we should error out in case that it fails even if we have procfs compiled in. I am slightly in favour of just printing a warning in !CONFIG_PROC_FS case. Also this is probably why you blame commit dd68314ccf3fb918c1fb6471817edbc60ece4b52 for the regression, since this check was added with this commit. But I think that there is no reasong to #ifdef everything procfs related. > +#endif > > err = ext4_init_feat_adverts(); > if (err) > @@ -5022,8 +5028,10 @@ out2: > out3: > ext4_exit_feat_adverts(); > out4: > +#ifdef CONFIG_PROC_FS > remove_proc_entry("fs/ext4", NULL); same here. > out5: > +#endif > kset_unregister(ext4_kset); > out6: > ext4_exit_system_zone(); > -Lukas ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ext4: don't work without procfs 2011-10-04 9:20 ` Lukas Czerner @ 2011-10-04 11:03 ` yargil 2011-10-04 11:33 ` Lukas Czerner 0 siblings, 1 reply; 4+ messages in thread From: yargil @ 2011-10-04 11:03 UTC (permalink / raw) To: Lukas Czerner; +Cc: linux-ext4 Selon Lukas Czerner <lczerner@redhat.com>: > On Mon, 3 Oct 2011, Yargil wrote: > > > Regression from commit dd68314ccf3fb918c1fb6471817edbc60ece4b52 > > Hi, > > the commit you've mentioned does not have anything to do with procfs. > Maybe you meant commit > > c9de560ded61faa5b754137b7753da252391c55a > ext4: Add multi block allocator for ext4 > > which adds multi block allocator for ext4 (mballoc.c) and has been added > in Jan 29 2008. > > > --- > > fs/ext4/super.c | 8 ++++++++ > > 1 files changed, 8 insertions(+), 0 deletions(-) > > > > diff --git a/fs/ext4/super.c b/fs/ext4/super.c > > index 44d0c8d..8e7298d 100644 > > --- a/fs/ext4/super.c > > +++ b/fs/ext4/super.c > > @@ -53,7 +53,9 @@ > > #define CREATE_TRACE_POINTS > > #include <trace/events/ext4.h> > > > > +#ifdef CONFIG_PROC_FS > > static struct proc_dir_entry *ext4_proc_root; > > +#endif > > This is not needed I think, since proc_dir_entry is defined in > linux/proc_fs.h even if CONFIG_PROC_FS is not defined. > > > static struct kset *ext4_kset; > > static struct ext4_lazy_init *ext4_li_info; > > static struct mutex ext4_li_mtx; > > @@ -812,9 +814,11 @@ static void ext4_put_super(struct super_block *sb) > > es->s_state = cpu_to_le16(sbi->s_mount_state); > > ext4_commit_super(sb, 1); > > } > > +#ifdef CONFIG_PROC_FS > > if (sbi->s_proc) { > > remove_proc_entry(sb->s_id, ext4_proc_root); > > This function exists even if CONFIG_PROC_FS is not defined. > > #define remove_proc_entry(name, parent) do {} while (0) > > > > } > > +#endif > > kobject_del(&sbi->s_kobj); > > > > for (i = 0; i < sbi->s_gdb_count; i++) > > @@ -4984,9 +4988,11 @@ static int __init ext4_init_fs(void) > > ext4_kset = kset_create_and_add("ext4", NULL, fs_kobj); > > if (!ext4_kset) > > goto out6; > > +#ifdef CONFIG_PROC_FS > > ext4_proc_root = proc_mkdir("fs/ext4", NULL); > > The same here > > static inline struct proc_dir_entry *proc_mkdir(const char *name, > struct proc_dir_entry *parent) {return NULL;} > > > > if (!ext4_proc_root) > > goto out5; > However this is wrong, because in case that procfs is not compiled in > ext4_proc_root will be NULL, but there is no reason to error out in this > case. > > The question is whether we should error out in case that it fails even > if we have procfs compiled in. I am slightly in favour of just printing > a warning in !CONFIG_PROC_FS case. > > Also this is probably why you blame commit > dd68314ccf3fb918c1fb6471817edbc60ece4b52 for the regression, since this > check was added with this commit. But I think that there is no reasong > to #ifdef everything procfs related. I did that to avoid warning. But if printing warning is not a problem then the only part of code to be #ifdef is the test of the return value of proc_mkdir in the ext4_init_fs function. > > > > +#endif > > > > err = ext4_init_feat_adverts(); > > if (err) > > @@ -5022,8 +5028,10 @@ out2: > > out3: > > ext4_exit_feat_adverts(); > > out4: > > +#ifdef CONFIG_PROC_FS > > remove_proc_entry("fs/ext4", NULL); > > same here. > > > out5: > > +#endif > > kset_unregister(ext4_kset); > > out6: > > ext4_exit_system_zone(); > > > > -Lukas > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ext4: don't work without procfs 2011-10-04 11:03 ` yargil @ 2011-10-04 11:33 ` Lukas Czerner 0 siblings, 0 replies; 4+ messages in thread From: Lukas Czerner @ 2011-10-04 11:33 UTC (permalink / raw) To: yargil; +Cc: Lukas Czerner, linux-ext4 On Tue, 4 Oct 2011, yargil@free.fr wrote: > Selon Lukas Czerner <lczerner@redhat.com>: > > > On Mon, 3 Oct 2011, Yargil wrote: > > > > > Regression from commit dd68314ccf3fb918c1fb6471817edbc60ece4b52 > > > > Hi, > > > > the commit you've mentioned does not have anything to do with procfs. > > Maybe you meant commit > > > > c9de560ded61faa5b754137b7753da252391c55a > > ext4: Add multi block allocator for ext4 > > > > which adds multi block allocator for ext4 (mballoc.c) and has been added > > in Jan 29 2008. > > > > > --- > > > fs/ext4/super.c | 8 ++++++++ > > > 1 files changed, 8 insertions(+), 0 deletions(-) > > > > > > diff --git a/fs/ext4/super.c b/fs/ext4/super.c > > > index 44d0c8d..8e7298d 100644 > > > --- a/fs/ext4/super.c > > > +++ b/fs/ext4/super.c > > > @@ -53,7 +53,9 @@ > > > #define CREATE_TRACE_POINTS > > > #include <trace/events/ext4.h> > > > > > > +#ifdef CONFIG_PROC_FS > > > static struct proc_dir_entry *ext4_proc_root; > > > +#endif > > > > This is not needed I think, since proc_dir_entry is defined in > > linux/proc_fs.h even if CONFIG_PROC_FS is not defined. > > > > > static struct kset *ext4_kset; > > > static struct ext4_lazy_init *ext4_li_info; > > > static struct mutex ext4_li_mtx; > > > @@ -812,9 +814,11 @@ static void ext4_put_super(struct super_block *sb) > > > es->s_state = cpu_to_le16(sbi->s_mount_state); > > > ext4_commit_super(sb, 1); > > > } > > > +#ifdef CONFIG_PROC_FS > > > if (sbi->s_proc) { > > > remove_proc_entry(sb->s_id, ext4_proc_root); > > > > This function exists even if CONFIG_PROC_FS is not defined. > > > > #define remove_proc_entry(name, parent) do {} while (0) > > > > > > > } > > > +#endif > > > kobject_del(&sbi->s_kobj); > > > > > > for (i = 0; i < sbi->s_gdb_count; i++) > > > @@ -4984,9 +4988,11 @@ static int __init ext4_init_fs(void) > > > ext4_kset = kset_create_and_add("ext4", NULL, fs_kobj); > > > if (!ext4_kset) > > > goto out6; > > > +#ifdef CONFIG_PROC_FS > > > ext4_proc_root = proc_mkdir("fs/ext4", NULL); > > > > The same here > > > > static inline struct proc_dir_entry *proc_mkdir(const char *name, > > struct proc_dir_entry *parent) {return NULL;} > > > > > > > if (!ext4_proc_root) > > > goto out5; > > However this is wrong, because in case that procfs is not compiled in > > ext4_proc_root will be NULL, but there is no reason to error out in this > > case. > > > > The question is whether we should error out in case that it fails even > > if we have procfs compiled in. I am slightly in favour of just printing > > a warning in !CONFIG_PROC_FS case. > > > > Also this is probably why you blame commit > > dd68314ccf3fb918c1fb6471817edbc60ece4b52 for the regression, since this > > check was added with this commit. But I think that there is no reasong > > to #ifdef everything procfs related. > > I did that to avoid warning. But if printing warning is not a problem then the > only part of code to be #ifdef is the test of the return value of proc_mkdir in > the ext4_init_fs function. Exactly, it has the advantage of not spawning #ifdefs in super.c and the advantage of not preventing the mount in case that the proc_mkdir() fails for some reason, because we actually do not need that for file system to work properly. Thanks! -Lukas > > > > > > > > +#endif > > > > > > err = ext4_init_feat_adverts(); > > > if (err) > > > @@ -5022,8 +5028,10 @@ out2: > > > out3: > > > ext4_exit_feat_adverts(); > > > out4: > > > +#ifdef CONFIG_PROC_FS > > > remove_proc_entry("fs/ext4", NULL); > > > > same here. > > > > > out5: > > > +#endif > > > kset_unregister(ext4_kset); > > > out6: > > > ext4_exit_system_zone(); > > > > > > > -Lukas > > > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-ext4" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-10-04 11:33 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-10-03 20:46 [PATCH] ext4: don't work without procfs Yargil 2011-10-04 9:20 ` Lukas Czerner 2011-10-04 11:03 ` yargil 2011-10-04 11:33 ` Lukas Czerner
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox