HI ,
In the latest patch mentioned, k should be their instead of p:
-WARN_ON_ONCE(!wait_task_inactive(p, TASK_PARKED))
+WARN_ON_ONCE(!wait_task_inactive(k, TASK_PARKED))
Regards
Gaurav
On 6/7/2018 12:29 AM, Peter Zijlstra wrote:
On Wed, Jun 06, 2018 at 03:51:16PM +0200, Oleg Nesterov wrote:
On Wed, Jun 06, 2018 at 03:51:16PM +0200, Oleg Nesterov wrote:
> IIUC, this will only affect smpboot_update_cpumask_percpu_thread() which can
> hit
> an already parked thread, but it doesn't need to wait.
>
> And it seems that smpboot_update_cpumask_percpu_thread() in turn needs some
> cleanups.
On Wed, Jun 06, 2018 at 03:51:16PM +0200, Oleg Nesterov wrote:
> On 06/05, Peter Zijlstra wrote:
> >
> > Also, I think we still need TASK_PARKED as a special state for that.
>
> I think it would be nice to kill the TASK_PARKED state altogether. But I don't
> know how. I'll try to look at this code
On Wed, Jun 06, 2018 at 03:51:16PM +0200, Oleg Nesterov wrote:
> And I really think we should unexport kthread_park/unpark(), only
> smpboot_thread_fn()
> should use them. kthread() should not play with __kthread_parkme(). And even
> KTHREAD_SHOULD_PARK must die, I mean it should live in struct
>
On Wed, Jun 06, 2018 at 03:51:16PM +0200, Oleg Nesterov wrote:
> In particular, I think that kthread_park() on a parked kthread
> must not be possible.
It happens though; I put in a WARN and someone triggered it -- although
I could not reproduce :/
On 06/05, Peter Zijlstra wrote:
>
> Also, I think we still need TASK_PARKED as a special state for that.
I think it would be nice to kill the TASK_PARKED state altogether. But I don't
know how. I'll try to look at this code later, but I am not sure I will find a
way to cleanup it...
> --- a/kern
On Tue, Jun 05, 2018 at 06:35:16PM +0200, Oleg Nesterov wrote:
> Yes, but this won't fix the race decribed by Kohli...
Clearly I'm not going strong today... and yes, looking at that again I
didn't address that.
> Plus this complicates the schedule() paths for the very special case
I checked, we
Hi,
Just for info , the patch that I have shared earlier with pi_lock
approach has been tested since last one month and no issue has been
observed,
https://lkml.org/lkml/2018/4/25/189
Can we take this if it looks good?
Regards
Gaurav
On 6/5/2018 10:05 PM, Oleg Nesterov wrote:
On 06/05, Pe
On 06/05, Peter Zijlstra wrote:
>
> On Tue, Jun 05, 2018 at 05:22:12PM +0200, Peter Zijlstra wrote:
>
> > > OK, but __kthread_parkme() can be preempted before it calls schedule(),
> > > so the
> > > caller still can be migrated? Plus kthread_park_complete() can be called
> > > twice.
> >
> > Ar
On Tue, Jun 05, 2018 at 05:22:12PM +0200, Peter Zijlstra wrote:
> > OK, but __kthread_parkme() can be preempted before it calls schedule(), so
> > the
> > caller still can be migrated? Plus kthread_park_complete() can be called
> > twice.
>
> Argh... I forgot TASK_DEAD does the whole thing with
On Tue, Jun 05, 2018 at 05:08:41PM +0200, Oleg Nesterov wrote:
> On 06/05, Kohli, Gaurav wrote:
> >
> > As last mentioned on mail, we are still seeing issue with the latest
> > approach and below is the susceptible race as mentioned earlier..
> > controller Thread CPU
I have to admit that I didn't try to follow this discussion, somehow I thought
that the plan was to use set_special_state(PARKED)...
On 06/05, Kohli, Gaurav wrote:
>
> As last mentioned on mail, we are still seeing issue with the latest
> approach and below is the susceptible race as mentioned ear
Hi Peter,
As last mentioned on mail, we are still seeing issue with the latest
approach and below is the susceptible race as mentioned earlier..
controller Thread CPUHP Thread
takedown_cpu
kthread_park
kthread_parkme
Set KTHREAD_SHOULD_PARK
Corrected the formatting, Sorry for spam.
HI Peter,
We have tested with new patch and still seeing same issue, in this dumps
we don't have debug traces, but seems there still exist race from code
review , Can you please check it once:
Controller Thread CPUHP
On 5/2/2018 3:43 PM, Kohli, Gaurav wrote:
On 5/2/2018 1:50 PM, Peter Zijlstra wrote:
On Wed, May 02, 2018 at 10:45:52AM +0530, Kohli, Gaurav wrote:
On 5/1/2018 6:49 PM, Peter Zijlstra wrote:
- complete(&kthread->parked), which we can do inside schedule();
this
solves the problem
On 5/2/2018 1:50 PM, Peter Zijlstra wrote:
On Wed, May 02, 2018 at 10:45:52AM +0530, Kohli, Gaurav wrote:
On 5/1/2018 6:49 PM, Peter Zijlstra wrote:
- complete(&kthread->parked), which we can do inside schedule(); this
solves the problem because then kthread_park() will not return e
On Wed, May 02, 2018 at 10:45:52AM +0530, Kohli, Gaurav wrote:
> On 5/1/2018 6:49 PM, Peter Zijlstra wrote:
> > - complete(&kthread->parked), which we can do inside schedule(); this
> > solves the problem because then kthread_park() will not return early
> > and the task really is blocke
On 5/1/2018 6:49 PM, Peter Zijlstra wrote:
On 5/1/2018 5:01 PM, Peter Zijlstra wrote:
Let me ponder what the best solution is, it's a bit of a mess.
So there's:
- TASK_PARKED, which we can make a special state; this solves the
problem because then wait_task_inactive() is guaranteed
> On 5/1/2018 5:01 PM, Peter Zijlstra wrote:
> > Let me ponder what the best solution is, it's a bit of a mess.
So there's:
- TASK_PARKED, which we can make a special state; this solves the
problem because then wait_task_inactive() is guaranteed to see
TASK_PARKED and DTRT.
- complete(&
On 5/1/2018 5:01 PM, Peter Zijlstra wrote:
On Tue, May 01, 2018 at 04:10:53PM +0530, Kohli, Gaurav wrote:
Yes with loop, it will reset TASK_PARKED but that is not happening in the
dumps we have seen.
But was that with or without the fixed wait-loop? I don't care about
stuff you might have se
On Tue, May 01, 2018 at 04:10:53PM +0530, Kohli, Gaurav wrote:
> Yes with loop, it will reset TASK_PARKED but that is not happening in the
> dumps we have seen.
But was that with or without the fixed wait-loop? I don't care about
stuff you might have seen with the current code, that is clearly bro
On Tue, May 01, 2018 at 12:18:45PM +0200, Peter Zijlstra wrote:
> Aaaah... I think I've spotted a problem there. We clear SHOULD_PARK
> before we rebind, so if the thread lost the first PARKED store,
> does the completion, gets migrated, cycles through the loop and now
> observes !SHOULD_PARK and b
On 5/1/2018 3:48 PM, Peter Zijlstra wrote:
On Tue, May 01, 2018 at 01:20:26PM +0530, Kohli, Gaurav wrote:
But In our older case, where we have seen failure below is the wake up path
and ftraces, Wakeup occured and completed before schedule call only.
So final state of CPUHP is running not pa
On Tue, May 01, 2018 at 12:18:45PM +0200, Peter Zijlstra wrote:
> Aaaah... I think I've spotted a problem there. We clear SHOULD_PARK
> before we rebind, so if the thread lost the first PARKED store,
> does the completion, gets migrated, cycles through the loop and now
> observes !SHOULD_PARK and b
On Tue, May 01, 2018 at 01:20:26PM +0530, Kohli, Gaurav wrote:
> But In our older case, where we have seen failure below is the wake up path
> and ftraces, Wakeup occured and completed before schedule call only.
>
> So final state of CPUHP is running not parked. I have also pasted debug
> ftraces
sorry for spam, Adding list
On 4/30/2018 4:47 PM, Peter Zijlstra wrote:
On Thu, Apr 26, 2018 at 09:23:25PM +0530, Kohli, Gaurav wrote:
On 4/26/2018 2:27 PM, Peter Zijlstra wrote:
On Thu, Apr 26, 2018 at 10:41:31AM +0200, Peter Zijlstra wrote:
diff --git a/kernel/kthread.c b/kernel/kthread.c
On Mon, Apr 30, 2018 at 01:20:29PM +0200, Peter Zijlstra wrote:
> On Thu, Apr 26, 2018 at 06:18:20PM +0200, Oleg Nesterov wrote:
> > On 04/26, Peter Zijlstra wrote:
> > >
> > > For the others, I think we want to do something like the below. I still
> > > need to look at TASK_TRACED, which I suspect
On Thu, Apr 26, 2018 at 06:18:20PM +0200, Oleg Nesterov wrote:
> On 04/26, Peter Zijlstra wrote:
> >
> > For the others, I think we want to do something like the below. I still
> > need to look at TASK_TRACED, which I suspect is also special,
>
> Yes, and TASK_STOPPED.
I already did that one, rig
On Thu, Apr 26, 2018 at 09:23:25PM +0530, Kohli, Gaurav wrote:
> On 4/26/2018 2:27 PM, Peter Zijlstra wrote:
>
> > On Thu, Apr 26, 2018 at 10:41:31AM +0200, Peter Zijlstra wrote:
> > > diff --git a/kernel/kthread.c b/kernel/kthread.c
> > > index cd50e99202b0..4b6503c6a029 100644
> > > --- a/kernel
On 04/26, Peter Zijlstra wrote:
>
> For the others, I think we want to do something like the below. I still
> need to look at TASK_TRACED, which I suspect is also special,
Yes, and TASK_STOPPED.
ptrace_freeze_traced() and ptrace_unfreeze_traced() should be fine, but
ptrace_stop() wants set_specia
On Thu, Apr 26, 2018 at 10:41:31AM +0200, Peter Zijlstra wrote:
[...]
> +/*
> + * Special states are those that do not use the normal wait-loop pattern. See
> + * the comment with set_special_state().
> + */
> +#define is_special_state(state) \
> + ((state) == TASK_DEAD ||
On 4/26/2018 2:27 PM, Peter Zijlstra wrote:
On Thu, Apr 26, 2018 at 10:41:31AM +0200, Peter Zijlstra wrote:
diff --git a/kernel/kthread.c b/kernel/kthread.c
index cd50e99202b0..4b6503c6a029 100644
--- a/kernel/kthread.c
+++ b/kernel/kthread.c
@@ -177,12 +177,13 @@ void *kthread_probe_data(struc
On Thu, Apr 26, 2018 at 09:34:36AM +0530, Kohli, Gaurav wrote:
> On 4/26/2018 1:39 AM, Peter Zijlstra wrote:
>
> > On Wed, Apr 25, 2018 at 02:03:19PM +0530, Gaurav Kohli wrote:
> > > diff --git a/kernel/smpboot.c b/kernel/smpboot.c
> > > index 5043e74..c5c5184 100644
> > > --- a/kernel/smpboot.c
>
On Thu, Apr 26, 2018 at 10:41:31AM +0200, Peter Zijlstra wrote:
> diff --git a/kernel/kthread.c b/kernel/kthread.c
> index cd50e99202b0..4b6503c6a029 100644
> --- a/kernel/kthread.c
> +++ b/kernel/kthread.c
> @@ -177,12 +177,13 @@ void *kthread_probe_data(struct task_struct *task)
>
> static voi
On Wed, Apr 25, 2018 at 10:09:17PM +0200, Peter Zijlstra wrote:
> On Wed, Apr 25, 2018 at 02:03:19PM +0530, Gaurav Kohli wrote:
> > diff --git a/kernel/smpboot.c b/kernel/smpboot.c
> > index 5043e74..c5c5184 100644
> > --- a/kernel/smpboot.c
> > +++ b/kernel/smpboot.c
> > @@ -122,7 +122,45 @@ stati
On 4/26/2018 1:39 AM, Peter Zijlstra wrote:
On Wed, Apr 25, 2018 at 02:03:19PM +0530, Gaurav Kohli wrote:
diff --git a/kernel/smpboot.c b/kernel/smpboot.c
index 5043e74..c5c5184 100644
--- a/kernel/smpboot.c
+++ b/kernel/smpboot.c
@@ -122,7 +122,45 @@ static int smpboot_thread_fn(void *data)
On Wed, Apr 25, 2018 at 02:03:19PM +0530, Gaurav Kohli wrote:
> diff --git a/kernel/smpboot.c b/kernel/smpboot.c
> index 5043e74..c5c5184 100644
> --- a/kernel/smpboot.c
> +++ b/kernel/smpboot.c
> @@ -122,7 +122,45 @@ static int smpboot_thread_fn(void *data)
> }
>
> if
37 matches
Mail list logo
