From: Petr Mladek <pmladek@suse.com>
To: Yiwei Zhang <zzyiwei@android.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Felix Kuehling <Felix.Kuehling@amd.com>,
Jens Axboe <axboe@kernel.dk>,
"J. Bruce Fields" <bfields@redhat.com>,
Peter Zijlstra <peterz@infradead.org>,
Frederic Weisbecker <frederic@kernel.org>,
Marcelo Tosatti <mtosatti@redhat.com>,
Ilias Stamatis <stamatis.iliass@gmail.com>,
Rob Clark <robdclark@chromium.org>,
Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
Liang Chen <cl@rock-chips.com>,
linux-kernel@vger.kernel.org, kernel-team@android.com
Subject: Re: [PATCH] kthread: add kthread_mod_pending_delayed_work api
Date: Fri, 19 Feb 2021 11:56:00 +0100 [thread overview]
Message-ID: <YC+ZQAwwb4RGgjDf@alley> (raw)
In-Reply-To: <20210214000611.2169820-1-zzyiwei@android.com>
On Sun 2021-02-14 00:06:11, Yiwei Zhang wrote:
> The existing kthread_mod_delayed_work api will queue a new work if
> failing to cancel the current work due to no longer being pending.
> However, there's a case that the same work can be enqueued from both
> an async request and a delayed work, and a racing could happen if the
> async request comes right after the timeout delayed work gets scheduled,
> because the clean up work may not be safe to run twice.
Please, provide more details about the use case. Why the work is
originally sheduled with a delay. And and why it suddenly can/should
be proceed immediately.
>
> Signed-off-by: Yiwei Zhang <zzyiwei@android.com>
> ---
> include/linux/kthread.h | 3 +++
> kernel/kthread.c | 48 +++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 51 insertions(+)
>
> --- a/kernel/kthread.c
> +++ b/kernel/kthread.c
> @@ -1142,6 +1142,54 @@ bool kthread_mod_delayed_work(struct kthread_worker *worker,
> }
> EXPORT_SYMBOL_GPL(kthread_mod_delayed_work);
>
> +/**
> + * kthread_mod_pending_delayed_work - modify delay of a pending delayed work
> + * @worker: kthread worker to use
> + * @dwork: kthread delayed work to queue
> + * @delay: number of jiffies to wait before queuing
> + *
> + * If @dwork is still pending modify @dwork's timer so that it expires after
> + * @delay. If @dwork is still pending and @delay is zero, @work is guaranteed to
> + * be queued immediately.
> + *
> + * Return: %true if @dwork was pending and its timer was modified,
> + * %false otherwise.
> + *
> + * A special case is when the work is being canceled in parallel.
> + * It might be caused either by the real kthread_cancel_delayed_work_sync()
> + * or yet another kthread_mod_delayed_work() call. We let the other command
> + * win and return %false here. The caller is supposed to synchronize these
> + * operations a reasonable way.
> + *
> + * This function is safe to call from any context including IRQ handler.
> + * See __kthread_cancel_work() and kthread_delayed_work_timer_fn()
> + * for details.
> + */
> +bool kthread_mod_pending_delayed_work(struct kthread_worker *worker,
> + struct kthread_delayed_work *dwork,
> + unsigned long delay)
> +{
kthread_worker API tries to follow the workqueue API. It helps to use and
switch between them easily.
workqueue API does not provide this possibility. Instead it has
flush_delayed_work(). It queues the work when it was pending and
waits until the work is procced. So, we might do:
bool kthread_flush_delayed_work(struct kthread_delayed_work *dwork)
> + struct kthread_work *work = &dwork->work;
> + unsigned long flags;
> + int ret = true;
> +
> + raw_spin_lock_irqsave(&worker->lock, flags);
> + if (!work->worker || work->canceling ||
> + !__kthread_cancel_work(work, true, &flags)) {
> + ret = false;
> + goto out;
> + }
Please, use separate checks with comments as it is done, for example,
in kthread_mod_delayed_work()
struct kthread_work *work = &dwork->work;
unsigned long flags;
int ret;
raw_spin_lock_irqsave(&worker->lock, flags);
/* Do not bother with canceling when never queued. */
if (!work->worker)
goto nope;
/* Do not fight with another command that is canceling this work. */
if (work->canceling)
goto nope;
/* Nope when the work was not pending. */
ret = __kthread_cancel_work(work, true, &flags);
if (!ret)
nope;
/* Queue the work immediately. */
kthread_insert_work(worker, work, &worker->work_list);
raw_spin_unlock_irqrestore(&worker->lock, flags);
return kthread_flush_work(work);
nope:
raw_spin_unlock_irqrestore(&worker->lock, flags);
return false;
Will this work for you?
Best Regards,
Petr
next prev parent reply other threads:[~2021-02-19 10:59 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-14 0:06 [PATCH] kthread: add kthread_mod_pending_delayed_work api Yiwei Zhang
2021-02-15 13:28 ` Petr Mladek
2021-02-16 9:11 ` Christoph Hellwig
2021-02-16 18:58 ` Yiwei Zhang
2021-02-17 11:14 ` Petr Mladek
2021-02-19 6:29 ` Yiwei Zhang
2021-02-19 10:27 ` Petr Mladek
2021-02-19 10:30 ` Christoph Hellwig
2021-02-19 10:56 ` Petr Mladek [this message]
2021-02-23 0:39 ` Yiwei Zhang
2021-02-23 0:58 ` Yiwei Zhang
2021-02-23 15:52 ` Petr Mladek
2021-02-23 22:29 ` Yiwei Zhang
2021-02-24 9:34 ` Petr Mladek
2021-02-25 22:17 ` Yiwei Zhang
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=YC+ZQAwwb4RGgjDf@alley \
--to=pmladek@suse.com \
--cc=Felix.Kuehling@amd.com \
--cc=akpm@linux-foundation.org \
--cc=axboe@kernel.dk \
--cc=bfields@redhat.com \
--cc=cl@rock-chips.com \
--cc=frederic@kernel.org \
--cc=kernel-team@android.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mathieu.desnoyers@efficios.com \
--cc=mtosatti@redhat.com \
--cc=peterz@infradead.org \
--cc=robdclark@chromium.org \
--cc=stamatis.iliass@gmail.com \
--cc=zzyiwei@android.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).