Re: [PATCH v3 3/7] kunit: Handle logging of lines longer than the fragment buffer size

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/8/23 15:38, David Gow wrote:
On Wed, 9 Aug 2023 at 23:54, Richard Fitzgerald
<rf@xxxxxxxxxxxxxxxxxxxxx> wrote:

Add handling to kunit_log_append() for log messages that are longer than
a single buffer fragment.

The initial implementation of fragmented buffers did not change the logic
of the original kunit_log_append(). A consequence was that it still had
the original assumption that a log line will fit into one buffer.

This patch checks for log messages that are larger than one fragment
buffer. In that case, kvasprintf() is used to format it into a temporary
buffer and that content is then split across as many fragments as
necessary.

Signed-off-by: Richard Fitzgerald <rf@xxxxxxxxxxxxxxxxxxxxx>
---

I think this looks good (and is a long-overdue addition to the logging
functionality).

One thought I have (and I'm kicking myself for not thinking of it
earlier) is that this is starting to get very similar to the "string
stream" functionality in lib/kunit/string-stream.{h,c}. Now, I
actually think this implementation is much more efficient (using
larger fragments, whereas the string stream uses variable-sized ones).
Particularly since there are a lot of cases where string streams are
created, converted to a string, and then logged, there's almost
certainly a bunch of redundant work being done here.

My gut feeling is that we should stick with this as-is, and maybe try
to either work out some better integration between string streams and
logging (to avoid that extra string allocation) or find some way of
combining them.


I completely failed to notice string_stream. I could re-implement this
to use string_stream. I wonder whether appending newlines gets
a bit inefficient with the current string_stream implementation.
Could add newline support to string_stream and alloc one extra byte for
each fragment just in case we need to add a newline.

The string_stream implementation would waste a lot a memory if you log
many short lines. My current code wastes memory if you log lots of lines
that don't fit in available space in the current fragment - though it's
simple to shuffle the formatted string backwards to fill up the previous
fragment (I just haven't done that yet).



[Index of Archives]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux