All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: peterz@infradead.org
Cc: Hou Tao <houtao1@huawei.com>, Ingo Molnar <mingo@redhat.com>,
	Will Deacon <will@kernel.org>, Dennis Zhou <dennis@kernel.org>,
	Tejun Heo <tj@kernel.org>, Christoph Lameter <cl@linux.com>,
	linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org
Subject: Re: [RFC PATCH] locking/percpu-rwsem: use this_cpu_{inc|dec}() for read_count
Date: Tue, 15 Sep 2020 17:31:14 +0200	[thread overview]
Message-ID: <20200915153113.GA6881@redhat.com> (raw)
In-Reply-To: <20200915150610.GC2674@hirez.programming.kicks-ass.net>

On 09/15, Peter Zijlstra wrote:
>
> On Tue, Sep 15, 2020 at 10:07:50PM +0800, Hou Tao wrote:
> > Under aarch64, __this_cpu_inc() is neither IRQ-safe nor atomic, so
> > when percpu_up_read() is invoked under IRQ-context (e.g. aio completion),
> > and it interrupts the process on the same CPU which is invoking
> > percpu_down_read(), the decreasement on read_count may lost and
> > the final value of read_count on the CPU will be unexpected
> > as shown below:
>
> > Fixing it by using the IRQ-safe helper this_cpu_inc|dec() for
> > operations on read_count.
> >
> > Another plausible fix is to state that percpu-rwsem can NOT be
> > used under IRQ context and convert all users which may
> > use it under IRQ context.
>
> *groan*...
>
> So yeah, fs/super totally abuses percpu_rwsem, and yes, using it from
> IRQ context is totally out of spec. That said, we've (grudgingly)
> accomodated them before.

Yes, I didn't expect percpu_up_ can be called from IRQ :/

> This seems to be a fairly long standing issue, and certainly not unique
> to ARM64 either (Power, and anyone else using asm-gemeric/percpu.h,
> should be similarly affected I think). The issue seems to stem from
> Oleg's original rewrite:
>
>   a1fd3e24d8a4 ("percpu_rw_semaphore: reimplement to not block the readers unnecessarily")

Not really... I think it was 70fe2f48152e ("aio: fix freeze protection of aio writes").
And iiuc io_uring does the same.

> and is certainly an understandable mistake.
>
> I'm torn on what to do, using this_cpu over __this_cpu is going to
> adversely affect code-gen (and possibly performance) for all the
> percpu-rwsem users that are not quite so 'creative'.

Yes, but what else can we do?

Oleg.


  reply	other threads:[~2020-09-15 22:52 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-15 14:07 [RFC PATCH] locking/percpu-rwsem: use this_cpu_{inc|dec}() for read_count Hou Tao
2020-09-15 15:06 ` peterz
2020-09-15 15:31   ` Oleg Nesterov [this message]
2020-09-15 15:51     ` peterz
2020-09-15 16:03       ` peterz
2020-09-15 16:11         ` Will Deacon
2020-09-15 18:11           ` peterz
2020-09-16  8:20             ` Will Deacon
2020-09-15 16:47         ` Oleg Nesterov
2020-09-16 12:32         ` Hou Tao
2020-09-16 12:51           ` peterz
2020-09-17  8:48           ` Will Deacon
2020-09-24 11:55             ` Hou Tao
2020-09-29 17:49               ` Will Deacon
2020-09-29 18:07                 ` Ard Biesheuvel
2020-09-17 10:51           ` Boaz Harrosh
2020-09-17 12:01             ` Oleg Nesterov
2020-09-17 12:48               ` Matthew Wilcox
2020-09-17 13:22                 ` peterz
2020-09-17 13:34                 ` Oleg Nesterov
2020-09-17 13:46                 ` Boaz Harrosh
2020-09-17 14:46                   ` Christoph Hellwig
2020-09-18  9:07               ` Jan Kara
2020-09-18 10:01                 ` peterz
2020-09-18 10:04                   ` peterz
2020-09-18 10:07                     ` peterz
2020-09-18 10:12                   ` peterz
2020-09-18 10:48                     ` Oleg Nesterov
2020-09-18 11:03                       ` peterz
2020-09-18 13:09                         ` Oleg Nesterov
2020-09-18 13:26                           ` Jan Kara
2020-09-20 23:49                             ` Dave Chinner
2020-09-18  8:36 ` [tip: locking/urgent] locking/percpu-rwsem: Use this_cpu_{inc,dec}() " tip-bot2 for Hou Tao

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=20200915153113.GA6881@redhat.com \
    --to=oleg@redhat.com \
    --cc=cl@linux.com \
    --cc=dennis@kernel.org \
    --cc=houtao1@huawei.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=tj@kernel.org \
    --cc=will@kernel.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.