linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: Sasha Levin <levinsasha928@gmail.com>
Cc: torvalds@linux-foundation.org, akpm@linux-foundation.org,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	paul.gortmaker@windriver.com, davem@davemloft.net,
	rostedt@goodmis.org, mingo@elte.hu, ebiederm@xmission.com,
	aarcange@redhat.com, ericvh@gmail.com, netdev@vger.kernel.org
Subject: Re: [RFC v2 1/7] hashtable: introduce a small and naive hashtable
Date: Fri, 3 Aug 2012 14:44:14 -0700	[thread overview]
Message-ID: <20120803214414.GL15477@google.com> (raw)
In-Reply-To: <501C4471.4090706@gmail.com>

Hello, Sasha.

On Fri, Aug 03, 2012 at 11:36:49PM +0200, Sasha Levin wrote:
> On 08/03/2012 11:30 PM, Tejun Heo wrote:
> The function definition itself is just a macro, for example:
> 
> 	#define MM_SLOTS_HASH_CMP(mm_slot, obj) ((mm_slot)->mm == (obj))

It seems like it would make things more difficult to follow and
error-prone.  I'd definitely prefer just using functions.

> As an alternative, what do you think about simplifying that to be
> just a 'cond' instead of a function? Something like:
> 
> 	hash_get(&mm_slots_hash, mm, struct mm_slot, hash, mm);
> 
> In that case, the last param ("mm") will get unrolled to a condition like this:
> 
> 	if ((obj)->mm == key)
> 
> Which will be simple and easy for the user.

It seems a bit too magical(tm) to me. ;)

> The only reason I want to keep this interface is that most cases
> I've stumbled so far were easy short comparisons of a struct member
> with the key, and I don't want to make them more complex than they
> need to be. I probably will switch hash_get() to use
> hash_for_each_possible() as well, which will cut down on how
> hash_get() is a separate case.

I can understand that but I think the benefit we're talking about is a
bit too miniscule to matter and to have two different interfaces.
What do others think?

Thanks.

-- 
tejun

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2012-08-03 21:44 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-03 14:23 [RFC v2 0/7] generic hashtable implementation Sasha Levin
2012-08-03 14:23 ` [RFC v2 1/7] hashtable: introduce a small and naive hashtable Sasha Levin
2012-08-03 17:15   ` Tejun Heo
2012-08-03 17:16     ` Tejun Heo
2012-08-03 21:19     ` Sasha Levin
2012-08-03 21:30       ` Tejun Heo
2012-08-03 21:36         ` Sasha Levin
2012-08-03 21:44           ` Tejun Heo [this message]
2012-08-03 21:41         ` Sasha Levin
2012-08-03 21:48           ` Tejun Heo
2012-08-03 22:20             ` Sasha Levin
2012-08-03 22:23               ` Tejun Heo
2012-08-03 22:26                 ` Sasha Levin
2012-08-03 22:29                 ` Linus Torvalds
2012-08-03 22:36                   ` Tejun Heo
2012-08-03 23:47                     ` Linus Torvalds
2012-08-04  0:03                       ` Sasha Levin
2012-08-04  0:05                         ` Linus Torvalds
2012-08-04  0:33                           ` Sasha Levin
2012-08-04  0:05                       ` Tejun Heo
2012-08-03 17:39   ` Eric Dumazet
2012-08-03 14:23 ` [RFC v2 2/7] user_ns: use new hashtable implementation Sasha Levin
2012-08-05  0:58   ` Eric W. Biederman
2012-08-03 14:23 ` [RFC v2 3/7] mm,ksm: " Sasha Levin
2012-08-03 14:23 ` [RFC v2 4/7] workqueue: " Sasha Levin
2012-08-03 14:23 ` [RFC v2 5/7] mm/huge_memory: " Sasha Levin
2012-08-03 14:23 ` [RFC v2 6/7] tracepoint: " Sasha Levin
2012-08-05  0:36   ` Steven Rostedt
2012-08-05 16:31     ` Mathieu Desnoyers
2012-08-05 17:03       ` Sasha Levin
2012-08-05 17:12         ` Mathieu Desnoyers
2012-08-03 14:23 ` [RFC v2 7/7] net,9p: " Sasha Levin
2012-08-03 18:00   ` Eric Dumazet
2012-08-03 21:14     ` Sasha Levin

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=20120803214414.GL15477@google.com \
    --to=tj@kernel.org \
    --cc=aarcange@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=davem@davemloft.net \
    --cc=ebiederm@xmission.com \
    --cc=ericvh@gmail.com \
    --cc=levinsasha928@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mingo@elte.hu \
    --cc=netdev@vger.kernel.org \
    --cc=paul.gortmaker@windriver.com \
    --cc=rostedt@goodmis.org \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

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

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).