linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Sasha Levin <levinsasha928@gmail.com>
To: Tejun Heo <tj@kernel.org>
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,
	josh@joshtriplett.org, eric.dumazet@gmail.com,
	mathieu.desnoyers@efficios.com, axboe@kernel.dk, agk@redhat.com,
	dm-devel@redhat.com, neilb@suse.de, ccaulfie@redhat.com,
	teigland@redhat.com, Trond.Myklebust@netapp.com,
	bfields@fieldses.org, fweisbec@gmail.com, jesse@nicira.com,
	venkat.x.venkatsubra@oracle.com, ejt@redhat.com,
	snitzer@redhat.com, edumazet@google.com,
	linux-nfs@vger.kernel.org, dev@openvswitch.org,
	rds-devel@oss.oracle.com, lw@cn.fujitsu.com
Subject: Re: [PATCH v3 01/17] hashtable: introduce a small and naive hashtable
Date: Sat, 25 Aug 2012 00:59:25 +0200	[thread overview]
Message-ID: <5038074D.300@gmail.com> (raw)
In-Reply-To: <20120824212348.GK21325@google.com>

>> Why do we need hash_head/hash_for_each_head()? I haven't stumbled on a place yet
>> that needed direct access to the bucket itself.
> 
> Because whole hash table walking is much less common and we can avoid
> another full set of iterators.

I don't agree. Out of 32 places which now use a hashtable iterator of some kind,
12 of them (38%) walk the entire table.

The thing is that usually data structures are indexable by more than one key, so
usually hashtables are fully walked in cold paths to look for different keys.

Take kernel/workqueue.c for example: There are 4 places which do a key lookup
(find_worker_executing_work()) and 3 places which fully walk the entire table
(for_each_busy_worker()).

>> This basically means 11 macros/functions that would let us have full
>> encapsulation and will make it very easy for future implementations to work with
>> this API instead of making up a new one. It's also not significantly (+~2-3)
>> more than the ones you listed.
> 
> I'm not sure whether full encapsulation is a good idea for trivial
> hashtable.  For higher level stuff, sure but at this level I think
> benefits coming from known obvious implementation can be larger.
> e.g. suppose the caller knows certain entries to be way colder than
> others and wants to put them at the end of the chain.

Thats the thing, the amount of things of things you can do with a given bucket
is very limited. You can't add entries to any point besides the head (without
walking the entire list).

Basically you can do only two things with a bucket:

 - Add something to it at a very specific place.
 - Walk it

So I don't understand whats the point in exposing the internal structure of the
hashtable if there's nothing significant that can be gained from it by the user.

> 
> So, I think implmenting the minimal set of helpers which reflect the
> underlying trivial implementation explicitly could actually be better
> even when discounting the reduced number of wrappers.
> 
> Thanks.
> 

--
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-24 22:59 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-22  2:26 [PATCH v3 00/17] generic hashtable implementation Sasha Levin
2012-08-22  2:26 ` [PATCH v3 01/17] hashtable: introduce a small and naive hashtable Sasha Levin
2012-08-22 18:01   ` Tejun Heo
2012-08-22 23:54     ` Ryan Mallon
2012-08-23  0:24     ` Sasha Levin
2012-08-23 20:04       ` Tejun Heo
2012-08-24 19:47         ` Sasha Levin
2012-08-24 19:59           ` Tejun Heo
2012-08-24 20:11             ` Sasha Levin
2012-08-24 20:33               ` Tejun Heo
2012-08-24 20:53                 ` Sasha Levin
2012-08-24 21:23                   ` Tejun Heo
2012-08-24 22:59                     ` Sasha Levin [this message]
2012-08-24 23:07                       ` Tejun Heo
2012-08-25  4:24                         ` Mathieu Desnoyers
2012-08-28  9:56                           ` Sasha Levin
2012-08-28 10:11                             ` Mathieu Desnoyers
2012-08-28 11:27                               ` Sasha Levin
2012-08-28 11:56                                 ` Mathieu Desnoyers
2012-08-28 23:00                                   ` Mathieu Desnoyers
2012-09-04 15:35                                     ` Steven Rostedt
2012-09-04 16:30                                       ` Pedro Alves
2012-09-04 16:40                                         ` Pedro Alves
2012-09-04 17:01                                           ` Mathieu Desnoyers
2012-09-06 13:53                                             ` Sasha Levin
2012-09-06 14:19                                               ` Pedro Alves
2012-09-06 14:33                                               ` Mathieu Desnoyers
2012-09-06 14:36                                               ` David Laight
2012-09-06 14:55                                               ` Josh Triplett
2012-09-06 15:11                                                 ` Steven Rostedt
2012-09-06 15:49                                                 ` Sasha Levin
2012-09-06 16:00                                                   ` Steven Rostedt
2012-09-06 16:21                                                     ` Sasha Levin
2012-09-06 16:50                                                       ` Mathieu Desnoyers
2012-09-06 17:01                                                         ` Sasha Levin
2012-09-06 17:02                                                           ` Mathieu Desnoyers
2012-09-06 17:15                                                       ` Steven Rostedt
2012-09-04 17:17                                           ` Steven Rostedt
2012-09-04 17:21                                             ` Pedro Alves
2012-09-04 20:59                                               ` Steven Rostedt
2012-09-04 21:51                                                 ` Pedro Alves
2012-09-04 22:41                                                   ` Steven Rostedt
2012-09-04 22:58                                                     ` Pedro Alves
2012-09-04 23:27                                                       ` Steven Rostedt
2012-09-04 16:32                                       ` Mathieu Desnoyers
2012-08-22  2:26 ` [PATCH v3 02/17] userns: use new hashtable implementation Sasha Levin
2012-08-22  2:26 ` [PATCH v3 03/17] mm,ksm: " Sasha Levin
2012-08-22  2:26 ` [PATCH v3 04/17] workqueue: " Sasha Levin
2012-08-22 18:05   ` Tejun Heo
2012-08-22  2:27 ` [PATCH v3 05/17] mm/huge_memory: " Sasha Levin
2012-08-22  2:27 ` [PATCH v3 06/17] tracepoint: " Sasha Levin
2012-08-22  2:27 ` [PATCH v3 07/17] net,9p: " Sasha Levin
2012-08-22  2:27 ` [PATCH v3 08/17] block,elevator: " Sasha Levin
2012-08-22  2:27 ` [PATCH v3 09/17] SUNRPC/cache: " Sasha Levin
2012-08-22  2:27 ` [PATCH v3 10/17] dlm: " Sasha Levin
2012-08-22  2:27 ` [PATCH v3 11/17] net,l2tp: " Sasha Levin
2012-08-22  2:27 ` [PATCH v3 12/17] dm: " Sasha Levin
2012-08-22  2:27 ` [PATCH v3 13/17] lockd: " Sasha Levin
2012-08-22 11:47   ` J. Bruce Fields
2012-08-22 12:13     ` Sasha Levin
2012-08-22 13:12       ` J. Bruce Fields
2012-08-22 13:22       ` Mathieu Desnoyers
2012-08-22 17:32         ` Sasha Levin
2012-08-22  2:27 ` [PATCH v3 14/17] net,rds: " Sasha Levin
2012-08-22  2:27 ` [PATCH v3 15/17] openvswitch: " Sasha Levin
2012-08-22  2:27 ` [PATCH v3 16/17] tracing output: " Sasha Levin
2012-08-22  2:27 ` [PATCH v3 17/17] SUNRPC: use new hashtable implementation in auth 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=5038074D.300@gmail.com \
    --to=levinsasha928@gmail.com \
    --cc=Trond.Myklebust@netapp.com \
    --cc=aarcange@redhat.com \
    --cc=agk@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=axboe@kernel.dk \
    --cc=bfields@fieldses.org \
    --cc=ccaulfie@redhat.com \
    --cc=davem@davemloft.net \
    --cc=dev@openvswitch.org \
    --cc=dm-devel@redhat.com \
    --cc=ebiederm@xmission.com \
    --cc=edumazet@google.com \
    --cc=ejt@redhat.com \
    --cc=eric.dumazet@gmail.com \
    --cc=ericvh@gmail.com \
    --cc=fweisbec@gmail.com \
    --cc=jesse@nicira.com \
    --cc=josh@joshtriplett.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=lw@cn.fujitsu.com \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mingo@elte.hu \
    --cc=neilb@suse.de \
    --cc=netdev@vger.kernel.org \
    --cc=paul.gortmaker@windriver.com \
    --cc=rds-devel@oss.oracle.com \
    --cc=rostedt@goodmis.org \
    --cc=snitzer@redhat.com \
    --cc=teigland@redhat.com \
    --cc=tj@kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=venkat.x.venkatsubra@oracle.com \
    /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).