All of lore.kernel.org
 help / color / mirror / Atom feed
From: Simo <idra-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
To: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org,
	matthew-Ztpu424NOJ8@public.gmane.org,
	dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
	sage-4GqslpFJ+cxBDgjK7y7TUQ@public.gmane.org,
	smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	swhiteho-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
	Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org,
	akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-afs-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	ceph-devel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-cifs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	samba-technical-w/Ol4Ecudpl8XjKLYN78aQ@public.gmane.org,
	cluster-devel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
Subject: Re: [PATCH v2 06/14] locks: don't walk inode->i_flock list in locks_show
Date: Sat, 15 Jun 2013 11:04:11 -0400	[thread overview]
Message-ID: <51BC826B.5060805@samba.org> (raw)
In-Reply-To: <20130615070535.6367eed9-9yPaYZwiELC+kQycOl6kW4xkIHaj4LzF@public.gmane.org>

On 06/15/2013 07:05 AM, Jeff Layton wrote:
> On Fri, 14 Jun 2013 07:52:44 -0400
> Simo <idra-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:
>
>> On 06/13/2013 04:26 PM, Jeff Layton wrote:
>>> The only real solution I can think of is to put flock locks into the
>>> blocked_list/blocked_hash too, or maybe giving them a simple hlist to
>>> sit on.
>>>
>>> I'll fix that up in the next iteration. It'll probably make flock()
>>> tests run slower, but such is the cost of preserving this procfile...
>> How hard would it be to make the procfile stuff optional ?
>> So that those that need performance can decide to not use it ?
>> Maybe even something that can be disabled at run time ? Not just compile
>> time.
>>
> (re-adding back the cc lists...)
>
> It'd be tricky, especially if you want to do it at runtime. The
> procfile itself is not a problem per-se. The real problem is the
> tracking you have to do in order to eventually present the procfile. So
> a boot-time or compile-time switch might be reasonable, but a runtime
> switch will probably never really be.

Just to be clear, I meant for a switch to turn it off at runtime, I 
understand very well that it would be way too hard to turn on at 
runtime. But killing the perf problem might be desirable on a system you 
cannot just reboot.

> I have a new patchset that I'm testing now though that should address
> Bruce's concerns about iterating over that global list. So far, it
> seems to be at least as fast as the latest patchset I posted.
>
> It makes the (spin)locking a bit more complex, but hopefully I can
> document this well enough that it's not a great concern.
>
> Stay tuned...

Thanks Jeff,
this is very valuable work.

Simo.

WARNING: multiple messages have this Message-ID (diff)
From: Simo <idra@samba.org>
To: cluster-devel.redhat.com
Subject: [Cluster-devel] [PATCH v2 06/14] locks: don't walk inode->i_flock list in locks_show
Date: Sat, 15 Jun 2013 11:04:11 -0400	[thread overview]
Message-ID: <51BC826B.5060805@samba.org> (raw)
In-Reply-To: <20130615070535.6367eed9@tlielax.poochiereds.net>

On 06/15/2013 07:05 AM, Jeff Layton wrote:
> On Fri, 14 Jun 2013 07:52:44 -0400
> Simo <idra@samba.org> wrote:
>
>> On 06/13/2013 04:26 PM, Jeff Layton wrote:
>>> The only real solution I can think of is to put flock locks into the
>>> blocked_list/blocked_hash too, or maybe giving them a simple hlist to
>>> sit on.
>>>
>>> I'll fix that up in the next iteration. It'll probably make flock()
>>> tests run slower, but such is the cost of preserving this procfile...
>> How hard would it be to make the procfile stuff optional ?
>> So that those that need performance can decide to not use it ?
>> Maybe even something that can be disabled at run time ? Not just compile
>> time.
>>
> (re-adding back the cc lists...)
>
> It'd be tricky, especially if you want to do it at runtime. The
> procfile itself is not a problem per-se. The real problem is the
> tracking you have to do in order to eventually present the procfile. So
> a boot-time or compile-time switch might be reasonable, but a runtime
> switch will probably never really be.

Just to be clear, I meant for a switch to turn it off at runtime, I 
understand very well that it would be way too hard to turn on at 
runtime. But killing the perf problem might be desirable on a system you 
cannot just reboot.

> I have a new patchset that I'm testing now though that should address
> Bruce's concerns about iterating over that global list. So far, it
> seems to be at least as fast as the latest patchset I posted.
>
> It makes the (spin)locking a bit more complex, but hopefully I can
> document this well enough that it's not a great concern.
>
> Stay tuned...

Thanks Jeff,
this is very valuable work.

Simo.



WARNING: multiple messages have this Message-ID (diff)
From: Simo <idra@samba.org>
To: Jeff Layton <jlayton@redhat.com>
Cc: viro@zeniv.linux.org.uk, matthew@wil.cx, dhowells@redhat.com,
	sage@inktank.com, smfrench@gmail.com, swhiteho@redhat.com,
	Trond.Myklebust@netapp.com, akpm@linux-foundation.org,
	linux-kernel@vger.kernel.org, linux-afs@lists.infradead.org,
	ceph-devel@vger.kernel.org, linux-cifs@vger.kernel.org,
	samba-technical@lists.samba.org, cluster-devel@redhat.com,
	linux-nfs@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	piastryyy@gmail.com
Subject: Re: [PATCH v2 06/14] locks: don't walk inode->i_flock list in locks_show
Date: Sat, 15 Jun 2013 11:04:11 -0400	[thread overview]
Message-ID: <51BC826B.5060805@samba.org> (raw)
In-Reply-To: <20130615070535.6367eed9@tlielax.poochiereds.net>

On 06/15/2013 07:05 AM, Jeff Layton wrote:
> On Fri, 14 Jun 2013 07:52:44 -0400
> Simo <idra@samba.org> wrote:
>
>> On 06/13/2013 04:26 PM, Jeff Layton wrote:
>>> The only real solution I can think of is to put flock locks into the
>>> blocked_list/blocked_hash too, or maybe giving them a simple hlist to
>>> sit on.
>>>
>>> I'll fix that up in the next iteration. It'll probably make flock()
>>> tests run slower, but such is the cost of preserving this procfile...
>> How hard would it be to make the procfile stuff optional ?
>> So that those that need performance can decide to not use it ?
>> Maybe even something that can be disabled at run time ? Not just compile
>> time.
>>
> (re-adding back the cc lists...)
>
> It'd be tricky, especially if you want to do it at runtime. The
> procfile itself is not a problem per-se. The real problem is the
> tracking you have to do in order to eventually present the procfile. So
> a boot-time or compile-time switch might be reasonable, but a runtime
> switch will probably never really be.

Just to be clear, I meant for a switch to turn it off at runtime, I 
understand very well that it would be way too hard to turn on at 
runtime. But killing the perf problem might be desirable on a system you 
cannot just reboot.

> I have a new patchset that I'm testing now though that should address
> Bruce's concerns about iterating over that global list. So far, it
> seems to be at least as fast as the latest patchset I posted.
>
> It makes the (spin)locking a bit more complex, but hopefully I can
> document this well enough that it's not a great concern.
>
> Stay tuned...

Thanks Jeff,
this is very valuable work.

Simo.


  parent reply	other threads:[~2013-06-15 15:04 UTC|newest]

Thread overview: 82+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-11 11:08 [PATCH v2 00/14] locks: scalability improvements for file locking Jeff Layton
2013-06-11 11:08 ` Jeff Layton
2013-06-11 11:08 ` [Cluster-devel] " Jeff Layton
     [not found] ` <1370948948-31784-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2013-06-11 11:08   ` [PATCH v2 01/14] cifs: use posix_unblock_lock instead of locks_delete_block Jeff Layton
2013-06-11 11:08     ` Jeff Layton
2013-06-11 11:08     ` [Cluster-devel] " Jeff Layton
2013-06-11 11:09   ` [PATCH v2 06/14] locks: don't walk inode->i_flock list in locks_show Jeff Layton
2013-06-11 11:09     ` Jeff Layton
2013-06-11 11:09     ` [Cluster-devel] " Jeff Layton
2013-06-13 19:45     ` J. Bruce Fields
2013-06-13 19:45       ` [Cluster-devel] " J. Bruce Fields
2013-06-13 20:26       ` Jeff Layton
2013-06-13 20:26         ` [Cluster-devel] " Jeff Layton
     [not found]         ` <51BB040C.3050101@samba.org>
2013-06-15 11:05           ` Jeff Layton
2013-06-15 11:05             ` [Cluster-devel] " Jeff Layton
     [not found]             ` <20130615070535.6367eed9-9yPaYZwiELC+kQycOl6kW4xkIHaj4LzF@public.gmane.org>
2013-06-15 15:04               ` Simo [this message]
2013-06-15 15:04                 ` Simo
2013-06-15 15:04                 ` [Cluster-devel] " Simo
2013-06-11 11:09   ` [PATCH v2 12/14] locks: give the blocked_hash its own spinlock Jeff Layton
2013-06-11 11:09     ` Jeff Layton
2013-06-11 11:09     ` [Cluster-devel] " Jeff Layton
     [not found]     ` <1370948948-31784-13-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2013-06-13 15:02       ` J. Bruce Fields
2013-06-13 15:02         ` J. Bruce Fields
2013-06-13 15:02         ` [Cluster-devel] " J. Bruce Fields
2013-06-13 15:18         ` Jeff Layton
2013-06-13 15:18           ` [Cluster-devel] " Jeff Layton
     [not found]           ` <20130613111844.59421622-4QP7MXygkU+dMjc06nkz3ljfA9RmPOcC@public.gmane.org>
2013-06-13 15:20             ` J. Bruce Fields
2013-06-13 15:20               ` J. Bruce Fields
2013-06-13 15:20               ` [Cluster-devel] " J. Bruce Fields
2013-06-11 16:04   ` [PATCH v2 00/14] locks: scalability improvements for file locking J. Bruce Fields
2013-06-11 16:04     ` J. Bruce Fields
2013-06-11 16:04     ` [Cluster-devel] " J. Bruce Fields
2013-06-11 16:35     ` Jeff Layton
2013-06-11 16:35       ` Jeff Layton
2013-06-11 16:35       ` [Cluster-devel] " Jeff Layton
2013-06-11 11:08 ` [PATCH v2 02/14] locks: make generic_add_lease and generic_delete_lease static Jeff Layton
2013-06-11 11:08   ` Jeff Layton
2013-06-11 11:08   ` [Cluster-devel] " Jeff Layton
2013-06-11 11:08 ` [PATCH v2 03/14] locks: comment cleanups and clarifications Jeff Layton
2013-06-11 11:08   ` Jeff Layton
2013-06-11 11:08   ` [Cluster-devel] " Jeff Layton
2013-06-11 11:08 ` [PATCH v2 04/14] locks: make "added" in __posix_lock_file a bool Jeff Layton
2013-06-11 11:08   ` Jeff Layton
2013-06-11 11:08   ` [Cluster-devel] " Jeff Layton
2013-06-11 11:08 ` [PATCH v2 05/14] locks: encapsulate the fl_link list handling Jeff Layton
2013-06-11 11:08   ` Jeff Layton
2013-06-11 11:08   ` [Cluster-devel] " Jeff Layton
2013-06-11 11:09 ` [PATCH v2 07/14] locks: convert to i_lock to protect i_flock list Jeff Layton
2013-06-11 11:09   ` Jeff Layton
2013-06-11 11:09   ` [Cluster-devel] " Jeff Layton
2013-06-13 14:41   ` J. Bruce Fields
2013-06-13 14:41     ` [Cluster-devel] " J. Bruce Fields
2013-06-13 15:09     ` Jeff Layton
2013-06-13 15:09       ` [Cluster-devel] " Jeff Layton
2013-06-11 11:09 ` [PATCH v2 08/14] locks: ensure that deadlock detection is atomic with respect to blocked_list modification Jeff Layton
2013-06-11 11:09   ` Jeff Layton
2013-06-11 11:09   ` [Cluster-devel] " Jeff Layton
2013-06-11 11:09 ` [PATCH v2 09/14] locks: convert fl_link to a hlist_node Jeff Layton
2013-06-11 11:09   ` Jeff Layton
2013-06-11 11:09   ` [Cluster-devel] " Jeff Layton
2013-06-11 11:09 ` [PATCH v2 10/14] locks: turn the blocked_list into a hashtable Jeff Layton
2013-06-11 11:09   ` Jeff Layton
2013-06-11 11:09   ` [Cluster-devel] " Jeff Layton
2013-06-13 14:50   ` J. Bruce Fields
2013-06-13 14:50     ` [Cluster-devel] " J. Bruce Fields
2013-06-11 11:09 ` [PATCH v2 11/14] locks: add a new "lm_owner_key" lock operation Jeff Layton
2013-06-11 11:09   ` Jeff Layton
2013-06-11 11:09   ` [Cluster-devel] " Jeff Layton
     [not found]   ` <1370948948-31784-12-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2013-06-13 15:00     ` J. Bruce Fields
2013-06-13 15:00       ` J. Bruce Fields
2013-06-13 15:00       ` [Cluster-devel] " J. Bruce Fields
2013-06-11 11:09 ` [PATCH v2 13/14] seq_file: add seq_list_*_percpu helpers Jeff Layton
2013-06-11 11:09   ` [Cluster-devel] " Jeff Layton
     [not found]   ` <1370948948-31784-14-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2013-06-13 15:27     ` J. Bruce Fields
2013-06-13 15:27       ` J. Bruce Fields
2013-06-13 15:27       ` [Cluster-devel] " J. Bruce Fields
2013-06-11 11:09 ` [PATCH v2 14/14] locks: move file_lock_list to a set of percpu hlist_heads and convert file_lock_lock to an lglock Jeff Layton
2013-06-11 11:09   ` Jeff Layton
2013-06-11 11:09   ` [Cluster-devel] " Jeff Layton
     [not found]   ` <1370948948-31784-15-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2013-06-13 15:37     ` J. Bruce Fields
2013-06-13 15:37       ` J. Bruce Fields
2013-06-13 15:37       ` [Cluster-devel] " J. Bruce Fields

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=51BC826B.5060805@samba.org \
    --to=idra-eunubhrolfbytjvyw6ydsg@public.gmane.org \
    --cc=Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org \
    --cc=akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org \
    --cc=ceph-devel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=cluster-devel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
    --cc=dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
    --cc=jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
    --cc=linux-afs-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \
    --cc=linux-cifs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=matthew-Ztpu424NOJ8@public.gmane.org \
    --cc=piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=sage-4GqslpFJ+cxBDgjK7y7TUQ@public.gmane.org \
    --cc=samba-technical-w/Ol4Ecudpl8XjKLYN78aQ@public.gmane.org \
    --cc=smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=swhiteho-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
    --cc=viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.