public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Marco Elver <elver@google.com>
To: Christian Brauner <christian.brauner@ubuntu.com>
Cc: Oleg Nesterov <oleg@redhat.com>,
	dvyukov@google.com, ebiederm@xmission.com,
	linux-kernel@vger.kernel.org, syzkaller-bugs@googlegroups.com
Subject: Re: [PATCH cgroup/for-5.5] cgroup: remove cgroup_enable_task_cg_lists() optimization
Date: Mon, 28 Oct 2019 18:46:49 +0100	[thread overview]
Message-ID: <20191028174649.GA63741@google.com> (raw)
In-Reply-To: <20191028173031.m32p5e3ek764hnre@wittgenstein>


On Mon, 28 Oct 2019, Christian Brauner wrote:

> On Mon, Oct 28, 2019 at 05:48:52PM +0100, Oleg Nesterov wrote:
> > On 10/25, Christian Brauner wrote:
> > 
> > It is not that this code lacked READ_ONCE(). I am sure me and others
> > understood that this code can read ->exit_state more than once, just
> > nobody noticed that in this case this is really wrong.
> > 
> > IOW, if we simply change the code before 3245d6acab98 to use READ_ONCE()
> > the code will be equally wrong, and
> > 
> > > and as far as I understand this would also help kcsan to better detect
> > > races.
> > 
> > this change will simply hide the problem from kcsan.
> 
> I can't speak to that since the claim that read_once() helps them even
> if it's not really doing anything. But maybe I misunderstood the
> k{c,t}san manpage.

What Oleg said is right: marking things _ONCE will make these accesses
no longer be data races, and  KCSAN would then no longer report these as
data race, even if the code is still buggy due to there being a race
condition. In this case, the race condition would stop also being a data
race, and we'd no longer find it.

Thanks,
-- Marco

      reply	other threads:[~2019-10-28 17:47 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-21 10:34 KCSAN: data-race in exit_signals / prepare_signal syzbot
2019-10-21 11:19 ` Christian Brauner
2019-10-21 12:00   ` Oleg Nesterov
2019-10-21 12:15     ` Marco Elver
2019-10-21 13:47       ` Oleg Nesterov
2019-10-21 12:51     ` Christian Brauner
2019-10-21 14:21 ` cgroup_enable_task_cg_lists && PF_EXITING (Was: KCSAN: data-race in exit_signals / prepare_signal) Oleg Nesterov
2019-10-24 17:54   ` Tejun Heo
2019-10-24 19:03   ` [PATCH cgroup/for-5.5] cgroup: remove cgroup_enable_task_cg_lists() optimization Tejun Heo
2019-10-25 12:56     ` Tejun Heo
2019-10-25 13:33       ` Christian Brauner
2019-10-25 14:13         ` Oleg Nesterov
2019-10-25 14:32           ` Christian Brauner
2019-10-25 15:52             ` Oleg Nesterov
2019-10-25 17:05               ` Christian Brauner
2019-10-28 16:48                 ` Oleg Nesterov
2019-10-28 17:30                   ` Christian Brauner
2019-10-28 17:46                     ` Marco Elver [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191028174649.GA63741@google.com \
    --to=elver@google.com \
    --cc=christian.brauner@ubuntu.com \
    --cc=dvyukov@google.com \
    --cc=ebiederm@xmission.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oleg@redhat.com \
    --cc=syzkaller-bugs@googlegroups.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox