All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mathias Krause <minipli@grsecurity.net>
To: Peter Zijlstra <peterz@infradead.org>
Cc: "Vincent Guittot" <vincent.guittot@linaro.org>,
	"Michal Koutný" <mkoutny@suse.com>,
	"Benjamin Segall" <bsegall@google.com>,
	"Ingo Molnar" <mingo@redhat.com>,
	"Juri Lelli" <juri.lelli@redhat.com>,
	"Dietmar Eggemann" <dietmar.eggemann@arm.com>,
	"Steven Rostedt" <rostedt@goodmis.org>,
	"Mel Gorman" <mgorman@suse.de>,
	"Daniel Bristot de Oliveira" <bristot@redhat.com>,
	"Valentin Schneider" <Valentin.Schneider@arm.com>,
	linux-kernel@vger.kernel.org, "Odin Ugedal" <odin@uged.al>,
	"Kevin Tanguy" <kevin.tanguy@corp.ovh.com>,
	"Brad Spengler" <spender@grsecurity.net>
Subject: Re: [PATCH] sched/fair: Prevent dead task groups from regaining cfs_rq's
Date: Fri, 5 Nov 2021 18:40:11 +0100	[thread overview]
Message-ID: <00ec3c8e-6462-5cc4-ab79-e8887bcedad8@grsecurity.net> (raw)
In-Reply-To: <YYVpm7LRWMZMzxId@hirez.programming.kicks-ass.net>

Am 05.11.21 um 18:27 schrieb Peter Zijlstra:
> On Fri, Nov 05, 2021 at 06:14:33PM +0100, Mathias Krause wrote:
>> Am 05.11.21 um 17:58 schrieb Peter Zijlstra:
>>> On Fri, Nov 05, 2021 at 05:29:14PM +0100, Mathias Krause wrote:
>>>>> Looks like it needs to be the kfree_rcu() one in this case. I'll prepare
>>>>> a patch.
>>>>
>>>> Testing the below patch right now. Looking good so far. Will prepare a
>>>> proper patch later, if we all can agree that this covers all cases.
>>>>
>>>> But the basic idea is to defer the kfree()'s to after the next RCU GP,
>>>> which also means we need to free the tg object itself later. Slightly
>>>> ugly. :/
>>>
>>> Can't we add an rcu_head to struct task_group and simply call_rcu() the
>>> thing with a free function?
>>
>> There is already one and this patch makes use of it for the second RCU
>> GP requirement. Basically, the patch is doing what you request, no? See
>> the new free_fair_sched_group().
> 
> For some reason I thought you still did kfree_rcu(), I suppose reading
> is hard. I'll give it another go after dinner.

I wanted to go for kfree_rcu() initially, true. But after realizing,
that adding a rcu_head to struct cfs_rg, sched_entity and task_group
(which already has one) might be too much for what's needed, I went the
call_rcu() route instead and re-used the rcu_head of task_group.

Actually re-using the rcu_head in task_group is safe, as we'll use it
only sequentially: first in sched_destroy_group() to schedule
sched_free_group_rcu() and then, when it's executing, in
free_fair_sched_group() to schedule free_tg().

rcu_head's get unlinked prior to getting their callback function
invoked, which makes the above a valid use case.

Thanks,
Mathias

  reply	other threads:[~2021-11-05 17:40 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-11 17:22 [PATCH] sched/fair: Use rq->lock when checking cfs_rq list presence Michal Koutný
2021-10-11 19:12 ` Odin Ugedal
2021-10-12 18:32   ` Tao Zhou
2021-10-13 18:52     ` Odin Ugedal
2021-10-13 14:39   ` Michal Koutný
2021-10-13 18:45     ` Odin Ugedal
2021-10-13  7:57 ` Vincent Guittot
2021-10-13 14:26   ` Michal Koutný
2021-11-02 16:02     ` task_group unthrottling and removal race (was Re: [PATCH] sched/fair: Use rq->lock when checking cfs_rq list) presence Michal Koutný
2021-11-02 20:20       ` Odin Ugedal
2021-11-03  9:51       ` Mathias Krause
2021-11-03 10:51         ` Mathias Krause
2021-11-03 11:10           ` Michal Koutný
2021-11-03 14:16             ` Mathias Krause
2021-11-03 19:06               ` [PATCH] sched/fair: Prevent dead task groups from regaining cfs_rq's Mathias Krause
2021-11-03 22:03                 ` Benjamin Segall
2021-11-04  8:50                   ` Vincent Guittot
2021-11-04 15:13                     ` Mathias Krause
2021-11-04 16:49                       ` Vincent Guittot
2021-11-04 17:37                         ` Mathias Krause
2021-11-05 14:25                           ` Vincent Guittot
2021-11-05 14:44                             ` Mathias Krause
2021-11-05 16:29                               ` Mathias Krause
2021-11-05 16:58                                 ` Peter Zijlstra
2021-11-05 17:14                                   ` Mathias Krause
2021-11-05 17:27                                     ` Peter Zijlstra
2021-11-05 17:40                                       ` Mathias Krause [this message]
2021-11-06 10:48                                 ` Peter Zijlstra
2021-11-08 10:27                                   ` Mathias Krause
2021-11-08 11:40                                     ` Peter Zijlstra
2021-11-08 15:06                                       ` Mathias Krause
2021-11-10 15:14                                         ` Vincent Guittot
2021-11-09 18:47                                       ` Michal Koutný
2021-11-09 18:47                                         ` Michal Koutný
2021-11-10 15:17                                         ` Vincent Guittot
2021-11-10 15:17                                           ` Vincent Guittot
2021-11-04 20:46                       ` Benjamin Segall
2021-11-04 18:49                 ` Michal Koutný
2021-11-05 14:55                   ` Mathias Krause
2021-11-05 14:58                 ` Peter Zijlstra

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=00ec3c8e-6462-5cc4-ab79-e8887bcedad8@grsecurity.net \
    --to=minipli@grsecurity.net \
    --cc=Valentin.Schneider@arm.com \
    --cc=bristot@redhat.com \
    --cc=bsegall@google.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=juri.lelli@redhat.com \
    --cc=kevin.tanguy@corp.ovh.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mgorman@suse.de \
    --cc=mingo@redhat.com \
    --cc=mkoutny@suse.com \
    --cc=odin@uged.al \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=spender@grsecurity.net \
    --cc=vincent.guittot@linaro.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.