Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <317b4a96-f28d-aab5-57cc-f0222b7e4901@intel.com>
Date:   Wed, 26 Oct 2022 14:12:28 -0700
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Firefox/102.0 Thunderbird/102.4.0
Subject: Re: [RFD] resctrl: reassigning a running container's CTRL_MON group
Content-Language: en-US
To:     Peter Newman <peternewman@google.com>,
        James Morse <james.morse@arm.com>
CC:     Tony Luck <tony.luck@intel.com>,
        "Yu, Fenghua" <fenghua.yu@intel.com>,
        "Eranian, Stephane" <eranian@google.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        "Babu Moger" <Babu.Moger@amd.com>,
        Gaurang Upasani <gupasani@google.com>
References: <CALPaoCj-zav4x6H3ffXo_O+RFan8Qb-uLy-DdtkaQTfuxY4a0w@mail.gmail.com>
 <b2e020b1-f6b2-e236-a042-4eb2fd27d8b0@intel.com>
 <IA1PR11MB6097236CFF891041DBA42ECB9B5F9@IA1PR11MB6097.namprd11.prod.outlook.com>
 <Y0BhzKkksSjSeE3W@agluck-desk3.sc.intel.com>
 <81a7b4f6-fbb5-380e-532d-f2c1fc49b515@intel.com>
 <CALPaoCjdeRjyX5L6BBX688ZM21eMwetuL9QLF1+GEDUskGcU2w@mail.gmail.com>
 <76bb4dc9-ab7c-4cb6-d1bf-26436c88c6e2@arm.com>
 <CALPaoCiKUQC+LxDwKQ0gE5AQniJi_nbzrXi_HA9ZBRtiXdw_dg@mail.gmail.com>
 <835d769b-3662-7be5-dcdd-804cb1f3999a@arm.com>
 <CALPaoCgiwtvxZMJ6f-BOTNMPXDbMB8QM42HyCAvYRi4grPiphQ@mail.gmail.com>
 <CALPaoCg2-9ARbK+MEgdvdcjJtSy_2H6YeRkLrT97zgy8Aro3Vg@mail.gmail.com>
 <715e4123-fdb3-a71e-4069-91d16a56a308@arm.com>
 <CALPaoCjUk31HvhaqWKfkLNLyZJzbDLJOnEqTsqn882CToLCGcw@mail.gmail.com>
From:   Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <CALPaoCjUk31HvhaqWKfkLNLyZJzbDLJOnEqTsqn882CToLCGcw@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?clpMemYwWWcwVmhiSnVONkJVMGYvOHpSYWxMVmY5RnYydjF6T0piZkE2ekxO?=
 =?utf-8?B?ejY5d3BpY2orOGVNZExEbzk2blBjOWpRNnUwUStXNnB4NjQwbVREZzAyRkVv?=
 =?utf-8?B?UzZEcjUwYS9ickU5VG9hNFNFQnp3UFplN3F6YUp1VTd2eWllRGpxR3I0bk9W?=
 =?utf-8?B?dHRITHQ3ZGZpWk95NndEeDh2c1RaY3lYNWY0aGorWUNQcHdaM0VWRkhFNThQ?=
 =?utf-8?B?dHUzd2hSZ2grUEtqU2IvMmF4R091Y2xnQUQzOXJ1WkY1a2c0UlY2RS9EWTVV?=
 =?utf-8?B?eGNqM1R5NndLUHlNR0RpS0RBYXVEcWhPRVlncjd6eDJiZU9Oblc2YTRzM3pS?=
 =?utf-8?B?RUVDV013d1BSaGdybFRtVEdzNzZGY1VLTEhZb2owN0RzQVdLTC9QTlZiWlVP?=
 =?utf-8?B?QmJOcHE5WTVWdC9yTHFEc09VcDVMSGpwZHMrUXhCT2lWTVYydVNyZzEwWXIy?=
 =?utf-8?B?eERyOW1LWmN0ejAvOXdHVjVNaE15WnlhMlZ2OUNuU2Y2NkIrVjcvS2UyRGZM?=
 =?utf-8?B?VFpWZkNQcDZWQVFOVjBnYjgreWltUVpCNk5WOUcxZEozTGd4Z2kzazMveEhq?=
 =?utf-8?B?Y2pDdEF6dEtiS2lZZXp2ZDRDWGVFbThpaWMzMGlEZUQ3SmhmRXVhYUtLNzhH?=
 =?utf-8?B?NG5xR2YrdCtaRlhvYUM4b0FEYlNaUkM1NXpCTWhIV2lmNGwzNDM4R3hFNFlD?=
 =?utf-8?B?aW5mQUxhNkJWMGM4V1pVRDI0NXZOdmNCTFROZ3oyUmJ6QlViNFJxdnhNeXZW?=
 =?utf-8?B?SzlWajZlUCtoK3RZSEN3ZWFTRnNuSHlhbW9jbUtyM1Q3ZVQrakh2WkU4TzNq?=
 =?utf-8?B?T3ptcXBwMytJTlRNWkkzY1FjOTFnWnNzUFRVamt1a0g4RnhnQjRFTGt6ajhv?=
 =?utf-8?B?WlBTQnlRVnJvQnJGNXNmdmoyVXlQTDUvSk9ab2RIOEd1Z0lobkRHMytNZUt6?=
 =?utf-8?B?Z21nOUZFUzZLdFRsa1dQckMyMm5jVVNUc0pOTEdHeVgzVjFRVmVvV3NvNU05?=
 =?utf-8?B?dnYwQTJDMHNlYlpHd0dGZWhraGxZaC8rNjRMd2hZdDQ4RDltR0xmWGd0TndK?=
 =?utf-8?B?V3NpMGZBYjJITWFzWnNqSjAwZnFweDRvRm5VcjYyaWFUdEJ4RkVVbEgwbDB1?=
 =?utf-8?B?MHc2ZVpwUmNTMlR6Y3ovb2NiVlJvVlNEbStlVzQxeWdrTUk2cnFWUWFaK0h1?=
 =?utf-8?B?aVhEZW44RHQ2ZmxZcERwUGNmYVZMQzFHWmZRQklQbGFxalF5OFc3QnhoUVU4?=
 =?utf-8?B?M1U2UG4rNE5zSG9sT2pOY252cldOSmt1WVUvYmpIOVludDNEVlRvNnFiR1li?=
 =?utf-8?B?cUdvMm9wdjcrMjJsclNMRVVJNFIvZVh0SGVkRkI4aGJLV3lIa3pmM01yTEI0?=
 =?utf-8?B?eVJkaUNBMFF5KzdwZjJMaVJSWUVLQnpQYnpTZXlwODkzUVpjNHgyZjR6NXZm?=
 =?utf-8?B?dVRKb1UrZEV3ZGVUbmNqcUJLam9VZnVLNUZlWHoxYlB2Znp6UlBKZkxOTDFa?=
 =?utf-8?B?S3JCS2ZDRnJOM3RlemxJR0Y3UmU1eEdkNWlZcDJFekwrcE43OC9Eb0Vob3ZQ?=
 =?utf-8?B?UVBuenlhWVdlQkhoYUJJQUs4dG9aNDV5OUYrN1pYY0ZwcE1rM1lENERqbVcx?=
 =?utf-8?B?SkJGdUNzaWFZeGsxZGM4Zlc1QmZQaHRjRjRSbjV2d2h2TSswejJmaHU4ZlQ0?=
 =?utf-8?B?ajI3UlJHbFgwZUJNeWZhVFZrQU81NFVGV2NHSzh5NFpSVXZQWkl3UWdOTlpC?=
 =?utf-8?B?c0J2STZjSkZ5b0gxYS82dzluM0hKZm82elVaZW41dE14dkdTOWpnS1VoSEpy?=
 =?utf-8?B?alRpUDJVZXpHQ1QxQTgraUdxa0xTQm9IdlhSbmNZNHNQQUJHTkdzNFg2U3Zv?=
 =?utf-8?B?TzUzQThtSFU3VlhjUHd5d2JuVlI5cXpOWDkzVXFYSTBkT1hkSGYzVm9xMFJu?=
 =?utf-8?B?YnNKRU9NeDRJYTE1M0hKS0orT2RTbldzSHdGN09ydTh3d2FTV0VUaUlWUTVq?=
 =?utf-8?B?RE1ERWk3WkF6ZHpZdXBMRUdRSnBLcUN0RVBnRnlYRnhiT0cyNlArSHpSQWdH?=
 =?utf-8?B?ZjZ0VGRPd0Z3S1AxeUpBd0xlekRjM3FzZkd3WnR3KzQxK3NRVVVaeW5XYzFU?=
 =?utf-8?B?SkpEVVdva2ZNNStQb2xXMWpBdnZYZ2RBenQzZ2ttZnhBdHZBMS9ZQlFnNG9N?=
 =?utf-8?B?UUE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 8dff0d19-cd5e-4dda-3235-08dab796cb64
X-MS-Exchange-CrossTenant-AuthSource: CY4PR11MB1862.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 26 Oct 2022 21:12:31.4321
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: PajtCkY+QVyOt14t8WJ+NXmYm/dyRQzS3P52kWtNOByi6nTch+unTKuEZJ8kUrPzSNz3pP5phyXKdFXuLd4N7R2f9ROQd1BpVo38C8iDfwM=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW3PR11MB4762
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.9 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        RCVD_IN_DNSWL_MED,SPF_HELO_PASS,SPF_NONE,URIBL_BLOCKED autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Peter,

On 10/26/2022 1:52 AM, Peter Newman wrote:
> On Tue, Oct 25, 2022 at 5:55 PM James Morse <james.morse@arm.com> wrote:
>> On 21/10/2022 13:42, Peter Newman wrote:
>>> 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?
> 
> Sure, when I think of a solution. For an smp_mb() to be effective above,
> we would need to execute another smp_mb() unconditionally before reading
> the closid/rmid fields when switching a task in.
> 
> The only quick fix I know will work without badly hurting context switch
> time would be to go back to pinging all CPUs following a mass
> task-movement operation.
> 
> I'll see if I can come up with anything better, though.
> 

The original concern is "the stores to t->closid and t->rmid could be
reordered with the task_curr(t) and task_cpu(t) reads which follow". I can see
that issue. Have you considered using the compiler barrier, barrier(), instead?
From what I understand it will prevent the compiler from moving the memory accesses.
This is what is currently done in __rdtgroup_move_task() and could be done here also?

Reinette