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

[Greg KH]

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@xxxxxxxxxx>
> > > ---
> > >   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@xxxxxxxxxxxxxxxx
> > >   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@xxxxxxxxxxxxxxxx
> > > +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?

> But you're right that the manual newline append and the PAGE_SIZE - 1
> math were unnecessary.  In the next version I'll rework the callback
> contract so the driver writes the complete output (including any
> trailing newlines) and dev_usage_show() just returns the callback
> result directly:
> 
>     static ssize_t dev_usage_show(struct device *dev,
>                                   struct device_attribute *attr, char *buf)
>     {
>         struct uacce_device *uacce = to_uacce_device(dev);
> 
>         return uacce->ops->get_dev_usage(uacce, buf, PAGE_SIZE);

Why not have the get_dev_usage() return the value that you want to write
to the buffer, and then use sysfs_emit() to write it?  Then no need to
worry about PAGE_SIZE at all.

thanks,

greg k-h