All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hidetoshi Seto <seto.hidetoshi@jp.fujitsu.com>
To: Rusty Russell <rusty@rustcorp.com.au>
Cc: linux-kernel@vger.kernel.org
Subject: Re: [PATCH] stopmachine: add stopmachine_timeout
Date: Wed, 16 Jul 2008 13:05:31 +0900	[thread overview]
Message-ID: <487D738B.4070104@jp.fujitsu.com> (raw)
In-Reply-To: <200807151750.12131.rusty@rustcorp.com.au>

Hi Rusty,

Rusty Russell wrote:
> On Tuesday 15 July 2008 11:11:34 Hidetoshi Seto wrote:
>> However we need to be careful that the stuck CPU can restart unexpectedly.
> 
> OK, if you are worried about that race, I think we can still fix it...

After having a relaxing day, once I said:
 "I like your idea that if we did not want to do something on the stuck CPU
  then treat the CPU as stopped."
but now I noticed that the stuck CPU can harm what we want to do if it is
not real stuck... ex. busy loop in a subsystem, and we want to touch the
core of the subsystem exclusively.
So "force progress" is not safe, on some rare case.  I'd like to make this
timeout feature as a safe-net, therefore we should return error without
taking a risk even it would be small, I think.

> Hmm, there's still the vague possibility that the thread doesn't schedule 
> until we start a new stop_machine (and clear prepared_cpus).  We could simply 
> loop in the main thread if any threads are alive, before freeing them (inside 
> the lock).  A counter and notifier is the other way, but it seems like 
> overkill for a very unlikely event.

I suppose my current implementation, returning control to user immediately,
is better than looping in main thread.  In my implementation, num_threads is
initialized to num_online_cpus() by main thread, and decremented 1 by 1
each child thread.  If time out happen, main thread will return without
waiting completion but set state STOPMACHINE_EXIT.  Then child threads are now
detached from usual procedure, so they exit soon without do any work.

At the beginning of new stop_machine, we can check the num_threads to know
whether there are remaining child threads.  If there are, something is wrong
since the system cannot run MAX_PRIO RT thread, not binded to typical cpu now.
So we can return error in such case, assuming that the new stop_machine will
fail in same way.

Anyway, I also think we can better thing here, but we don't need to do all
at once.  Making steps by incremental patches would be nice, I think.

Thanks,
H.Seto

  reply	other threads:[~2008-07-16  4:06 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-07-14  7:52 [PATCH] stopmachine: add stopmachine_timeout Hidetoshi Seto
2008-07-14  8:19 ` Hidetoshi Seto
2008-07-14 10:43 ` Rusty Russell
2008-07-15  1:11   ` Hidetoshi Seto
2008-07-15  7:50     ` Rusty Russell
2008-07-16  4:05       ` Hidetoshi Seto [this message]
2008-07-20  9:45         ` Rusty Russell
2008-07-22  3:28           ` [PATCH] stopmachine: allow force progress on timeout Hidetoshi Seto
2008-07-14 11:51 ` [PATCH] stopmachine: add stopmachine_timeout Christian Borntraeger
2008-07-14 12:34   ` Rusty Russell
2008-07-14 18:56     ` Jeremy Fitzhardinge
2008-07-14 18:56     ` Jeremy Fitzhardinge
2008-07-14 21:20       ` Heiko Carstens
2008-07-15  1:14         ` Rusty Russell
2008-07-15  1:14         ` Rusty Russell
2008-07-15  2:24         ` Hidetoshi Seto
2008-07-15  2:37           ` Max Krasnyansky
2008-07-15  2:37             ` Max Krasnyansky
2008-07-15  2:24         ` Hidetoshi Seto
2008-07-15  2:24         ` Max Krasnyansky
2008-07-15  2:24         ` Max Krasnyansky
2008-07-15  6:09           ` Heiko Carstens
2008-07-15  6:09           ` Heiko Carstens
2008-07-15  8:09           ` Rusty Russell
2008-07-15  8:39             ` Heiko Carstens
2008-07-15  8:39             ` Heiko Carstens
2008-07-15  8:51             ` Max Krasnyansky
2008-07-15  8:51             ` Max Krasnyansky
2008-07-16  9:15             ` Christian Borntraeger
2008-07-16  9:15             ` Christian Borntraeger
2008-07-15  8:09           ` Rusty Russell
2008-07-14 21:20       ` Heiko Carstens
2008-07-14 12:34   ` Rusty Russell
2008-07-16  4:27 ` [PATCH] stopmachine: add stopmachine_timeout v2 Hidetoshi Seto
2008-07-16  6:23   ` Max Krasnyansky
2008-07-16  6:35     ` Hidetoshi Seto
2008-07-16  6:51       ` [PATCH] stopmachine: add stopmachine_timeout v3 Hidetoshi Seto
2008-07-16  6:51       ` Hidetoshi Seto
2008-07-16  7:33         ` Peter Zijlstra
2008-07-16  8:12           ` Hidetoshi Seto
2008-07-16  8:12           ` Hidetoshi Seto
2008-07-16  7:33         ` Peter Zijlstra
2008-07-16  6:35     ` [PATCH] stopmachine: add stopmachine_timeout v2 Hidetoshi Seto
2008-07-16  6:23   ` Max Krasnyansky
2008-07-16 10:11   ` Jeremy Fitzhardinge
2008-07-16 10:11   ` Jeremy Fitzhardinge
2008-07-17  3:40     ` Hidetoshi Seto
2008-07-17  3:40     ` Hidetoshi Seto
2008-07-17  5:37       ` Jeremy Fitzhardinge
2008-07-17  5:37       ` Jeremy Fitzhardinge
2008-07-18  4:18       ` Rusty Russell
2008-07-18  4:18       ` Rusty Russell
2008-07-16  4:27 ` Hidetoshi Seto
2008-07-17  6:12 ` [PATCH] stopmachine: add stopmachine_timeout v4 Hidetoshi Seto
2008-07-17  6:12   ` Hidetoshi Seto
2008-07-17  7:09   ` Max Krasnyansky
2008-07-17  7:09   ` Max Krasnyansky

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=487D738B.4070104@jp.fujitsu.com \
    --to=seto.hidetoshi@jp.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rusty@rustcorp.com.au \
    /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.