Messages in this thread | | | From | Linus Torvalds <> | Date | Mon, 28 Jun 2021 12:02:56 -0700 | Subject | Re: [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
| |