Received-SPF: pass (google.com: domain of linux-kernel+bounces-70133-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
From: "Wu, Wentong" <wentong.wu@intel.com>
To: Sakari Ailus <sakari.ailus@linux.intel.com>
CC: Hans de Goede <hdegoede@redhat.com>, "Winkler, Tomas"
	<tomas.winkler@intel.com>, Arnd Bergmann <arnd@arndb.de>, Greg Kroah-Hartman
	<gregkh@linuxfoundation.org>, "linux-kernel@vger.kernel.org"
	<linux-kernel@vger.kernel.org>
Subject: RE: [PATCH 1/3] mei: vsc: Call wake_up() and event handler in a
 workqueue
Thread-Topic: [PATCH 1/3] mei: vsc: Call wake_up() and event handler in a
 workqueue
Thread-Index: AQHaXZha9/Lhx161gUKUqkYDIUauW7EPVLgA
Date: Sun, 18 Feb 2024 01:23:30 +0000
Message-ID: <MW5PR11MB5787CBE779AE23D84847EBB48D522@MW5PR11MB5787.namprd11.prod.outlook.com>
References: <20240212094618.344921-1-sakari.ailus@linux.intel.com>
 <20240212094618.344921-2-sakari.ailus@linux.intel.com>
In-Reply-To: <20240212094618.344921-2-sakari.ailus@linux.intel.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?ALHk+IB2zqVwffu/Lh4BGococN0wBci305AclzCZcCgBdUk1dLNXGXUkW8HX?=
 =?us-ascii?Q?HdpVZ0O/l2iPt5Xzh9dczuCFPsTy93a81tVcXYRPK8S9hHifTpQTpwPflkWx?=
 =?us-ascii?Q?7vpvBmWK0bllbtp1X6QdksgrHbFD0lWFKkwU4mrQoDEv5z1nt/CCEoab5V5m?=
 =?us-ascii?Q?zg04O9aer41n2t84L9vJfEHADpJzaT9H+iYAIC3sQnj/NIOvqNwI3t6hk9Rv?=
 =?us-ascii?Q?hzE0a/ZIHtEmOiFK42Fy9v+Nv+LO5BE5boQ9516LVJnUuT95WtGpeuBo12Iw?=
 =?us-ascii?Q?vnUlQsjXzHAryFrLzzaAV6m/ZKvd9d0m/6fllVmNB/luJbJU9GLgHT12WFLU?=
 =?us-ascii?Q?FRWI0ibvI/nQZaY8ZR2S4dR0iguFU9NhCmd4ocg/APITuONbEo3p5X4N/kk+?=
 =?us-ascii?Q?q6T+Jv5j70yzjU87W4wmS2D2oFK16eowqeXaHZfnnAAtXrXt0Vq1SGLv2Q5k?=
 =?us-ascii?Q?r/elu1220lAHMNdUvWsrejQZjnJ345OU/0yeuunu/t7vcQ8UVJ3EwIK4qiVI?=
 =?us-ascii?Q?FB6jYdQsUOpLR9Kd6kqKAevDrP3Q2KYF6jOLhAzG+hiRUvwXOVPAy9i+Mskq?=
 =?us-ascii?Q?G07silp1LfBHYHGFdIRrjW7/e6KrzSFCXkluUGFMCn/LPJ1A7ceIeK4SDEVi?=
 =?us-ascii?Q?Afn2M6fo++swsYx1CJJR5njk9bQxKh79X0Wy46kaZlf9RWK5fi7DmVYIdOaR?=
 =?us-ascii?Q?1yFg3vtDUxuocKqM6zO9FApycQm4mlr1K7RtmfEaLB83WBjwiW2ys+HiE0vb?=
 =?us-ascii?Q?UsBlRSucrxxV9zYyg/BCXwxLyeJquZuiOL9AQkwTwLAnQds0XD1w9FFS2WpV?=
 =?us-ascii?Q?rh3OdeI/WFgOS+v2FiXOtUnsQlu13jHh7K6DAwj9xAkH05fC5Hyl83nOxjjd?=
 =?us-ascii?Q?cKeQSwE3+E4XKaPz1QYM5lOqXTaz6vs7NHuTzV8bMXlRTN0dEAJHgiDyXEiz?=
 =?us-ascii?Q?j+eA+binFv1W8GQjKBSVyQTgHNV9Rb3KXRMF0Z+2wevhrJhM6fQbLmzvNvIa?=
 =?us-ascii?Q?nH0yoehPP6SLOVwNajuQBAe3beF26N5tgZCytn+r50uuTBhcGN2C5dCM17TO?=
 =?us-ascii?Q?A6XTXGIo/Mmshu9VdgaRQjdD+eshltDekA69crIXPa8z8G/5WFd6EQVeHPib?=
 =?us-ascii?Q?cHZL2C9Lr0sJUwuVA91VRDwcA1KFeYWaIWYJnzUColUjwUb1sMMH3NkSlmZp?=
 =?us-ascii?Q?eF6fVqErxuquX4jKZW3EVeekMiKE7pb/oeMCyifIoztdOKx+uf8vIpb26zBY?=
 =?us-ascii?Q?WYAsYT6KJRZaR8AS3jseolE1BFPTq0He0LViegMF3QhF1Zgmy68jjvaUhPis?=
 =?us-ascii?Q?eoDlST5715A1NI4m78ZynrZfmYfhglAY4LHA7YojDsI3CI23RjiuUcpYrDBF?=
 =?us-ascii?Q?Y3kBnFcrkiCrUiVp5L1vLr2Z+w5kdrXqgxxoj4kwJ2R10fEIgfLbK44Ww/Lv?=
 =?us-ascii?Q?AzYgh8dSre7chGh9H8ITILlk7M43IFXHtDXEI2VA5Enr6NFj7MXnszATWM9r?=
 =?us-ascii?Q?bIlpCZRfULB0bUQPazSchPA4Ff0VvhVJR5IwqKCvKUl5+hzXlbQYOVSKXdgs?=
 =?us-ascii?Q?SZxz8xmYz+xf61G2MIZZh6P0fdP0NIv+ogIegCol?=
Content-Type: text/plain; charset="us-ascii"
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-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: MW5PR11MB5787.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: d7d440de-7681-42d1-f529-08dc30203799
X-MS-Exchange-CrossTenant-originalarrivaltime: 18 Feb 2024 01:23:30.9639
 (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: TxUuU31WT8wVyT8pjfvpWj7TEZ2xElRYEtqr2d6gK95+CxVaeAwQFSDo/3qqnygesQ0jLtUvD+qm/NBvze8MYw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR11MB4727
X-OriginatorOrg: intel.com

Hi Sakari,

Thanks, and sorry for the late response because I'm in vacation this week.

> From: Sakari Ailus <sakari.ailus@linux.intel.com>
>=20
> The event handler, in this case that of mei_vsc_event_cb() of platform-vs=
c.c,
> is called from a threaded interrupt handler in uninterruptible context.

But why this thread is uninterruptible?

https://github.com/torvalds/linux/blob/master/kernel/irq/manage.c#L1294

https://lwn.net/Articles/302043/

BR,
Wentong
> However there are multiple places where the handler may sleep. This patch
> creates a per-device workqueue and calls
> wake_up() and the event callback from queued work where sleeping is
> allowed.
>=20
> Fixes: 566f5ca97680 ("mei: Add transport driver for IVSC device")
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/misc/mei/vsc-tp.c | 35 ++++++++++++++++++++++++-----------
>  1 file changed, 24 insertions(+), 11 deletions(-)
>=20
> diff --git a/drivers/misc/mei/vsc-tp.c b/drivers/misc/mei/vsc-tp.c index
> 6f4a4be6ccb5..264ece72f0bf 100644
> --- a/drivers/misc/mei/vsc-tp.c
> +++ b/drivers/misc/mei/vsc-tp.c
> @@ -72,6 +72,8 @@ struct vsc_tp {
>  	atomic_t assert_cnt;
>  	wait_queue_head_t xfer_wait;
>=20
> +	struct workqueue_struct *event_workqueue;
> +	struct work_struct event_work;
>  	vsc_tp_event_cb_t event_notify;
>  	void *event_notify_context;
>=20
> @@ -416,19 +418,19 @@ static irqreturn_t vsc_tp_isr(int irq, void *data)
>=20
>  	atomic_inc(&tp->assert_cnt);
>=20
> -	wake_up(&tp->xfer_wait);
> +	queue_work(tp->event_workqueue, &tp->event_work);
>=20
> -	return IRQ_WAKE_THREAD;
> +	return IRQ_HANDLED;
>  }
>=20
> -static irqreturn_t vsc_tp_thread_isr(int irq, void *data)
> +static void vsc_tp_event_work(struct work_struct *work)
>  {
> -	struct vsc_tp *tp =3D data;
> +	struct vsc_tp *tp =3D container_of(work, struct vsc_tp, event_work);;
> +
> +	wake_up(&tp->xfer_wait);
>=20
>  	if (tp->event_notify)
>  		tp->event_notify(tp->event_notify_context);
> -
> -	return IRQ_HANDLED;
>  }
>=20
>  static int vsc_tp_match_any(struct acpi_device *adev, void *data) @@ -
> 481,13 +483,18 @@ static int vsc_tp_probe(struct spi_device *spi)
>  	init_waitqueue_head(&tp->xfer_wait);
>  	tp->spi =3D spi;
>=20
> +	tp->event_workqueue =3D
> create_singlethread_workqueue(dev_name(dev));
> +	if (!tp->event_workqueue)
> +		return -ENOMEM;
> +
> +	INIT_WORK(&tp->event_work, vsc_tp_event_work);
> +
>  	irq_set_status_flags(spi->irq, IRQ_DISABLE_UNLAZY);
> -	ret =3D devm_request_threaded_irq(dev, spi->irq, vsc_tp_isr,
> -					vsc_tp_thread_isr,
> -					IRQF_TRIGGER_FALLING |
> IRQF_ONESHOT,
> -					dev_name(dev), tp);
> +	ret =3D devm_request_irq(dev, spi->irq, vsc_tp_isr,
> +			       IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> +			       dev_name(dev), tp);
>  	if (ret)
> -		return ret;
> +		goto err_destroy_workqueue;
>=20
>  	mutex_init(&tp->mutex);
>=20
> @@ -516,6 +523,10 @@ static int vsc_tp_probe(struct spi_device *spi)
>=20
>  	return 0;
>=20
> +err_destroy_workqueue:
> +	destroy_workqueue(tp->event_workqueue);
> +	kfree(tp->event_workqueue);
> +
>  err_destroy_lock:
>  	mutex_destroy(&tp->mutex);
>=20
> @@ -528,6 +539,8 @@ static void vsc_tp_remove(struct spi_device *spi)
>=20
>  	platform_device_unregister(tp->pdev);
>=20
> +	destroy_workqueue(tp->event_workqueue);
> +	kfree(tp->event_workqueue);
>  	mutex_destroy(&tp->mutex);
>  }
>=20
> --
> 2.39.2