[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <715e4123-fdb3-a71e-4069-91d16a56a308@arm.com>
Date: Tue, 25 Oct 2022 16:55:47 +0100
From: James Morse <james.morse@....com>
To: Peter Newman <peternewman@...gle.com>
Cc: Reinette Chatre <reinette.chatre@...el.com>,
Tony Luck <tony.luck@...el.com>,
"Yu, Fenghua" <fenghua.yu@...el.com>,
"Eranian, Stephane" <eranian@...gle.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Thomas Gleixner <tglx@...utronix.de>,
Babu Moger <Babu.Moger@....com>,
Gaurang Upasani <gupasani@...gle.com>
Subject: Re: [RFD] resctrl: reassigning a running container's CTRL_MON group
Hi Peter,
On 21/10/2022 13:42, Peter Newman wrote:
> On Thu, Oct 20, 2022 at 12:39 PM Peter Newman <peternewman@...gle.com> wrote:
>>
>> On Wed, Oct 19, 2022 at 3:58 PM James Morse <james.morse@....com> wrote:
>>> The devil is in the detail, I'm not sure how it serialises with a fork()ing process, I'd
>>> hope to do better than relying on the kernel walking the list of processes a lot quicker
>>> than user-space can.
>>
>> I wasn't planning to do it any more optimally than the rmdir
>> implementation today when looking for all tasks impacted by a
>> CLOSID/RMID deletion.
>
> This is probably a separate topic, but I noticed this when looking at how rmdir
> moves tasks to a new closid/rmid...
>
> In rdt_move_group_tasks(), how do we know that a task switching in on another
> CPU will observe the updated closid and rmid values soon enough?
>
> Even on x86, without an smp_mb(), the stores to t->closid and t->rmid could be
> reordered with the task_curr(t) and task_cpu(t) reads which follow. The original
> description of this scenario seemed to assume that accesses below would happen
> in program order:
>
> WRITE_ONCE(t->closid, to->closid);
> WRITE_ONCE(t->rmid, to->mon.rmid);
>
> /*
> * If the task is on a CPU, set the CPU in the mask.
> * The detection is inaccurate as tasks might move or
> * schedule before the smp function call takes place.
> * In such a case the function call is pointless, but
> * there is no other side effect.
> */
> if (IS_ENABLED(CONFIG_SMP) && mask && task_curr(t))
> cpumask_set_cpu(task_cpu(t), mask);
>
> If the task concurrently switches in on another CPU, the code above may not
> observed that it's running, and the CPU running the task may not have observed
> the updated rmid and closid yet, so it could continue with the old rmid/closid
> and not get interrupted.
Makes sense to me - do you want to send a patch to fix it?
Thanks,
James
Powered by blists - more mailing lists