On Fri, Oct 25, 2019 at 04:13:25PM +0200, Oleg Nesterov wrote: > On 10/25, Christian Brauner wrote: > > > > [+Dmitry] > > > > On Fri, Oct 25, 2019 at 05:56:06AM -0700, Tejun Heo wrote: > > > On Thu, Oct 24, 2019 at 12:03:51PM -0700, Tejun Heo wrote: > > > > cgroup_enable_task_cg_lists() is used to lazyily initialize task > > > > cgroup associations on the first use to reduce fork / exit overheads > > > > on systems which don't use cgroup. Unfortunately, locking around it > > > > has never been actually correct and its value is dubious given how the > > > > vast majority of systems use cgroup right away from boot. > > > > > > > > This patch removes the optimization. For now, replace the cg_list > > > > based branches with WARN_ON_ONCE()'s to be on the safe side. We can > > > > simplify the logic further in the future. > > > > > > > > Signed-off-by: Tejun Heo <tj@xxxxxxxxxx> > > > > Reported-by: Oleg Nesterov <oleg@xxxxxxxxxx> > > > > > > Applying to cgroup/for-5.5. > > > > The code you removed was the only place where task->flags was set from > > !current. > > No, that code doesn't modify task->flags. It checks PF_EXITING under siglock > but this makes no sense and can't avoid the race with cgroup_exit(). Sorry, you are right. I misread Ah right, sorry I misremembered this from the prior thread where we discussed where ->flags is set from [1]. > > > So I think this fixes the syzbot data-race report in: > > https://lore.kernel.org/r/0000000000003b1e8005956939f1@xxxxxxxxxx > > No. > > Almost every usage of task->flags (load or sore) can be reported as "data race". > > Say, you do > > if (task->flags & PF_KTHREAD) > > while this task does > > current->flags |= PF_FREEZER_SKIP; > schedule(). > > this is data race. Right, but I thought we agreed on WONTFIX in those scenarios? The alternative is to READ_ONCE()/WRITE_ONCE() all of these. [1]: https://lore.kernel.org/r/20191021134659.GA1339@xxxxxxxxxx Anyway, accidental noise on my part. Christian
