BACKPORT: ANDROID: 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 <hridya@google.com> Signed-off-by: T.J. Mercier <tjmercier@google.com> Bug: 206979019 Bug: 254192604 Link: https://lore.kernel.org/lkml/CABdmKX2dNYhgOYdrrJU6-jt6F=LjCidbKhR6t4F7yaa0SPr+-A@mail.gmail.com/T/ Conflicts: include/linux/dma-buf.h 1. The android12-5.10 KMI is frozen, and the modification to struct dma_buf_sysfs_entry in the original patch triggers ABI check failures. Instead of an anonymous union, use the existing struct kobject directly as a work_struct with type punning. (cherry picked from commit fc02d3582a6668bfff235c78f601fca0a15125da https://android.git.corp.google.com/kernel/common android13-5.10) Signed-off-by: T.J. Mercier <tjmercier@google.com> Change-Id: Ic0386849b6b248b0a72215633fc1a50782455bac
This commit is contained in:
@@ -11,6 +11,7 @@
|
|||||||
#include <linux/printk.h>
|
#include <linux/printk.h>
|
||||||
#include <linux/slab.h>
|
#include <linux/slab.h>
|
||||||
#include <linux/sysfs.h>
|
#include <linux/sysfs.h>
|
||||||
|
#include <linux/workqueue.h>
|
||||||
|
|
||||||
#include "dma-buf-sysfs-stats.h"
|
#include "dma-buf-sysfs-stats.h"
|
||||||
|
|
||||||
@@ -135,10 +136,51 @@ void dma_buf_uninit_sysfs_statistics(void)
|
|||||||
kset_unregister(dma_buf_stats_kset);
|
kset_unregister(dma_buf_stats_kset);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
static void sysfs_add_workfn(struct work_struct *work)
|
||||||
|
{
|
||||||
|
/* The ABI would have to change for this to be false, but let's be paranoid. */
|
||||||
|
_Static_assert(sizeof(struct kobject) >= sizeof(struct work_struct),
|
||||||
|
"kobject is smaller than work_struct");
|
||||||
|
|
||||||
|
struct dma_buf_sysfs_entry *sysfs_entry =
|
||||||
|
container_of((struct kobject *)work, struct dma_buf_sysfs_entry, kobj);
|
||||||
|
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
|
||||||
|
* 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)
|
int dma_buf_stats_setup(struct dma_buf *dmabuf)
|
||||||
{
|
{
|
||||||
struct dma_buf_sysfs_entry *sysfs_entry;
|
struct dma_buf_sysfs_entry *sysfs_entry;
|
||||||
int ret;
|
struct work_struct *work;
|
||||||
|
|
||||||
if (!dmabuf || !dmabuf->file)
|
if (!dmabuf || !dmabuf->file)
|
||||||
return -EINVAL;
|
return -EINVAL;
|
||||||
@@ -148,25 +190,21 @@ int dma_buf_stats_setup(struct dma_buf *dmabuf)
|
|||||||
return -EINVAL;
|
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)
|
if (!sysfs_entry)
|
||||||
return -ENOMEM;
|
return -ENOMEM;
|
||||||
|
|
||||||
sysfs_entry->kobj.kset = dma_buf_per_buffer_stats_kset;
|
|
||||||
sysfs_entry->dmabuf = dmabuf;
|
sysfs_entry->dmabuf = dmabuf;
|
||||||
|
|
||||||
dmabuf->sysfs_entry = sysfs_entry;
|
dmabuf->sysfs_entry = sysfs_entry;
|
||||||
|
|
||||||
/* create the directory for buffer stats */
|
/*
|
||||||
ret = kobject_init_and_add(&sysfs_entry->kobj, &dma_buf_ktype, NULL,
|
* The use of kobj as a work_struct is an ugly hack
|
||||||
"%lu", file_inode(dmabuf->file)->i_ino);
|
* to avoid an ABI break in this frozen kernel.
|
||||||
if (ret)
|
*/
|
||||||
goto err_sysfs_dmabuf;
|
work = (struct work_struct *)&dmabuf->sysfs_entry->kobj;
|
||||||
|
INIT_WORK(work, sysfs_add_workfn);
|
||||||
|
get_dma_buf(dmabuf); /* This reference will be dropped in sysfs_add_workfn. */
|
||||||
|
schedule_work(work);
|
||||||
|
|
||||||
return 0;
|
return 0;
|
||||||
|
|
||||||
err_sysfs_dmabuf:
|
|
||||||
kobject_put(&sysfs_entry->kobj);
|
|
||||||
dmabuf->sysfs_entry = NULL;
|
|
||||||
return ret;
|
|
||||||
}
|
}
|
||||||
|
Reference in New Issue
Block a user