[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <30f0e085-b636-45be-960b-68bf6a136f59@redhat.com>
Date: Mon, 11 Aug 2025 10:49:00 +0000 (UTC)
From: Gabriele Monaco <gmonaco@...hat.com>
To: Akhilesh Patil <akhilesh@...iitb.ac.in>
Cc: rostedt@...dmis.org, mhiramat@...nel.org, mathieu.desnoyers@...icios.com,
linux-trace-kernel@...r.kernel.org, linux-kernel@...r.kernel.org,
akhileshpatilvnit@...il.com, skhan@...uxfoundation.org
Subject: Re: [PATCH] include/linux/rv.h: remove redundant include file
2025-08-09T06:36:58Z Akhilesh Patil <akhilesh@...iitb.ac.in>:
> Remove redundant include <linux/types.h> to clean up the code.
> Fix this redundancy introduced by commit [1].
>
> Fixes: 24cbfe18d55a ("rv: Merge struct rv_monitor_def into struct rv_monitor") [1]
> Reported-by: kernel test robot <lkp@...el.com>
> Closes: https://lore.kernel.org/r/202507312017.oyD08TL5-lkp@intel.com/
> Signed-off-by: Akhilesh Patil <akhilesh@...iitb.ac.in>
> ---
Thanks for the patch!
I'm really being picky here, but, since you're touching this, isn't it cleaner to keep all includes inside ifdef CONFIG_RV ?
When CONFIG_RV is not enabled, the header only defines constants so it doesn't need other includes.
This would mean you could remove the other #include <linux/types.h>, instead, and move the #include <linux/list.h> down.
I think you can keep the Fixes: tag and make clear the reason for cleanup in the commit message.
Thanks,
Gabriele
> include/linux/rv.h | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/include/linux/rv.h b/include/linux/rv.h
> index 14410a42faef..8b968b8ed77b 100644
> --- a/include/linux/rv.h
> +++ b/include/linux/rv.h
> @@ -15,7 +15,6 @@
>
> #ifdef CONFIG_RV
> #include <linux/bitops.h>
> -#include <linux/types.h>
> #include <linux/array_size.h>
>
> /*
> --
> 2.34.1
Powered by blists - more mailing lists