On Fri, 2019-08-30 at 23:22 +0000, Tim.Bird@sony.com wrote:
-----Original Message----- From: Brendan Higgins
On Fri, Aug 30, 2019 at 3:46 PM Joe Perches joe@perches.com wrote:
On Fri, 2019-08-30 at 21:58 +0000, Tim.Bird@sony.com wrote:
From: Joe Perches
[]
IMHO %pV should be avoided if possible. Just because people are doing it doesn't mean it should be used when it is not necessary.
Well, as the guy that created %pV, I of course have a different opinion.
then wouldn't it be easier to pass in the
kernel level as a separate parameter and then strip off all printk headers like this:
Depends on whether or not you care for overall object size. Consolidated formats with the embedded KERN_<LEVEL> like suggested are smaller overall object size.
This is an argument I can agree with. I'm generally in favor of things that lessen kernel size creep. :-)
As am I.
Sorry, to be clear, we are talking about the object size penalty due to adding a single parameter to a function. Is that right?
Not exactly. The argument is that pre-pending the different KERN_LEVEL strings onto format strings can result in several versions of nearly identical strings being compiled into the object file. By parameterizing this (that is, adding '%s' into the format string, and putting the level into the string as an argument), it prevents this duplication of format strings.
I haven't seen the data on duplication of format strings, and how much this affects it, but little things can add up. Whether it matters in this case depends on whether the format strings that kunit uses are also used elsewhere in the kernel, and whether these same format strings are used with multiple kernel message levels.
deduplication can matter as well, but so far there is little content with kunit_(err|warn|info(=)
kunit/example-test.c: kunit_info(test, "initializing\n"); kunit/test.c: kunit_err(test, kunit/test.c: kunit_err(test, "%s", fragment->fragment); kunit/test.c: kunit_err(test, "\n"); kunit/test.c: kunit_err(test, "%s", buf); kunit/test.c: kunit_err(test, "failed to initialize: %d\n", ret); kunit/test.c: kunit_err(test, "test case timed out\n"); kunit/test.c: kunit_err(test, "internal error occurred preventing test case from running: %d\n", kunit/try-catch.c: kunit_err(test, "try timed out\n"); kunit/try-catch.c: kunit_err(test, "wake_up_process() was never called\n"); kunit/try-catch.c: kunit_err(test, "Unknown error: %d\n", exit_code);
Of these, only two do match other kernel uses.
"initializing\n", "failed to initialize: %d\n"