public inbox for netdev@vger.kernel.org 
 help / color / mirror / Atom feed

From: Ying Xue <ying.xue@windriver•com>
To: Thomas Graf <tgraf@suug•ch>
Cc: <davem@davemloft•net>, <eric.dumazet@gmail•com>,
	<netdev@vger•kernel.org>
Subject: Re: [PATCH net-next v3] lib/rhashtable: allow user to set the minimum shifts of shrinking
Date: Mon, 1 Sep 2014 17:28:37 +0800	[thread overview]
Message-ID: <54043C45.7090005@windriver.com> (raw)
In-Reply-To: <20140901090302.GB27933@casper.infradead.org>

On 09/01/2014 05:03 PM, Thomas Graf wrote:
> On 08/30/14 at 02:48pm, Ying Xue wrote:
>> Although rhashtable library allows user to specify a quiet big size
>> for user's created hash table, the table may be shrunk to a
>> very small size - HASH_MIN_SIZE(4) after object is removed from
>> the table at the first time. Subsequently, even if the total amount
>> of objects saved in the table is quite lower than user's initial
>> setting in a long time, the hash table size is still dynamically
>> adjusted by rhashtable_shrink() or rhashtable_expand() each time
>> object is inserted or removed from the table. However, as
>> synchronize_rcu() has to be called when table is shrunk or
>> expanded by the two functions, we should permit user to set the
>> minimum table size through configuring the minimum number of shifts
>> according to user specific requirement, avoiding these expensive
>> actions of shrinking or expanding because of calling synchronize_rcu().
>>
>> Signed-off-by: Ying Xue <ying.xue@windriver•com>
> 
> I see the following warning when compiling:
> 
> In file included from lib/rhashtable.c:17:0:
> lib/rhashtable.c: In function ‘rhashtable_init’:
> include/linux/kernel.h:715:17: warning: comparison of distinct pointer types lacks a cast [enabled by default]
>   (void) (&_max1 == &_max2);  \
>                  ^
> lib/rhashtable.c:570:22: note: in expansion of macro ‘max’
>   params->min_shift = max(params->min_shift, ilog2(HASH_MIN_SIZE));
> 
> 

Thanks for your check.

After I made below change, the compiling warning disappears.

+       params->min_shift = max(params->min_shift,
+                               (size_t)ilog2(HASH_MIN_SIZE));
+

But when I check the patch format with scripts/checkpatch.pl, below
warning occurs:

./scripts/checkpatch.pl
0001-lib-rhashtable-allow-user-to-set-the-minimum-shifts-.patch
WARNING: max() should probably be max_t(size_t, params->min_shift,
ilog2(HASH_MIN_SIZE))
#77: FILE: lib/rhashtable.c:570:
+	params->min_shift = max(params->min_shift,
 				(size_t)ilog2(HASH_MIN_SIZE));

total: 0 errors, 1 warnings, 46 lines checked

0001-lib-rhashtable-allow-user-to-set-the-minimum-shifts-.patch has
style problems, please review.

Do you think it's worth replacing max() macro with max_t() regarding
above suggestion?

By the way, if the replacement should do, all max() macro in the
lib/rhashtable.c should be replaced as well.

Thanks,
Ying

next prev parent reply	other threads:[~2014-09-01  9:29 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-08-30  6:48 [PATCH net-next v3] lib/rhashtable: allow user to set the minimum shifts of shrinking Ying Xue
2014-09-01  9:03 ` Thomas Graf
2014-09-01  9:28   ` Ying Xue [this message]
2014-09-01  9:41     ` Thomas Graf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=54043C45.7090005@windriver.com \
    --to=ying.xue@windriver$(echo .)com \
    --cc=davem@davemloft$(echo .)net \
    --cc=eric.dumazet@gmail$(echo .)com \
    --cc=netdev@vger$(echo .)kernel.org \
    --cc=tgraf@suug$(echo .)ch \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox