Received-SPF: pass (google.com: domain of linux-kernel+bounces-50455-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Fri, 2 Feb 2024 10:58:08 -0800
From: Dan Williams <dan.j.williams@intel.com>
To: Ira Weiny <ira.weiny@intel.com>, "Rafael J. Wysocki" <rafael@kernel.org>,
	Dan Williams <dan.j.williams@intel.com>, Jonathan Cameron
	<jonathan.cameron@huawei.com>
CC: <linux-acpi@vger.kernel.org>, <linux-cxl@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, Dan Carpenter <dan.carpenter@linaro.org>,
	"Ira Weiny" <ira.weiny@intel.com>
Subject: Re: [PATCH] acpi/ghes: Prevent sleeping with spinlock held
Message-ID: <65bd3b4073afd_7193229489@dwillia2-mobl3.amr.corp.intel.com.notmuch>
References: <20240202-cxl-cper-smatch-v1-1-7a4103c7f5a0@intel.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20240202-cxl-cper-smatch-v1-1-7a4103c7f5a0@intel.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?Dzlmq4qIVXGJR3YZeZP3qVnu4WZzj8WFk6kQ79n0VKTmya6GzWeAncfSavg1?=
 =?us-ascii?Q?aUHPZJeiSF3dWptaGjA0pKOA/WCHmEgbJ/yjMW1m8/8X4qptOzAXhI92QVoY?=
 =?us-ascii?Q?rQdIbhBaEhb/LnsRCurppoA1rJD9OwM8ebuLEisHC2wJWIixZbQsbqku2nDp?=
 =?us-ascii?Q?r4gVQjqX+LAUAGDlLU3sH+pfBZhcSOqsk747cAGyjcWKGPE/2a5XlnFhNy/r?=
 =?us-ascii?Q?QFEECQsUDz08/mwmkWtaoUK4FuNnGWv3JeQmIeLvpkf1sBcLRdBw0a5tlnv6?=
 =?us-ascii?Q?Q34wDZoBwZRfj+IhVj+X9TWlGF4e9b35AIq9ZF55wBqZqrRqkQbyUrM7rwez?=
 =?us-ascii?Q?WIdMNfgivduhL+tYGVb2pUkqhagjS4sFlAj5WHQ1XO520leKHyqsEPsJljD7?=
 =?us-ascii?Q?y36I94jugLYhxGbAYcaMSAXrcNy/91f0cxytl+YfI5lIwwboXXE3w1ISFiyH?=
 =?us-ascii?Q?HbxE0IbbcNQ5vsQ6VkNs4iycqTi+VhGPi1VxAC5uohq+5N3TAauixOiJdgZ7?=
 =?us-ascii?Q?zfji26ZHsaU0CbDAAEE/zXdbMyLLt4qeICRQ14rG9Dpleg2/QDvXcmQb7HhJ?=
 =?us-ascii?Q?Fk/ix3tatJPbZ1ELiExqXstzWWadb2LXk8R7YZ1Aw3hS3G8gO9E1qU6UKh/0?=
 =?us-ascii?Q?HV8Bq4cCNW8dYe3i15iSHLeW3zIkoWrlthIVmAkRhx0IkQJ5LCVfqhQFMLWV?=
 =?us-ascii?Q?rLeScLVyukk7eOJnCJS/p6V3McjqYevKtw9Wqe8rqYXMynvO5r2KRAcmhcit?=
 =?us-ascii?Q?Y9ZZaXfRvQWDv+KDOfuKXdPew6WF19Y58P3SiX+jUvvvVzOfK6Aa2lKg0Aw6?=
 =?us-ascii?Q?nCWM5KhJxmByakwNHLnY3HK2d5uIYTMiZHtI9zVIygyQ3KApIaOi+lF0oezI?=
 =?us-ascii?Q?cbKeVcGSF7+MuUhlU+b7FBmH7mFmIWooOiRujoi6zuaKqQvCop61MjpvmGt5?=
 =?us-ascii?Q?gkCj0jGy+unlSFcnCsj7W4k0KkT7GC9KHg0uH6WgT8jinLS1KwWD+xy2FckD?=
 =?us-ascii?Q?PJ+BOpUH2WbFKir4iFSHWQo74bgh/y2cu2I5ReEkzwi9IOC9gmXwRgukOs8p?=
 =?us-ascii?Q?zDMPYNsA0qaOEepmYOZBv4t04N9lPxCSIilKWfmaSmGx7MGT+EQl9V3bJnjw?=
 =?us-ascii?Q?5WNABgt5E0k0Y+vk1lX7FuXG1uoVHOnWhF+V7ZSwzguqBcFttkAp2qFnBDS8?=
 =?us-ascii?Q?KvZ9HfdZlnS0eHejiFN0gG9aOo97ORRFejrwkOp8bYAp5AT9PcbqQu5yX3f+?=
 =?us-ascii?Q?8CjxQhYHHKkSfL7rRSANEgdpXKE2BUvBoPsOa1iJDBfD5VXIoF1TlerDy+vg?=
 =?us-ascii?Q?W0/aJdcl/obIOyZZufAPHQAZlfvYuL2VEUBazEurHZy5KkgTZ1u99fnu3Em4?=
 =?us-ascii?Q?m0LA4rsnIPJevxzXFU5XHmd4vD4pdOERSFMKGlWssyAsS9pPJif6N4ipZXEw?=
 =?us-ascii?Q?44rk3WpR7tDFOqS3BruMFkvDQL49mXWugQoCVVv/etFtzfohikJ25fnjoRgJ?=
 =?us-ascii?Q?jlRDmf0VpuLKqHj+ehYf2CvwmAVKqQNm3r5eHJNCwbra9kTX4MM+miQuZbTx?=
 =?us-ascii?Q?8My8kr+fLmFNL2betZqGz0/QTO95kJOicSoM1VFU1ZCKL0a4MCXKleHlraEi?=
 =?us-ascii?Q?uw=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 234091a1-6b77-42a1-191c-08dc2420e6ce
X-MS-Exchange-CrossTenant-AuthSource: PH8PR11MB8107.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 02 Feb 2024 18:58:11.1729
 (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: tlhOP2jK9pWfHd6sU7Crcp6+H9NvJVlSIl3afngG5mZkS+T+rm+yh2DRq6a74qJ/wl6JyG0OdtXP8dDtuSJeAiXD3gs5BVyveJlAyBBi2k4=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR11MB6378
X-OriginatorOrg: intel.com

Ira Weiny wrote:
> Smatch caught that cxl_cper_post_event() is called with a spinlock held
> or preemption disabled.[1]  There is no need for the callback to sleep.
> 
> Replace the RW semaphore with a RW lock.
> 
> A static call was considered but ARM does not select HAVE_STATIC_CALL
> and in that case setting the function pointer uses a RW semaphore.
> 
> [1] https://lore.kernel.org/all/b963c490-2c13-4b79-bbe7-34c6568423c7@moroto.mountain/
> 
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> ---
>  drivers/acpi/apei/ghes.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index 7b7c605166e0..bdc0ec2813a3 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
> @@ -39,6 +39,7 @@
>  #include <linux/aer.h>
>  #include <linux/nmi.h>
>  #include <linux/sched/clock.h>
> +#include <linux/spinlock.h>
>  #include <linux/uuid.h>
>  #include <linux/ras.h>
>  #include <linux/task_work.h>
> @@ -677,7 +678,7 @@ static void ghes_defer_non_standard_event(struct acpi_hest_generic_data *gdata,
>  /*
>   * Only a single callback can be registered for CXL CPER events.
>   */
> -static DECLARE_RWSEM(cxl_cper_rw_sem);
> +static DEFINE_RWLOCK(cxl_callback_lock);
>  static cxl_cper_callback cper_callback;
>  
>  /* CXL Event record UUIDs are formatted as GUIDs and reported in section type */
> @@ -721,14 +722,14 @@ static void cxl_cper_post_event(enum cxl_event_type event_type,
>  		return;
>  	}
>  
> -	guard(rwsem_read)(&cxl_cper_rw_sem);
> +	guard(read_lock_irqsave)(&cxl_callback_lock);
>  	if (cper_callback)
>  		cper_callback(event_type, rec);

This does not help because cxl_cper_event_call() takes a sleeping
device_lock().