Re: [PATCH 1/2] uacce: add device usage sysfs interface

[Greg KH <gregkh@linuxfoundation.org>]

On Tue, Sep 22, 2026 at 07:57:14PM +0800, Weili Qian wrote:
> 
> 
> On 2026/9/22 17:21, Greg KH wrote:
> > On Tue, Sep 22, 2026 at 05:11:55PM +0800, qianweili wrote:
> > > 
> > > On 2026/9/21 16:12, Greg KH wrote:
> > > > On Mon, Sep 21, 2026 at 03:51:30PM +0800, Weili Qian wrote:
> > > > > Userspace has no way to query the runtime usage of a UACCE
> > > > > device; it can only be inferred indirectly from queue state, which is
> > > > > neither accurate nor uniform across drivers.
> > > > > 
> > > > > Add a read-only dev_usage sysfs attribute and a get_dev_usage callback
> > > > > in struct uacce_ops. A driver implementing the callback writes the
> > > > > current usage as a percentage (0-100) string into the caller-provided
> > > > > buffer and returns the number of bytes written; dev_usage_show()
> > > > > appends the trailing newline. The attribute is hidden via
> > > > > uacce_dev_is_visible() when the driver does not provide the callback.
> > > > > 
> > > > > The corresponding ABI entry is added to Documentation/ABI/testing/
> > > > > sysfs-driver-uacce.
> > > > > 
> > > > > Signed-off-by: Weili Qian <qianweili@huawei.com>
> > > > > ---
> > > > >    Documentation/ABI/testing/sysfs-driver-uacce |  9 +++++++++
> > > > >    drivers/misc/uacce/uacce.c                   | 19 +++++++++++++++++++
> > > > >    include/linux/uacce.h                        |  5 +++++
> > > > >    3 files changed, 33 insertions(+)
> > > > > 
> > > > > diff --git a/Documentation/ABI/testing/sysfs-driver-uacce b/Documentation/ABI/testing/sysfs-driver-uacce
> > > > > index d3f0b8f3c589..3e4af4c1e5a9 100644
> > > > > --- a/Documentation/ABI/testing/sysfs-driver-uacce
> > > > > +++ b/Documentation/ABI/testing/sysfs-driver-uacce
> > > > > @@ -55,3 +55,12 @@ Date:           Feb 2020
> > > > >    KernelVersion:  5.7
> > > > >    Contact:        linux-accelerators@lists.ozlabs.org
> > > > >    Description:    Size (bytes) of dus region queue file
> > > > > +
> > > > > +What:           /sys/class/uacce/<dev_name>/dev_usage
> > > > > +Date:           Sep 2026
> > > > > +KernelVersion:  7.3
> > > > That's not going to happen here :(
> > > I'll change it to 7.4 in the next version.
> > > > > +Contact:        linux-accelerators@lists.ozlabs.org
> > > > > +Description:    (R) Current usage of the device, reported as a driver-defined
> > > > > +                string of up to PAGE_SIZE - 1 bytes. Usage is expressed as a
> > > > > +                percentage (0-100). The attribute is hidden if the driver does
> > > > > +                not implement the get_dev_usage callback.
> > > > > diff --git a/drivers/misc/uacce/uacce.c b/drivers/misc/uacce/uacce.c
> > > > > index 45521d4a56d1..545ba35a590b 100644
> > > > > --- a/drivers/misc/uacce/uacce.c
> > > > > +++ b/drivers/misc/uacce/uacce.c
> > > > > @@ -433,6 +433,20 @@ static ssize_t isolate_strategy_store(struct device *dev, struct device_attribut
> > > > >    	return count;
> > > > >    }
> > > > > +static ssize_t dev_usage_show(struct device *dev, struct device_attribute *attr, char *buf)
> > > > > +{
> > > > > +	struct uacce_device *uacce = to_uacce_device(dev);
> > > > > +	int ret;
> > > > > +
> > > > > +	ret = uacce->ops->get_dev_usage(uacce, buf, PAGE_SIZE - 1);
> > > > Why can't you use sysfs_emit()?  That way you don't have to worry about
> > > > PAGE_SIZE, and you don't have to do:
> > > > 
> > > > > +	if (ret < 0)
> > > > > +		return ret;
> > > > > +
> > > > > +	buf[ret++] = '\n';
> > > > That type of thing :(
> > > > 
> > > > Also, you got your math wrong above :(
> > > sysfs_emit() is useful when the framework side knows the format string
> > > upfront.  Here get_dev_usage is a driver callback that dynamically
> > > generates content -- the format is not known to the framework, so
> > > there is no format string to emit.  Having the callback write into a
> > > temporary buffer and then sysfs_emit(buf, "%s", tmp) in the show
> > > function would just add an unnecessary copy without gaining the
> > > overflow protection that sysfs_emit normally provides.
> > Then that is going to be a mess, sysfs files should be in a consistant
> > way, don't have random formats for the same filename depending on random
> > hardware types.  Use different sysfs files if you want to do that.
> > 
> > And this is just going to be a single value, nothing complex, so why do
> > you need a callback for that?
> A single device may run multiple independent algorithms in parallel
> -- e.g. the HiSilicon ZIP device has separate compression and
> decompression engines whose usage rates are independent and cannot
> be aggregated into one meaningful number.

Then that can not be a sysfs file, as sysfs files are "one value per
file".

> Following your suggestion of different sysfs files, one approach is
> to expose one file per algorithm.  Each file contains a single int
> (0-100) formatted with sysfs_emit(), and the callback returns int.
> But the number of algorithms is driver-specific (1 to 3 in the
> HiSilicon drivers), so the framework would need to create attributes
> dynamically at registration time, which adds complexity.
> 
> I'm not sure this is the best approach.  Do you have a better
> suggestion for handling this case?

I don't know, just don't violate the one-version-per-file rule AND
always have the same type of data in the file with the same name (i.e.
don't have a file that can contain different types of data.)

This propose api seems to violate all of that, so I wouldn't recommend
it at all.

Why is this info needed in userspace at all?  What is userspace going to
do with it?

thanks,

greg k-h