Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: quoted-printable
From:   Qian Cai <cai@lca.pw>
Mime-Version: 1.0 (1.0)
Subject: Re: [PATCH] tick/sched: fix data races at tick_do_timer_cpu
Date:   Wed, 4 Mar 2020 06:20:56 -0500
Message-Id: <1C65422C-FFA4-4651-893B-300FAF9C49DE@lca.pw>
References: <87tv34laqq.fsf@nanos.tec.linutronix.de>
Cc:     fweisbec@gmail.com, mingo@kernel.org, elver@google.com,
        linux-kernel@vger.kernel.org, Peter Zijlstra <peterz@infradead.org>
In-Reply-To: <87tv34laqq.fsf@nanos.tec.linutronix.de>
To:     Thomas Gleixner <tglx@linutronix.de>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


> On Mar 4, 2020, at 4:39 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
>=20
> They are reported, but are they actually a real problem?
>=20
> This completely lacks analysis why these 8 places need the
> READ/WRITE_ONCE() treatment at all and if so why the other 14 places
> accessing tick_do_timer_cpu are safe without it.
>=20
> I definitely appreciate the work done with KCSAN, but just making the
> tool shut up does not cut it.

Looks at tick_sched_do_timer(), for example,

if (unlikely(tick_do_timer_cpu =3D=3D TICK_DO_TIMER_NONE)) {
#ifdef CONFIG_NO_HZ_FULL
		WARN_ON(tick_nohz_full_running);
#endif
		tick_do_timer_cpu =3D cpu;
	}
#endif

/* Check, if the jiffies need an update */
if (tick_do_timer_cpu =3D=3D cpu)
	tick_do_update_jiffies64(now);

Could we rule out all compilers and archs will not optimize it if !CONFIG_NO=
_HZ_FULL to,

if (unlikely(tick_do_timer_cpu =3D=3D TICK_DO_TIMER_NONE) || tick_do_timer_c=
pu =3D=3D cpu)
         tick_do_update_jiffies64(now);

So it could save a branch or/and realized that tick_do_timer_cpu is not used=
 later in this cpu, so it could discard the store?

I am not all that familiar with all other 14 places if it is possible to hap=
pen concurrently as well, so I took a pragmatic approach that for now only d=
eal with the places that KCSAN confirmed, and then look forward for an incre=
mental approach if there are more places needs treatments later once confirm=
ed.=