All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Tamir Duberstein <tamird@gmail.com>
Cc: Petr Mladek <pmladek@suse.com>, David Gow <davidgow@google.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	Sergey Senozhatsky <senozhatsky@chromium.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Shuah Khan <shuah@kernel.org>,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org
Subject: Re: [PATCH v8 4/4] scanf: break kunit into test cases
Date: Wed, 5 Mar 2025 21:35:21 +0200	[thread overview]
Message-ID: <Z8ineT_g9gzcftWe@smile.fi.intel.com> (raw)
In-Reply-To: <CAJ-ks9kz-fEH1YLiCn3fHR9AtYQLCZS77GKfOObifEL4GLwk8A@mail.gmail.com>

On Wed, Mar 05, 2025 at 10:57:47AM -0500, Tamir Duberstein wrote:
> On Wed, Mar 5, 2025 at 10:55 AM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Wed, Mar 05, 2025 at 10:25:51AM -0500, Tamir Duberstein wrote:
> > > On Wed, Mar 5, 2025 at 10:01 AM Petr Mladek <pmladek@suse.com> wrote:
> > > > On Fri 2025-02-14 11:20:01, Tamir Duberstein wrote:

...

> > > > >  #include <kunit/test.h>
> > > > > -#include <linux/bitops.h>
> > > > > -#include <linux/kernel.h>
> > > > >  #include <linux/module.h>
> > > > > -#include <linux/overflow.h>
> > > > > -#include <linux/printk.h>
> > > > >  #include <linux/prandom.h>
> > > > >  #include <linux/slab.h>
> > > > > -#include <linux/string.h>
> > > > > +#include <linux/sprintf.h>
> > > > >
> > > > >  #define BUF_SIZE 1024
> > > >
> > > > It would make more sense to do this clean up in the 3rd patch
> > > > where some code was replaced by the kunit macros.
> > > >
> > > > Also I am not sure about the choice. It might make sense to remove
> > > > <include/printk.h> because the pr_*() calls were removed.
> > > > But what about the others? Did anyone request the clean up, please?
> > > >
> > > > I do not want to open a bike shadding because different people
> > > > have different opinion.
> > > >
> > > > I would personally prefer to keep the explicit includes when the
> > > > related API is still used. It helps to optimize nested includes
> > > > in the header files which helps to speedup build. AFAIK, there
> > > > are people working in this optimization and they might need
> > > > to revert this change.
> > >
> > > Yeah, I don't feel strongly. I'll just restore all the includes.
> >
> > It will be blind approach. Please, try to look at them closely and include what
> > you use (IWYU principle). I don't think anybody uses kernel.h here, for
> > example.
> 
> I think I'm getting conflicting instructions here. IWYU is indeed what
> I did: bitops, kernel, overflow, printk are all unused; string is used
> only for sprintf, so I made that replacement.
> 
> However Petr said "Did anyone request the clean up, please?" which
> implies to me an aversion to unwanted cleanup. So, which is it please?

I believe he asks the background of the change. And if it made in a separate
patch it would be clearer to begin with (e.g., Suggested-by tag).

But I don't know how you deducted that it's unwanted. With a separate patch
we may discuss and see if it's wanted or not. In any case I would like to see
such a patch.

-- 
With Best Regards,
Andy Shevchenko



  reply	other threads:[~2025-03-05 19:35 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-14 16:19 [PATCH v8 0/4] scanf: convert self-test to KUnit Tamir Duberstein
2025-02-14 16:19 ` [PATCH v8 1/4] scanf: implicate test line in failure messages Tamir Duberstein
2025-03-05  8:55   ` Petr Mladek
2025-03-05  9:56   ` Petr Mladek
2025-03-05 11:35     ` Tamir Duberstein
2025-02-14 16:19 ` [PATCH v8 2/4] scanf: remove redundant debug logs Tamir Duberstein
2025-03-05  8:59   ` Petr Mladek
2025-02-14 16:20 ` [PATCH v8 3/4] scanf: convert self-test to KUnit Tamir Duberstein
2025-02-15 18:50   ` kernel test robot
2025-02-15 19:52     ` Tamir Duberstein
2025-02-16 20:22       ` Andy Shevchenko
2025-02-17 14:29         ` Tamir Duberstein
2025-03-05  9:47       ` Petr Mladek
2025-03-05 11:36         ` Tamir Duberstein
2025-03-05 12:46   ` Petr Mladek
2025-02-14 16:20 ` [PATCH v8 4/4] scanf: break kunit into test cases Tamir Duberstein
2025-03-05 15:01   ` Petr Mladek
2025-03-05 15:25     ` Tamir Duberstein
2025-03-05 15:55       ` Andy Shevchenko
2025-03-05 15:57         ` Tamir Duberstein
2025-03-05 19:35           ` Andy Shevchenko [this message]
2025-03-05 20:09             ` Tamir Duberstein
2025-03-06  9:38           ` Petr Mladek
2025-03-06 10:56             ` Tamir Duberstein
2025-03-05 15:54     ` Andy Shevchenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Z8ineT_g9gzcftWe@smile.fi.intel.com \
    --to=andriy.shevchenko@linux.intel.com \
    --cc=akpm@linux-foundation.org \
    --cc=davidgow@google.com \
    --cc=geert@linux-m68k.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=pmladek@suse.com \
    --cc=rostedt@goodmis.org \
    --cc=senozhatsky@chromium.org \
    --cc=shuah@kernel.org \
    --cc=tamird@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.