linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Davidlohr Bueso <dave@stgolabs.net>
To: Waiman Long <longman@redhat.com>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>,
	Jan Kara <jack@suse.com>, Jeff Layton <jlayton@poochiereds.net>,
	"J. Bruce Fields" <bfields@fieldses.org>,
	Tejun Heo <tj@kernel.org>,
	Christoph Lameter <cl@linux-foundation.org>,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Andi Kleen <andi@firstfloor.org>,
	Dave Chinner <dchinner@redhat.com>,
	Boqun Feng <boqun.feng@gmail.com>
Subject: Re: [PATCH v7 5/6] lib/dlock-list: Enable faster lookup with hashing
Date: Mon, 9 Oct 2017 09:03:04 -0700	[thread overview]
Message-ID: <20171009160304.GD5131@linux-80c1.suse> (raw)
In-Reply-To: <95cbbab7-246d-fd42-23e3-19aa5bd8efe5@redhat.com>

On Mon, 09 Oct 2017, Waiman Long wrote:

>On 10/09/2017 09:08 AM, Davidlohr Bueso wrote:
>> On Thu, 05 Oct 2017, Waiman Long wrote:
>>
>>> This patch provides an alternative way of list selection. The caller
>>> can provide a object context which will be hashed to one of the list
>>> in a dlock-list. The object can then be added into that particular
>>> list. Lookup can be done by iterating elements in the provided list
>>> only instead of all the lists in a dlock-list.
>>
>> Unless I'm misusing the api, I could not find a standard way of
>> iterating a _particular_ list head (the one the dlock_list_hash()
>> returned). This is because iterators always want the all the heads.
>>
>> Also, in my particular epoll case I'd need the head->lock _not_ to
>> be dropped after the iteration, and therefore is pretty adhoc.
>> Currently we do:
>>
>> dlist_for_each_entry() {
>>     // acquire head->lock for each list
>> }
>> // no locks held
>> dlist_add()
>>
>> I'm thinking perhaps we could have dlist_check_add() which passes a
>> callback to ensure we want to add the node. The function could acquire
>> the head->lock and not release it until the very end.
>
>With the dlock_list_hash(), dlock-list is degenerated into a pool of
>list where one is chosen by hashing. So the regular list iteration
>macros like list_for_each_entry() can then be used. Of course, you have
>to explicitly do the lock and unlock operation.

Right, which seemed rather asymmetric and fragile, albeit adhoc to epoll.
Particularly not having the iter structure, makes use directly have to
deal with the spinlock, and there goes irq awareness out the window.

>I could also encapsulate it a bit with inlined function like

Probably not worth it, unless some other use cases came up. This just
bulks the api even more.

Thanks,
Davidlohr

  reply	other threads:[~2017-10-09 16:03 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-05 18:43 [PATCH v7 0/6] vfs: Use dlock list for SB's s_inodes list Waiman Long
2017-10-05 18:43 ` [PATCH v7 1/6] lib/dlock-list: Distributed and lock-protected lists Waiman Long
2017-10-10  5:35   ` Boqun Feng
2017-10-13 21:10     ` Waiman Long
2017-10-18  8:55   ` Boqun Feng
2017-10-05 18:43 ` [PATCH v7 2/6] vfs: Remove unnecessary list_for_each_entry_safe() variants Waiman Long
2017-10-05 18:43 ` [PATCH v7 3/6] vfs: Use dlock list for superblock's inode list Waiman Long
2017-10-05 18:43 ` [PATCH v7 4/6] lib/dlock-list: Make sibling CPUs share the same linked list Waiman Long
2017-10-09 15:40   ` Jan Kara
2017-10-09 16:14     ` Waiman Long
2017-10-05 18:43 ` [PATCH v7 5/6] lib/dlock-list: Enable faster lookup with hashing Waiman Long
2017-10-09 13:08   ` Davidlohr Bueso
2017-10-09 14:16     ` Waiman Long
2017-10-09 16:03       ` Davidlohr Bueso [this message]
2017-10-09 16:11         ` Waiman Long
2017-10-05 18:43 ` [PATCH v7 6/6] lib/dlock-list: Add an IRQ-safe mode to be used in interrupt handler Waiman Long
2017-10-13 15:45 ` [PATCH v7 7/6] fs/epoll: scale nested callbacks Davidlohr Bueso
2017-10-16 19:30   ` Jason Baron
2017-10-17 15:53     ` Davidlohr Bueso
2017-10-18 14:06       ` Jason Baron
2017-10-18 15:44         ` Davidlohr Bueso
2017-10-17 19:36 ` [PATCH v7 8/9] lib/dlock-list: Export symbols and add warnings Waiman Long
2017-10-17 19:36   ` [PATCH v7 9/9] lib/dlock-list: Unique lock class key for each allocation call site Waiman Long
2017-10-26 18:28 ` [PATCH v7 0/6] vfs: Use dlock list for SB's s_inodes list Waiman Long
2017-10-27  0:58   ` Boqun Feng
2017-10-27 20:19     ` Waiman Long
2017-10-27 20:10 ` [PATCH v7 10/10] lib/dlock-list: Fix use-after-unlock problem in dlist_for_each_entry_safe() Waiman Long
2017-10-30  9:06   ` Jan Kara
2017-10-30 14:06     ` Boqun Feng
2017-10-30 14:11   ` Davidlohr Bueso
2017-10-30 14:15     ` Waiman Long

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=20171009160304.GD5131@linux-80c1.suse \
    --to=dave@stgolabs.net \
    --cc=andi@firstfloor.org \
    --cc=bfields@fieldses.org \
    --cc=boqun.feng@gmail.com \
    --cc=cl@linux-foundation.org \
    --cc=dchinner@redhat.com \
    --cc=jack@suse.com \
    --cc=jlayton@poochiereds.net \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=longman@redhat.com \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=tj@kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /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).