From nobody Sat Feb 7 09:34:52 2026 Delivered-To: importer@patchew.org Received-SPF: none (zohomail.com: 8.43.85.245 is neither permitted nor denied by domain of lists.libvirt.org) client-ip=8.43.85.245; envelope-from=devel-bounces@lists.libvirt.org; helo=lists.libvirt.org; Authentication-Results: mx.zohomail.com; spf=none (zohomail.com: 8.43.85.245 is neither permitted nor denied by domain of lists.libvirt.org) smtp.mailfrom=devel-bounces@lists.libvirt.org; dmarc=fail(p=reject dis=none) header.from=linux.ibm.com Return-Path: Received: from lists.libvirt.org (lists.libvirt.org [8.43.85.245]) by mx.zohomail.com with SMTPS id 1713538997771567.3840156606298; Fri, 19 Apr 2024 08:03:17 -0700 (PDT) Received: by lists.libvirt.org (Postfix, from userid 996) id 930A61F44; Fri, 19 Apr 2024 11:03:16 -0400 (EDT) Received: from lists.libvirt.org (localhost [IPv6:::1]) by lists.libvirt.org (Postfix) with ESMTP id 538D51E47; Fri, 19 Apr 2024 10:50:59 -0400 (EDT) Received: by lists.libvirt.org (Postfix, from userid 996) id E66B91D7F; Fri, 19 Apr 2024 10:50:15 -0400 (EDT) Received: from mx0b-001b2d01.pphosted.com (mx0b-001b2d01.pphosted.com [148.163.158.5]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by lists.libvirt.org (Postfix) with ESMTPS id A77E51D7F for ; Fri, 19 Apr 2024 10:49:56 -0400 (EDT) Received: from pps.filterd (m0360072.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 43JEnbZJ020853 for ; Fri, 19 Apr 2024 14:49:56 GMT Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3xktq9r01t-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Fri, 19 Apr 2024 14:49:55 +0000 Received: from m0360072.ppops.net (m0360072.ppops.net [127.0.0.1]) by pps.reinject (8.17.1.5/8.17.1.5) with ESMTP id 43JEnt7Q020998 for ; Fri, 19 Apr 2024 14:49:55 GMT Received: from ppma12.dal12v.mail.ibm.com (dc.9e.1632.ip4.static.sl-reverse.com [50.22.158.220]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3xktq9r01q-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 19 Apr 2024 14:49:55 +0000 Received: from pps.filterd (ppma12.dal12v.mail.ibm.com [127.0.0.1]) by ppma12.dal12v.mail.ibm.com (8.17.1.19/8.17.1.19) with ESMTP id 43JCEhtl030331; Fri, 19 Apr 2024 14:49:54 GMT Received: from smtprelay06.fra02v.mail.ibm.com ([9.218.2.230]) by ppma12.dal12v.mail.ibm.com (PPS) with ESMTPS id 3xkbmcm6yn-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 19 Apr 2024 14:49:54 +0000 Received: from smtpav06.fra02v.mail.ibm.com (smtpav06.fra02v.mail.ibm.com [10.20.54.105]) by smtprelay06.fra02v.mail.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 43JEnnaQ21365434 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 19 Apr 2024 14:49:51 GMT Received: from smtpav06.fra02v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 63A4A2005A; Fri, 19 Apr 2024 14:49:49 +0000 (GMT) Received: from smtpav06.fra02v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id E69482004E; Fri, 19 Apr 2024 14:49:48 +0000 (GMT) Received: from li-1de7cd4c-3205-11b2-a85c-d27f97db1fe1.fritz.box (unknown [9.171.43.6]) by smtpav06.fra02v.mail.ibm.com (Postfix) with ESMTP; Fri, 19 Apr 2024 14:49:48 +0000 (GMT) X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on lists.libvirt.org X-Spam-Level: X-Spam-Status: No, score=-0.7 required=5.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE autolearn=unavailable autolearn_force=no version=3.4.4 From: Marc Hartmayer To: Subject: [PATCH v1 16/20] node_device_udev: Use a worker pool for processing events and emitting nodedev event Date: Fri, 19 Apr 2024 16:49:35 +0200 Message-ID: <20240419144939.107773-17-mhartmay@linux.ibm.com> X-Mailer: git-send-email 2.44.0 In-Reply-To: <20240419144939.107773-1-mhartmay@linux.ibm.com> References: <20240419144939.107773-1-mhartmay@linux.ibm.com> MIME-Version: 1.0 X-TM-AS-GCONF: 00 X-Proofpoint-ORIG-GUID: K5HiYqTDFYu7ZQnz6oDS_lp2fpL-5_2a X-Proofpoint-GUID: 88Mv8YRLCnUHSBIcLBa1PBHT4qUuoD-g X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.293,Aquarius:18.0.1011,Hydra:6.0.619,FMLib:17.11.176.26 definitions=2024-04-19_09,2024-04-19_01,2023-05-22_02 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 mlxscore=0 impostorscore=0 lowpriorityscore=0 adultscore=0 phishscore=0 mlxlogscore=999 suspectscore=0 clxscore=1015 priorityscore=1501 malwarescore=0 bulkscore=0 spamscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2404010000 definitions=main-2404190110 Message-ID-Hash: N5XBPFYQLEQBJ3RVK7GWCMT3T2VWRBKW X-Message-ID-Hash: N5XBPFYQLEQBJ3RVK7GWCMT3T2VWRBKW X-MailFrom: mhartmay@linux.ibm.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; header-match-config-1; header-match-config-2; header-match-config-3; header-match-devel.lists.libvirt.org-0; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; suspicious-header CC: Boris Fiuczynski , Jonathon Jongsma X-Mailman-Version: 3.2.2 Precedence: list List-Id: Development discussions about the libvirt library & tools Archived-At: List-Archive: List-Help: List-Post: List-Subscribe: List-Unsubscribe: Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-ZM-MESSAGEID: 1713538998325100001 Use a worker pool for processing the events (e.g. udev, mdevctl config chan= ges) and the initialization instead of a separate initThread and a mdevctl-threa= d. This has the large advantage that we can leverage the job API and now this thread pool is responsible to do all the "costly-work" and emitting the lib= virt nodedev events. Signed-off-by: Marc Hartmayer Reviewed-by: Boris Fiuczynski Reviewed-by: Jonathon Jongsma --- src/node_device/node_device_udev.c | 244 +++++++++++++++++++++-------- 1 file changed, 179 insertions(+), 65 deletions(-) diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_devi= ce_udev.c index e4b1532dc385..67a8b5cd7132 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -43,6 +43,7 @@ #include "virnetdev.h" #include "virmdev.h" #include "virutil.h" +#include "virthreadpool.h" =20 #include "configmake.h" =20 @@ -69,13 +70,13 @@ struct _udevEventData { bool udevThreadQuit; bool udevDataReady; =20 - /* init thread */ - virThread *initThread; - /* Protects @mdevctlMonitors */ virMutex mdevctlLock; GList *mdevctlMonitors; int mdevctlTimeout; + + /* Immutable pointer, self-locking APIs */ + virThreadPool *workerPool; }; =20 static virClass *udevEventDataClass; @@ -86,8 +87,6 @@ udevEventDataDispose(void *obj) struct udev *udev =3D NULL; udevEventData *priv =3D obj; =20 - g_clear_pointer(&priv->initThread, g_free); - VIR_WITH_MUTEX_LOCK_GUARD(&priv->mdevctlLock) { g_list_free_full(g_steal_pointer(&priv->mdevctlMonitors), g_object= _unref); } @@ -100,6 +99,8 @@ udevEventDataDispose(void *obj) udev_unref(udev); } =20 + g_clear_pointer(&priv->workerPool, virThreadPoolFree); + virMutexDestroy(&priv->mdevctlLock); =20 virCondDestroy(&priv->udevThreadCond); @@ -143,6 +144,66 @@ udevEventDataNew(void) return ret; } =20 +typedef enum { + NODE_DEVICE_EVENT_INIT =3D 0, + NODE_DEVICE_EVENT_UDEV_ADD, + NODE_DEVICE_EVENT_UDEV_REMOVE, + NODE_DEVICE_EVENT_UDEV_CHANGE, + NODE_DEVICE_EVENT_UDEV_MOVE, + NODE_DEVICE_EVENT_MDEVCTL_CONFIG_CHANGED, + + NODE_DEVICE_EVENT_LAST +} nodeDeviceEventType; + +struct _nodeDeviceEvent { + nodeDeviceEventType eventType; + void *data; + virFreeCallback dataFreeFunc; +}; +typedef struct _nodeDeviceEvent nodeDeviceEvent; + +static void +nodeDeviceEventFree(nodeDeviceEvent *event) +{ + if (!event) + return; + + if (event->dataFreeFunc) + event->dataFreeFunc(event->data); + g_free(event); +} +G_DEFINE_AUTOPTR_CLEANUP_FUNC(nodeDeviceEvent, nodeDeviceEventFree); + + /** + * nodeDeviceEventSubmit: + * @eventType: the event to be processed + * @data: additional data for the event processor (the pointer is stolen = and it + * will be properly freed using @dataFreeFunc) + * @dataFreeFunc: callback to free @data + * + * Submits @eventType to be processed by the asynchronous event handling + * thread. + */ +static int nodeDeviceEventSubmit(nodeDeviceEventType eventType, void *data= , virFreeCallback dataFreeFunc) +{ + nodeDeviceEvent *event =3D g_new0(nodeDeviceEvent, 1); + udevEventData *priv =3D NULL; + + if (!driver) + return -1; + + priv =3D driver->privateData; + + event->eventType =3D eventType; + event->data =3D data; + event->dataFreeFunc =3D dataFreeFunc; + if (virThreadPoolSendJob(priv->workerPool, 0, event) < 0) { + nodeDeviceEventFree(event); + return -1; + } + return 0; +} + =20 static bool udevHasDeviceProperty(struct udev_device *dev, @@ -1447,7 +1508,7 @@ static void scheduleMdevctlUpdate(udevEventData *data= , bool force); =20 =20 static int -udevRemoveOneDeviceSysPath(virNodeDeviceDriverState *driver_state, const c= har *path) +processNodeDeviceRemoveEvent(virNodeDeviceDriverState *driver_state, const= char *path) { virNodeDeviceObj *obj =3D NULL; virNodeDeviceDef *def; @@ -1529,7 +1590,7 @@ udevSetParent(virNodeDeviceDriverState *driver_state,= struct udev_device *device } =20 static int -udevAddOneDevice(virNodeDeviceDriverState *driver_state, struct udev_devic= e *device) +processNodeDeviceAddAndChangeEvent(virNodeDeviceDriverState *driver_state,= struct udev_device *device) { g_autofree char *sysfs_path =3D NULL; virNodeDeviceDef *def =3D NULL; @@ -1647,7 +1708,7 @@ udevProcessDeviceListEntry(virNodeDeviceDriverState *= driver_state, struct udev * device =3D udev_device_new_from_syspath(udev, name); =20 if (device !=3D NULL) { - if (udevAddOneDevice(driver_state, device) !=3D 0) { + if (processNodeDeviceAddAndChangeEvent(driver_state, device) !=3D = 0) { VIR_DEBUG("Failed to create node device for udev device '%s'", name); } @@ -1755,26 +1816,23 @@ udevHandleOneDevice(struct udev_device *device) =20 VIR_DEBUG("udev action: '%s': %s", action, udev_device_get_syspath(dev= ice)); =20 - if (STREQ(action, "add") || STREQ(action, "change")) - return udevAddOneDevice(driver, device); - - if (STREQ(action, "remove")) { - const char *path =3D udev_device_get_syspath(device); - - return udevRemoveOneDeviceSysPath(driver, path); - } - - if (STREQ(action, "move")) { - const char *devpath_old =3D udevGetDeviceProperty(device, "DEVPATH= _OLD"); - - if (devpath_old) { - g_autofree char *devpath_old_fixed =3D g_strdup_printf("/sys%s= ", devpath_old); - - udevRemoveOneDeviceSysPath(driver, devpath_old_fixed); - } - - return udevAddOneDevice(driver, device); + /* Reference is either released via workerpool logic or at the end of = this + * function. */ + device =3D udev_device_ref(device); + if (STREQ(action, "add")) { + return nodeDeviceEventSubmit(NODE_DEVICE_EVENT_UDEV_ADD, device, + (virFreeCallback)udev_device_unref); + } else if (STREQ(action, "change")) { + return nodeDeviceEventSubmit(NODE_DEVICE_EVENT_UDEV_CHANGE, device, + (virFreeCallback)udev_device_unref); + } else if (STREQ(action, "remove")) { + return nodeDeviceEventSubmit(NODE_DEVICE_EVENT_UDEV_REMOVE, device, + (virFreeCallback)udev_device_unref); + } else if (STREQ(action, "move")) { + return nodeDeviceEventSubmit(NODE_DEVICE_EVENT_UDEV_MOVE, device, + (virFreeCallback)udev_device_unref); } + udev_device_unref(device); =20 return 0; } @@ -1993,23 +2051,23 @@ udevSetupSystemDev(void) =20 =20 static void -nodeStateInitializeEnumerate(void *opaque) +processNodeStateInitializeEnumerate(virNodeDeviceDriverState *driver_state= , void *opaque) { - udevEventData *priv =3D driver->privateData; + udevEventData *priv =3D driver_state->privateData; struct udev *udev =3D opaque; =20 /* Populate with known devices */ - if (udevEnumerateDevices(driver, udev) !=3D 0) + if (udevEnumerateDevices(driver_state, udev) !=3D 0) goto error; /* Load persistent mdevs (which might not be activated yet) and additi= onal * information about active mediated devices from mdevctl */ - if (nodeDeviceUpdateMediatedDevices(driver) !=3D 0) + if (nodeDeviceUpdateMediatedDevices(driver_state) !=3D 0) goto error; =20 cleanup: - VIR_WITH_MUTEX_LOCK_GUARD(&driver->lock) { - driver->initialized =3D true; - virCondBroadcast(&driver->initCond); + VIR_WITH_MUTEX_LOCK_GUARD(&driver_state->lock) { + driver_state->initialized =3D true; + virCondBroadcast(&driver_state->initCond); } =20 return; @@ -2051,31 +2109,16 @@ udevPCITranslateInit(bool privileged G_GNUC_UNUSED) =20 =20 static void -mdevctlUpdateThreadFunc(void *opaque) -{ - virNodeDeviceDriverState *driver_state =3D opaque; - - if (nodeDeviceUpdateMediatedDevices(driver_state) < 0) - VIR_WARN("mdevctl failed to update mediated devices"); -} - - -static void -launchMdevctlUpdateThread(int timer G_GNUC_UNUSED, void *opaque) +submitMdevctlUpdate(int timer G_GNUC_UNUSED, void *opaque) { udevEventData *priv =3D opaque; - virThread thread; =20 if (priv->mdevctlTimeout !=3D -1) { virEventRemoveTimeout(priv->mdevctlTimeout); priv->mdevctlTimeout =3D -1; } =20 - if (virThreadCreateFull(&thread, false, mdevctlUpdateThreadFunc, - "mdevctl-thread", false, driver) < 0) { - virReportSystemError(errno, "%s", - _("failed to create mdevctl thread")); - } + nodeDeviceEventSubmit(NODE_DEVICE_EVENT_MDEVCTL_CONFIG_CHANGED, NULL,= NULL); } =20 =20 @@ -2170,7 +2213,7 @@ mdevctlEnableMonitor(udevEventData *priv) /* Schedules an mdevctl update for 100ms in the future, canceling any exis= ting * timeout that may have been set. In this way, multiple update requests in * quick succession can be collapsed into a single update. if @force is tr= ue, - * an update thread will be spawned immediately. */ + * the worker job is submitted immediately. */ static void scheduleMdevctlUpdate(udevEventData *data, bool force) @@ -2178,12 +2221,12 @@ scheduleMdevctlUpdate(udevEventData *data, if (!force) { if (data->mdevctlTimeout !=3D -1) virEventRemoveTimeout(data->mdevctlTimeout); - data->mdevctlTimeout =3D virEventAddTimeout(100, launchMdevctlUpda= teThread, + data->mdevctlTimeout =3D virEventAddTimeout(100, submitMdevctlUpda= te, data, NULL); return; } =20 - launchMdevctlUpdateThread(-1, data); + submitMdevctlUpdate(-1, data); } =20 =20 @@ -2223,6 +2266,62 @@ mdevctlEventHandleCallback(GFileMonitor *monitor G_G= NUC_UNUSED, } =20 =20 +static void nodeDeviceEventHandler(void *data, void *opaque) +{ + virNodeDeviceDriverState *driver_state =3D opaque; + g_autoptr(nodeDeviceEvent) processEvent =3D data; + + switch (processEvent->eventType) { + case NODE_DEVICE_EVENT_INIT: + { + struct udev *udev =3D processEvent->data; + + processNodeStateInitializeEnumerate(driver_state, udev); + } + break; + case NODE_DEVICE_EVENT_UDEV_ADD: + case NODE_DEVICE_EVENT_UDEV_CHANGE: + { + struct udev_device *device =3D processEvent->data; + + processNodeDeviceAddAndChangeEvent(driver_state, device); + } + break; + case NODE_DEVICE_EVENT_UDEV_REMOVE: + { + struct udev_device *device =3D processEvent->data; + const char *path =3D udev_device_get_syspath(device); + + processNodeDeviceRemoveEvent(driver_state, path); + } + break; + case NODE_DEVICE_EVENT_UDEV_MOVE: + { + struct udev_device *device =3D processEvent->data; + const char *devpath_old =3D udevGetDeviceProperty(device, "DEVPATH= _OLD"); + + if (devpath_old) { + g_autofree char *devpath_old_fixed =3D g_strdup_printf("/sys%s= ", devpath_old); + + processNodeDeviceRemoveEvent(driver_state, devpath_old_fixed); + } + + processNodeDeviceAddAndChangeEvent(driver_state, device); + } + break; + case NODE_DEVICE_EVENT_MDEVCTL_CONFIG_CHANGED: + { + if (nodeDeviceUpdateMediatedDevices(driver_state) < 0) + VIR_WARN("mdevctl failed to update mediated devices"); + } + break; + case NODE_DEVICE_EVENT_LAST: + g_assert_not_reached(); + break; + } +} + + /* Note: It must be safe to call this function even if the driver was not * successfully initialized. This must be considered when changing t= his * function. */ @@ -2258,6 +2357,9 @@ nodeStateShutdownPrepare(void) priv->udevThreadQuit =3D true; virCondSignal(&priv->udevThreadCond); } + + if (priv->workerPool) + virThreadPoolStop(priv->workerPool); return 0; } =20 @@ -2278,11 +2380,19 @@ nodeStateShutdownWait(void) return 0; =20 VIR_WITH_OBJECT_LOCK_GUARD(priv) { - if (priv->initThread) - virThreadJoin(priv->initThread); - if (priv->udevThread) - virThreadJoin(priv->udevThread); + if (priv->mdevctlTimeout !=3D -1) { + virEventRemoveTimeout(priv->mdevctlTimeout); + priv->mdevctlTimeout =3D -1; + } + + if (priv->watch) { + virEventRemoveHandle(priv->watch); + priv->watch =3D -1; + } } + + if (priv->workerPool) + virThreadPoolDrain(priv->workerPool); return 0; } =20 @@ -2353,6 +2463,16 @@ nodeStateInitialize(bool privileged, driver->parserCallbacks.postParse =3D nodeDeviceDefPostParse; driver->parserCallbacks.validate =3D nodeDeviceDefValidate; =20 + /* must be initialized before trying to reconnect to all the running m= devs + * since there might occur some mdevctl monitor events that will be + * dispatched to the worker pool */ + priv->workerPool =3D virThreadPoolNewFull(1, 1, 0, nodeDeviceEventHand= ler, + "nodev-device-event", + NULL, + driver); + if (!priv->workerPool) + goto unlock; + if (udevPCITranslateInit(privileged) < 0) goto unlock; =20 @@ -2410,13 +2530,7 @@ nodeStateInitialize(bool privileged, if (udevSetupSystemDev() !=3D 0) goto cleanup; =20 - priv->initThread =3D g_new0(virThread, 1); - if (virThreadCreateFull(priv->initThread, true, nodeStateInitializeEnu= merate, - "nodedev-init", false, udev) < 0) { - virReportSystemError(errno, "%s", - _("failed to create udev enumerate thread")); - goto cleanup; - } + nodeDeviceEventSubmit(NODE_DEVICE_EVENT_INIT, udev_ref(udev), (virFree= Callback)udev_unref); =20 return VIR_DRV_STATE_INIT_COMPLETE; =20 --=20 2.34.1 _______________________________________________ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-leave@lists.libvirt.org