lkml.org 
[lkml]   [2016]   [Jun]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: performance delta after VFS i_mutex=>i_rwsem conversion
    On Tue, Jun 07, 2016 at 01:40:58AM +0100, Al Viro wrote:
    > On Mon, Jun 06, 2016 at 04:50:59PM -0700, Linus Torvalds wrote:
    > >
    > >
    > > On Mon, 6 Jun 2016, Al Viro wrote:
    > > >
    > > > True in general, but here we really do a lot under that ->d_lock - all
    > > > list traversals are under it. So I suspect that contention on nested
    > > > lock is not an issue in that particular load. It's certainly a separate
    > > > commit, so we'll see how much does it give on its own, but I doubt that
    > > > it'll be anywhere near enough.
    > >
    > > Hmm. Maybe. 
    > >
    > > But at least we can try to minimize everything that happens under the
    > > dentry->d_lock spinlock. 
    > >
    > > So how about this patch? It's entirely untested, but it rewrites that
    > > readdir() function to try to do the minimum possible under the d_lock
    > > spinlock.
    > >
    > > I say "rewrite", because it really is totally different. It's not just
    > > that the nested "next" locking is gone, it also treats the cursor very
    > > differently and tries to avoid doing any unnecessary cursor list
    > > operations.
    >
    > Similar to what I've got here, except that mine has a couple of helper
    > functions usable in dcache_dir_lseek() as well:
    >
    > next_positive(parent, child, n) - returns nth positive child after that one
    > or NULL if there's less than n such. NULL as the second argument => search
    > from the beginning.
    >
    > move_cursor(cursor, child) - moves cursor immediately past child *or* to
    > the very end if child is NULL.
    >
    > The third commit in series will be the lockless replacement for
    > for next_positive(). move_cursor() is easy - it became simply
    > struct dentry *parent = cursor->d_parent;
    > unsigned n, *seq = &parent->d_inode->i_dir_seq;
    > spin_lock(&parent->d_lock);
    > for (;;) {
    > n = *seq;
    > if (!(n & 1) && cmpxchg(seq, n, n + 1) == n)
    > break;
    > cpu_relax();
    > }
    > __list_del(cursor->d_child.prev, cursor->d_child.next);
    > if (child)
    > list_add(&cursor->d_child, &child->d_child);
    > else
    > list_add_tail(&cursor->d_child, &parent->d_subdirs);
    > smp_store_release(seq, n + 2);
    > spin_unlock(&parent->d_lock);
    >
    > with
    >
    > static struct dentry *next_positive(struct dentry *parent,
    > struct dentry *child, int count)
    > {
    > struct list_head *p = child ? &child->d_child : &parent->d_subdirs;
    > unsigned *seq = &parent->d_inode->i_dir_seq, n;
    > do {
    > int i = count;
    > n = smp_load_acquire(seq) & ~1;
    > rcu_read_lock();
    > do {
    > p = p->next;
    > if (p == &parent->d_subdirs) {
    > child = NULL;
    > break;
    > }
    > child = list_entry(p, struct dentry, d_child);
    > } while (!simple_positive(child) || --i);
    > rcu_read_unlock();
    > } while (unlikely(smp_load_acquire(seq) != n));
    > return child;
    > }
    > as initial attempt at lockless next_positive(); barriers are probably wrong,
    > though...

    FWIW,
    loff_t dcache_dir_lseek(struct file *file, loff_t offset, int whence)
    {
    struct dentry *dentry = file->f_path.dentry;
    switch (whence) {
    case 1:
    offset += file->f_pos;
    case 0:
    if (offset >= 0)
    break;
    default:
    return -EINVAL;
    }
    if (offset != file->f_pos) {
    file->f_pos = offset;
    if (file->f_pos >= 2) {
    struct dentry *cursor = file->private_data;
    loff_t n = file->f_pos - 2;

    inode_lock_shared(dentry->d_inode);
    move_cursor(cursor, next_positive(dentry, NULL, n));
    inode_unlock_shared(dentry->d_inode);
    }
    }
    return offset;
    }

    and
    int dcache_readdir(struct file *file, struct dir_context *ctx)
    {
    struct dentry *dentry = file->f_path.dentry;
    struct dentry *cursor = file->private_data;
    struct dentry *child, *next;
    bool moved = false;

    if (!dir_emit_dots(file, ctx))
    return 0;

    child = ctx->pos != 2 ? cursor : NULL;
    while ((next = next_positive(dentry, child, 1)) != NULL) {
    if (!dir_emit(ctx, next->d_name.name, next->d_name.len,
    d_inode(next)->i_ino, dt_type(d_inode(next))))
    break;
    moved = true;
    child = next;
    ctx->pos++;
    }
    if (moved)
    move_cursor(cursor, child);
    return 0;
    }

    is what the methods themselves became.

    \
     
     \ /
      Last update: 2016-06-07 03:01    [W:3.415 / U:1.964 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site