Re: [PATCH] cpu/hotplug: Fix rollback during error-out in takedown_cpu()

From: Thomas Gleixner <tglx@linutronix.de>
To: Mukesh Ojha <mojha@codeaurora.org>
Cc: Neeraj Upadhyay <neeraju@codeaurora.org>,
	josh@joshtriplett.org, peterz@infradead.org, mingo@kernel.org,
	jiangshanlai@gmail.com, dzickus@redhat.com,
	brendan.jackman@arm.com, malat@debian.org,
	linux-kernel@vger.kernel.org, sramana@codeaurora.org,
	linux-arm-msm@vger.kernel.org
Subject: Re: [PATCH] cpu/hotplug: Fix rollback during error-out in takedown_cpu()
Date: Wed, 5 Sep 2018 15:14:01 +0200 (CEST)	[thread overview]
Message-ID: <alpine.DEB.2.21.1809051512450.1416@nanos.tec.linutronix.de> (raw)
In-Reply-To: <cf91ee04-a4b6-6f2c-d194-71b7e92fbf45@codeaurora.org>

[-- Attachment #1: Type: text/plain, Size: 1003 bytes --]

On Wed, 5 Sep 2018, Mukesh Ojha wrote:
> On 9/5/2018 5:03 PM, Thomas Gleixner wrote:
> > > +		st->rollback = true;
> > > +		st->target = prev_state;
> > > +		st->bringup = !st->bringup;
> > No, this is just papering over the actual problem.
> > 
> > The state inconsistency happens in take_cpu_down() when it returns with a
> > failure from __cpu_disable() because that returns with state = TEARDOWN_CPU
> > and st->state is then incremented in undo_cpu_down().
> > 
> > That's the real issue and we need to analyze the whole cpu_down rollback
> > logic first.
> 
> Could this be done like below ?

IOW, more papering over the real root cause.

> diff --git a/kernel/cpu.c b/kernel/cpu.c
> index aa7fe85..47bce90 100644
> --- a/kernel/cpu.c
> +++ b/kernel/cpu.c
> @@ -802,17 +802,18 @@ static int take_cpu_down(void *_param)
>         int err, cpu = smp_processor_id();
>         int ret;

  ^^^^^^^

Please fix your mailer or your editor. That patch is whitespace damaged.

Thanks,

	tglx