All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: LKML <linux-kernel@vger.kernel.org>,
	x86@kernel.org, "Eric W. Biederman" <ebiederm@xmission.com>,
	Frederic Weisbecker <frederic@kernel.org>,
	John Stultz <john.stultz@linaro.org>,
	Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: [patch V2 3/5] posix-cpu-timers: Provide mechanisms to defer timer handling to task_work
Date: Fri, 17 Jul 2020 19:26:28 +0200	[thread overview]
Message-ID: <20200717172627.GC6067@redhat.com> (raw)
In-Reply-To: <20200716202044.734067877@linutronix.de>

Looks correct to me, but I forgot everything about posix-timers.c

this obviously means that the expired timer won't fire until the
task returns to user-mode but probably we don't care.

One cosmetic nit below,

On 07/16, Thomas Gleixner wrote:
>
> +#ifdef CONFIG_POSIX_CPU_TIMERS_TASK_WORK
> +void posix_cpu_timers_work(struct callback_head *work);
> +
> +static inline void posix_cputimer_init_work(struct posix_cputimers *pct)
> +{
> +	pct->task_work.func = posix_cpu_timers_work;

init_task_work() ?

> +}
> +#else
> +static inline void posix_cputimer_init_work(struct posix_cputimers *pct) { }
> +#endif
> +
>  static inline void posix_cputimers_init(struct posix_cputimers *pct)
>  {
>  	memset(pct, 0, sizeof(*pct));
>  	pct->bases[0].nextevt = U64_MAX;
>  	pct->bases[1].nextevt = U64_MAX;
>  	pct->bases[2].nextevt = U64_MAX;
> +	posix_cputimer_init_work(pct);
>  }

And I can't resist. I know this is a common practice, please ignore, but to me

	static inline void posix_cputimers_init(struct posix_cputimers *pct)
	{
		memset(pct, 0, sizeof(*pct));
		pct->bases[0].nextevt = U64_MAX;
		pct->bases[1].nextevt = U64_MAX;
		pct->bases[2].nextevt = U64_MAX;
	#ifdef CONFIG_POSIX_CPU_TIMERS_TASK_WORK
		init_task_work(&pct->task_work, posix_cpu_timers_work);
	#endif
	}

looks better than 2 posix_cputimer_init_work() definitions above.

Note also that signal_struct->posix_cputimers.task_work is never used, perhaps
it would be better to move this task_work into task_struct? This way we do not
even need to change posix_cputimers_init(), we call simply initialize
init_task.posix_task_work.

Oleg.


  parent reply	other threads:[~2020-07-17 17:26 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-16 20:19 [patch V2 0/5] posix-cpu-timers: Move expiry into task work context Thomas Gleixner
2020-07-16 20:19 ` [patch V2 1/5] posix-cpu-timers: Split run_posix_cpu_timers() Thomas Gleixner
2020-07-16 20:19 ` [patch V2 2/5] posix-cpu-timers: Convert the flags to a bitmap Thomas Gleixner
2020-07-21 12:34   ` Frederic Weisbecker
2020-07-21 16:10     ` Thomas Gleixner
2020-07-21 16:23       ` David Laight
2020-07-21 18:30         ` Thomas Gleixner
2020-07-16 20:19 ` [patch V2 3/5] posix-cpu-timers: Provide mechanisms to defer timer handling to task_work Thomas Gleixner
2020-07-16 22:50   ` Peter Zijlstra
2020-07-17 18:37     ` Thomas Gleixner
2020-07-23  1:03     ` Frederic Weisbecker
2020-07-23  8:32       ` Thomas Gleixner
2020-07-23 12:15         ` Frederic Weisbecker
2020-07-16 22:54   ` Peter Zijlstra
2020-07-17 18:38     ` Thomas Gleixner
2020-07-19 19:33       ` Thomas Gleixner
2020-07-21 18:50         ` Thomas Gleixner
2020-07-17 17:26   ` Oleg Nesterov [this message]
2020-07-17 18:35     ` Thomas Gleixner
2020-07-16 20:19 ` [patch V2 4/5] posix-cpu-timers: Expiry timers directly when in task work context Thomas Gleixner
2020-07-16 20:19 ` [patch V2 5/5] x86: Select POSIX_CPU_TIMERS_TASK_WORK Thomas Gleixner

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=20200717172627.GC6067@redhat.com \
    --to=oleg@redhat.com \
    --cc=ebiederm@xmission.com \
    --cc=frederic@kernel.org \
    --cc=john.stultz@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=x86@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.