The DMA-BUF attachment statistics form a subset of the DMA-BUF sysfs statistics that recently merged to the drm-misc tree. They are not UABI yet since they have not merged to the upstream Linux kernel.
Since there has been a reported a performance regression due to the overhead of sysfs directory creation/teardown during dma_buf_attach()/dma_buf_detach(), this patch deletes the DMA-BUF attachment statistics from sysfs.
Fixes: bdb8d06dfefd (dmabuf: Add the capability to expose DMA-BUF stats in sysfs) Signed-off-by: Hridya Valsaraju hridya@google.com Reviewed-by: Christian König christian.koenig@amd.com Reviewed-by: Greg Kroah-Hartman gregkh@linuxfoundation.org ---
Changes in v2: Updated commit message to clarify that the sysfs files being removed have not yet merged to upstream Linux and are hence not ABI.
Hi Christian,
I have updated the commit message as per your suggestion. Please do take another look when you get a chance.
Thanks, Hridya
.../ABI/testing/sysfs-kernel-dmabuf-buffers | 28 ---- drivers/dma-buf/dma-buf-sysfs-stats.c | 140 +----------------- drivers/dma-buf/dma-buf-sysfs-stats.h | 27 ---- drivers/dma-buf/dma-buf.c | 16 -- include/linux/dma-buf.h | 17 --- 5 files changed, 4 insertions(+), 224 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers b/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers index a243984ed420..5d3bc997dc64 100644 --- a/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers +++ b/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers @@ -22,31 +22,3 @@ KernelVersion: v5.13 Contact: Hridya Valsaraju hridya@google.com Description: This file is read-only and specifies the size of the DMA-BUF in bytes. - -What: /sys/kernel/dmabuf/buffers/<inode_number>/attachments -Date: May 2021 -KernelVersion: v5.13 -Contact: Hridya Valsaraju hridya@google.com -Description: This directory will contain subdirectories representing every - attachment of the DMA-BUF. - -What: /sys/kernel/dmabuf/buffers/<inode_number>/attachments/<attachment_uid> -Date: May 2021 -KernelVersion: v5.13 -Contact: Hridya Valsaraju hridya@google.com -Description: This directory will contain information on the attached device - and the number of current distinct device mappings. - -What: /sys/kernel/dmabuf/buffers/<inode_number>/attachments/<attachment_uid>/device -Date: May 2021 -KernelVersion: v5.13 -Contact: Hridya Valsaraju hridya@google.com -Description: This file is read-only and is a symlink to the attached device's - sysfs entry. - -What: /sys/kernel/dmabuf/buffers/<inode_number>/attachments/<attachment_uid>/map_counter -Date: May 2021 -KernelVersion: v5.13 -Contact: Hridya Valsaraju hridya@google.com -Description: This file is read-only and contains a map_counter indicating the - number of distinct device mappings of the attachment. diff --git a/drivers/dma-buf/dma-buf-sysfs-stats.c b/drivers/dma-buf/dma-buf-sysfs-stats.c index a2638e84199c..053baadcada9 100644 --- a/drivers/dma-buf/dma-buf-sysfs-stats.c +++ b/drivers/dma-buf/dma-buf-sysfs-stats.c @@ -40,14 +40,11 @@ * * * ``/sys/kernel/dmabuf/buffers/<inode_number>/exporter_name`` * * ``/sys/kernel/dmabuf/buffers/<inode_number>/size`` - * * ``/sys/kernel/dmabuf/buffers/<inode_number>/attachments/<attach_uid>/device`` - * * ``/sys/kernel/dmabuf/buffers/<inode_number>/attachments/<attach_uid>/map_counter`` * - * The information in the interface can also be used to derive per-exporter and - * per-device usage statistics. The data from the interface can be gathered - * on error conditions or other important events to provide a snapshot of - * DMA-BUF usage. It can also be collected periodically by telemetry to monitor - * various metrics. + * The information in the interface can also be used to derive per-exporter + * statistics. The data from the interface can be gathered on error conditions + * or other important events to provide a snapshot of DMA-BUF usage. + * It can also be collected periodically by telemetry to monitor various metrics. * * Detailed documentation about the interface is present in * Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers. @@ -121,120 +118,6 @@ static struct kobj_type dma_buf_ktype = { .default_groups = dma_buf_stats_default_groups, };
-#define to_dma_buf_attach_entry_from_kobj(x) container_of(x, struct dma_buf_attach_sysfs_entry, kobj) - -struct dma_buf_attach_stats_attribute { - struct attribute attr; - ssize_t (*show)(struct dma_buf_attach_sysfs_entry *sysfs_entry, - struct dma_buf_attach_stats_attribute *attr, char *buf); -}; -#define to_dma_buf_attach_stats_attr(x) container_of(x, struct dma_buf_attach_stats_attribute, attr) - -static ssize_t dma_buf_attach_stats_attribute_show(struct kobject *kobj, - struct attribute *attr, - char *buf) -{ - struct dma_buf_attach_stats_attribute *attribute; - struct dma_buf_attach_sysfs_entry *sysfs_entry; - - attribute = to_dma_buf_attach_stats_attr(attr); - sysfs_entry = to_dma_buf_attach_entry_from_kobj(kobj); - - if (!attribute->show) - return -EIO; - - return attribute->show(sysfs_entry, attribute, buf); -} - -static const struct sysfs_ops dma_buf_attach_stats_sysfs_ops = { - .show = dma_buf_attach_stats_attribute_show, -}; - -static ssize_t map_counter_show(struct dma_buf_attach_sysfs_entry *sysfs_entry, - struct dma_buf_attach_stats_attribute *attr, - char *buf) -{ - return sysfs_emit(buf, "%u\n", sysfs_entry->map_counter); -} - -static struct dma_buf_attach_stats_attribute map_counter_attribute = - __ATTR_RO(map_counter); - -static struct attribute *dma_buf_attach_stats_default_attrs[] = { - &map_counter_attribute.attr, - NULL, -}; -ATTRIBUTE_GROUPS(dma_buf_attach_stats_default); - -static void dma_buf_attach_sysfs_release(struct kobject *kobj) -{ - struct dma_buf_attach_sysfs_entry *sysfs_entry; - - sysfs_entry = to_dma_buf_attach_entry_from_kobj(kobj); - kfree(sysfs_entry); -} - -static struct kobj_type dma_buf_attach_ktype = { - .sysfs_ops = &dma_buf_attach_stats_sysfs_ops, - .release = dma_buf_attach_sysfs_release, - .default_groups = dma_buf_attach_stats_default_groups, -}; - -void dma_buf_attach_stats_teardown(struct dma_buf_attachment *attach) -{ - struct dma_buf_attach_sysfs_entry *sysfs_entry; - - sysfs_entry = attach->sysfs_entry; - if (!sysfs_entry) - return; - - sysfs_delete_link(&sysfs_entry->kobj, &attach->dev->kobj, "device"); - - kobject_del(&sysfs_entry->kobj); - kobject_put(&sysfs_entry->kobj); -} - -int dma_buf_attach_stats_setup(struct dma_buf_attachment *attach, - unsigned int uid) -{ - struct dma_buf_attach_sysfs_entry *sysfs_entry; - int ret; - struct dma_buf *dmabuf; - - if (!attach) - return -EINVAL; - - dmabuf = attach->dmabuf; - - sysfs_entry = kzalloc(sizeof(struct dma_buf_attach_sysfs_entry), - GFP_KERNEL); - if (!sysfs_entry) - return -ENOMEM; - - sysfs_entry->kobj.kset = dmabuf->sysfs_entry->attach_stats_kset; - - attach->sysfs_entry = sysfs_entry; - - ret = kobject_init_and_add(&sysfs_entry->kobj, &dma_buf_attach_ktype, - NULL, "%u", uid); - if (ret) - goto kobj_err; - - ret = sysfs_create_link(&sysfs_entry->kobj, &attach->dev->kobj, - "device"); - if (ret) - goto link_err; - - return 0; - -link_err: - kobject_del(&sysfs_entry->kobj); -kobj_err: - kobject_put(&sysfs_entry->kobj); - attach->sysfs_entry = NULL; - - return ret; -} void dma_buf_stats_teardown(struct dma_buf *dmabuf) { struct dma_buf_sysfs_entry *sysfs_entry; @@ -243,7 +126,6 @@ void dma_buf_stats_teardown(struct dma_buf *dmabuf) if (!sysfs_entry) return;
- kset_unregister(sysfs_entry->attach_stats_kset); kobject_del(&sysfs_entry->kobj); kobject_put(&sysfs_entry->kobj); } @@ -290,7 +172,6 @@ int dma_buf_stats_setup(struct dma_buf *dmabuf) { struct dma_buf_sysfs_entry *sysfs_entry; int ret; - struct kset *attach_stats_kset;
if (!dmabuf || !dmabuf->file) return -EINVAL; @@ -315,21 +196,8 @@ int dma_buf_stats_setup(struct dma_buf *dmabuf) if (ret) goto err_sysfs_dmabuf;
- /* create the directory for attachment stats */ - attach_stats_kset = kset_create_and_add("attachments", - &dmabuf_sysfs_no_uevent_ops, - &sysfs_entry->kobj); - if (!attach_stats_kset) { - ret = -ENOMEM; - goto err_sysfs_attach; - } - - sysfs_entry->attach_stats_kset = attach_stats_kset; - return 0;
-err_sysfs_attach: - kobject_del(&sysfs_entry->kobj); err_sysfs_dmabuf: kobject_put(&sysfs_entry->kobj); dmabuf->sysfs_entry = NULL; diff --git a/drivers/dma-buf/dma-buf-sysfs-stats.h b/drivers/dma-buf/dma-buf-sysfs-stats.h index 5f4703249117..a49c6e2650cc 100644 --- a/drivers/dma-buf/dma-buf-sysfs-stats.h +++ b/drivers/dma-buf/dma-buf-sysfs-stats.h @@ -14,23 +14,8 @@ int dma_buf_init_sysfs_statistics(void); void dma_buf_uninit_sysfs_statistics(void);
int dma_buf_stats_setup(struct dma_buf *dmabuf); -int dma_buf_attach_stats_setup(struct dma_buf_attachment *attach, - unsigned int uid); -static inline void dma_buf_update_attachment_map_count(struct dma_buf_attachment *attach, - int delta) -{ - struct dma_buf_attach_sysfs_entry *entry = attach->sysfs_entry;
- entry->map_counter += delta; -} void dma_buf_stats_teardown(struct dma_buf *dmabuf); -void dma_buf_attach_stats_teardown(struct dma_buf_attachment *attach); -static inline unsigned int dma_buf_update_attach_uid(struct dma_buf *dmabuf) -{ - struct dma_buf_sysfs_entry *entry = dmabuf->sysfs_entry; - - return entry->attachment_uid++; -} #else
static inline int dma_buf_init_sysfs_statistics(void) @@ -44,19 +29,7 @@ static inline int dma_buf_stats_setup(struct dma_buf *dmabuf) { return 0; } -static inline int dma_buf_attach_stats_setup(struct dma_buf_attachment *attach, - unsigned int uid) -{ - return 0; -}
static inline void dma_buf_stats_teardown(struct dma_buf *dmabuf) {} -static inline void dma_buf_attach_stats_teardown(struct dma_buf_attachment *attach) {} -static inline void dma_buf_update_attachment_map_count(struct dma_buf_attachment *attach, - int delta) {} -static inline unsigned int dma_buf_update_attach_uid(struct dma_buf *dmabuf) -{ - return 0; -} #endif #endif // _DMA_BUF_SYSFS_STATS_H diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 510b42771974..b1a6db71c656 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -738,7 +738,6 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev, { struct dma_buf_attachment *attach; int ret; - unsigned int attach_uid;
if (WARN_ON(!dmabuf || !dev)) return ERR_PTR(-EINVAL); @@ -764,13 +763,8 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev, } dma_resv_lock(dmabuf->resv, NULL); list_add(&attach->node, &dmabuf->attachments); - attach_uid = dma_buf_update_attach_uid(dmabuf); dma_resv_unlock(dmabuf->resv);
- ret = dma_buf_attach_stats_setup(attach, attach_uid); - if (ret) - goto err_sysfs; - /* When either the importer or the exporter can't handle dynamic * mappings we cache the mapping here to avoid issues with the * reservation object lock. @@ -797,7 +791,6 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev, dma_resv_unlock(attach->dmabuf->resv); attach->sgt = sgt; attach->dir = DMA_BIDIRECTIONAL; - dma_buf_update_attachment_map_count(attach, 1 /* delta */); }
return attach; @@ -814,7 +807,6 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev, if (dma_buf_is_dynamic(attach->dmabuf)) dma_resv_unlock(attach->dmabuf->resv);
-err_sysfs: dma_buf_detach(dmabuf, attach); return ERR_PTR(ret); } @@ -864,7 +856,6 @@ void dma_buf_detach(struct dma_buf *dmabuf, struct dma_buf_attachment *attach) dma_resv_lock(attach->dmabuf->resv, NULL);
__unmap_dma_buf(attach, attach->sgt, attach->dir); - dma_buf_update_attachment_map_count(attach, -1 /* delta */);
if (dma_buf_is_dynamic(attach->dmabuf)) { dmabuf->ops->unpin(attach); @@ -878,7 +869,6 @@ void dma_buf_detach(struct dma_buf *dmabuf, struct dma_buf_attachment *attach) if (dmabuf->ops->detach) dmabuf->ops->detach(dmabuf, attach);
- dma_buf_attach_stats_teardown(attach); kfree(attach); } EXPORT_SYMBOL_GPL(dma_buf_detach); @@ -1020,10 +1010,6 @@ struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *attach, } } #endif /* CONFIG_DMA_API_DEBUG */ - - if (!IS_ERR(sg_table)) - dma_buf_update_attachment_map_count(attach, 1 /* delta */); - return sg_table; } EXPORT_SYMBOL_GPL(dma_buf_map_attachment); @@ -1061,8 +1047,6 @@ void dma_buf_unmap_attachment(struct dma_buf_attachment *attach, if (dma_buf_is_dynamic(attach->dmabuf) && !IS_ENABLED(CONFIG_DMABUF_MOVE_NOTIFY)) dma_buf_unpin(attach); - - dma_buf_update_attachment_map_count(attach, -1 /* delta */); } EXPORT_SYMBOL_GPL(dma_buf_unmap_attachment);
diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h index 2b814fde0d11..678b2006be78 100644 --- a/include/linux/dma-buf.h +++ b/include/linux/dma-buf.h @@ -444,15 +444,6 @@ struct dma_buf { struct dma_buf_sysfs_entry { struct kobject kobj; struct dma_buf *dmabuf; - - /** - * @sysfs_entry.attachment_uid: - * - * This is protected by the dma_resv_lock() on @resv and is - * incremented on each attach. - */ - unsigned int attachment_uid; - struct kset *attach_stats_kset; } *sysfs_entry; #endif }; @@ -504,7 +495,6 @@ struct dma_buf_attach_ops { * @importer_ops: importer operations for this attachment, if provided * dma_buf_map/unmap_attachment() must be called with the dma_resv lock held. * @importer_priv: importer specific attachment data. - * @sysfs_entry: For exposing information about this attachment in sysfs. * * This structure holds the attachment information between the dma_buf buffer * and its user device(s). The list contains one attachment struct per device @@ -525,13 +515,6 @@ struct dma_buf_attachment { const struct dma_buf_attach_ops *importer_ops; void *importer_priv; void *priv; -#ifdef CONFIG_DMABUF_SYSFS_STATS - /* for sysfs stats */ - struct dma_buf_attach_sysfs_entry { - struct kobject kobj; - unsigned int map_counter; - } *sysfs_entry; -#endif };
/**
Am 13.07.21 um 06:07 schrieb Hridya Valsaraju:
The DMA-BUF attachment statistics form a subset of the DMA-BUF sysfs statistics that recently merged to the drm-misc tree. They are not UABI yet since they have not merged to the upstream Linux kernel.
Since there has been a reported a performance regression due to the overhead of sysfs directory creation/teardown during dma_buf_attach()/dma_buf_detach(), this patch deletes the DMA-BUF attachment statistics from sysfs.
Fixes: bdb8d06dfefd (dmabuf: Add the capability to expose DMA-BUF stats in sysfs) Signed-off-by: Hridya Valsaraju hridya@google.com Reviewed-by: Christian König christian.koenig@amd.com Reviewed-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
Changes in v2: Updated commit message to clarify that the sysfs files being removed have not yet merged to upstream Linux and are hence not ABI.
Hi Christian,
I have updated the commit message as per your suggestion. Please do take another look when you get a chance.
I've just pushed that one to drm-misc-next. Sorry for the delay.
Regards, Christian.
Thanks, Hridya
.../ABI/testing/sysfs-kernel-dmabuf-buffers | 28 ---- drivers/dma-buf/dma-buf-sysfs-stats.c | 140 +----------------- drivers/dma-buf/dma-buf-sysfs-stats.h | 27 ---- drivers/dma-buf/dma-buf.c | 16 -- include/linux/dma-buf.h | 17 --- 5 files changed, 4 insertions(+), 224 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers b/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers index a243984ed420..5d3bc997dc64 100644 --- a/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers +++ b/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers @@ -22,31 +22,3 @@ KernelVersion: v5.13 Contact: Hridya Valsaraju hridya@google.com Description: This file is read-only and specifies the size of the DMA-BUF in bytes.
-What: /sys/kernel/dmabuf/buffers/<inode_number>/attachments -Date: May 2021 -KernelVersion: v5.13 -Contact: Hridya Valsaraju hridya@google.com -Description: This directory will contain subdirectories representing every
attachment of the DMA-BUF.-What: /sys/kernel/dmabuf/buffers/<inode_number>/attachments/<attachment_uid> -Date: May 2021 -KernelVersion: v5.13 -Contact: Hridya Valsaraju hridya@google.com -Description: This directory will contain information on the attached device
and the number of current distinct device mappings.-What: /sys/kernel/dmabuf/buffers/<inode_number>/attachments/<attachment_uid>/device -Date: May 2021 -KernelVersion: v5.13 -Contact: Hridya Valsaraju hridya@google.com -Description: This file is read-only and is a symlink to the attached device's
sysfs entry.-What: /sys/kernel/dmabuf/buffers/<inode_number>/attachments/<attachment_uid>/map_counter -Date: May 2021 -KernelVersion: v5.13 -Contact: Hridya Valsaraju hridya@google.com -Description: This file is read-only and contains a map_counter indicating the
number of distinct device mappings of the attachment.diff --git a/drivers/dma-buf/dma-buf-sysfs-stats.c b/drivers/dma-buf/dma-buf-sysfs-stats.c index a2638e84199c..053baadcada9 100644 --- a/drivers/dma-buf/dma-buf-sysfs-stats.c +++ b/drivers/dma-buf/dma-buf-sysfs-stats.c @@ -40,14 +40,11 @@
- ``/sys/kernel/dmabuf/buffers/<inode_number>/exporter_name``
- ``/sys/kernel/dmabuf/buffers/<inode_number>/size``
- ``/sys/kernel/dmabuf/buffers/<inode_number>/attachments/<attach_uid>/device``
- ``/sys/kernel/dmabuf/buffers/<inode_number>/attachments/<attach_uid>/map_counter``
- The information in the interface can also be used to derive per-exporter and
- per-device usage statistics. The data from the interface can be gathered
- on error conditions or other important events to provide a snapshot of
- DMA-BUF usage. It can also be collected periodically by telemetry to monitor
- various metrics.
- The information in the interface can also be used to derive per-exporter
- statistics. The data from the interface can be gathered on error conditions
- or other important events to provide a snapshot of DMA-BUF usage.
- It can also be collected periodically by telemetry to monitor various metrics.
- Detailed documentation about the interface is present in
- Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers.
@@ -121,120 +118,6 @@ static struct kobj_type dma_buf_ktype = { .default_groups = dma_buf_stats_default_groups, }; -#define to_dma_buf_attach_entry_from_kobj(x) container_of(x, struct dma_buf_attach_sysfs_entry, kobj)
-struct dma_buf_attach_stats_attribute {
- struct attribute attr;
- ssize_t (*show)(struct dma_buf_attach_sysfs_entry *sysfs_entry,
struct dma_buf_attach_stats_attribute *attr, char *buf);-}; -#define to_dma_buf_attach_stats_attr(x) container_of(x, struct dma_buf_attach_stats_attribute, attr)
-static ssize_t dma_buf_attach_stats_attribute_show(struct kobject *kobj,
struct attribute *attr,char *buf)-{
- struct dma_buf_attach_stats_attribute *attribute;
- struct dma_buf_attach_sysfs_entry *sysfs_entry;
- attribute = to_dma_buf_attach_stats_attr(attr);
- sysfs_entry = to_dma_buf_attach_entry_from_kobj(kobj);
- if (!attribute->show)
return -EIO;- return attribute->show(sysfs_entry, attribute, buf);
-}
-static const struct sysfs_ops dma_buf_attach_stats_sysfs_ops = {
- .show = dma_buf_attach_stats_attribute_show,
-};
-static ssize_t map_counter_show(struct dma_buf_attach_sysfs_entry *sysfs_entry,
struct dma_buf_attach_stats_attribute *attr,char *buf)-{
- return sysfs_emit(buf, "%u\n", sysfs_entry->map_counter);
-}
-static struct dma_buf_attach_stats_attribute map_counter_attribute =
- __ATTR_RO(map_counter);
-static struct attribute *dma_buf_attach_stats_default_attrs[] = {
- &map_counter_attribute.attr,
- NULL,
-}; -ATTRIBUTE_GROUPS(dma_buf_attach_stats_default);
-static void dma_buf_attach_sysfs_release(struct kobject *kobj) -{
- struct dma_buf_attach_sysfs_entry *sysfs_entry;
- sysfs_entry = to_dma_buf_attach_entry_from_kobj(kobj);
- kfree(sysfs_entry);
-}
-static struct kobj_type dma_buf_attach_ktype = {
- .sysfs_ops = &dma_buf_attach_stats_sysfs_ops,
- .release = dma_buf_attach_sysfs_release,
- .default_groups = dma_buf_attach_stats_default_groups,
-};
-void dma_buf_attach_stats_teardown(struct dma_buf_attachment *attach) -{
- struct dma_buf_attach_sysfs_entry *sysfs_entry;
- sysfs_entry = attach->sysfs_entry;
- if (!sysfs_entry)
return;- sysfs_delete_link(&sysfs_entry->kobj, &attach->dev->kobj, "device");
- kobject_del(&sysfs_entry->kobj);
- kobject_put(&sysfs_entry->kobj);
-}
-int dma_buf_attach_stats_setup(struct dma_buf_attachment *attach,
unsigned int uid)-{
- struct dma_buf_attach_sysfs_entry *sysfs_entry;
- int ret;
- struct dma_buf *dmabuf;
- if (!attach)
return -EINVAL;- dmabuf = attach->dmabuf;
- sysfs_entry = kzalloc(sizeof(struct dma_buf_attach_sysfs_entry),
GFP_KERNEL);- if (!sysfs_entry)
return -ENOMEM;- sysfs_entry->kobj.kset = dmabuf->sysfs_entry->attach_stats_kset;
- attach->sysfs_entry = sysfs_entry;
- ret = kobject_init_and_add(&sysfs_entry->kobj, &dma_buf_attach_ktype,
NULL, "%u", uid);- if (ret)
goto kobj_err;- ret = sysfs_create_link(&sysfs_entry->kobj, &attach->dev->kobj,
"device");- if (ret)
goto link_err;- return 0;
-link_err:
- kobject_del(&sysfs_entry->kobj);
-kobj_err:
- kobject_put(&sysfs_entry->kobj);
- attach->sysfs_entry = NULL;
- return ret;
-} void dma_buf_stats_teardown(struct dma_buf *dmabuf) { struct dma_buf_sysfs_entry *sysfs_entry; @@ -243,7 +126,6 @@ void dma_buf_stats_teardown(struct dma_buf *dmabuf) if (!sysfs_entry) return;
- kset_unregister(sysfs_entry->attach_stats_kset); kobject_del(&sysfs_entry->kobj); kobject_put(&sysfs_entry->kobj); }
@@ -290,7 +172,6 @@ int dma_buf_stats_setup(struct dma_buf *dmabuf) { struct dma_buf_sysfs_entry *sysfs_entry; int ret;
- struct kset *attach_stats_kset;
if (!dmabuf || !dmabuf->file) return -EINVAL; @@ -315,21 +196,8 @@ int dma_buf_stats_setup(struct dma_buf *dmabuf) if (ret) goto err_sysfs_dmabuf;
- /* create the directory for attachment stats */
- attach_stats_kset = kset_create_and_add("attachments",
&dmabuf_sysfs_no_uevent_ops,&sysfs_entry->kobj);- if (!attach_stats_kset) {
ret = -ENOMEM;goto err_sysfs_attach;- }
- sysfs_entry->attach_stats_kset = attach_stats_kset;
- return 0;
-err_sysfs_attach:
- kobject_del(&sysfs_entry->kobj); err_sysfs_dmabuf: kobject_put(&sysfs_entry->kobj); dmabuf->sysfs_entry = NULL;
diff --git a/drivers/dma-buf/dma-buf-sysfs-stats.h b/drivers/dma-buf/dma-buf-sysfs-stats.h index 5f4703249117..a49c6e2650cc 100644 --- a/drivers/dma-buf/dma-buf-sysfs-stats.h +++ b/drivers/dma-buf/dma-buf-sysfs-stats.h @@ -14,23 +14,8 @@ int dma_buf_init_sysfs_statistics(void); void dma_buf_uninit_sysfs_statistics(void); int dma_buf_stats_setup(struct dma_buf *dmabuf); -int dma_buf_attach_stats_setup(struct dma_buf_attachment *attach,
unsigned int uid);-static inline void dma_buf_update_attachment_map_count(struct dma_buf_attachment *attach,
int delta)-{
- struct dma_buf_attach_sysfs_entry *entry = attach->sysfs_entry;
- entry->map_counter += delta;
-} void dma_buf_stats_teardown(struct dma_buf *dmabuf); -void dma_buf_attach_stats_teardown(struct dma_buf_attachment *attach); -static inline unsigned int dma_buf_update_attach_uid(struct dma_buf *dmabuf) -{
- struct dma_buf_sysfs_entry *entry = dmabuf->sysfs_entry;
- return entry->attachment_uid++;
-} #else static inline int dma_buf_init_sysfs_statistics(void) @@ -44,19 +29,7 @@ static inline int dma_buf_stats_setup(struct dma_buf *dmabuf) { return 0; } -static inline int dma_buf_attach_stats_setup(struct dma_buf_attachment *attach,
unsigned int uid)-{
- return 0;
-} static inline void dma_buf_stats_teardown(struct dma_buf *dmabuf) {} -static inline void dma_buf_attach_stats_teardown(struct dma_buf_attachment *attach) {} -static inline void dma_buf_update_attachment_map_count(struct dma_buf_attachment *attach,
int delta) {}-static inline unsigned int dma_buf_update_attach_uid(struct dma_buf *dmabuf) -{
- return 0;
-} #endif #endif // _DMA_BUF_SYSFS_STATS_H diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 510b42771974..b1a6db71c656 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -738,7 +738,6 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev, { struct dma_buf_attachment *attach; int ret;
- unsigned int attach_uid;
if (WARN_ON(!dmabuf || !dev)) return ERR_PTR(-EINVAL); @@ -764,13 +763,8 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev, } dma_resv_lock(dmabuf->resv, NULL); list_add(&attach->node, &dmabuf->attachments);
- attach_uid = dma_buf_update_attach_uid(dmabuf); dma_resv_unlock(dmabuf->resv);
- ret = dma_buf_attach_stats_setup(attach, attach_uid);
- if (ret)
goto err_sysfs;- /* When either the importer or the exporter can't handle dynamic
- mappings we cache the mapping here to avoid issues with the
- reservation object lock.
@@ -797,7 +791,6 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev, dma_resv_unlock(attach->dmabuf->resv); attach->sgt = sgt; attach->dir = DMA_BIDIRECTIONAL;
dma_buf_update_attachment_map_count(attach, 1 /* delta */);return attach; @@ -814,7 +807,6 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev, if (dma_buf_is_dynamic(attach->dmabuf)) dma_resv_unlock(attach->dmabuf->resv); -err_sysfs: dma_buf_detach(dmabuf, attach); return ERR_PTR(ret); } @@ -864,7 +856,6 @@ void dma_buf_detach(struct dma_buf *dmabuf, struct dma_buf_attachment *attach) dma_resv_lock(attach->dmabuf->resv, NULL); __unmap_dma_buf(attach, attach->sgt, attach->dir);
dma_buf_update_attachment_map_count(attach, -1 /* delta */);if (dma_buf_is_dynamic(attach->dmabuf)) { dmabuf->ops->unpin(attach); @@ -878,7 +869,6 @@ void dma_buf_detach(struct dma_buf *dmabuf, struct dma_buf_attachment *attach) if (dmabuf->ops->detach) dmabuf->ops->detach(dmabuf, attach);
- dma_buf_attach_stats_teardown(attach); kfree(attach); } EXPORT_SYMBOL_GPL(dma_buf_detach);
@@ -1020,10 +1010,6 @@ struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *attach, } } #endif /* CONFIG_DMA_API_DEBUG */
- if (!IS_ERR(sg_table))
dma_buf_update_attachment_map_count(attach, 1 /* delta */);- return sg_table; } EXPORT_SYMBOL_GPL(dma_buf_map_attachment);
@@ -1061,8 +1047,6 @@ void dma_buf_unmap_attachment(struct dma_buf_attachment *attach, if (dma_buf_is_dynamic(attach->dmabuf) && !IS_ENABLED(CONFIG_DMABUF_MOVE_NOTIFY)) dma_buf_unpin(attach);
- dma_buf_update_attachment_map_count(attach, -1 /* delta */); } EXPORT_SYMBOL_GPL(dma_buf_unmap_attachment);
diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h index 2b814fde0d11..678b2006be78 100644 --- a/include/linux/dma-buf.h +++ b/include/linux/dma-buf.h @@ -444,15 +444,6 @@ struct dma_buf { struct dma_buf_sysfs_entry { struct kobject kobj; struct dma_buf *dmabuf;
/*** @sysfs_entry.attachment_uid:** This is protected by the dma_resv_lock() on @resv and is* incremented on each attach.*/unsigned int attachment_uid;struct kset *attach_stats_kset;@@ -504,7 +495,6 @@ struct dma_buf_attach_ops {
- @importer_ops: importer operations for this attachment, if provided
- dma_buf_map/unmap_attachment() must be called with the dma_resv lock held.
- @importer_priv: importer specific attachment data.
- @sysfs_entry: For exposing information about this attachment in sysfs.
- This structure holds the attachment information between the dma_buf buffer
- and its user device(s). The list contains one attachment struct per device
@@ -525,13 +515,6 @@ struct dma_buf_attachment { const struct dma_buf_attach_ops *importer_ops; void *importer_priv; void *priv; -#ifdef CONFIG_DMABUF_SYSFS_STATS
- /* for sysfs stats */
- struct dma_buf_attach_sysfs_entry {
struct kobject kobj;unsigned int map_counter;- } *sysfs_entry;
-#endif }; /**
On Tue, Jul 20, 2021 at 2:11 AM Christian König christian.koenig@amd.com wrote:
Am 13.07.21 um 06:07 schrieb Hridya Valsaraju:
The DMA-BUF attachment statistics form a subset of the DMA-BUF sysfs statistics that recently merged to the drm-misc tree. They are not UABI yet since they have not merged to the upstream Linux kernel.
Since there has been a reported a performance regression due to the overhead of sysfs directory creation/teardown during dma_buf_attach()/dma_buf_detach(), this patch deletes the DMA-BUF attachment statistics from sysfs.
Fixes: bdb8d06dfefd (dmabuf: Add the capability to expose DMA-BUF stats in sysfs) Signed-off-by: Hridya Valsaraju hridya@google.com Reviewed-by: Christian König christian.koenig@amd.com Reviewed-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
Changes in v2: Updated commit message to clarify that the sysfs files being removed have not yet merged to upstream Linux and are hence not ABI.
Hi Christian,
I have updated the commit message as per your suggestion. Please do take another look when you get a chance.
I've just pushed that one to drm-misc-next. Sorry for the delay.
No worries at all, thank you Christian.
Regards, Hridya
Regards, Christian.
Thanks, Hridya
.../ABI/testing/sysfs-kernel-dmabuf-buffers | 28 ---- drivers/dma-buf/dma-buf-sysfs-stats.c | 140 +----------------- drivers/dma-buf/dma-buf-sysfs-stats.h | 27 ---- drivers/dma-buf/dma-buf.c | 16 -- include/linux/dma-buf.h | 17 --- 5 files changed, 4 insertions(+), 224 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers b/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers index a243984ed420..5d3bc997dc64 100644 --- a/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers +++ b/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers @@ -22,31 +22,3 @@ KernelVersion: v5.13 Contact: Hridya Valsaraju hridya@google.com Description: This file is read-only and specifies the size of the DMA-BUF in bytes.
-What: /sys/kernel/dmabuf/buffers/<inode_number>/attachments -Date: May 2021 -KernelVersion: v5.13 -Contact: Hridya Valsaraju hridya@google.com -Description: This directory will contain subdirectories representing every
attachment of the DMA-BUF.-What: /sys/kernel/dmabuf/buffers/<inode_number>/attachments/<attachment_uid> -Date: May 2021 -KernelVersion: v5.13 -Contact: Hridya Valsaraju hridya@google.com -Description: This directory will contain information on the attached device
and the number of current distinct device mappings.-What: /sys/kernel/dmabuf/buffers/<inode_number>/attachments/<attachment_uid>/device -Date: May 2021 -KernelVersion: v5.13 -Contact: Hridya Valsaraju hridya@google.com -Description: This file is read-only and is a symlink to the attached device's
sysfs entry.-What: /sys/kernel/dmabuf/buffers/<inode_number>/attachments/<attachment_uid>/map_counter -Date: May 2021 -KernelVersion: v5.13 -Contact: Hridya Valsaraju hridya@google.com -Description: This file is read-only and contains a map_counter indicating the
number of distinct device mappings of the attachment.diff --git a/drivers/dma-buf/dma-buf-sysfs-stats.c b/drivers/dma-buf/dma-buf-sysfs-stats.c index a2638e84199c..053baadcada9 100644 --- a/drivers/dma-buf/dma-buf-sysfs-stats.c +++ b/drivers/dma-buf/dma-buf-sysfs-stats.c @@ -40,14 +40,11 @@
- ``/sys/kernel/dmabuf/buffers/<inode_number>/exporter_name``
- ``/sys/kernel/dmabuf/buffers/<inode_number>/size``
- ``/sys/kernel/dmabuf/buffers/<inode_number>/attachments/<attach_uid>/device``
- ``/sys/kernel/dmabuf/buffers/<inode_number>/attachments/<attach_uid>/map_counter``
- The information in the interface can also be used to derive per-exporter and
- per-device usage statistics. The data from the interface can be gathered
- on error conditions or other important events to provide a snapshot of
- DMA-BUF usage. It can also be collected periodically by telemetry to monitor
- various metrics.
- The information in the interface can also be used to derive per-exporter
- statistics. The data from the interface can be gathered on error conditions
- or other important events to provide a snapshot of DMA-BUF usage.
- It can also be collected periodically by telemetry to monitor various metrics.
- Detailed documentation about the interface is present in
- Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers.
@@ -121,120 +118,6 @@ static struct kobj_type dma_buf_ktype = { .default_groups = dma_buf_stats_default_groups, };
-#define to_dma_buf_attach_entry_from_kobj(x) container_of(x, struct dma_buf_attach_sysfs_entry, kobj)
-struct dma_buf_attach_stats_attribute {
struct attribute attr;ssize_t (*show)(struct dma_buf_attach_sysfs_entry *sysfs_entry,struct dma_buf_attach_stats_attribute *attr, char *buf);-}; -#define to_dma_buf_attach_stats_attr(x) container_of(x, struct dma_buf_attach_stats_attribute, attr)
-static ssize_t dma_buf_attach_stats_attribute_show(struct kobject *kobj,
struct attribute *attr,char *buf)-{
struct dma_buf_attach_stats_attribute *attribute;struct dma_buf_attach_sysfs_entry *sysfs_entry;attribute = to_dma_buf_attach_stats_attr(attr);sysfs_entry = to_dma_buf_attach_entry_from_kobj(kobj);if (!attribute->show)return -EIO;return attribute->show(sysfs_entry, attribute, buf);-}
-static const struct sysfs_ops dma_buf_attach_stats_sysfs_ops = {
.show = dma_buf_attach_stats_attribute_show,-};
-static ssize_t map_counter_show(struct dma_buf_attach_sysfs_entry *sysfs_entry,
struct dma_buf_attach_stats_attribute *attr,char *buf)-{
return sysfs_emit(buf, "%u\n", sysfs_entry->map_counter);-}
-static struct dma_buf_attach_stats_attribute map_counter_attribute =
__ATTR_RO(map_counter);-static struct attribute *dma_buf_attach_stats_default_attrs[] = {
&map_counter_attribute.attr,NULL,-}; -ATTRIBUTE_GROUPS(dma_buf_attach_stats_default);
-static void dma_buf_attach_sysfs_release(struct kobject *kobj) -{
struct dma_buf_attach_sysfs_entry *sysfs_entry;sysfs_entry = to_dma_buf_attach_entry_from_kobj(kobj);kfree(sysfs_entry);-}
-static struct kobj_type dma_buf_attach_ktype = {
.sysfs_ops = &dma_buf_attach_stats_sysfs_ops,.release = dma_buf_attach_sysfs_release,.default_groups = dma_buf_attach_stats_default_groups,-};
-void dma_buf_attach_stats_teardown(struct dma_buf_attachment *attach) -{
struct dma_buf_attach_sysfs_entry *sysfs_entry;sysfs_entry = attach->sysfs_entry;if (!sysfs_entry)return;sysfs_delete_link(&sysfs_entry->kobj, &attach->dev->kobj, "device");kobject_del(&sysfs_entry->kobj);kobject_put(&sysfs_entry->kobj);-}
-int dma_buf_attach_stats_setup(struct dma_buf_attachment *attach,
unsigned int uid)-{
struct dma_buf_attach_sysfs_entry *sysfs_entry;int ret;struct dma_buf *dmabuf;if (!attach)return -EINVAL;dmabuf = attach->dmabuf;sysfs_entry = kzalloc(sizeof(struct dma_buf_attach_sysfs_entry),GFP_KERNEL);if (!sysfs_entry)return -ENOMEM;sysfs_entry->kobj.kset = dmabuf->sysfs_entry->attach_stats_kset;attach->sysfs_entry = sysfs_entry;ret = kobject_init_and_add(&sysfs_entry->kobj, &dma_buf_attach_ktype,NULL, "%u", uid);if (ret)goto kobj_err;ret = sysfs_create_link(&sysfs_entry->kobj, &attach->dev->kobj,"device");if (ret)goto link_err;return 0;-link_err:
kobject_del(&sysfs_entry->kobj);-kobj_err:
kobject_put(&sysfs_entry->kobj);attach->sysfs_entry = NULL;return ret;-} void dma_buf_stats_teardown(struct dma_buf *dmabuf) { struct dma_buf_sysfs_entry *sysfs_entry; @@ -243,7 +126,6 @@ void dma_buf_stats_teardown(struct dma_buf *dmabuf) if (!sysfs_entry) return;
kset_unregister(sysfs_entry->attach_stats_kset); kobject_del(&sysfs_entry->kobj); kobject_put(&sysfs_entry->kobj);@@ -290,7 +172,6 @@ int dma_buf_stats_setup(struct dma_buf *dmabuf) { struct dma_buf_sysfs_entry *sysfs_entry; int ret;
struct kset *attach_stats_kset; if (!dmabuf || !dmabuf->file) return -EINVAL;@@ -315,21 +196,8 @@ int dma_buf_stats_setup(struct dma_buf *dmabuf) if (ret) goto err_sysfs_dmabuf;
/* create the directory for attachment stats */attach_stats_kset = kset_create_and_add("attachments",&dmabuf_sysfs_no_uevent_ops,&sysfs_entry->kobj);if (!attach_stats_kset) {ret = -ENOMEM;goto err_sysfs_attach;}sysfs_entry->attach_stats_kset = attach_stats_kset;return 0;-err_sysfs_attach:
kobject_del(&sysfs_entry->kobj);diff --git a/drivers/dma-buf/dma-buf-sysfs-stats.h b/drivers/dma-buf/dma-buf-sysfs-stats.h index 5f4703249117..a49c6e2650cc 100644 --- a/drivers/dma-buf/dma-buf-sysfs-stats.h +++ b/drivers/dma-buf/dma-buf-sysfs-stats.h @@ -14,23 +14,8 @@ int dma_buf_init_sysfs_statistics(void); void dma_buf_uninit_sysfs_statistics(void);
int dma_buf_stats_setup(struct dma_buf *dmabuf); -int dma_buf_attach_stats_setup(struct dma_buf_attachment *attach,
unsigned int uid);-static inline void dma_buf_update_attachment_map_count(struct dma_buf_attachment *attach,
int delta)-{
struct dma_buf_attach_sysfs_entry *entry = attach->sysfs_entry;entry->map_counter += delta;-} void dma_buf_stats_teardown(struct dma_buf *dmabuf); -void dma_buf_attach_stats_teardown(struct dma_buf_attachment *attach); -static inline unsigned int dma_buf_update_attach_uid(struct dma_buf *dmabuf) -{
struct dma_buf_sysfs_entry *entry = dmabuf->sysfs_entry;return entry->attachment_uid++;-} #else
static inline int dma_buf_init_sysfs_statistics(void) @@ -44,19 +29,7 @@ static inline int dma_buf_stats_setup(struct dma_buf *dmabuf) { return 0; } -static inline int dma_buf_attach_stats_setup(struct dma_buf_attachment *attach,
unsigned int uid)-{
return 0;-}
static inline void dma_buf_stats_teardown(struct dma_buf *dmabuf) {} -static inline void dma_buf_attach_stats_teardown(struct dma_buf_attachment *attach) {} -static inline void dma_buf_update_attachment_map_count(struct dma_buf_attachment *attach,
int delta) {}-static inline unsigned int dma_buf_update_attach_uid(struct dma_buf *dmabuf) -{
return 0;-} #endif #endif // _DMA_BUF_SYSFS_STATS_H diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 510b42771974..b1a6db71c656 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -738,7 +738,6 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev, { struct dma_buf_attachment *attach; int ret;
unsigned int attach_uid; if (WARN_ON(!dmabuf || !dev)) return ERR_PTR(-EINVAL);@@ -764,13 +763,8 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev, } dma_resv_lock(dmabuf->resv, NULL); list_add(&attach->node, &dmabuf->attachments);
attach_uid = dma_buf_update_attach_uid(dmabuf); dma_resv_unlock(dmabuf->resv);ret = dma_buf_attach_stats_setup(attach, attach_uid);if (ret)goto err_sysfs;/* When either the importer or the exporter can't handle dynamic * mappings we cache the mapping here to avoid issues with the * reservation object lock.@@ -797,7 +791,6 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev, dma_resv_unlock(attach->dmabuf->resv); attach->sgt = sgt; attach->dir = DMA_BIDIRECTIONAL;
dma_buf_update_attachment_map_count(attach, 1 /* delta */); } return attach;@@ -814,7 +807,6 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev, if (dma_buf_is_dynamic(attach->dmabuf)) dma_resv_unlock(attach->dmabuf->resv);
-err_sysfs: dma_buf_detach(dmabuf, attach); return ERR_PTR(ret); } @@ -864,7 +856,6 @@ void dma_buf_detach(struct dma_buf *dmabuf, struct dma_buf_attachment *attach) dma_resv_lock(attach->dmabuf->resv, NULL);
__unmap_dma_buf(attach, attach->sgt, attach->dir);
dma_buf_update_attachment_map_count(attach, -1 /* delta */); if (dma_buf_is_dynamic(attach->dmabuf)) { dmabuf->ops->unpin(attach);@@ -878,7 +869,6 @@ void dma_buf_detach(struct dma_buf *dmabuf, struct dma_buf_attachment *attach) if (dmabuf->ops->detach) dmabuf->ops->detach(dmabuf, attach);
dma_buf_attach_stats_teardown(attach); kfree(attach);@@ -1020,10 +1010,6 @@ struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *attach, } } #endif /* CONFIG_DMA_API_DEBUG */
if (!IS_ERR(sg_table))dma_buf_update_attachment_map_count(attach, 1 /* delta */);return sg_table;@@ -1061,8 +1047,6 @@ void dma_buf_unmap_attachment(struct dma_buf_attachment *attach, if (dma_buf_is_dynamic(attach->dmabuf) && !IS_ENABLED(CONFIG_DMABUF_MOVE_NOTIFY)) dma_buf_unpin(attach);
dma_buf_update_attachment_map_count(attach, -1 /* delta */);diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h index 2b814fde0d11..678b2006be78 100644 --- a/include/linux/dma-buf.h +++ b/include/linux/dma-buf.h @@ -444,15 +444,6 @@ struct dma_buf { struct dma_buf_sysfs_entry { struct kobject kobj; struct dma_buf *dmabuf;
/*** @sysfs_entry.attachment_uid:** This is protected by the dma_resv_lock() on @resv and is* incremented on each attach.*/unsigned int attachment_uid;struct kset *attach_stats_kset; } *sysfs_entry;@@ -504,7 +495,6 @@ struct dma_buf_attach_ops {
- @importer_ops: importer operations for this attachment, if provided
- dma_buf_map/unmap_attachment() must be called with the dma_resv lock held.
- @importer_priv: importer specific attachment data.
- @sysfs_entry: For exposing information about this attachment in sysfs.
- This structure holds the attachment information between the dma_buf buffer
- and its user device(s). The list contains one attachment struct per device
@@ -525,13 +515,6 @@ struct dma_buf_attachment { const struct dma_buf_attach_ops *importer_ops; void *importer_priv; void *priv; -#ifdef CONFIG_DMABUF_SYSFS_STATS
/* for sysfs stats */struct dma_buf_attach_sysfs_entry {struct kobject kobj;unsigned int map_counter;} *sysfs_entry;-#endif };
/**
linaro-mm-sig@lists.linaro.org
-
Christian König -
Hridya Valsaraju