From nobody Fri May 8 06:00:32 2026 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id D191EC433F5 for ; Tue, 10 May 2022 05:50:48 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236825AbiEJFyl (ORCPT ); Tue, 10 May 2022 01:54:41 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51356 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232053AbiEJFyi (ORCPT ); Tue, 10 May 2022 01:54:38 -0400 Received: from out30-56.freemail.mail.aliyun.com (out30-56.freemail.mail.aliyun.com [115.124.30.56]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id BC1123B28C for ; Mon, 9 May 2022 22:50:39 -0700 (PDT) X-Alimail-AntiSpam: AC=PASS;BC=-1|-1;BR=01201311R421e4;CH=green;DM=||false|;DS=||;FP=0|-1|-1|-1|0|-1|-1|-1;HT=e01e04426;MF=kanie@linux.alibaba.com;NM=1;PH=DS;RN=2;SR=0;TI=SMTPD_---0VCpEu9u_1652161831; Received: from localhost(mailfrom:kanie@linux.alibaba.com fp:SMTPD_---0VCpEu9u_1652161831) by smtp.aliyun-inc.com(127.0.0.1); Tue, 10 May 2022 13:50:37 +0800 From: Guixin Liu To: gregkh@linuxfoundation.org Cc: linux-kernel@vger.kernel.org Subject: [PATCH] uio: Replace mutex info_lock with percpu_ref Date: Tue, 10 May 2022 13:50:31 +0800 Message-Id: <1652161831-77791-1-git-send-email-kanie@linux.alibaba.com> X-Mailer: git-send-email 1.8.3.1 Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" If the underlying driver works in parallel, the mutex info_lock in uio=20 will force driver to work sequentially, so that become performance=20 bottleneck. Lets replace it with percpu_ref for better performance.=20 Use tcm_loop and tcmu(backstore is file, and I did some work to make tcmu work in parallel at uio_write() path) to evaluate performance, fio job: fio -filename=3D/dev/sdb -direct=3D1 -size=3D2G -name=3D1 -thread -runtime=3D60 -time_based -rw=3Drandread -numjobs=3D16 -iodepth=3D16 -bs= =3D128k Without this patch: READ: bw=3D2828MiB/s (2965MB/s), 176MiB/s-177MiB/s (185MB/s-186MB/s),=20 io=3D166GiB (178GB), run=3D60000-60001msec With this patch: READ: bw=3D3382MiB/s (3546MB/s), 211MiB/s-212MiB/s (221MB/s-222MB/s),=20 io=3D198GiB (213GB), run=3D60001-60001msec Reviewed-by: Xiaoguang Wang Signed-off-by: Guixin Liu Reported-by: kernel test robot --- drivers/uio/uio.c | 95 ++++++++++++++++++++++++++++++++++--------= ---- include/linux/uio_driver.h | 5 ++- 2 files changed, 75 insertions(+), 25 deletions(-) diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c index 43afbb7..72c16ba 100644 --- a/drivers/uio/uio.c +++ b/drivers/uio/uio.c @@ -24,6 +24,8 @@ #include #include #include +#include +#include =20 #define UIO_MAX_DEVICES (1U << MINORBITS) =20 @@ -218,7 +220,9 @@ static ssize_t name_show(struct device *dev, struct uio_device *idev =3D dev_get_drvdata(dev); int ret; =20 - mutex_lock(&idev->info_lock); + if (!percpu_ref_tryget_live(&idev->info_ref)) + return -EINVAL; + if (!idev->info) { ret =3D -EINVAL; dev_err(dev, "the device has been unregistered\n"); @@ -228,7 +232,7 @@ static ssize_t name_show(struct device *dev, ret =3D sprintf(buf, "%s\n", idev->info->name); =20 out: - mutex_unlock(&idev->info_lock); + percpu_ref_put(&idev->info_ref); return ret; } static DEVICE_ATTR_RO(name); @@ -239,7 +243,9 @@ static ssize_t version_show(struct device *dev, struct uio_device *idev =3D dev_get_drvdata(dev); int ret; =20 - mutex_lock(&idev->info_lock); + if (!percpu_ref_tryget_live(&idev->info_ref)) + return -EINVAL; + if (!idev->info) { ret =3D -EINVAL; dev_err(dev, "the device has been unregistered\n"); @@ -249,7 +255,7 @@ static ssize_t version_show(struct device *dev, ret =3D sprintf(buf, "%s\n", idev->info->version); =20 out: - mutex_unlock(&idev->info_lock); + percpu_ref_put(&idev->info_ref); return ret; } static DEVICE_ATTR_RO(version); @@ -489,16 +495,20 @@ static int uio_open(struct inode *inode, struct file = *filep) listener->event_count =3D atomic_read(&idev->event); filep->private_data =3D listener; =20 - mutex_lock(&idev->info_lock); + if (!percpu_ref_tryget_live(&idev->info_ref)) { + ret =3D -EINVAL; + goto err_alloc_listener; + } + if (!idev->info) { - mutex_unlock(&idev->info_lock); + percpu_ref_put(&idev->info_ref); ret =3D -EINVAL; goto err_infoopen; } =20 if (idev->info->open) ret =3D idev->info->open(idev->info, inode); - mutex_unlock(&idev->info_lock); + percpu_ref_put(&idev->info_ref); if (ret) goto err_infoopen; =20 @@ -531,10 +541,12 @@ static int uio_release(struct inode *inode, struct fi= le *filep) struct uio_listener *listener =3D filep->private_data; struct uio_device *idev =3D listener->dev; =20 - mutex_lock(&idev->info_lock); + if (!percpu_ref_tryget_live(&idev->info_ref)) + return -EINVAL; + if (idev->info && idev->info->release) ret =3D idev->info->release(idev->info, inode); - mutex_unlock(&idev->info_lock); + percpu_ref_put(&idev->info_ref); =20 module_put(idev->owner); kfree(listener); @@ -548,10 +560,12 @@ static __poll_t uio_poll(struct file *filep, poll_tab= le *wait) struct uio_device *idev =3D listener->dev; __poll_t ret =3D 0; =20 - mutex_lock(&idev->info_lock); + if (!percpu_ref_tryget_live(&idev->info_ref)) + return -EIO; + if (!idev->info || !idev->info->irq) ret =3D -EIO; - mutex_unlock(&idev->info_lock); + percpu_ref_put(&idev->info_ref); =20 if (ret) return ret; @@ -577,13 +591,17 @@ static ssize_t uio_read(struct file *filep, char __us= er *buf, add_wait_queue(&idev->wait, &wait); =20 do { - mutex_lock(&idev->info_lock); + if (!percpu_ref_tryget_live(&idev->info_ref)) { + retval =3D -EINVAL; + break; + } + if (!idev->info || !idev->info->irq) { retval =3D -EIO; - mutex_unlock(&idev->info_lock); + percpu_ref_put(&idev->info_ref); break; } - mutex_unlock(&idev->info_lock); + percpu_ref_put(&idev->info_ref); =20 set_current_state(TASK_INTERRUPTIBLE); =20 @@ -631,7 +649,9 @@ static ssize_t uio_write(struct file *filep, const char= __user *buf, if (copy_from_user(&irq_on, buf, count)) return -EFAULT; =20 - mutex_lock(&idev->info_lock); + if (!percpu_ref_tryget_live(&idev->info_ref)) + return -EINVAL; + if (!idev->info) { retval =3D -EINVAL; goto out; @@ -650,7 +670,7 @@ static ssize_t uio_write(struct file *filep, const char= __user *buf, retval =3D idev->info->irqcontrol(idev->info, irq_on); =20 out: - mutex_unlock(&idev->info_lock); + percpu_ref_put(&idev->info_ref); return retval ? retval : sizeof(s32); } =20 @@ -675,7 +695,9 @@ static vm_fault_t uio_vma_fault(struct vm_fault *vmf) vm_fault_t ret =3D 0; int mi; =20 - mutex_lock(&idev->info_lock); + if (!percpu_ref_tryget_live(&idev->info_ref)) + return VM_FAULT_SIGBUS; + if (!idev->info) { ret =3D VM_FAULT_SIGBUS; goto out; @@ -702,8 +724,7 @@ static vm_fault_t uio_vma_fault(struct vm_fault *vmf) vmf->page =3D page; =20 out: - mutex_unlock(&idev->info_lock); - + percpu_ref_put(&idev->info_ref); return ret; } =20 @@ -772,7 +793,9 @@ static int uio_mmap(struct file *filep, struct vm_area_= struct *vma) =20 vma->vm_private_data =3D idev; =20 - mutex_lock(&idev->info_lock); + if (!percpu_ref_tryget_live(&idev->info_ref)) + return -EINVAL; + if (!idev->info) { ret =3D -EINVAL; goto out; @@ -811,7 +834,7 @@ static int uio_mmap(struct file *filep, struct vm_area_= struct *vma) } =20 out: - mutex_unlock(&idev->info_lock); + percpu_ref_put(&idev->info_ref); return ret; } =20 @@ -907,6 +930,13 @@ static void uio_device_release(struct device *dev) kfree(idev); } =20 +static void uio_info_free(struct percpu_ref *ref) +{ + struct uio_device *idev =3D container_of(ref, struct uio_device, info_ref= ); + + complete(&idev->free_done); +} + /** * __uio_register_device - register a new userspace IO device * @owner: module that creates the new device @@ -937,10 +967,17 @@ int __uio_register_device(struct module *owner, =20 idev->owner =3D owner; idev->info =3D info; - mutex_init(&idev->info_lock); init_waitqueue_head(&idev->wait); atomic_set(&idev->event, 0); =20 + ret =3D percpu_ref_init(&idev->info_ref, uio_info_free, 0, GFP_KERNEL); + if (ret) { + pr_err("percpu_ref init failed!\n"); + return ret; + } + init_completion(&idev->confirm_done); + init_completion(&idev->free_done); + ret =3D uio_get_minor(idev); if (ret) { kfree(idev); @@ -1036,6 +1073,13 @@ int __devm_uio_register_device(struct module *owner, } EXPORT_SYMBOL_GPL(__devm_uio_register_device); =20 +static void uio_confirm_info(struct percpu_ref *ref) +{ + struct uio_device *idev =3D container_of(ref, struct uio_device, info_ref= ); + + complete(&idev->confirm_done); +} + /** * uio_unregister_device - unregister a industrial IO device * @info: UIO device capabilities @@ -1052,14 +1096,17 @@ void uio_unregister_device(struct uio_info *info) idev =3D info->uio_dev; minor =3D idev->minor; =20 - mutex_lock(&idev->info_lock); + percpu_ref_kill_and_confirm(&idev->info_ref, uio_confirm_info); + wait_for_completion(&idev->confirm_done); + wait_for_completion(&idev->free_done); + + /* now, we can set info to NULL */ uio_dev_del_attributes(idev); =20 if (info->irq && info->irq !=3D UIO_IRQ_CUSTOM) free_irq(info->irq, idev); =20 idev->info =3D NULL; - mutex_unlock(&idev->info_lock); =20 wake_up_interruptible(&idev->wait); kill_fasync(&idev->async_queue, SIGIO, POLL_HUP); diff --git a/include/linux/uio_driver.h b/include/linux/uio_driver.h index 47c5962..6d3d87f 100644 --- a/include/linux/uio_driver.h +++ b/include/linux/uio_driver.h @@ -16,6 +16,7 @@ #include #include #include +#include =20 struct module; struct uio_map; @@ -74,9 +75,11 @@ struct uio_device { struct fasync_struct *async_queue; wait_queue_head_t wait; struct uio_info *info; - struct mutex info_lock; struct kobject *map_dir; struct kobject *portio_dir; + struct percpu_ref info_ref; + struct completion confirm_done; + struct completion free_done; }; =20 /** --=20 1.8.3.1