[PATCH v2] ext4: group info caches set to SLAB_MEM_SPREAD flags.

[PATCH v2] ext4: group info caches set to SLAB_MEM_SPREAD flags.

[Namjae Jeon]

I try to set to SLAB_MEM_SPREAD flags in groups info caches accoding to http://lwn.net/Articles/173654/.
And other filesystems have already set to this flags when using slab caches in fs.
I believe that it is useful by original Paul jackson's patch and other fs is currently using this flags.
Ted's opinion is that theoretically it would be sound to have that flag set with groups info slab cache.

Signed-off-by: Namjae Jeon <linkinjeon@gmail.com>
Signed-off-by: Amit Sahrawat <amit.sahrawat83@gmail.com>
---
 fs/ext4/mballoc.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index e2d8be8..7aacbbe 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -2407,7 +2407,8 @@ static int ext4_groupinfo_create_slab(size_t size)
 				bb_counters[blocksize_bits + 2]);
 
 	cachep = kmem_cache_create(ext4_groupinfo_slab_names[cache_index],
-					slab_size, 0, SLAB_RECLAIM_ACCOUNT,
+					slab_size, 0, SLAB_RECLAIM_ACCOUNT |
+					SLAB_MEM_SPREAD,
 					NULL);
 
 	ext4_groupinfo_caches[cache_index] = cachep;
-- 
1.7.4.4

Re: [PATCH v2] ext4: group info caches set to SLAB_MEM_SPREAD flags.

[NamJae Jeon]

2011/11/20 Namjae Jeon <linkinjeon@gmail.com>:
> I try to set to SLAB_MEM_SPREAD flags in groups info caches accoding to http://lwn.net/Articles/173654/.
> And other filesystems have already set to this flags when using slab caches in fs.
> I believe that it is useful by original Paul jackson's patch and other fs is currently using this flags.
> Ted's opinion is that theoretically it would be sound to have that flag set with groups info slab cache.
>
> Signed-off-by: Namjae Jeon <linkinjeon@gmail.com>
> Signed-off-by: Amit Sahrawat <amit.sahrawat83@gmail.com>
> ---
>  fs/ext4/mballoc.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index e2d8be8..7aacbbe 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -2407,7 +2407,8 @@ static int ext4_groupinfo_create_slab(size_t size)
>                                bb_counters[blocksize_bits + 2]);
>
>        cachep = kmem_cache_create(ext4_groupinfo_slab_names[cache_index],
> -                                       slab_size, 0, SLAB_RECLAIM_ACCOUNT,
> +                                       slab_size, 0, SLAB_RECLAIM_ACCOUNT |
> +                                       SLAB_MEM_SPREAD,
>                                        NULL);
>
>        ext4_groupinfo_caches[cache_index] = cachep;
> --
> 1.7.4.4
>
>
Hi. Ted.
This patch is not really meaningful ? or you need to see performance
data although cpuset mem spread flags patch is proved before.
I am waiting for your decision.
Thanks.

Re: [PATCH v2] ext4: group info caches set to SLAB_MEM_SPREAD flags.

[Ted Ts'o]

On Sun, Nov 20, 2011 at 02:45:32PM +0900, NamJae Jeon wrote:
> This patch is not really meaningful ? or you need to see performance
> data although cpuset mem spread flags patch is proved before.
> I am waiting for your decision.

I don't think it's really going to make much of a difference, to be
honest.  It's probably technically a little better, but at the end of
the day, it's not really worth the tiny amount of effort to process
the patch.

I put it in the same category as people who like to remove whitespaces
from files or remove unused variables.  It marginally makes the code a
tiny amount better, but at the moment, review bandwidth in the ext4
subsystem is a critically tight resource, and in terms of the grand
scheme of things, there are higher priority things that I need to
worry about, as opposed to people constantly resending trivial patches
and demanding a decision.

So let me ask a counter question --- why are you sending these
patches?  Especially when you don't even have a system that would
benefit from such changes?  Is it just ego to have your name in the
kernel, or to be counted amongst kernel developers?  Is this a warm up
because you'd like to do more substantive work in ext4, or other parts
of the kernel?  Is this a University exercise?  And if you're from a
company, which company, and what's your interest in ext4?  I
understand how Whamcloud uses ext4, and how the Android folks are
using ext4, and to a lesser extent how Tao Bao is using ext4, and that
helps me understand where they are going with ext4 and why they submit
the kind of patches that they do.

But I don't understand why *you* are submitting these sorts of
patches.  If there's substantive work that you plan to do with ext4,
then the investment I might make in helping you become more proficient
ext4 developers is quite different from say, the sort of attention I
might give someone who is working on ext4 for a class project or for
ego reasons.

Best regards,

						- Ted

Re: [PATCH v2] ext4: group info caches set to SLAB_MEM_SPREAD flags.

[Amit Sahrawat]

Hi Ted,

First of all extremely sorry for causing you inconvenience and making
you use of your time to review such trivial patches.

Yes, we are part of some company and we were looking for Ext4 as the
next big thing and looking forward to adopting that. We really need
complete understanding of Ext4 to stand out and gain confidence
ourselves first. We started with the initial anlalysis of Ext4 and
side-by-side looking at the source code.
As far as submitting patch is concerned - yes, It was part of our
learning curve - that we started the comparision.

In the past, I have literally focussed on our runtime environment and
raised issues. This was the firsttime - that there was something we
saw for code submission(may be just out of our childish curiosity).

I will make sure that there is no more inconvenience of that sort.

Thanks & Regards,
Amit Sahrawat


On Mon, Nov 21, 2011 at 12:08 AM, Ted Ts'o <tytso@mit.edu> wrote:
> On Sun, Nov 20, 2011 at 02:45:32PM +0900, NamJae Jeon wrote:
>> This patch is not really meaningful ? or you need to see performance
>> data although cpuset mem spread flags patch is proved before.
>> I am waiting for your decision.
>
> I don't think it's really going to make much of a difference, to be
> honest.  It's probably technically a little better, but at the end of
> the day, it's not really worth the tiny amount of effort to process
> the patch.
>
> I put it in the same category as people who like to remove whitespaces
> from files or remove unused variables.  It marginally makes the code a
> tiny amount better, but at the moment, review bandwidth in the ext4
> subsystem is a critically tight resource, and in terms of the grand
> scheme of things, there are higher priority things that I need to
> worry about, as opposed to people constantly resending trivial patches
> and demanding a decision.
>
> So let me ask a counter question --- why are you sending these
> patches?  Especially when you don't even have a system that would
> benefit from such changes?  Is it just ego to have your name in the
> kernel, or to be counted amongst kernel developers?  Is this a warm up
> because you'd like to do more substantive work in ext4, or other parts
> of the kernel?  Is this a University exercise?  And if you're from a
> company, which company, and what's your interest in ext4?  I
> understand how Whamcloud uses ext4, and how the Android folks are
> using ext4, and to a lesser extent how Tao Bao is using ext4, and that
> helps me understand where they are going with ext4 and why they submit
> the kind of patches that they do.
>
> But I don't understand why *you* are submitting these sorts of
> patches.  If there's substantive work that you plan to do with ext4,
> then the investment I might make in helping you become more proficient
> ext4 developers is quite different from say, the sort of attention I
> might give someone who is working on ext4 for a class project or for
> ego reasons.
>
> Best regards,
>
>                                                - Ted
>