linux-kernel - Re: [patch V2 3/5] posix-cpu-timers: Provide mechanisms to defer timer handling to task

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <875zaezl55.fsf@nanos.tec.linutronix.de>
Date:   Thu, 23 Jul 2020 10:32:54 +0200
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Frederic Weisbecker <frederic@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>
Cc:     LKML <linux-kernel@...r.kernel.org>, x86@...nel.org,
        Oleg Nesterov <oleg@...hat.com>,
        "Eric W. Biederman" <ebiederm@...ssion.com>,
        John Stultz <john.stultz@...aro.org>,
        Paolo Bonzini <pbonzini@...hat.com>
Subject: Re: [patch V2 3/5] posix-cpu-timers: Provide mechanisms to defer timer handling to task_work

Frederic Weisbecker <frederic@...nel.org> writes:
> On Fri, Jul 17, 2020 at 12:50:34AM +0200, Peter Zijlstra wrote:
>> On Thu, Jul 16, 2020 at 10:19:26PM +0200, Thomas Gleixner wrote:
>> > +static void __run_posix_cpu_timers(struct task_struct *tsk)
>> > +{
>> > +	struct posix_cputimers *pct = &tsk->posix_cputimers;
>> > +
>> > +	if (!test_and_set_bit(CPUTIMERS_WORK_SCHEDULED, &pct->flags))
>> > +		task_work_add(tsk, &pct->task_work, true);
>> > +}
>> > +
>> > +static inline void posix_cpu_timers_enable_work(struct task_struct *tsk)
>> > +{
>> > +	clear_bit(CPUTIMERS_WORK_SCHEDULED, &tsk->posix_cputimers.flags);
>> 	/*
>> 	 * Ensure we observe everything before a failing test_and_set()
>> 	 * in __run_posix_cpu_timers().
>> 	 */
>> 	smp_mb__after_atomic();
>> > +}
>> 
>> Such that when another timer interrupt happens while we run this, we're
>> guaranteed to either see it, or get re-queued and thus re-run the
>> function.
>
> But each thread in the process enqueues its own task work and flips its
> own flags. So if task A runs the task work and task B runs __run_posix_cpu_timers(),
> they wouldn't be ordering against the same flags.

If two tasks queue work independent of each other then one of them will
find it done already, which is the same as if two tasks of the same
process execute run_posix_cpu_timers() in parallel.

I really don't want to go into the rathole of making the work or the
synchronization process wide. That's a guarantee for disaster.

Handling task work strictly per task is straight forward and simple. The
eventually resulting contention on sighand lock in task work is
unavoidable, but that's a reasonable tradeoff vs. the complexity you
need to handle task work process wide.

Thanks,

        tglx