Messages in this thread | | | Date | Mon, 9 Feb 2009 20:54:47 +0100 | From | Frederic Weisbecker <> | Subject | Re: [PATCH] softlockup: remove timestamp checking from hung_task |
| |
On Mon, Feb 09, 2009 at 11:29:56AM -0800, Mandeep Singh Baines wrote: > Frederic Weisbecker (fweisbec@gmail.com) wrote: > > BTW, here is a small fixlet on top of your patch about what I commented concerning > > the tasks than weren't yet scheduled once: > > > > -- > > From e7120e424b031978e482b5fe311d90916ffb8b7e Mon Sep 17 00:00:00 2001 > > From: Frederic Weisbecker <fweisbec@gmail.com> > > Date: Sat, 7 Feb 2009 17:45:12 +0100 > > Subject: [PATCH] softlockup: ensure the task has been scheduled once > > > > When we check if the task has been scheduled since the last scan, we might > > have a race condition if the task has been inserted on the task list but not > > yet scheduled once. So we just add a small check to ensure it has been switched > > in at least one time to avoid false positive. > > > > Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com> > > --- > > kernel/hung_task.c | 6 +++++- > > 1 files changed, 5 insertions(+), 1 deletions(-) > > > > diff --git a/kernel/hung_task.c b/kernel/hung_task.c > > index 4a10756..7f57a71 100644 > > --- a/kernel/hung_task.c > > +++ b/kernel/hung_task.c > > @@ -72,7 +72,11 @@ static void check_hung_task(struct task_struct *t, unsigned long timeout) > > { > > unsigned long switch_count = t->nvcsw + t->nivcsw; > > > > - if (t->flags & PF_FROZEN) > > + /* > > + * Ensure the task is not frozen and that it has been scheduled > > + * at least once. > > + */ > > + if (t->flags & PF_FROZEN || !switch_count) > > return; > > > > if (switch_count != t->last_switch_count) { > > -- > > 1.6.1 > > > > Good catch! I would change the description though. The race is a little > more subtle than your current description. For a newly forked process, the race > can occur if check_hung_task() processes the task in the time between the task > changing its state to UNINTERRUPTIBLE and the the scheduler updating the > switch_count (nivcsw or nvcsw). > > One minor change to the code comment. You want to ensure that the task has > context switched at least once, NOT that the task has been scheduled at least > once.
Ooh ok. I thought that the first schedule updated nivcsw/nvcsw, that's why I cancelled my comment.
Ok so I will resend the patch with the appropriate comment.
> > If you re-send the patch with the fixed comment and description, I'll > ack it.
Thanks! :-)
| |