From cf175a59effa2f84f03f932da2ea95d548cb159a Mon Sep 17 00:00:00 2001 From: "T.J. Mercier" Date: Tue, 24 May 2022 23:36:36 +0000 Subject: [PATCH] FROMLIST: dma-buf: Move sysfs work out of DMA-BUF export path Recently, we noticed an issue where a process went into direct reclaim while holding the kernfs rw semaphore for sysfs in write (exclusive) mode. This caused processes who were doing DMA-BUF exports and releases to go into uninterruptible sleep since they needed to acquire the same semaphore for the DMA-BUF sysfs entry creation/deletion. In order to avoid blocking DMA-BUF export for an indeterminate amount of time while another process is holding the sysfs rw semaphore in exclusive mode, this patch moves the per-buffer sysfs file creation to the default work queue. Note that this can lead to a short-term inaccuracy in the dmabuf sysfs statistics, but this is a tradeoff to prevent the hot path from being blocked. A work_struct is added to dma_buf to achieve this, but as it is unioned with the kobject in the sysfs_entry, dma_buf does not increase in size. Fixes: bdb8d06dfefd ("dmabuf: Add the capability to expose DMA-BUF stats in sysfs") Originally-by: Hridya Valsaraju Signed-off-by: T.J. Mercier Bug: 206979019 Link: https://lore.kernel.org/lkml/CABdmKX2dNYhgOYdrrJU6-jt6F=LjCidbKhR6t4F7yaa0SPr+-A@mail.gmail.com/T/ Change-Id: Ic0386849b6b248b0a72215633fc1a50782455bac --- drivers/dma-buf/dma-buf-sysfs-stats.c | 56 ++++++++++++++++++++------- include/linux/dma-buf.h | 11 +++++- 2 files changed, 52 insertions(+), 15 deletions(-) diff --git a/drivers/dma-buf/dma-buf-sysfs-stats.c b/drivers/dma-buf/dma-buf-sysfs-stats.c index 053baadcada9..b989b9c79d20 100644 --- a/drivers/dma-buf/dma-buf-sysfs-stats.c +++ b/drivers/dma-buf/dma-buf-sysfs-stats.c @@ -11,6 +11,7 @@ #include #include #include +#include #include "dma-buf-sysfs-stats.h" @@ -168,10 +169,46 @@ void dma_buf_uninit_sysfs_statistics(void) kset_unregister(dma_buf_stats_kset); } +static void sysfs_add_workfn(struct work_struct *work) +{ + struct dma_buf_sysfs_entry *sysfs_entry = + container_of(work, struct dma_buf_sysfs_entry, sysfs_add_work); + struct dma_buf *dmabuf = sysfs_entry->dmabuf; + + /* + * A dmabuf is ref-counted via its file member. If this handler holds the only + * reference to the dmabuf, there is no need for sysfs kobject creation. This is an + * optimization and a race; when the reference count drops to 1 immediately after + * this check it is not harmful as the sysfs entry will still get cleaned up in + * dma_buf_stats_teardown, which won't get called until the final dmabuf reference + * is released, and that can't happen until the end of this function. + */ + if (file_count(dmabuf->file) > 1) { + /* + * kobject_init_and_add expects kobject to be zero-filled, but we have populated it + * (the sysfs_add_work union member) to trigger this work function. + */ + memset(&dmabuf->sysfs_entry->kobj, 0, sizeof(dmabuf->sysfs_entry->kobj)); + dmabuf->sysfs_entry->kobj.kset = dma_buf_per_buffer_stats_kset; + if (kobject_init_and_add(&dmabuf->sysfs_entry->kobj, &dma_buf_ktype, NULL, + "%lu", file_inode(dmabuf->file)->i_ino)) { + kobject_put(&dmabuf->sysfs_entry->kobj); + dmabuf->sysfs_entry = NULL; + } + } else { + /* + * Free the sysfs_entry and reset the pointer so dma_buf_stats_teardown doesn't + * attempt to operate on it. + */ + kfree(dmabuf->sysfs_entry); + dmabuf->sysfs_entry = NULL; + } + dma_buf_put(dmabuf); +} + int dma_buf_stats_setup(struct dma_buf *dmabuf) { struct dma_buf_sysfs_entry *sysfs_entry; - int ret; if (!dmabuf || !dmabuf->file) return -EINVAL; @@ -181,25 +218,16 @@ int dma_buf_stats_setup(struct dma_buf *dmabuf) return -EINVAL; } - sysfs_entry = kzalloc(sizeof(struct dma_buf_sysfs_entry), GFP_KERNEL); + sysfs_entry = kmalloc(sizeof(struct dma_buf_sysfs_entry), GFP_KERNEL); if (!sysfs_entry) return -ENOMEM; - sysfs_entry->kobj.kset = dma_buf_per_buffer_stats_kset; sysfs_entry->dmabuf = dmabuf; - dmabuf->sysfs_entry = sysfs_entry; - /* create the directory for buffer stats */ - ret = kobject_init_and_add(&sysfs_entry->kobj, &dma_buf_ktype, NULL, - "%lu", file_inode(dmabuf->file)->i_ino); - if (ret) - goto err_sysfs_dmabuf; + INIT_WORK(&dmabuf->sysfs_entry->sysfs_add_work, sysfs_add_workfn); + get_dma_buf(dmabuf); /* This reference will be dropped in sysfs_add_workfn. */ + schedule_work(&dmabuf->sysfs_entry->sysfs_add_work); return 0; - -err_sysfs_dmabuf: - kobject_put(&sysfs_entry->kobj); - dmabuf->sysfs_entry = NULL; - return ret; } diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h index c539f4a86728..1b935d19b992 100644 --- a/include/linux/dma-buf.h +++ b/include/linux/dma-buf.h @@ -23,6 +23,7 @@ #include #include #include +#include struct device; struct dma_buf; @@ -517,7 +518,15 @@ struct dma_buf { * `DMA-BUF statistics`_ for the uapi this enables. */ struct dma_buf_sysfs_entry { - struct kobject kobj; + union { + struct kobject kobj; + + /** @sysfs_add_work: + * + * For deferred sysfs kobject creation using a workqueue. + */ + struct work_struct sysfs_add_work; + }; struct dma_buf *dmabuf; } *sysfs_entry; #endif