linux-kernel - Re: [PATCH] clocksource: exynos_mct: Fix stall after CPU hotplugging

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-id: <1395755672.24026.0.camel@AMDC1943>
Date:	Tue, 25 Mar 2014 14:54:32 +0100
From:	Krzysztof Kozlowski <k.kozlowski@...sung.com>
To:	Tomasz Figa <t.figa@...sung.com>
Cc:	Daniel Lezcano <daniel.lezcano@...aro.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Kukjin Kim <kgene.kim@...sung.com>,
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	linux-samsung-soc@...r.kernel.org,
	Kyungmin Park <kyungmin.park@...sung.com>,
	Marek Szyprowski <m.szyprowski@...sung.com>,
	Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>
Subject: Re: [PATCH] clocksource: exynos_mct: Fix stall after CPU hotplugging

On Tue, 2014-03-25 at 12:32 +0100, Tomasz Figa wrote:
> Hi Krzysztof,
> 
> Two comments inline.
> 
> On 25.03.2014 11:41, Krzysztof Kozlowski wrote:
> 
> [snip]
> 
> > diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c
> > index 48f76bc05da0..0b49b09dd1a9 100644
> > --- a/drivers/clocksource/exynos_mct.c
> > +++ b/drivers/clocksource/exynos_mct.c
> > @@ -339,7 +339,14 @@ static void exynos4_mct_tick_start(unsigned long cycles,
> >   static int exynos4_tick_set_next_event(unsigned long cycles,
> >   				       struct clock_event_device *evt)
> >   {
> > -	struct mct_clock_event_device *mevt = this_cpu_ptr(&percpu_mct_tick);
> > +	/*
> > +	 * In case of hotplugging non-boot CPU, the set_next_event could be
> > +	 * called on CPU0 by ISR before IRQ affinity is set to proper CPU.
> 
> Hmm, is this a desired behavior? I guess this is a question for Thomas 
> and Daniel.
> 
> > +	 * Thus for accessing proper MCT Lx timer, 'per_cpu' for cpumask
> > +	 * in event must be used instead of 'this_cpu_ptr'.
> > +	 */
> > +	struct mct_clock_event_device *mevt = &per_cpu(percpu_mct_tick,
> > +			cpumask_first(evt->cpumask));
> >
> >   	exynos4_mct_tick_start(cycles, mevt);
> >
> > @@ -371,23 +378,13 @@ static inline void exynos4_tick_set_mode(enum clock_event_mode mode,
> >
> >   static int exynos4_mct_tick_clear(struct mct_clock_event_device *mevt)
> >   {
> > -	struct clock_event_device *evt = &mevt->evt;
> > -
> > -	/*
> > -	 * This is for supporting oneshot mode.
> > -	 * Mct would generate interrupt periodically
> > -	 * without explicit stopping.
> > -	 */
> > -	if (evt->mode != CLOCK_EVT_MODE_PERIODIC)
> > -		exynos4_mct_tick_stop(mevt);
> > -
> >   	/* Clear the MCT tick interrupt */
> >   	if (__raw_readl(reg_base + mevt->base + MCT_L_INT_CSTAT_OFFSET) & 1) {
> >   		exynos4_mct_write(0x1, mevt->base + MCT_L_INT_CSTAT_OFFSET);
> >   		return 1;
> > -	} else {
> > -		return 0;
> >   	}
> > +
> > +	return 0;
> 
> Nobody seems to be checking return value of this function (and I don't 
> see what it could be used for anyway), so I guess it could be simply 
> made void.

I'll send a another patch for it.

Best regards,
Krzysztof

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/