linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Waiman Long <longman@redhat.com>
To: Oleg Nesterov <oleg@redhat.com>
Cc: Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	Davidlohr Bueso <dave@stgolabs.net>,
	"Theodore Y. Ts'o" <tytso@mit.edu>,
	Amir Goldstein <amir73il@gmail.com>, Jan Kara <jack@suse.cz>,
	Matthew Wilcox <willy@infradead.org>
Subject: Re: [PATCH v4 1/2] locking/rwsem: Add a new RWSEM_ANONYMOUSLY_OWNED flag
Date: Wed, 16 May 2018 09:11:59 -0400	[thread overview]
Message-ID: <3481c82e-81b2-3a33-48a6-4b0d46c0bd71@redhat.com> (raw)
In-Reply-To: <20180516104829.GA24332@redhat.com>

On 05/16/2018 06:48 AM, Oleg Nesterov wrote:
> On 05/15, Waiman Long wrote:
>> There are use cases where a rwsem can be acquired by one task, but
>> released by another task. In thess cases, optimistic spinning may need
>> to be disabled.  One example will be the filesystem freeze/thaw code
> You do not read my emails ;)
>
> Let me repeat once again that in this particular case the writer will
> never spin because of owner == NULL. freeze_super() checks SB_UNFROZEN
> under sb->s_umount and only then calls sb_wait_write(). IOW, sb_wait_write()
> can only be called when this rwsem was already released by the previous
> writer.
>
> I am not arguing with this change, percpu_rwsem_release/acquire may have
> another user sometime, but the changelog is not accurate.

I know the change may not be necessary in this particular case, but it
is a correctness issue. Optimistic spinning should be disabled when the
exact time delay between percpu_rwsem_release() and
percpu_rwsem_acquire() is indeterminate even though no one is supposed
to spin on the rwsem during that time.

If we don't do that now, we may forget this issue when some other use
cases show up or we extend rwsem to do reader optimistic spinning, for
instance. So it is better to address that now than debugging the same
issue again in the future.

Cheers,
Longman

  parent reply	other threads:[~2018-05-16 13:12 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-15 21:49 [PATCH v4 0/2] locking/rwsem: Fix DEBUG_RWSEM warning from thaw_super() Waiman Long
2018-05-15 21:49 ` [PATCH v4 1/2] locking/rwsem: Add a new RWSEM_ANONYMOUSLY_OWNED flag Waiman Long
2018-05-16 10:48   ` Oleg Nesterov
2018-05-16 11:59     ` Peter Zijlstra
2018-05-16 13:11     ` Waiman Long [this message]
2018-05-16 15:27       ` Oleg Nesterov
2018-05-16 12:19   ` Matthew Wilcox
2018-05-18  7:02     ` Ingo Molnar
2018-05-18  8:41       ` Oleg Nesterov
2018-05-18  9:40         ` Ingo Molnar
2018-05-18 16:55           ` [PATCH] locking/rwsem: simplify the is-owner-spinnable checks Oleg Nesterov
2018-05-18 17:00             ` Waiman Long
2018-05-15 21:49 ` [PATCH v4 2/2] locking/percpu-rwsem: Annotate rwsem ownership transfer by setting RWSEM_OWNER_UNKNOWN Waiman Long
2018-05-16  5:37   ` Amir Goldstein
2018-05-16 13:17     ` 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=3481c82e-81b2-3a33-48a6-4b0d46c0bd71@redhat.com \
    --to=longman@redhat.com \
    --cc=amir73il@gmail.com \
    --cc=dave@stgolabs.net \
    --cc=jack@suse.cz \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=oleg@redhat.com \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=tytso@mit.edu \
    --cc=willy@infradead.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).