Page MenuHomeFreeBSD

linuxkpi: Implement <linux/seq_buf.h>
ClosedPublic

Authored by dumbbell on Sun, Jan 4, 10:18 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Feb 1, 8:03 PM
Unknown Object (File)
Tue, Jan 27, 8:41 PM
Unknown Object (File)
Tue, Jan 27, 12:02 AM
Unknown Object (File)
Mon, Jan 26, 6:19 PM
Unknown Object (File)
Mon, Jan 26, 5:50 PM
Unknown Object (File)
Mon, Jan 26, 2:12 AM
Unknown Object (File)
Wed, Jan 21, 4:58 AM
Unknown Object (File)
Tue, Jan 20, 9:09 PM

Details

Summary

It is a wrapper above a char * to track the overall available space in the buffer as well as the used space. This wrapper does not manage memory allocation.

The DRM generic code started to use this in Linux 6.10.

This is part of the update of DRM drivers to Linux 6.10.

Sponsored by: The FreeBSD Foundation

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Use SPDX tag and remove license text.

bz requested changes to this revision.Tue, Jan 6, 12:48 AM
bz added a subscriber: bz.
bz added inline comments.
sys/compat/linuxkpi/common/include/linux/seq_buf.h
2

Not needed anymore.

3

Could move this down in this case given it's not committed yet.

9

use SPDX?

37

While this is probably sufficient, does that mean we can leave actual data behind in the buffer? Should we zero the entire buffer? I do not know how this is used, hence asking as there is a cost to zeroing...

55

I would prefer if the public KPI functions would be named linuxkpi_ and "internal" functions get the "lkpi_" prefix.
I think during the last year we slowly got to that and it would help if we try to converge to that for everything new.

sys/compat/linuxkpi/common/src/linux_seq_buf.c
41

You probably want to print to buffer + len and only do that if len < size (not overflowed yet) and then the s->size needs to be reduced by len as well?

43

An overflow of the buffer is also considered an error so we'll need to check if len + ret < size and only if so update len and return success.

This likely needs to be += then.

45

According to documentation this returns 0 on success and -1 on error, not the length it would have printed.

This revision now requires changes to proceed.Tue, Jan 6, 12:48 AM
christos added inline comments.
sys/compat/linuxkpi/common/include/linux/seq_buf.h
37

I asked the same in an email a while ago. Isn't it risky to simply prepend a \0 at the beginning and leave the previous data still there, instead of zeroing out the whole buffer?

sys/compat/linuxkpi/common/include/linux/seq_buf.h
37

Linux only puts a \0 in the first byte so assuming we don't use this in any kernel-user contexts beyond what Linux does, and assuming an absence of Linux bugs, it's OK.

I see use in 3 files in drm:

drivers/gpu/drm/drm_edid.c
drivers/gpu/drm/i915/display/intel_ddi.c
drivers/gpu/drm/i915/display/intel_dp.c

Used only to craft strings followed by seq_buf_str() to get a char * to pass to e.g. printf. So OK.

(By comparison sbuf_clear leaves the buffer intact, it does not even put a \0 in the first byte.)

dumbbell marked 10 inline comments as done.

Address feedback from @bz.

sys/compat/linuxkpi/common/include/linux/seq_buf.h
9

I moved the SPDX line at the bottom.

55

Fixed

sys/compat/linuxkpi/common/src/linux_seq_buf.c
41

Indeed, I misunderstood the function description. Fixed.

This revision is now accepted and ready to land.Thu, Jan 22, 10:47 PM
This revision was automatically updated to reflect the committed changes.