lkml.org 
[lkml]   [2021]   [Jun]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [GIT PULL] sigqueue cache fix
On Mon, Jun 28, 2021 at 11:47 AM Ingo Molnar <mingo@kernel.org> wrote:
>
> But even if release_posix_timer() is changed to call sigqueue_free() with
> IRQs disabled, or sigqueue_free() disables interrupts itself, I think we
> need to be mindful of the Consumer <-> Producer SMP races, which only
> appear to be safe due to an accidental barrier by free_uid().

Oh, I agree. The SMP memory ordering issues are subtle and while I
suspect they aren't something that can be triggered in reality, it's
an example of how broken some WRITE_ONCE -> READ_ONCE serialization
is.

I don't mind READ_ONCE/WRITE_ONCE in general, but I absolutely detest
them when they are used to hide KCSAN things, and when they are used
randomly for pointers.

I think they are much better suited for one-time flag things, and
pretty much every time you see a READ_ONCE/WRITE_ONCE you should
likely see a barrier somewhere.

And no, we don't want them to be some subtle barriers that are just
"in the context of this code, we can depend on the barrier in that
other function".

In general, if you have a WRITE_ONCE -> READ_ONCE chain an dnot some
obvious required barrier for other reasons right *there*, it should
almost certainly not be one of those ONCE things at all. It should be
a proper smp_store_release() -> smp_load_acquire(). Those have real -
and on sane hardware cheap - memory orderings.

And as the whole - and only - point of the sigqueue cache was a very
subtle performance and latency issue, I don't think we want to use
locks or atomics. It's why my revert commit suggests re-purposing the
"percpu_cmpxchg()" functionality: that would likely be a good model at
least for x86 and arm.

But while we have "percpu_cmpxchg()", I don't think we currently don't
really have that kind of operation where it

(a) works on a non-percpu variable (but we can force casts, I guess)

(b) has "acquire" semantics

We do have the *atomic* cmpxchg with acquire semantics, but atomics
are rather more expensive than we'd probably really want.

Linus

\
 
 \ /
  Last update: 2021-06-28 21:04    [W:0.044 / U:0.960 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site