Received-SPF: pass (google.com: domain of linux-kernel+bounces-140303-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
From: Miguel Luis <miguel.luis@oracle.com>
To: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
CC: "Rafael J. Wysocki" <rafael@kernel.org>, Len Brown <lenb@kernel.org>,
        "linux-acpi@vger.kernel.org" <linux-acpi@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "rmk+kernel@armlinux.org.uk" <rmk+kernel@armlinux.org.uk>
Subject: Re: [RFC PATCH 4/4] ACPI: processor: refactor acpi_processor_remove:
 isolate acpi_unmap_cpu under CONFIG_ACPI_HOTPLUG_CPU
Thread-Topic: [RFC PATCH 4/4] ACPI: processor: refactor acpi_processor_remove:
 isolate acpi_unmap_cpu under CONFIG_ACPI_HOTPLUG_CPU
Thread-Index: AQHaio+eWHoiqm80uEG8ucHr7P+8e7FhgYiAgAFo0gA=
Date: Thu, 11 Apr 2024 11:02:37 +0000
Message-ID: <25259BC7-8CDB-4714-AEDE-CFFD14986148@oracle.com>
References: <20240409150536.9933-1-miguel.luis@oracle.com>
 <20240409150536.9933-5-miguel.luis@oracle.com>
 <20240410143101.00001f5a@Huawei.com>
In-Reply-To: <20240410143101.00001f5a@Huawei.com>
Accept-Language: en-GB, en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: 
 =?us-ascii?Q?hP59epU5mGbkejuZbUVS0wpy+KcyJBKc1YQNcltZVAaq98rmFbvX6q+qP+7N?=
 =?us-ascii?Q?u6la38TvY1i5pL0KPQ4Y9azpkpPlS1sSy0MbqGYVa18hr5ezLZ+dB4a43B43?=
 =?us-ascii?Q?rWZkpDIA6JF101FhTue7ejiw9Jy/2hxNAs43Tr23rjXG9MD1zVwkZ+GCb8Vi?=
 =?us-ascii?Q?LW69E7KsTExqhhTamMfnMYLR1Zr1ObPVvDkSj3hXgxojFYwmYrnS7mDqx5oa?=
 =?us-ascii?Q?r5TOM0l1TmbH8YacObR2W0WUEq0tbILund9iVw9iWbvUZX8tsqBUY38vTqpU?=
 =?us-ascii?Q?DOEOAnS/aHPNWQPDk5y9aaWxf/rhYDBBrLDF6CGVkezUdeGPM80jLho/ueaD?=
 =?us-ascii?Q?cOjHZlIO3TB0hA0bPCJUrqbxwxlZe0EmVloxsDBAw5+Lgj3BBSxuwqYnY8iL?=
 =?us-ascii?Q?pwAu4yMb4YPIY476kXSY9vhPErXjlIiiwOfDRWIJmBmZ3vk5tlTvChjSFhwi?=
 =?us-ascii?Q?jV152YjcWP25IUz42yIk7/wHEuDEBozZDewsUP6vJtGERSdKp39LRYftdEZA?=
 =?us-ascii?Q?860mpBZvJrtPa5SfJOuexPDIjLeNHzmmXq4+k3qfU+SiAfMNpty5aPTyGB6E?=
 =?us-ascii?Q?6ZyqDg9MB0ULTGYUq4FgSTudRLYxR0l7JdSTC5K7y7ZoSS+TXtyh5V/2DNPq?=
 =?us-ascii?Q?HnzHzURwKIvqFKcCtAAqloJ8JIMRpRsyClKK9ze5M9parW45p2fUJNEgmwzR?=
 =?us-ascii?Q?frigV6lAkOX9LBHh+G++uvySJ3K47VY5oN1e7G2zrBRHGEN57OrRzWZVdZw+?=
 =?us-ascii?Q?qO3ud0wsbxGVjg+Dm/YOlLDMuhJaiIam4C7Gb7LK2YZDl+/0LtucHX1TzloV?=
 =?us-ascii?Q?kzb1PLmy02Cp84O6pR4a+KVXRGq9R5zW2dx9CAdWdKZ3pwD2fuMmSuWO9rt2?=
 =?us-ascii?Q?RqXB1l8Tnt4b+J8khoKe+UJ3dU6x5dNwBfmJUPE1hzkbJrATIC/hPis+hqNR?=
 =?us-ascii?Q?2ubHP1pqG7D9tFsSkd34Jg/PccJXfHQkiv6quHdAHw0K3yUrexG+RNVElCPH?=
 =?us-ascii?Q?qA2wFbAhnuF4lysYAn0RCoJJgfES8+Db1YowfxM+8qaHQEpokp3vMTXLdlQB?=
 =?us-ascii?Q?IstG53QcXSYP0K6gdoNsFASJrRPkTVutXStkP2g5pK1aVdmbyePjKcsqK4pd?=
 =?us-ascii?Q?WhUzBsipJHHDyWe4G+j4AS49RnYZGNIxCtvngLRtA3jqgkHwVD1wWnN8hBXL?=
 =?us-ascii?Q?wmP6OzbvJf1PFEik2USSw1Vwopmc5RH27ZSRMPaapwmdHgqhIMC7jR79fARY?=
 =?us-ascii?Q?oiY4u75N/iB9CkZ5vW2SKmYTJZ5KCP5MyPAYHOLgLCQJF/zbybhznCZ5PNWz?=
 =?us-ascii?Q?AoME5qDxK3fGRMHFKYiEjfKORc5K8cuPCV9rzcvLf+IU4jrrRiUTsdoA+StY?=
 =?us-ascii?Q?WG3iH3Hocx2aQW6DLwnaWRdHp2lICYpZ1SxTVSaN84Jci0J8XKeQl7TYjBOg?=
 =?us-ascii?Q?2XOzrbh4Zf/MoOwVdy3FAWIYVv/3oVd4LTaGcEUBHRyhjRD+GZsnbt1d8wN/?=
 =?us-ascii?Q?VAlX0sMb+5FbMDOkG+kSy2Dx/wH7zIj+qXb+S0aIHXL0j5AwdjipZBr8WOO+?=
 =?us-ascii?Q?66RemyXFLmP40+jRsH0M8PEaqBieKiPo0bypDl7dEeUvynDlZatCQhGUlxwc?=
 =?us-ascii?Q?1Q=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <17906F31F50AD248B0FB2F258884B7F0@namprd10.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	T7/97ciLSjpUU/0VLz9Xm+FT07PYd1asfl88IqT4WPPe6iwqs7AsGjoSoOVR2yNHf91ekgw5wjJxEmjU1Mrt3pY0XGK0LtIJVf+jdVrDqoAclCIhOzPTWiVPPLxSvzaHHCKYrU6Fy9+vjALpncVRsbsKEc/utVcvU0FKfitH74pN4ppFkeLYUncOv2O2vKQTCQ3bVQ4JqJKEG8iBBMd69niIGA6sgNXkhVUSIZrGxyf70yj2RRaObuizyWVuMoCwQZQ+VaUzJ3su0bWQ3iAEHcxcfhFPoKEyn5N7bMphBGhNokEjKMVBgzidinKaGJSgDDK1QqT+NdOlR9EfnAD5Tjf3S054Xk2XmvU2mpIdnrsCBuWKewYixVZqdprmBc+t1fPbU5ZScTCSxFov2SVSO4ina4xKpL8NsPk23//avlh2OqfRLet4cuR5Dbt3wrZbeV0H2Js+io/kjlighbxK3qvbI7vufLGyL8z+iRKxXiUYg2vEXL0YhoU9BelM7INqZOLpHLXYcH5ZxANVO4ERhP5vCmlyPP8q6/55RMQAK/IyEpalcqQL6vCHwTr5hnpKcFpWQaBRxMRZKLp73RuQ2bJ7aT+W1vFCekYKNcvGX4g=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: PH0PR10MB5433.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 8562eb35-19ec-4879-0e32-08dc5a16e61b
X-MS-Exchange-CrossTenant-originalarrivaltime: 11 Apr 2024 11:02:37.5539
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: BnweWAk4Kis8yaq56mJCxsvaRO0jcGlhgzKYaoLRKTEbrF+ZTpj74O0hLfOBP0diQdIE53yfVvgvYnXOU2hYlQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR10MB7174
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.1011,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2024-04-11_04,2024-04-09_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 bulkscore=0 mlxscore=0 adultscore=0
 phishscore=0 spamscore=0 malwarescore=0 mlxlogscore=999 suspectscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2404010000
 definitions=main-2404110079
X-Proofpoint-ORIG-GUID: FXLpLYEXS8dUvruqUYWcqymVkyqY9uGl
X-Proofpoint-GUID: FXLpLYEXS8dUvruqUYWcqymVkyqY9uGl


> On 10 Apr 2024, at 13:31, Jonathan Cameron <Jonathan.Cameron@Huawei.com> =
wrote:
>=20
> On Tue,  9 Apr 2024 15:05:33 +0000
> Miguel Luis <miguel.luis@oracle.com> wrote:
>=20
>> acpi_unmap_cpu is architecture dependent. Isolate it.
>> The pre-processor guard for detach may now be restricted to
>> cpu unmap.
>>=20
>> Signed-off-by: Miguel Luis <miguel.luis@oracle.com>
> Again the why question isn't answered by the patch description.
>=20
> I assume this is to try and resolve the remove question of releasing
> resources that was outstanding on vCPU HP v4 series Russell posted.
>=20
> I've not looked as closely at the remove path as the add one yet, but
> my gut feeling is same issue applies.
> This code that runs in here should not be dependent on whether
> CONFIG_ACPI_HOTPLUG_CPU is enabled or not.

I agree.

>  What we do for the
> make disabled flow should not run a few of the steps in=20
> acpi_processor_remove() we should make that clear by calling
> a different function that doesn't have those steps.
>=20

Perhaps this got answered already elsewhere but is it OK for the detach han=
dler
to be out of CONFIG_ACPI_HOTPLUG_CPU ?

Miguel

>> ---
>> drivers/acpi/acpi_processor.c | 13 +++++++++----
>> 1 file changed, 9 insertions(+), 4 deletions(-)
>>=20
>> diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor=
c
>> index c6e2f64a056b..edcd6a8d4735 100644
>> --- a/drivers/acpi/acpi_processor.c
>> +++ b/drivers/acpi/acpi_processor.c
>> @@ -492,6 +492,14 @@ static int acpi_processor_add(struct acpi_device *d=
evice,
>> }
>>=20
>> #ifdef CONFIG_ACPI_HOTPLUG_CPU
>> +static void acpi_processor_hotunplug_unmap_cpu(struct acpi_processor *p=
r)
>> +{
>> + acpi_unmap_cpu(pr->id);
>> +}
>> +#else
>> +static void acpi_processor_hotunplug_unmap_cpu(struct acpi_processor *p=
r) {}
>> +#endif /* CONFIG_ACPI_HOTPLUG_CPU */
>> +
>> /* Removal */
>> static void acpi_processor_remove(struct acpi_device *device)
>> {
>> @@ -524,7 +532,7 @@ static void acpi_processor_remove(struct acpi_device=
 *device)
>>=20
>> /* Remove the CPU. */
>> arch_unregister_cpu(pr->id);
>> - acpi_unmap_cpu(pr->id);
>> + acpi_processor_hotunplug_unmap_cpu(pr);
>>=20
>> cpus_write_unlock();
>> cpu_maps_update_done();
>> @@ -535,7 +543,6 @@ static void acpi_processor_remove(struct acpi_device=
 *device)
>> free_cpumask_var(pr->throttling.shared_cpu_map);
>> kfree(pr);
>> }
>> -#endif /* CONFIG_ACPI_HOTPLUG_CPU */
>>=20
>> #ifdef CONFIG_ARCH_MIGHT_HAVE_ACPI_PDC
>> bool __init processor_physically_present(acpi_handle handle)
>> @@ -660,9 +667,7 @@ static const struct acpi_device_id processor_device_=
ids[] =3D {
>> static struct acpi_scan_handler processor_handler =3D {
>> .ids =3D processor_device_ids,
>> .attach =3D acpi_processor_add,
>> -#ifdef CONFIG_ACPI_HOTPLUG_CPU
>> .detach =3D acpi_processor_remove,
>> -#endif
>> .hotplug =3D {
>> .enabled =3D true,
>> },
>=20
>=20