[PATCH] cgroup.c: Some 'hlist_head' function fixes.

[PATCH] cgroup.c: Some 'hlist_head' function fixes.

[Rakib Mullick]

Hello guys, the following patch emphasizes on two things:
	1. We can carry out the following function with one variable.
	2. As hash_long returns with unsigned long we need a unsigned long to
handle this.
If anything else, please notice.
Thanks.

Signed-off-by: Md.Rakib H. Mullick (rakib.mullick@gmail.com)

--- linux-2.6.27-rc2.orig/kernel/cgroup.c	2008-08-06 16:23:26.000000000 +0600
+++ linux-2.6.27-rc2/kernel/cgroup.c	2008-08-08 19:06:53.000000000 +0600
@@ -200,17 +200,16 @@ static struct hlist_head css_set_table[C

 static struct hlist_head *css_set_hash(struct cgroup_subsys_state *css[])
 {
-	int i;
-	int index;
-	unsigned long tmp = 0UL;
+	unsigned long tmp = 0UL , i;

 	for (i = 0; i < CGROUP_SUBSYS_COUNT; i++)
 		tmp += (unsigned long)css[i];
 	tmp = (tmp >> 16) ^ tmp;

-	index = hash_long(tmp, CSS_SET_HASH_BITS);
+	i = 0;
+	i = hash_long(tmp, CSS_SET_HASH_BITS);

-	return &css_set_table[index];
+	return &css_set_table[i];
 }

 /* We don't maintain the lists running through each css_set to its

Re: [PATCH] cgroup.c: Some 'hlist_head' function fixes.

[Paul Jackson]

Rabik wrote:
> +	i = 0;
> +	i = hash_long(tmp, CSS_SET_HASH_BITS);

That "i = 0;" looks unnecessary to me, given
that i is unconditionally set again, on the
very next line.

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@sgi.com> 1.940.382.4214

Re: [PATCH] cgroup.c: Some 'hlist_head' function fixes.

[Rakib Mullick]

On 8/12/08, Paul Jackson <pj@sgi.com> wrote:
> > Is it [text size] the only criteria to judge this patch ?
>
>  No - not the only criteria, as the patch combines a couple of
>  changes.
>
>  > What about the use of "unsigned long", instead of int.
>
>  I had missed that change, even though you had explicitly
>
> described it in your patch comment, when you wrote:
>
>   2. As hash_long returns with unsigned long we need a unsigned long
>
>
> How about just casting the hash_long() result to int:
>
>   index = (int)hash_long(tmp, CSS_SET_HASH_BITS);
 Yes, it looks good.
>
>  Since we are using this 'index' to index an array,
>  it had better fit in an 'int', which indeed it does
>  as CSS_SET_HASH_BITS is 7, which constrains the output
>  of hash_long to [0 .. 2^7-1], that is between 0 and 127.
>
>  However ... looking around the kernel, I see that most other
>  uses of hash_long(), except in cases where the second argument
>  (bit size) might actually exceed 32 bits, either directly
>  index some array with the result, or else assign the result
>  to a temporary 'int'.
>
>  And the compiler does not complain that we're assigning a
>  long to an int.
>
>  So ... what's the problem?
Yes, maybe your right. Ok, I'll go through the code again. If it's
good then I'm happy.
>
>  I see nothing in this patch of value.
>
>  Am I missing something?
>
>  -----
>
>  I just noticed that you had dropped the other recipients
>  from this email thread, a couple of replies ago.  My
>  preference would have been to have this discussion in
>  public.  I prefer not to drop people from CC lists on
>  email threads.
Yes, I've noticed it too. I just forgot to do that. Actually, when I
reply to thread, I just think about that one. This could be a reason
for missing.
Thanks.
>

>
>  --
>
>                   I won't rest till it's the best ...
>                   Programmer, Linux Scalability
>                   Paul Jackson <pj@sgi.com> 1.940.382.4214
>