mbox series

[v5,00/10] kunit: Add dynamically-extending log

Message ID 20230824143129.1957914-1-rf@opensource.cirrus.com
Headers show
Series kunit: Add dynamically-extending log | expand

Message

Richard Fitzgerald Aug. 24, 2023, 2:31 p.m. UTC
This patch chain changes the logging implementation to use string_stream
so that the log will grow dynamically.

The first 8 patches add test code for string_stream, and make some
changes to string_stream needed to be able to use it for the log.

The final patch adds a performance report of string_stream.

CHANGES SINCE V4:
- Re-ordered the first 3 patches from V4 to squash the first two sets
  of string_stream tests into a single patch.
- Changed is_literal() so it doesn't need a struct kunit.
- Split out the new resource-managed alloc and free functions into
  a pre-patch to reduce the amount of code churn when the string_stream
  is decoupled from kunit.
- Wrapped the call to string_stream_geT_string() in string-stream-test
  in a local function to reduce the amount of code churn when the
  string_stream is decoupled from kunit.
- Some minor changes to test implementations.
- string_stream is now completely separated from kunit and the 'test'
  member of struct string_stream has been eliminated.

Richard Fitzgerald (10):
  kunit: string-stream: Don't create a fragment for empty strings
  kunit: string-stream: Improve testing of string_stream
  kunit: string-stream: Add option to make all lines end with newline
  kunit: string-stream: Add cases for string_stream newline appending
  kunit: Don't use a managed alloc in is_literal()
  kunit: string-stream: Add kunit_alloc_string_stream()
  kunit: string-stream: Decouple string_stream from kunit
  kunit: string-stream: Add tests for freeing resource-managed
    string_stream
  kunit: Use string_stream for test log
  kunit: string-stream: Test performance of string_stream

 include/kunit/test.h           |  14 +-
 lib/kunit/assert.c             |  14 +-
 lib/kunit/debugfs.c            |  36 ++-
 lib/kunit/kunit-test.c         |  46 ++-
 lib/kunit/string-stream-test.c | 508 +++++++++++++++++++++++++++++++--
 lib/kunit/string-stream.c      | 100 +++++--
 lib/kunit/string-stream.h      |  16 +-
 lib/kunit/test.c               |  50 +---
 8 files changed, 662 insertions(+), 122 deletions(-)

Comments

Rae Moar Aug. 24, 2023, 11:13 p.m. UTC | #1
On Thu, Aug 24, 2023 at 10:32 AM Richard Fitzgerald
<rf@opensource.cirrus.com> wrote:
>
> Add an optional feature to string_stream that will append a newline to
> any added string that does not already end with a newline. The purpose
> of this is so that string_stream can be used to collect log lines.
>
> This is enabled/disabled by calling string_stream_set_append_newlines().
>
> Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>

Hello,

This again looks good to me!

Reviewed-by: Rae Moar <rmoar@google.com>

Thanks!
-Rae

> ---
>  lib/kunit/string-stream.c | 28 +++++++++++++++++++++-------
>  lib/kunit/string-stream.h |  7 +++++++
>  2 files changed, 28 insertions(+), 7 deletions(-)
>
> diff --git a/lib/kunit/string-stream.c b/lib/kunit/string-stream.c
> index ed24d86af9f5..1dcf6513b692 100644
> --- a/lib/kunit/string-stream.c
> +++ b/lib/kunit/string-stream.c
> @@ -44,32 +44,46 @@ int string_stream_vadd(struct string_stream *stream,
>                        va_list args)
>  {
>         struct string_stream_fragment *frag_container;
> -       int len;
> +       int buf_len, result_len;
>         va_list args_for_counting;
>
>         /* Make a copy because `vsnprintf` could change it */
>         va_copy(args_for_counting, args);
>
>         /* Evaluate length of formatted string */
> -       len = vsnprintf(NULL, 0, fmt, args_for_counting);
> +       buf_len = vsnprintf(NULL, 0, fmt, args_for_counting);
>
>         va_end(args_for_counting);
>
> -       if (len == 0)
> +       if (buf_len == 0)
>                 return 0;
>
> +       /* Reserve one extra for possible appended newline. */
> +       if (stream->append_newlines)
> +               buf_len++;
> +
>         /* Need space for null byte. */
> -       len++;
> +       buf_len++;
>
>         frag_container = alloc_string_stream_fragment(stream->test,
> -                                                     len,
> +                                                     buf_len,
>                                                       stream->gfp);
>         if (IS_ERR(frag_container))
>                 return PTR_ERR(frag_container);
>
> -       len = vsnprintf(frag_container->fragment, len, fmt, args);
> +       if (stream->append_newlines) {
> +               /* Don't include reserved newline byte in writeable length. */
> +               result_len = vsnprintf(frag_container->fragment, buf_len - 1, fmt, args);
> +
> +               /* Append newline if necessary. */
> +               if (frag_container->fragment[result_len - 1] != '\n')
> +                       result_len = strlcat(frag_container->fragment, "\n", buf_len);
> +       } else {
> +               result_len = vsnprintf(frag_container->fragment, buf_len, fmt, args);
> +       }
> +
>         spin_lock(&stream->lock);
> -       stream->length += len;
> +       stream->length += result_len;
>         list_add_tail(&frag_container->node, &stream->fragments);
>         spin_unlock(&stream->lock);
>
> diff --git a/lib/kunit/string-stream.h b/lib/kunit/string-stream.h
> index b669f9a75a94..048930bf97f0 100644
> --- a/lib/kunit/string-stream.h
> +++ b/lib/kunit/string-stream.h
> @@ -25,6 +25,7 @@ struct string_stream {
>         spinlock_t lock;
>         struct kunit *test;
>         gfp_t gfp;
> +       bool append_newlines;
>  };
>
>  struct kunit;
> @@ -47,4 +48,10 @@ bool string_stream_is_empty(struct string_stream *stream);
>
>  void string_stream_destroy(struct string_stream *stream);
>
> +static inline void string_stream_set_append_newlines(struct string_stream *stream,
> +                                                    bool append_newlines)
> +{
> +       stream->append_newlines = append_newlines;
> +}
> +
>  #endif /* _KUNIT_STRING_STREAM_H */
> --
> 2.30.2
>
David Gow Aug. 25, 2023, 6:49 a.m. UTC | #2
On Thu, 24 Aug 2023 at 22:32, Richard Fitzgerald
<rf@opensource.cirrus.com> wrote:
>
> Add an optional feature to string_stream that will append a newline to
> any added string that does not already end with a newline. The purpose
> of this is so that string_stream can be used to collect log lines.
>
> This is enabled/disabled by calling string_stream_set_append_newlines().
>
> Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
> ---

This is the same as v4, patch 4, and still looks good.

(In the future, feel free to leave the Reviewed-by: tag from the
previous version, so long as there are no significant changes.)

Reviewed-by: David Gow <davidgow@google.com>

Cheers,
-- David


>  lib/kunit/string-stream.c | 28 +++++++++++++++++++++-------
>  lib/kunit/string-stream.h |  7 +++++++
>  2 files changed, 28 insertions(+), 7 deletions(-)
>
> diff --git a/lib/kunit/string-stream.c b/lib/kunit/string-stream.c
> index ed24d86af9f5..1dcf6513b692 100644
> --- a/lib/kunit/string-stream.c
> +++ b/lib/kunit/string-stream.c
> @@ -44,32 +44,46 @@ int string_stream_vadd(struct string_stream *stream,
>                        va_list args)
>  {
>         struct string_stream_fragment *frag_container;
> -       int len;
> +       int buf_len, result_len;
>         va_list args_for_counting;
>
>         /* Make a copy because `vsnprintf` could change it */
>         va_copy(args_for_counting, args);
>
>         /* Evaluate length of formatted string */
> -       len = vsnprintf(NULL, 0, fmt, args_for_counting);
> +       buf_len = vsnprintf(NULL, 0, fmt, args_for_counting);
>
>         va_end(args_for_counting);
>
> -       if (len == 0)
> +       if (buf_len == 0)
>                 return 0;
>
> +       /* Reserve one extra for possible appended newline. */
> +       if (stream->append_newlines)
> +               buf_len++;
> +
>         /* Need space for null byte. */
> -       len++;
> +       buf_len++;
>
>         frag_container = alloc_string_stream_fragment(stream->test,
> -                                                     len,
> +                                                     buf_len,
>                                                       stream->gfp);
>         if (IS_ERR(frag_container))
>                 return PTR_ERR(frag_container);
>
> -       len = vsnprintf(frag_container->fragment, len, fmt, args);
> +       if (stream->append_newlines) {
> +               /* Don't include reserved newline byte in writeable length. */
> +               result_len = vsnprintf(frag_container->fragment, buf_len - 1, fmt, args);
> +
> +               /* Append newline if necessary. */
> +               if (frag_container->fragment[result_len - 1] != '\n')
> +                       result_len = strlcat(frag_container->fragment, "\n", buf_len);
> +       } else {
> +               result_len = vsnprintf(frag_container->fragment, buf_len, fmt, args);
> +       }
> +
>         spin_lock(&stream->lock);
> -       stream->length += len;
> +       stream->length += result_len;
>         list_add_tail(&frag_container->node, &stream->fragments);
>         spin_unlock(&stream->lock);
>
> diff --git a/lib/kunit/string-stream.h b/lib/kunit/string-stream.h
> index b669f9a75a94..048930bf97f0 100644
> --- a/lib/kunit/string-stream.h
> +++ b/lib/kunit/string-stream.h
> @@ -25,6 +25,7 @@ struct string_stream {
>         spinlock_t lock;
>         struct kunit *test;
>         gfp_t gfp;
> +       bool append_newlines;
>  };
>
>  struct kunit;
> @@ -47,4 +48,10 @@ bool string_stream_is_empty(struct string_stream *stream);
>
>  void string_stream_destroy(struct string_stream *stream);
>
> +static inline void string_stream_set_append_newlines(struct string_stream *stream,
> +                                                    bool append_newlines)
> +{
> +       stream->append_newlines = append_newlines;
> +}
> +
>  #endif /* _KUNIT_STRING_STREAM_H */
> --
> 2.30.2
>
David Gow Aug. 25, 2023, 6:49 a.m. UTC | #3
On Thu, 24 Aug 2023 at 22:32, Richard Fitzgerald
<rf@opensource.cirrus.com> wrote:
>
> Add function kunit_alloc_string_stream() to do a resource-managed
> allocation of a string stream, and corresponding
> kunit_free_string_stream() to free the resource-managed stream.
>
> This is preparing for decoupling the string_stream
> implementation from struct kunit, to reduce the amount of code
> churn when that happens. Currently:
>  - kunit_alloc_string_stream() only calls alloc_string_stream().
>  - kunit_free_string_stream() takes a struct kunit* which
>    isn't used yet.
>
> Callers of the old alloc_string_stream() and
> string_stream_destroy() are all requesting a managed allocation
> so have been changed to use the new functions.
>
> alloc_string_stream() has been temporarily made static because
> its current behavior has been replaced with
> kunit_alloc_string_stream().
>
> Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
> ---

Looks good.

Reviewed-by: David Gow <davidgow@google.com>

Cheers,
-- David


>  lib/kunit/string-stream-test.c | 28 ++++++++++++++--------------
>  lib/kunit/string-stream.c      | 12 +++++++++++-
>  lib/kunit/string-stream.h      |  3 ++-
>  lib/kunit/test.c               |  4 ++--
>  4 files changed, 29 insertions(+), 18 deletions(-)
>
> diff --git a/lib/kunit/string-stream-test.c b/lib/kunit/string-stream-test.c
> index 2a9936db1b9f..89549c237069 100644
> --- a/lib/kunit/string-stream-test.c
> +++ b/lib/kunit/string-stream-test.c
> @@ -25,7 +25,7 @@ static void string_stream_init_test(struct kunit *test)
>  {
>         struct string_stream *stream;
>
> -       stream = alloc_string_stream(test, GFP_KERNEL);
> +       stream = kunit_alloc_string_stream(test, GFP_KERNEL);
>         KUNIT_ASSERT_NOT_ERR_OR_NULL(test, stream);
>
>         KUNIT_EXPECT_EQ(test, stream->length, 0);
> @@ -49,7 +49,7 @@ static void string_stream_line_add_test(struct kunit *test)
>         size_t len, total_len;
>         int num_lines, i;
>
> -       stream = alloc_string_stream(test, GFP_KERNEL);
> +       stream = kunit_alloc_string_stream(test, GFP_KERNEL);
>         KUNIT_ASSERT_NOT_ERR_OR_NULL(test, stream);
>
>         /* Add series of sequence numbered lines */
> @@ -105,7 +105,7 @@ static void string_stream_variable_length_line_test(struct kunit *test)
>         size_t offset, total_len;
>         int num_lines, i;
>
> -       stream = alloc_string_stream(test, GFP_KERNEL);
> +       stream = kunit_alloc_string_stream(test, GFP_KERNEL);
>         KUNIT_ASSERT_NOT_ERR_OR_NULL(test, stream);
>
>         /*
> @@ -165,10 +165,10 @@ static void string_stream_append_test(struct kunit *test)
>         size_t combined_length;
>         int i;
>
> -       stream_1 = alloc_string_stream(test, GFP_KERNEL);
> +       stream_1 = kunit_alloc_string_stream(test, GFP_KERNEL);
>         KUNIT_ASSERT_NOT_ERR_OR_NULL(test, stream_1);
>
> -       stream_2 = alloc_string_stream(test, GFP_KERNEL);
> +       stream_2 = kunit_alloc_string_stream(test, GFP_KERNEL);
>         KUNIT_ASSERT_NOT_ERR_OR_NULL(test, stream_2);
>
>         /* Append content of empty stream to empty stream */
> @@ -206,9 +206,9 @@ static void string_stream_append_test(struct kunit *test)
>         KUNIT_EXPECT_STREQ(test, get_concatenated_string(test, stream_1), combined_content);
>
>         /* Append content of non-empty stream to empty stream */
> -       string_stream_destroy(stream_1);
> +       kunit_free_string_stream(test, stream_1);
>
> -       stream_1 = alloc_string_stream(test, GFP_KERNEL);
> +       stream_1 = kunit_alloc_string_stream(test, GFP_KERNEL);
>         KUNIT_ASSERT_NOT_ERR_OR_NULL(test, stream_1);
>
>         string_stream_append(stream_1, stream_2);
> @@ -221,13 +221,13 @@ static void string_stream_append_auto_newline_test(struct kunit *test)
>         struct string_stream *stream_1, *stream_2;
>
>         /* Stream 1 has newline appending enabled */
> -       stream_1 = alloc_string_stream(test, GFP_KERNEL);
> +       stream_1 = kunit_alloc_string_stream(test, GFP_KERNEL);
>         KUNIT_ASSERT_NOT_ERR_OR_NULL(test, stream_1);
>         string_stream_set_append_newlines(stream_1, true);
>         KUNIT_EXPECT_TRUE(test, stream_1->append_newlines);
>
>         /* Stream 2 does not append newlines */
> -       stream_2 = alloc_string_stream(test, GFP_KERNEL);
> +       stream_2 = kunit_alloc_string_stream(test, GFP_KERNEL);
>         KUNIT_ASSERT_NOT_ERR_OR_NULL(test, stream_2);
>
>         /* Appending a stream with a newline should not add another newline */
> @@ -238,8 +238,8 @@ static void string_stream_append_auto_newline_test(struct kunit *test)
>         KUNIT_EXPECT_STREQ(test, get_concatenated_string(test, stream_1),
>                            "Original string\nAppended content\nMore stuff\n");
>
> -       string_stream_destroy(stream_2);
> -       stream_2 = alloc_string_stream(test, GFP_KERNEL);
> +       kunit_free_string_stream(test, stream_2);
> +       stream_2 = kunit_alloc_string_stream(test, GFP_KERNEL);
>         KUNIT_ASSERT_NOT_ERR_OR_NULL(test, stream_2);
>
>         /*
> @@ -260,7 +260,7 @@ static void string_stream_append_empty_string_test(struct kunit *test)
>         struct string_stream *stream;
>         int original_frag_count;
>
> -       stream = alloc_string_stream(test, GFP_KERNEL);
> +       stream = kunit_alloc_string_stream(test, GFP_KERNEL);
>         KUNIT_ASSERT_NOT_ERR_OR_NULL(test, stream);
>
>         /* Formatted empty string */
> @@ -282,7 +282,7 @@ static void string_stream_no_auto_newline_test(struct kunit *test)
>  {
>         struct string_stream *stream;
>
> -       stream = alloc_string_stream(test, GFP_KERNEL);
> +       stream = kunit_alloc_string_stream(test, GFP_KERNEL);
>         KUNIT_ASSERT_NOT_ERR_OR_NULL(test, stream);
>
>         /*
> @@ -305,7 +305,7 @@ static void string_stream_auto_newline_test(struct kunit *test)
>  {
>         struct string_stream *stream;
>
> -       stream = alloc_string_stream(test, GFP_KERNEL);
> +       stream = kunit_alloc_string_stream(test, GFP_KERNEL);
>         KUNIT_ASSERT_NOT_ERR_OR_NULL(test, stream);
>
>         string_stream_set_append_newlines(stream, true);
> diff --git a/lib/kunit/string-stream.c b/lib/kunit/string-stream.c
> index 1dcf6513b692..12ecf15e1f6b 100644
> --- a/lib/kunit/string-stream.c
> +++ b/lib/kunit/string-stream.c
> @@ -153,7 +153,7 @@ bool string_stream_is_empty(struct string_stream *stream)
>         return list_empty(&stream->fragments);
>  }
>
> -struct string_stream *alloc_string_stream(struct kunit *test, gfp_t gfp)
> +static struct string_stream *alloc_string_stream(struct kunit *test, gfp_t gfp)
>  {
>         struct string_stream *stream;
>
> @@ -173,3 +173,13 @@ void string_stream_destroy(struct string_stream *stream)
>  {
>         string_stream_clear(stream);
>  }
> +
> +struct string_stream *kunit_alloc_string_stream(struct kunit *test, gfp_t gfp)
> +{
> +       return alloc_string_stream(test, gfp);
> +}
> +
> +void kunit_free_string_stream(struct kunit *test, struct string_stream *stream)
> +{
> +       string_stream_destroy(stream);
> +}
> diff --git a/lib/kunit/string-stream.h b/lib/kunit/string-stream.h
> index 048930bf97f0..3e70ee9d66e9 100644
> --- a/lib/kunit/string-stream.h
> +++ b/lib/kunit/string-stream.h
> @@ -30,7 +30,8 @@ struct string_stream {
>
>  struct kunit;
>
> -struct string_stream *alloc_string_stream(struct kunit *test, gfp_t gfp);
> +struct string_stream *kunit_alloc_string_stream(struct kunit *test, gfp_t gfp);
> +void kunit_free_string_stream(struct kunit *test, struct string_stream *stream);
>
>  int __printf(2, 3) string_stream_add(struct string_stream *stream,
>                                      const char *fmt, ...);
> diff --git a/lib/kunit/test.c b/lib/kunit/test.c
> index 49698a168437..93d9225d61e3 100644
> --- a/lib/kunit/test.c
> +++ b/lib/kunit/test.c
> @@ -308,7 +308,7 @@ static void kunit_fail(struct kunit *test, const struct kunit_loc *loc,
>
>         kunit_set_failure(test);
>
> -       stream = alloc_string_stream(test, GFP_KERNEL);
> +       stream = kunit_alloc_string_stream(test, GFP_KERNEL);
>         if (IS_ERR(stream)) {
>                 WARN(true,
>                      "Could not allocate stream to print failed assertion in %s:%d\n",
> @@ -322,7 +322,7 @@ static void kunit_fail(struct kunit *test, const struct kunit_loc *loc,
>
>         kunit_print_string_stream(test, stream);
>
> -       string_stream_destroy(stream);
> +       kunit_free_string_stream(test, stream);
>  }
>
>  void __noreturn __kunit_abort(struct kunit *test)
> --
> 2.30.2
>
David Gow Aug. 25, 2023, 6:50 a.m. UTC | #4
On Thu, 24 Aug 2023 at 22:33, Richard Fitzgerald
<rf@opensource.cirrus.com> wrote:
>
> Add a test of the speed and memory use of string_stream.
>
> string_stream_performance_test() doesn't actually "test" anything (it
> cannot fail unless the system has run out of allocatable memory) but it
> measures the speed and memory consumption of the string_stream and reports
> the result.
>
> This allows changes in the string_stream implementation to be compared.
>
> Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
> ---

Still looks good to me, so still:

Reviewed-by: David Gow <davidgow@google.com>

My results are:
   # string_stream_performance_test: Time elapsed:           4457 us
   # string_stream_performance_test: Total string length:    573890
   # string_stream_performance_test: Bytes requested:        823922
   # string_stream_performance_test: Actual bytes allocated: 1048280

Cheers,
-- David



>  lib/kunit/string-stream-test.c | 54 ++++++++++++++++++++++++++++++++++
>  1 file changed, 54 insertions(+)
>
> diff --git a/lib/kunit/string-stream-test.c b/lib/kunit/string-stream-test.c
> index 6897c57e0db7..7d81d139b8f8 100644
> --- a/lib/kunit/string-stream-test.c
> +++ b/lib/kunit/string-stream-test.c
> @@ -8,7 +8,9 @@
>
>  #include <kunit/static_stub.h>
>  #include <kunit/test.h>
> +#include <linux/ktime.h>
>  #include <linux/slab.h>
> +#include <linux/timekeeping.h>
>
>  #include "string-stream.h"
>
> @@ -437,6 +439,57 @@ static void string_stream_auto_newline_test(struct kunit *test)
>                            "One\nTwo\nThree\nFour\nFive\nSix\nSeven\n\nEight\n");
>  }
>
> +/*
> + * This doesn't actually "test" anything. It reports time taken
> + * and memory used for logging a large number of lines.
> + */
> +static void string_stream_performance_test(struct kunit *test)
> +{
> +       struct string_stream_fragment *frag_container;
> +       struct string_stream *stream;
> +       char test_line[101];
> +       ktime_t start_time, end_time;
> +       size_t len, bytes_requested, actual_bytes_used, total_string_length;
> +       int offset, i;
> +
> +       stream = kunit_alloc_string_stream(test, GFP_KERNEL);
> +       KUNIT_ASSERT_NOT_ERR_OR_NULL(test, stream);
> +
> +       memset(test_line, 'x', sizeof(test_line) - 1);
> +       test_line[sizeof(test_line) - 1] = '\0';
> +
> +       start_time = ktime_get();
> +       for (i = 0; i < 10000; i++) {
> +               offset = i % (sizeof(test_line) - 1);
> +               string_stream_add(stream, "%s: %d\n", &test_line[offset], i);
> +       }
> +       end_time = ktime_get();
> +
> +       /*
> +        * Calculate memory used. This doesn't include invisible
> +        * overhead due to kernel allocator fragment size rounding.
> +        */
> +       bytes_requested = sizeof(*stream);
> +       actual_bytes_used = ksize(stream);
> +       total_string_length = 0;
> +
> +       list_for_each_entry(frag_container, &stream->fragments, node) {
> +               bytes_requested += sizeof(*frag_container);
> +               actual_bytes_used += ksize(frag_container);
> +
> +               len = strlen(frag_container->fragment);
> +               total_string_length += len;
> +               bytes_requested += len + 1; /* +1 for '\0' */
> +               actual_bytes_used += ksize(frag_container->fragment);
> +       }
> +
> +       kunit_info(test, "Time elapsed:           %lld us\n",
> +                  ktime_us_delta(end_time, start_time));
> +       kunit_info(test, "Total string length:    %zu\n", total_string_length);
> +       kunit_info(test, "Bytes requested:        %zu\n", bytes_requested);
> +       kunit_info(test, "Actual bytes allocated: %zu\n", actual_bytes_used);
> +}
> +
>  static int string_stream_test_init(struct kunit *test)
>  {
>         struct string_stream_test_priv *priv;
> @@ -462,6 +515,7 @@ static struct kunit_case string_stream_test_cases[] = {
>         KUNIT_CASE(string_stream_append_empty_string_test),
>         KUNIT_CASE(string_stream_no_auto_newline_test),
>         KUNIT_CASE(string_stream_auto_newline_test),
> +       KUNIT_CASE(string_stream_performance_test),
>         {}
>  };
>
> --
> 2.30.2
>
David Gow Aug. 25, 2023, 6:58 a.m. UTC | #5
On Thu, 24 Aug 2023 at 22:32, 'Richard Fitzgerald' via KUnit
Development <kunit-dev@googlegroups.com> wrote:
>
> This patch chain changes the logging implementation to use string_stream
> so that the log will grow dynamically.
>
> The first 8 patches add test code for string_stream, and make some
> changes to string_stream needed to be able to use it for the log.
>
> The final patch adds a performance report of string_stream.
>
> CHANGES SINCE V4:
> - Re-ordered the first 3 patches from V4 to squash the first two sets
>   of string_stream tests into a single patch.
> - Changed is_literal() so it doesn't need a struct kunit.
> - Split out the new resource-managed alloc and free functions into
>   a pre-patch to reduce the amount of code churn when the string_stream
>   is decoupled from kunit.
> - Wrapped the call to string_stream_geT_string() in string-stream-test
>   in a local function to reduce the amount of code churn when the
>   string_stream is decoupled from kunit.
> - Some minor changes to test implementations.
> - string_stream is now completely separated from kunit and the 'test'
>   member of struct string_stream has been eliminated.
>
> Richard Fitzgerald (10):
>   kunit: string-stream: Don't create a fragment for empty strings
>   kunit: string-stream: Improve testing of string_stream
>   kunit: string-stream: Add option to make all lines end with newline
>   kunit: string-stream: Add cases for string_stream newline appending
>   kunit: Don't use a managed alloc in is_literal()
>   kunit: string-stream: Add kunit_alloc_string_stream()
>   kunit: string-stream: Decouple string_stream from kunit
>   kunit: string-stream: Add tests for freeing resource-managed
>     string_stream
>   kunit: Use string_stream for test log
>   kunit: string-stream: Test performance of string_stream
>

Thanks a lot for sticking with this. I think we're in pretty good
shape now. There are a few minor comments, only one of which really
concerns me. That's the freeing of string streams in the
resource-managed string stream tests. I can't quite see how the actual
stream is freed after being "fake freed" by the stub. Is there
something I'm missing?

Otherwise, this seems good enough to go. I fear we're probably past
the point where it can make it into 6.6 (pull requests are already
being sent out, and I'd really rather have the final version of this
soak in linux-next for a while before sending it to Linus. But we'll
make it the first thing to go into 6.7, I think.

Cheers,
-- David


>  include/kunit/test.h           |  14 +-
>  lib/kunit/assert.c             |  14 +-
>  lib/kunit/debugfs.c            |  36 ++-
>  lib/kunit/kunit-test.c         |  46 ++-
>  lib/kunit/string-stream-test.c | 508 +++++++++++++++++++++++++++++++--
>  lib/kunit/string-stream.c      | 100 +++++--
>  lib/kunit/string-stream.h      |  16 +-
>  lib/kunit/test.c               |  50 +---
>  8 files changed, 662 insertions(+), 122 deletions(-)
>
> --
> 2.30.2
>
> --
> You received this message because you are subscribed to the Google Groups "KUnit Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/20230824143129.1957914-1-rf%40opensource.cirrus.com.