All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: Petr Mladek <pmladek@suse.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Oleg Nesterov <oleg@redhat.com>, Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
	Josh Triplett <josh@joshtriplett.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Jiri Kosina <jkosina@suse.cz>, Borislav Petkov <bp@suse.de>,
	Michal Hocko <mhocko@suse.cz>,
	linux-mm@kvack.org, Vlastimil Babka <vbabka@suse.cz>,
	live-patching@vger.kernel.org, linux-api@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [RFC v2 07/18] kthread: Allow to cancel kthread work
Date: Wed, 7 Oct 2015 07:24:46 -0700	[thread overview]
Message-ID: <20151007142446.GA2012@mtj.duckdns.org> (raw)
In-Reply-To: <20151007092130.GD3122@pathway.suse.cz>

Hello, Petr.

On Wed, Oct 07, 2015 at 11:21:30AM +0200, Petr Mladek wrote:
> Now, let's have one work: W, two workers: A, B, and try to queue
> the same work to the two workers at the same time:

It's a debug WARN condition to catch silly mistakes.  It can have
minor race conditions.

...
> Second, we still need the busy waiting for the pending timer callback.

Isn't that del_timer_sync()?

> Yes, we could set some flag so that the call back does not queue
> the work. But cancel_kthread_work_sync() still has to wait.
> It could not return if there is still some pending operation
> with the struct kthread_work. Otherwise, it never could
> be freed a safe way.
> 
> Also note that we still need the WORK_PENDING flag. Otherwise, we
> would not be able to detect the race when timer is removed but
> the callback has not run yet.

Yeah, just use a state field as I wrote before.

> Let me to repeat that using per-work and per-worker lock is not an
> option either. We would need some crazy hacks to avoid ABBA deadlocks.
> 
> 
> All in all, I would prefer to keep the original approach that is
> heavily inspired by the workqueues. I think that it is actually
> an advantage to reuse some working concept that reinventing wheels.

At each turn, you come up with non-issues and declare that it needs to
be full workqueue-like implementation but the issues you're raising
seem all rather irrelevant.  Can you please try to take a step back
and put some distance from the implementation details of workqueue?

Thanks.

-- 
tejun

WARNING: multiple messages have this Message-ID (diff)
From: Tejun Heo <tj@kernel.org>
To: Petr Mladek <pmladek@suse.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Oleg Nesterov <oleg@redhat.com>, Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
	Josh Triplett <josh@joshtriplett.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Jiri Kosina <jkosina@suse.cz>, Borislav Petkov <bp@suse.de>,
	Michal Hocko <mhocko@suse.cz>,
	linux-mm@kvack.org, Vlastimil Babka <vbabka@suse.cz>,
	live-patching@vger.kernel.org, linux-api@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [RFC v2 07/18] kthread: Allow to cancel kthread work
Date: Wed, 7 Oct 2015 07:24:46 -0700	[thread overview]
Message-ID: <20151007142446.GA2012@mtj.duckdns.org> (raw)
In-Reply-To: <20151007092130.GD3122@pathway.suse.cz>

Hello, Petr.

On Wed, Oct 07, 2015 at 11:21:30AM +0200, Petr Mladek wrote:
> Now, let's have one work: W, two workers: A, B, and try to queue
> the same work to the two workers at the same time:

It's a debug WARN condition to catch silly mistakes.  It can have
minor race conditions.

...
> Second, we still need the busy waiting for the pending timer callback.

Isn't that del_timer_sync()?

> Yes, we could set some flag so that the call back does not queue
> the work. But cancel_kthread_work_sync() still has to wait.
> It could not return if there is still some pending operation
> with the struct kthread_work. Otherwise, it never could
> be freed a safe way.
> 
> Also note that we still need the WORK_PENDING flag. Otherwise, we
> would not be able to detect the race when timer is removed but
> the callback has not run yet.

Yeah, just use a state field as I wrote before.

> Let me to repeat that using per-work and per-worker lock is not an
> option either. We would need some crazy hacks to avoid ABBA deadlocks.
> 
> 
> All in all, I would prefer to keep the original approach that is
> heavily inspired by the workqueues. I think that it is actually
> an advantage to reuse some working concept that reinventing wheels.

At each turn, you come up with non-issues and declare that it needs to
be full workqueue-like implementation but the issues you're raising
seem all rather irrelevant.  Can you please try to take a step back
and put some distance from the implementation details of workqueue?

Thanks.

-- 
tejun

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2015-10-07 14:24 UTC|newest]

Thread overview: 97+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-21 13:03 [RFC v2 00/18] kthread: Use kthread worker API more widely Petr Mladek
2015-09-21 13:03 ` Petr Mladek
2015-09-21 13:03 ` [RFC v2 01/18] kthread: Allow to call __kthread_create_on_node() with va_list args Petr Mladek
2015-09-21 13:03   ` Petr Mladek
2015-09-21 13:03 ` [RFC v2 02/18] kthread: Add create_kthread_worker*() Petr Mladek
2015-09-21 13:03   ` Petr Mladek
2015-09-22 18:20   ` Tejun Heo
2015-09-22 18:20     ` Tejun Heo
2015-09-21 13:03 ` [RFC v2 03/18] kthread: Add drain_kthread_worker() Petr Mladek
2015-09-21 13:03   ` Petr Mladek
2015-09-22 18:26   ` Tejun Heo
2015-09-22 18:26     ` Tejun Heo
2015-09-21 13:03 ` [RFC v2 04/18] kthread: Add destroy_kthread_worker() Petr Mladek
2015-09-21 13:03   ` Petr Mladek
2015-09-22 18:30   ` Tejun Heo
2015-09-22 18:30     ` Tejun Heo
2015-09-21 13:03 ` [RFC v2 05/18] kthread: Add pending flag to kthread work Petr Mladek
2015-09-21 13:03   ` Petr Mladek
2015-09-21 13:03 ` [RFC v2 06/18] kthread: Initial support for delayed " Petr Mladek
2015-09-21 13:03   ` Petr Mladek
2015-09-21 13:03 ` [RFC v2 07/18] kthread: Allow to cancel " Petr Mladek
2015-09-21 13:03   ` Petr Mladek
     [not found]   ` <1442840639-6963-8-git-send-email-pmladek-IBi9RG/b67k@public.gmane.org>
2015-09-22 19:35     ` Tejun Heo
2015-09-22 19:35       ` Tejun Heo
2015-09-22 19:35       ` Tejun Heo
     [not found]       ` <20150922193513.GE17659-qYNAdHglDFBN0TnZuCh8vA@public.gmane.org>
2015-09-25 11:26         ` Petr Mladek
2015-09-25 11:26           ` Petr Mladek
2015-09-25 11:26           ` Petr Mladek
2015-09-28 17:03           ` Tejun Heo
2015-09-28 17:03             ` Tejun Heo
2015-10-02 15:43             ` Petr Mladek
2015-10-02 15:43               ` Petr Mladek
     [not found]               ` <20151002154336.GC3122-KsEp0d+Q8qECVLCxKZUutA@public.gmane.org>
2015-10-02 19:24                 ` Tejun Heo
2015-10-02 19:24                   ` Tejun Heo
2015-10-02 19:24                   ` Tejun Heo
     [not found]                   ` <20151002192453.GA7564-qYNAdHglDFBN0TnZuCh8vA@public.gmane.org>
2015-10-05 10:07                     ` Petr Mladek
2015-10-05 10:07                       ` Petr Mladek
2015-10-05 10:07                       ` Petr Mladek
     [not found]                       ` <20151005100758.GK9603-KsEp0d+Q8qECVLCxKZUutA@public.gmane.org>
2015-10-05 11:09                         ` Petr Mladek
2015-10-05 11:09                           ` Petr Mladek
2015-10-05 11:09                           ` Petr Mladek
     [not found]                           ` <20151005110924.GL9603-KsEp0d+Q8qECVLCxKZUutA@public.gmane.org>
2015-10-07  9:21                             ` Petr Mladek
2015-10-07  9:21                               ` Petr Mladek
2015-10-07  9:21                               ` Petr Mladek
2015-10-07 14:24                               ` Tejun Heo [this message]
2015-10-07 14:24                                 ` Tejun Heo
     [not found]                                 ` <20151007142446.GA2012-qYNAdHglDFBN0TnZuCh8vA@public.gmane.org>
2015-10-14 10:20                                   ` Petr Mladek
2015-10-14 10:20                                     ` Petr Mladek
2015-10-14 10:20                                     ` Petr Mladek
2015-10-14 17:30                                     ` Tejun Heo
2015-10-14 17:30                                       ` Tejun Heo
2015-09-21 13:03 ` [RFC v2 08/18] kthread: Allow to modify delayed " Petr Mladek
2015-09-21 13:03   ` Petr Mladek
2015-09-21 13:03 ` [RFC v2 09/18] mm/huge_page: Convert khugepaged() into kthread worker API Petr Mladek
2015-09-21 13:03   ` Petr Mladek
2015-09-22 20:26   ` Tejun Heo
2015-09-22 20:26     ` Tejun Heo
2015-09-23  9:50     ` Petr Mladek
2015-09-23  9:50       ` Petr Mladek
2015-09-21 13:03 ` [RFC v2 10/18] ring_buffer: Do no not complete benchmark reader too early Petr Mladek
2015-09-21 13:03   ` Petr Mladek
2015-09-21 13:03 ` [RFC v2 11/18] ring_buffer: Fix more races when terminating the producer in the benchmark Petr Mladek
2015-09-21 13:03   ` Petr Mladek
2015-09-21 13:03 ` [RFC v2 12/18] ring_buffer: Convert benchmark kthreads into kthread worker API Petr Mladek
2015-09-21 13:03   ` Petr Mladek
2015-09-21 13:03 ` [RFC v2 13/18] rcu: Finish folding ->fqs_state into ->gp_state Petr Mladek
2015-09-21 13:03   ` Petr Mladek
2015-09-21 13:03 ` [RFC v2 14/18] rcu: Store first_gp_fqs into struct rcu_state Petr Mladek
2015-09-21 13:03   ` Petr Mladek
2015-09-21 13:03 ` [RFC v2 15/18] rcu: Clean up timeouts for forcing the quiescent state Petr Mladek
2015-09-21 13:03   ` Petr Mladek
2015-09-21 13:03 ` [RFC v2 16/18] rcu: Check actual RCU_GP_FLAG_FQS when handling " Petr Mladek
2015-09-21 13:03   ` Petr Mladek
2015-09-21 13:03 ` [RFC v2 17/18] rcu: Convert RCU gp kthreads into kthread worker API Petr Mladek
2015-09-21 13:03   ` Petr Mladek
2015-09-28 17:14   ` Paul E. McKenney
2015-09-28 17:14     ` Paul E. McKenney
     [not found]     ` <20150928171437.GB5182-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
2015-10-01 15:43       ` Petr Mladek
2015-10-01 15:43         ` Petr Mladek
2015-10-01 15:43         ` Petr Mladek
2015-10-01 16:33         ` Paul E. McKenney
2015-10-01 16:33           ` Paul E. McKenney
2015-09-21 13:03 ` [RFC v2 18/18] kthread: Better support freezable kthread workers Petr Mladek
2015-09-21 13:03   ` Petr Mladek
2015-09-22 20:32 ` [RFC v2 00/18] kthread: Use kthread worker API more widely Tejun Heo
2015-09-22 20:32   ` Tejun Heo
2015-09-30  5:08 ` Paul E. McKenney
2015-09-30  5:08   ` Paul E. McKenney
     [not found]   ` <20150930050833.GA4412-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
2015-10-01 15:59     ` Petr Mladek
2015-10-01 15:59       ` Petr Mladek
2015-10-01 15:59       ` Petr Mladek
2015-10-01 17:00       ` Paul E. McKenney
2015-10-01 17:00         ` Paul E. McKenney
2015-10-02 12:00         ` Petr Mladek
2015-10-02 12:00           ` Petr Mladek
2015-10-02 13:59           ` Paul E. McKenney
2015-10-02 13:59             ` Paul E. McKenney

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=20151007142446.GA2012@mtj.duckdns.org \
    --to=tj@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=bp@suse.de \
    --cc=jkosina@suse.cz \
    --cc=josh@joshtriplett.org \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=live-patching@vger.kernel.org \
    --cc=mhocko@suse.cz \
    --cc=mingo@redhat.com \
    --cc=oleg@redhat.com \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=peterz@infradead.org \
    --cc=pmladek@suse.com \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=vbabka@suse.cz \
    /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.