Thread (31 messages) 31 messages, 8 authors, 4d ago

Re: [PATCH 13/16] perf: Use sysfs_emit() for cpumask show callbacks

From: Yury Norov <yury.norov@gmail.com>
Date: 2026-06-27 17:49:29
Also in: driver-core, imx, linux-acpi, linux-amlogic, linux-arm-msm, linux-cxl, linux-fpga, linux-pci, linux-perf-users, linux-pm, linux-rdma, lkml, nvdimm

On Fri, May 29, 2026 at 01:06:19PM +0100, David Laight wrote:
On Fri, 29 May 2026 12:05:08 +0100
Robin Murphy [off-list ref] wrote:
quoted
On 2026-05-28 7:36 pm, Yury Norov wrote:
quoted
These callbacks are sysfs show paths.

Use sysfs_emit() and cpumask_pr_args() to emit the masks.

This prepares for removing cpumap_print_to_pagebuf().  
TBH, looking at this diff I think it only shows the value of having a 
helper to abstract the boilerplate...

I'm not sure I agree with the argument of removing something entirely 
just because it may occasionally be misused, but could we at least have 
something like:

#define sysfs_emit_cpumask(buf, mask) \
	sysfs_emit((buf), "%*pbl\n", cpumask_pr_args(mask))

to save the mess in all the many places where the current 
cpumap_print_to_pagebuf() usage _is_ entirely appropriate?
This way you have to add 2 wrappers:

 #define sysfs_emit_cpulist(buf, mask) \
 	sysfs_emit((buf), "%*pbl\n", cpumask_pr_args(mask))

 and

 #define sysfs_emit_cpumask(buf, mask) \
 	sysfs_emit((buf), "%*pb\n", cpumask_pr_args(mask))

There are people who complain even about DIV_ROUND_UP(), how hard it is
to keep all that helpers in memory, and all that things.

https://lore.kernel.org/all/20260304124805.GB2277644@noisy.programming.kicks-ass.net/ (local)

Disagree about DIV_ROUND_UP() (because yeah, I'm bad in math), but
this sysfs_emit_cpumask() is a complete syntax redundancy.

Once we have it, people will do this type of things:

        tmp = kmalloc(PAGE_SIZE);
        sysfs_emit_cpumask(tmp, mask);
        sysfs_emit(buf, "my prefix: %s\n", tmp);
        kfree(tmp);

Patch #1 in this series is one example. My series that removes
bitmap_print_to_pagebuf() will give you more:

https://lore.kernel.org/all/20260303200842.124996-2-ynorov@nvidia.com/ (local)

It doesn't mean that *you* will misuse the API. It means that *I* will
have to inspect the codebase for that type of bugs periodically.

So, the overall state is simple: we've got well-established
printf()-like functions that people know and understand, and we also
have exotic APIs here and there with a non-standard interface and a
clear potential to misuse. In this case, they have historical roots,
but now we don't need them.
That has the advantage of letting you change how it is done (again)
without having to find all the callers.
You mean things like silencing the prints or adding a prefix?

If you believe that perf subsystem would benefit from it - that's
OK. Just please keep it local. The kernel globally doesn't need to
'change how it is done' beyond the lib/vsprintf.  The kernel really
needs people to use something that the other people are familiar with.

Thanks,
Yury
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help