Messages in this thread | | | Date | Mon, 5 Oct 2020 10:38:29 +0200 | From | Petr Mladek <> | Subject | Re: [RFC PATCH] kthread: do not modify running work |
| |
On Sun 2020-10-04 10:12:13, Hillf Danton wrote: > > On Fri, 02 Oct 2020 10:32:32 Thomas Gleixner wrote: > > On Fri, Oct 02 2020 at 10:34, Hillf Danton wrote: > > > On Thu, 01 Oct 2020 15:59:38 +0200 Thomas Gleixner wrote: > > >> On Thu, Oct 01 2020 at 17:51, Hillf Danton wrote: > > >> Aside of that it's pretty irrelevant whether there is a user at the > > >> moment which reschedules work from the callback or not. > > >> > > >> It's something which needs to work because its possible from regular > > >> work queues as well and makes a lot of sense. > > > > > > https://lore.kernel.org/lkml/87eemheay8.fsf@nanos.tec.linutronix.de/ > > > > That's a completely different thing, really. This adds new functionality > > without users and exports it. > > > > kthread work is modeled after workqueue to address specific > > needs.
Exactly.
> > delayed work can be rescheduled from the callback and all > > variants of timers support rearming the timer from the callback as well.
This might be a bit confusing here. The timer callback just moves the work to the list of works that are going to be proceed by the kthread. It neither runs the work nor rearms the timer.
But the timer can be set again by any parallel kthread_queue_delayed_work() or kthread_mod_delayed_work() call even the the timer callback is still running. Module that some code is serialized by the lock.
> What is the difference of invoking kthread_queue_delayed_work() from the > callback from kthread_mod_delayed_work()?
kthread_queue_delayed_work() does nothing when the work is already queued. kthread_mod_delayed_work() removes the work from the queue if it is there and queue it again with the newly requested delay.
> > So having a consistent behaviour accross all these facilities makes > > absolutely sense and I don't agree with your sentiment in the changelog > > at all. > > > > Just because it does not make sense to you is not a justification for > > making stuff inconsistent. You still have not provided a technical > > reason why this change is needed. > > Given the queue method, it is no win to modify delayed work from callback > in any case because "we are not adding interfaces just because we can."
What about ipmi_kthread_worker_func()? It is delayed work that queues itself.
> Nor does it help much in regard of a running work that is delayed two > minutes because for instance it is not clear that it is a one-off work > with resources released in the callback.
This is up to the other API user to use the API the right way.
As it has already been mentioned, kthread_worker() API should behave the same as work_queue API. It is needed for kthreads that have special needs, for example, real time priority. It should be easy to migrate between the two APIs. Different behavior would be just a call for problems.
The dream is that all custom kthreads are converted into either the classic work_queues or kthread_worker API. People are really creative when doing the main loop and it is easy to do it wrong. It causes then problems, for example, with suspend/resume, livepatching.
What is the motivation for this patch, please? Does it solve some real problem?
Best Regards, Petr
| |