MIME-Version: 1.0
References: <20230126105128.2249938-1-dvyukov@google.com> <20230126154118.2393850-1-dvyukov@google.com>
 <CANpmjNM=PVigDZKu-H_-cLECUJKSx7TH+kxSjfF=4UHdrGBj+g@mail.gmail.com>
 <87o7qlgjce.ffs@tglx> <CACT4Y+aMLeCo9+nwXrFWo8FLG8rKHDe8v2ppkZ+mOaKAF6qtgw@mail.gmail.com>
 <20230128195641.GA14906@redhat.com>
In-Reply-To: <20230128195641.GA14906@redhat.com>
From:   Dmitry Vyukov <dvyukov@google.com>
Date:   Mon, 30 Jan 2023 10:00:20 +0100
Message-ID: <CACT4Y+YzTK8+gKm9pZ1hwGvaN-ExmVLuxZYKVia+dnakv6+b5A@mail.gmail.com>
Subject: Re: [PATCH v4] posix-timers: Prefer delivery of signals to the
 current thread
To:     Oleg Nesterov <oleg@redhat.com>
Cc:     Thomas Gleixner <tglx@linutronix.de>,
        Marco Elver <elver@google.com>, linux-kernel@vger.kernel.org,
        "Eric W . Biederman" <ebiederm@xmission.com>,
        Frederic Weisbecker <frederic@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Sat, 28 Jan 2023 at 20:56, Oleg Nesterov <oleg@redhat.com> wrote:
>
> Dmitry,
>
> I agree with what you said, just one note...
>
> On 01/27, Dmitry Vyukov wrote:
> >
> > After this change the test passes quickly (within a second for me).
>
> yet perhaps it makes sense to slightly change it? It does
>
>         +static void *distribution_thr(void *arg) {
>         +       while (__atomic_load_n(&remain, __ATOMIC_RELAXED));
>         +       return NULL;
>         +}
>
> so distribution_thr() eats CPU even after this thread gets a signal and thus
> (in theory) it can "steal" cpu_timer_fire() from other threads unpredictably
> long ? How about
>
>         -       while (__atomic_load_n(&remain, __ATOMIC_RELAXED));
>         +       while (__atomic_load_n(&got_signal, __ATOMIC_RELAXED));
> ?

But why?
IIUC this makes the test even "weaker". As Thomas notes it's already
somewhat "weak". And this would make it even "weaker". So if it passes
in the current version, I would keep it as is. It makes sense to relax
it only if it's known to fail sometimes. But it doesn't fail as far as
I know. And the intention is really that the current version must pass
-- all threads must get signals even if other threads are running.