On Wed, Jan 08, 2025 at 09:45:45AM -0800, Alexei Starovoitov wrote:

On Tue, Dec 31, 2024 at 2:30 AM Thomas Weißschuh [off-list ref] wrote:

quoted

On 2024-12-30 16:50:41-0800, Alexei Starovoitov wrote:

quoted

On Sat, Dec 28, 2024 at 12:43 AM Thomas Weißschuh [off-list ref] wrote:

quoted

Most users use this function through the BIN_ATTR_SIMPLE* macros,
they can handle the switch transparently.

This series is meant to be merged through the driver core tree.

hmm. why?

Patch 1 changes the signature of sysfs_bin_attr_simple_read().
Before patch 1 sysfs_bin_attr_simple_read() needs to be assigned to the
callback member .read, after patch 1 it's .read_new.
(Both callbacks work exactly the same, except for their signature,
.read_new is only a transition mechanism and will go away again)

quoted

I'd rather take patches 2 and 3 into bpf-next to avoid
potential conflicts.
Patch 1 looks orthogonal and independent.

If you pick up 2 and 3 through bpf-next you would need to adapt these
assignments. As soon as both patch 1 and the modified 2 and 3 hit
Linus' tree, the build would break due to mismatches function pointers.
(Casting function pointers to avoid the mismatch will blow up with KCFI)

I see. All these steps to constify is frankly a mess.
You're wasting cpu and memory for this read vs read_new
when const is not much more than syntactic sugar in C.
You should have done one tree wide patch without doing this _new() hack.

Anyway, rant over. Carry patches 2,3. Hopefully they won't conflict.
But I don't want to see any constification patches in bpf land
that come with such pointless runtime penalty.

The "pointless" penalty will go away once we convert all instances, and
really, it's just one pointer check, sysfs files should NOT be a hot
path for anything real, and one more pointer check should be cached and
not measurable compared to the real logic behind the binary data coming
from the hardware/kernel, right?

sysfs is NOT tuned for speed at all, so adding more checks like this
should be fine.

thanks,

greg k-h