linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH] f2fs:avoid creaing multi debugfs when mount more disks.
       [not found] <201301141929076532360@gmail.com>
@ 2013-01-15  7:08 ` Jaegeuk Kim
  2013-01-15 10:58   ` Namjae Jeon
  0 siblings, 1 reply; 6+ messages in thread
From: Jaegeuk Kim @ 2013-01-15  7:08 UTC (permalink / raw)
  To: majianpeng; +Cc: linux-f2fs-devel, linux-fsdevel

[-- Attachment #1: Type: text/plain, Size: 1355 bytes --]

Hi Jianpeng,

Could you review the following patch that was originally written by you?
I added some description and fixed typos.
Thanks,

From e80c3ff6f7cab22cb866a05f63897e950c5a6f9e Mon Sep 17 00:00:00 2001
From: majianpeng <majianpeng@gmail.com>
Date: Mon, 14 Jan 2013 19:29:10 +0800
Subject: [PATCH] f2fs: avoid creating multi debugfs entries when
mounting many disks

When f2fs is mounted on multiple disks, it should avoid redundant status
files.
One status file contains the whole f2fs information.

Signed-off-by: Jianpeng Ma <majianpeng@gmail.com>
[jaegeuk.kim@samsung.com: fix subject and add description]
Signed-off-by: Jaegeuk Kim <jaegeuk.kim@samsung.com>
---
 fs/f2fs/debug.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/fs/f2fs/debug.c b/fs/f2fs/debug.c
index b8ed7a7..0c374d1 100644
--- a/fs/f2fs/debug.c
+++ b/fs/f2fs/debug.c
@@ -336,10 +336,11 @@ int f2fs_build_stats(struct f2fs_sb_info *sbi)
 	if (retval)
 		return retval;
 
-	if (!debugfs_root)
+	if (!debugfs_root) {
 		debugfs_root = debugfs_create_dir("f2fs", NULL);
-
-	debugfs_create_file("status", S_IRUGO, debugfs_root, NULL,
&stat_fops);
+		debugfs_create_file("status", S_IRUGO, debugfs_root, NULL,
+							&stat_fops);
+	}
 	return 0;
 }
 
-- 
1.8.0.1.250.gb7973fb



-- 
Jaegeuk Kim
Samsung

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] f2fs:avoid creaing multi debugfs when mount more disks.
  2013-01-15  7:08 ` [PATCH] f2fs:avoid creaing multi debugfs when mount more disks Jaegeuk Kim
@ 2013-01-15 10:58   ` Namjae Jeon
  2013-01-15 11:29     ` Jaegeuk Kim
  0 siblings, 1 reply; 6+ messages in thread
From: Namjae Jeon @ 2013-01-15 10:58 UTC (permalink / raw)
  To: jaegeuk.kim; +Cc: majianpeng, linux-f2fs-devel, linux-fsdevel

2013/1/15, Jaegeuk Kim <jaegeuk.kim@samsung.com>:
> Hi Jianpeng,
>
> Could you review the following patch that was originally written by you?
> I added some description and fixed typos.
> Thanks,
Hi Jaegeuk.

How about change like this ?

--------------------------------------------------------------------------------------
Subject: [PATCH] f2fs: fix the debugfs entry creation path

As the "status" debugfs entry will be maintained for entire F2FS filesystem
irrespective of the number of partitions.
So, we can move the initialization to the init part of the f2fs and destroy will
be done from exit part. After making changes, for individual partition mount -
entry creation code will not be executed.

Signed-off-by: Jianpeng Ma <majianpeng@gmail.com>
Signed-off-by: Namjae Jeon <namjae.jeon@samsung.com>
Signed-off-by: Amit Sahrawat <a.sahrawat@samsung.com>
---
 fs/f2fs/debug.c |   27 ++++++++++-----------------
 fs/f2fs/f2fs.h  |    6 ++++--
 fs/f2fs/super.c |    7 +++++--
 3 files changed, 19 insertions(+), 21 deletions(-)

diff --git a/fs/f2fs/debug.c b/fs/f2fs/debug.c
index 0e0380a..5164fba 100644
--- a/fs/f2fs/debug.c
+++ b/fs/f2fs/debug.c
@@ -303,7 +303,7 @@ static const struct file_operations stat_fops = {
 	.release = single_release,
 };

-static int init_stats(struct f2fs_sb_info *sbi)
+int f2fs_build_stats(struct f2fs_sb_info *sbi)
 {
 	struct f2fs_super_block *raw_super = F2FS_RAW_SUPER(sbi);
 	struct f2fs_stat_info *si;
@@ -328,21 +328,6 @@ static int init_stats(struct f2fs_sb_info *sbi)
 	return 0;
 }

-int f2fs_build_stats(struct f2fs_sb_info *sbi)
-{
-	int retval;
-
-	retval = init_stats(sbi);
-	if (retval)
-		return retval;
-
-	if (!debugfs_root)
-		debugfs_root = debugfs_create_dir("f2fs", NULL);
-
-	debugfs_create_file("status", S_IRUGO, debugfs_root, NULL, &stat_fops);
-	return 0;
-}
-
 void f2fs_destroy_stats(struct f2fs_sb_info *sbi)
 {
 	struct f2fs_stat_info *si = sbi->stat_info;
@@ -354,7 +339,15 @@ void f2fs_destroy_stats(struct f2fs_sb_info *sbi)
 	kfree(sbi->stat_info);
 }

-void destroy_root_stats(void)
+void __init f2fs_create_root_stats(void)
+{
+	debugfs_root = debugfs_create_dir("f2fs", NULL);
+	if (debugfs_root)
+		debugfs_create_file("status", S_IRUGO, debugfs_root,
+					 NULL, &stat_fops);
+}
+
+void f2fs_destroy_root_stats(void)
 {
 	debugfs_remove_recursive(debugfs_root);
 	debugfs_root = NULL;
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index ee967fd..6278ff9 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1060,7 +1060,8 @@ struct f2fs_stat_info {

 int f2fs_build_stats(struct f2fs_sb_info *);
 void f2fs_destroy_stats(struct f2fs_sb_info *);
-void destroy_root_stats(void);
+void f2fs_create_root_stats(void);
+void f2fs_destroy_root_stats(void);
 #else
 #define stat_inc_call_count(si)
 #define stat_inc_seg_count(si, type)
@@ -1070,7 +1071,8 @@ void destroy_root_stats(void);

 static inline int f2fs_build_stats(struct f2fs_sb_info *sbi) { return 0; }
 static inline void f2fs_destroy_stats(struct f2fs_sb_info *sbi) { }
-static inline void destroy_root_stats(void) { }
+static inline void f2fs_create_root_stats(void) { }
+static inline void f2fs_destroy_root_stats(void) { }
 #endif

 extern const struct file_operations f2fs_dir_operations;
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 6cdaa69..f6714e2 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -671,14 +671,17 @@ static int __init init_f2fs_fs(void)
 	err = create_checkpoint_caches();
 	if (err)
 		goto fail;
-	return register_filesystem(&f2fs_fs_type);
+	err = register_filesystem(&f2fs_fs_type);
+	if (err)
+		goto fail;
+	f2fs_create_root_stats();
 fail:
 	return err;
 }

 static void __exit exit_f2fs_fs(void)
 {
-	destroy_root_stats();
+	f2fs_destroy_root_stats();
 	unregister_filesystem(&f2fs_fs_type);
 	destroy_checkpoint_caches();
 	destroy_gc_caches();
-- 
1.7.2.3



-----------------------

>
> From e80c3ff6f7cab22cb866a05f63897e950c5a6f9e Mon Sep 17 00:00:00 2001
> From: majianpeng <majianpeng@gmail.com>
> Date: Mon, 14 Jan 2013 19:29:10 +0800
> Subject: [PATCH] f2fs: avoid creating multi debugfs entries when
> mounting many disks
>
> When f2fs is mounted on multiple disks, it should avoid redundant status
> files.
> One status file contains the whole f2fs information.
>
> Signed-off-by: Jianpeng Ma <majianpeng@gmail.com>
> [jaegeuk.kim@samsung.com: fix subject and add description]
> Signed-off-by: Jaegeuk Kim <jaegeuk.kim@samsung.com>
> ---
>  fs/f2fs/debug.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/fs/f2fs/debug.c b/fs/f2fs/debug.c
> index b8ed7a7..0c374d1 100644
> --- a/fs/f2fs/debug.c
> +++ b/fs/f2fs/debug.c
> @@ -336,10 +336,11 @@ int f2fs_build_stats(struct f2fs_sb_info *sbi)
>  	if (retval)
>  		return retval;
>
> -	if (!debugfs_root)
> +	if (!debugfs_root) {
>  		debugfs_root = debugfs_create_dir("f2fs", NULL);
> -
> -	debugfs_create_file("status", S_IRUGO, debugfs_root, NULL,
> &stat_fops);
> +		debugfs_create_file("status", S_IRUGO, debugfs_root, NULL,
> +							&stat_fops);
> +	}
>  	return 0;
>  }
>
> --
> 1.8.0.1.250.gb7973fb
>
>
>
> --
> Jaegeuk Kim
> Samsung
>

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] f2fs:avoid creaing multi debugfs when mount more disks.
  2013-01-15 10:58   ` Namjae Jeon
@ 2013-01-15 11:29     ` Jaegeuk Kim
  2013-01-15 11:50       ` Namjae Jeon
  0 siblings, 1 reply; 6+ messages in thread
From: Jaegeuk Kim @ 2013-01-15 11:29 UTC (permalink / raw)
  To: Namjae Jeon; +Cc: majianpeng, linux-f2fs-devel, linux-fsdevel

[-- Attachment #1: Type: text/plain, Size: 470 bytes --]

[snip]

>  void f2fs_destroy_stats(struct f2fs_sb_info *sbi)
>  {
>  	struct f2fs_stat_info *si = sbi->stat_info;
> @@ -354,7 +339,15 @@ void f2fs_destroy_stats(struct f2fs_sb_info *sbi)
>  	kfree(sbi->stat_info);
>  }
> 
> -void destroy_root_stats(void)
> +void __init f2fs_create_root_stats(void)

We don't need to add __init here, right?
Anyway, this patch looks much better to me.

Jianpeng, how do you think about this?

-- 
Jaegeuk Kim
Samsung

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] f2fs:avoid creaing multi debugfs when mount more disks.
  2013-01-15 11:29     ` Jaegeuk Kim
@ 2013-01-15 11:50       ` Namjae Jeon
  2013-01-15 12:10         ` Jaegeuk Kim
  0 siblings, 1 reply; 6+ messages in thread
From: Namjae Jeon @ 2013-01-15 11:50 UTC (permalink / raw)
  To: jaegeuk.kim; +Cc: majianpeng, linux-f2fs-devel, linux-fsdevel

2013/1/15, Jaegeuk Kim <jaegeuk.kim@samsung.com>:
> [snip]
>
>>  void f2fs_destroy_stats(struct f2fs_sb_info *sbi)
>>  {
>>  	struct f2fs_stat_info *si = sbi->stat_info;
>> @@ -354,7 +339,15 @@ void f2fs_destroy_stats(struct f2fs_sb_info *sbi)
>>  	kfree(sbi->stat_info);
>>  }
>>
>> -void destroy_root_stats(void)
>> +void __init f2fs_create_root_stats(void)
>
> We don't need to add __init here, right?
As you know, the function f2fs_create_root_stats will only be needed
at the init time of F2FS filesystem. So, there is no harm making this
function part of __init section, as this function will never be called
again.
Thanks.
> Anyway, this patch looks much better to me.
>
> Jianpeng, how do you think about this?
>
> --
> Jaegeuk Kim
> Samsung
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] f2fs:avoid creaing multi debugfs when mount more disks.
  2013-01-15 11:50       ` Namjae Jeon
@ 2013-01-15 12:10         ` Jaegeuk Kim
  2013-01-15 22:52           ` Namjae Jeon
  0 siblings, 1 reply; 6+ messages in thread
From: Jaegeuk Kim @ 2013-01-15 12:10 UTC (permalink / raw)
  To: Namjae Jeon; +Cc: majianpeng, linux-f2fs-devel, linux-fsdevel

[-- Attachment #1: Type: text/plain, Size: 1064 bytes --]

2013-01-15 (화), 20:50 +0900, Namjae Jeon:
> 2013/1/15, Jaegeuk Kim <jaegeuk.kim@samsung.com>:
> > [snip]
> >
> >>  void f2fs_destroy_stats(struct f2fs_sb_info *sbi)
> >>  {
> >>  	struct f2fs_stat_info *si = sbi->stat_info;
> >> @@ -354,7 +339,15 @@ void f2fs_destroy_stats(struct f2fs_sb_info *sbi)
> >>  	kfree(sbi->stat_info);
> >>  }
> >>
> >> -void destroy_root_stats(void)
> >> +void __init f2fs_create_root_stats(void)
> >
> > We don't need to add __init here, right?
> As you know, the function f2fs_create_root_stats will only be needed
> at the init time of F2FS filesystem. So, there is no harm making this
> function part of __init section, as this function will never be called
> again.

Oh, I meant it for code consistency at this moment.
No other functions declare that though.
Do we need to write another patch to add __init globally?

> Thanks.
> > Anyway, this patch looks much better to me.
> >
> > Jianpeng, how do you think about this?
> >
> > --
> > Jaegeuk Kim
> > Samsung
> >

-- 
Jaegeuk Kim
Samsung

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] f2fs:avoid creaing multi debugfs when mount more disks.
  2013-01-15 12:10         ` Jaegeuk Kim
@ 2013-01-15 22:52           ` Namjae Jeon
  0 siblings, 0 replies; 6+ messages in thread
From: Namjae Jeon @ 2013-01-15 22:52 UTC (permalink / raw)
  To: jaegeuk.kim; +Cc: majianpeng, linux-f2fs-devel, linux-fsdevel

2013/1/15, Jaegeuk Kim <jaegeuk.kim@samsung.com>:
> 2013-01-15 (화), 20:50 +0900, Namjae Jeon:
>> 2013/1/15, Jaegeuk Kim <jaegeuk.kim@samsung.com>:
>> > [snip]
>> >
>> >>  void f2fs_destroy_stats(struct f2fs_sb_info *sbi)
>> >>  {
>> >>  	struct f2fs_stat_info *si = sbi->stat_info;
>> >> @@ -354,7 +339,15 @@ void f2fs_destroy_stats(struct f2fs_sb_info *sbi)
>> >>  	kfree(sbi->stat_info);
>> >>  }
>> >>
>> >> -void destroy_root_stats(void)
>> >> +void __init f2fs_create_root_stats(void)
>> >
>> > We don't need to add __init here, right?
>> As you know, the function f2fs_create_root_stats will only be needed
>> at the init time of F2FS filesystem. So, there is no harm making this
>> function part of __init section, as this function will never be called
>> again.
>
> Oh, I meant it for code consistency at this moment.
> No other functions declare that though.
> Do we need to write another patch to add __init globally?
Yes, needed. I will send you another patch for __init today.

Thank you !
>
>> Thanks.
>> > Anyway, this patch looks much better to me.
>> >
>> > Jianpeng, how do you think about this?
>> >
>> > --
>> > Jaegeuk Kim
>> > Samsung
>> >
>
> --
> Jaegeuk Kim
> Samsung
>
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" 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] 6+ messages in thread

end of thread, other threads:[~2013-01-15 22:52 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <201301141929076532360@gmail.com>
2013-01-15  7:08 ` [PATCH] f2fs:avoid creaing multi debugfs when mount more disks Jaegeuk Kim
2013-01-15 10:58   ` Namjae Jeon
2013-01-15 11:29     ` Jaegeuk Kim
2013-01-15 11:50       ` Namjae Jeon
2013-01-15 12:10         ` Jaegeuk Kim
2013-01-15 22:52           ` Namjae Jeon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).