linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] ia64: clean-up header dependency and build process, fix build warning
@ 2020-08-29  5:15 Masahiro Yamada
  2020-08-29  5:15 ` [PATCH 1/3] ia64: do not typedef struct pal_min_state_area_s Masahiro Yamada
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Masahiro Yamada @ 2020-08-29  5:15 UTC (permalink / raw)
  To: Tony Luck, Fenghua Yu, linux-ia64
  Cc: Randy Dunlap, linux-kbuild, Masahiro Yamada, Andrew Morton,
	Ard Biesheuvel, Arnd Bergmann, Dmitry Safonov, afzal mohammed,
	linux-efi, linux-kernel


Randy Dunlap reports the following warning with CONFIG_IA64_PALINFO=m:

../scripts/Makefile.build:68: 'arch/ia64/kernel/palinfo.ko' will not be built even though obj-m is specified.
../scripts/Makefile.build:69: You cannot use subdir-y/m to visit a module Makefile. Use obj-y/m instead.

This comes from the fact Kbuild descends into arch/ia64/kernel/ twice.

First, to generate <generated/nr-irqs.h>,
Second, to build kernel and module objects.

The warning is emitted in the first descend because it is not the
intended usage.

I looked into the code closely, and noticed arch/ia64/kernel/nr-irqs.c
was not needed in the first place.

It was separated out of arch/ia64/kernel/asm-offsets.c just because
<asm/mca.h> was including too many bogus headers.

IA64 is not actively maintained, and there exists unneeded obsolete code.

The first two patches are the outcome when I played with ARCH=ia64 builds,
but not prerequisites for 3/3. Anyway I believe they are nice cleanups
and folded in this patch set.

3/3 is the important one to fix the false positive warning,
and it is a nice cleanup too.



Masahiro Yamada (3):
  ia64: do not typedef struct pal_min_state_area_s
  ia64: remove unneeded header includes from <asm/mca.h>
  ia64: remove generated/nr-irqs.h generation to fix build warning

 arch/ia64/Makefile             |  6 ------
 arch/ia64/include/asm/irq.h    |  4 +++-
 arch/ia64/include/asm/mca.h    | 11 ++++-------
 arch/ia64/include/asm/pal.h    |  4 ++--
 arch/ia64/include/asm/sal.h    |  2 +-
 arch/ia64/kernel/Makefile      |  5 -----
 arch/ia64/kernel/asm-offsets.c | 18 +++++++++---------
 arch/ia64/kernel/efi.c         |  1 +
 arch/ia64/kernel/mca.c         |  5 +++--
 arch/ia64/kernel/mca_drv.c     |  2 +-
 arch/ia64/kernel/nr-irqs.c     | 22 ----------------------
 11 files changed, 24 insertions(+), 56 deletions(-)
 delete mode 100644 arch/ia64/kernel/nr-irqs.c

-- 
2.25.1


^ permalink raw reply	[flat|nested] 10+ messages in thread
* [PATCH 1/3] ia64: do not typedef struct pal_min_state_area_s
  2020-08-29  5:15 [PATCH 0/3] ia64: clean-up header dependency and build process, fix build warning Masahiro Yamada
@ 2020-08-29  5:15 ` Masahiro Yamada
  2020-08-29 18:21   ` Randy Dunlap
  2020-08-29  5:15 ` [PATCH 2/3] ia64: remove unneeded header includes from <asm/mca.h> Masahiro Yamada
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Masahiro Yamada @ 2020-08-29  5:15 UTC (permalink / raw)
  To: Tony Luck, Fenghua Yu, linux-ia64
  Cc: Randy Dunlap, linux-kbuild, Masahiro Yamada, Andrew Morton,
	Arnd Bergmann, Dmitry Safonov, afzal mohammed, linux-kernel

Documentation/process/coding-style.rst says:

  Please don't use things like ``vps_t``.
  It's a **mistake** to use typedef for structures and pointers.

This commit converts as follows:

  struct pal_min_state_area_s  ->  struct pal_min_state_area
         pal_min_state_area_t  ->  struct pal_min_state_area

My main motivation for this is to slim down the include directives
of <asm/mca.h> in the next commit.

Currently, <asm/mca.h> is required to include <asm/pal.h> directly
or indirectly due to (pal_min_state_area_t *). Otherwise, it would
have no idea what pal_min_state_area_t is.

Replacing it with (struct pal_min_state_area *) will relax the header
dependency since it is enough to tell it is a pointer to a structure,
and to resolve the size of struct pal_min_state_area. It will make
<asm/mca.h> independent of <asm/pal.h>.

<asm/pal.h> typedef's a lot of structures, but it is trivial to
convert the others in the same way.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 arch/ia64/include/asm/mca.h    |  2 +-
 arch/ia64/include/asm/pal.h    |  4 ++--
 arch/ia64/include/asm/sal.h    |  2 +-
 arch/ia64/kernel/asm-offsets.c | 18 +++++++++---------
 arch/ia64/kernel/mca.c         |  4 ++--
 arch/ia64/kernel/mca_drv.c     |  2 +-
 6 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/arch/ia64/include/asm/mca.h b/arch/ia64/include/asm/mca.h
index 726df17f1b51..c92b9c15962c 100644
--- a/arch/ia64/include/asm/mca.h
+++ b/arch/ia64/include/asm/mca.h
@@ -83,7 +83,7 @@ struct ia64_sal_os_state {
 	/* common */
 	unsigned long		sal_ra;			/* Return address in SAL, physical */
 	unsigned long		sal_gp;			/* GP of the SAL - physical */
-	pal_min_state_area_t	*pal_min_state;		/* from R17.  physical in asm, virtual in C */
+	struct pal_min_state_area *pal_min_state;	/* from R17.  physical in asm, virtual in C */
 	/* Previous values of IA64_KR(CURRENT) and IA64_KR(CURRENT_STACK).
 	 * Note: if the MCA/INIT recovery code wants to resume to a new context
 	 * then it must change these values to reflect the new kernel stack.
diff --git a/arch/ia64/include/asm/pal.h b/arch/ia64/include/asm/pal.h
index f9d2b3b2dfad..b1d87955e8cc 100644
--- a/arch/ia64/include/asm/pal.h
+++ b/arch/ia64/include/asm/pal.h
@@ -750,7 +750,7 @@ typedef union pal_mc_error_info_u {
  * for PAL.
  */
 
-typedef struct pal_min_state_area_s {
+struct pal_min_state_area {
 	u64	pmsa_nat_bits;		/* nat bits for saved GRs  */
 	u64	pmsa_gr[15];		/* GR1	- GR15		   */
 	u64	pmsa_bank0_gr[16];	/* GR16 - GR31		   */
@@ -766,7 +766,7 @@ typedef struct pal_min_state_area_s {
 	u64	pmsa_xfs;		/* previous ifs		   */
 	u64	pmsa_br1;		/* branch register 1	   */
 	u64	pmsa_reserved[70];	/* pal_min_state_area should total to 1KB */
-} pal_min_state_area_t;
+};
 
 
 struct ia64_pal_retval {
diff --git a/arch/ia64/include/asm/sal.h b/arch/ia64/include/asm/sal.h
index 08f5b6aaed73..78f4f7b40435 100644
--- a/arch/ia64/include/asm/sal.h
+++ b/arch/ia64/include/asm/sal.h
@@ -385,7 +385,7 @@ typedef struct sal_processor_static_info {
 		    fr              : 1,
 		    reserved        : 58;
 	} valid;
-	pal_min_state_area_t min_state_area;
+	struct pal_min_state_area min_state_area;
 	u64 br[8];
 	u64 cr[128];
 	u64 ar[128];
diff --git a/arch/ia64/kernel/asm-offsets.c b/arch/ia64/kernel/asm-offsets.c
index fb0deb8a4221..be3b90fef2e9 100644
--- a/arch/ia64/kernel/asm-offsets.c
+++ b/arch/ia64/kernel/asm-offsets.c
@@ -245,23 +245,23 @@ void foo(void)
 	BLANK();
 
 	DEFINE(IA64_PMSA_GR_OFFSET,
-	       offsetof (struct pal_min_state_area_s, pmsa_gr));
+	       offsetof(struct pal_min_state_area, pmsa_gr));
 	DEFINE(IA64_PMSA_BANK1_GR_OFFSET,
-	       offsetof (struct pal_min_state_area_s, pmsa_bank1_gr));
+	       offsetof(struct pal_min_state_area, pmsa_bank1_gr));
 	DEFINE(IA64_PMSA_PR_OFFSET,
-	       offsetof (struct pal_min_state_area_s, pmsa_pr));
+	       offsetof(struct pal_min_state_area, pmsa_pr));
 	DEFINE(IA64_PMSA_BR0_OFFSET,
-	       offsetof (struct pal_min_state_area_s, pmsa_br0));
+	       offsetof(struct pal_min_state_area, pmsa_br0));
 	DEFINE(IA64_PMSA_RSC_OFFSET,
-	       offsetof (struct pal_min_state_area_s, pmsa_rsc));
+	       offsetof(struct pal_min_state_area, pmsa_rsc));
 	DEFINE(IA64_PMSA_IIP_OFFSET,
-	       offsetof (struct pal_min_state_area_s, pmsa_iip));
+	       offsetof(struct pal_min_state_area, pmsa_iip));
 	DEFINE(IA64_PMSA_IPSR_OFFSET,
-	       offsetof (struct pal_min_state_area_s, pmsa_ipsr));
+	       offsetof(struct pal_min_state_area, pmsa_ipsr));
 	DEFINE(IA64_PMSA_IFS_OFFSET,
-	       offsetof (struct pal_min_state_area_s, pmsa_ifs));
+	       offsetof(struct pal_min_state_area, pmsa_ifs));
 	DEFINE(IA64_PMSA_XIP_OFFSET,
-	       offsetof (struct pal_min_state_area_s, pmsa_xip));
+	       offsetof(struct pal_min_state_area, pmsa_xip));
 	BLANK();
 
 	/* used by fsys_gettimeofday in arch/ia64/kernel/fsys.S */
diff --git a/arch/ia64/kernel/mca.c b/arch/ia64/kernel/mca.c
index 2703f7795672..17151269d655 100644
--- a/arch/ia64/kernel/mca.c
+++ b/arch/ia64/kernel/mca.c
@@ -894,7 +894,7 @@ static void
 finish_pt_regs(struct pt_regs *regs, struct ia64_sal_os_state *sos,
 		unsigned long *nat)
 {
-	const pal_min_state_area_t *ms = sos->pal_min_state;
+	const struct pal_min_state_area *ms = sos->pal_min_state;
 	const u64 *bank;
 
 	/* If ipsr.ic then use pmsa_{iip,ipsr,ifs}, else use
@@ -970,7 +970,7 @@ ia64_mca_modify_original_stack(struct pt_regs *regs,
 	char *p;
 	ia64_va va;
 	extern char ia64_leave_kernel[];	/* Need asm address, not function descriptor */
-	const pal_min_state_area_t *ms = sos->pal_min_state;
+	const struct pal_min_state_area *ms = sos->pal_min_state;
 	struct task_struct *previous_current;
 	struct pt_regs *old_regs;
 	struct switch_stack *old_sw;
diff --git a/arch/ia64/kernel/mca_drv.c b/arch/ia64/kernel/mca_drv.c
index 4d0ab323dee8..36a69b4e6169 100644
--- a/arch/ia64/kernel/mca_drv.c
+++ b/arch/ia64/kernel/mca_drv.c
@@ -496,7 +496,7 @@ recover_from_read_error(slidx_table_t *slidx,
 			struct ia64_sal_os_state *sos)
 {
 	u64 target_identifier;
-	pal_min_state_area_t *pmsa;
+	struct pal_min_state_area *pmsa;
 	struct ia64_psr *psr1, *psr2;
 	ia64_fptr_t *mca_hdlr_bh = (ia64_fptr_t*)mca_handler_bhhook;
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH 2/3] ia64: remove unneeded header includes from <asm/mca.h>
  2020-08-29  5:15 [PATCH 0/3] ia64: clean-up header dependency and build process, fix build warning Masahiro Yamada
  2020-08-29  5:15 ` [PATCH 1/3] ia64: do not typedef struct pal_min_state_area_s Masahiro Yamada
@ 2020-08-29  5:15 ` Masahiro Yamada
  2020-08-29 18:20   ` Randy Dunlap
  2020-08-31 15:00   ` Ard Biesheuvel
  2020-08-29  5:15 ` [PATCH 3/3] ia64: remove generated/nr-irqs.h generation to fix build warning Masahiro Yamada
  2020-12-04  5:43 ` [PATCH 0/3] ia64: clean-up header dependency and build process, " Masahiro Yamada
  3 siblings, 2 replies; 10+ messages in thread
From: Masahiro Yamada @ 2020-08-29  5:15 UTC (permalink / raw)
  To: Tony Luck, Fenghua Yu, linux-ia64
  Cc: Randy Dunlap, linux-kbuild, Masahiro Yamada, Andrew Morton,
	Ard Biesheuvel, Dmitry Safonov, afzal mohammed, linux-efi,
	linux-kernel

<asm/mca.h> includes too many unneeded headers.

This commit cuts off a lot of header includes.

What we need to include are:

 - <linux/percpu.h> for DECLARE_PER_CPU(u64, ia64_mca_pal_base)
 - <linux/threads.h> for NR_CPUS
 - <linux/types.h> for u8, u64, size_t, etc.
 - <asm/ptrace.h> for KERNEL_STACK_SIZE

The other header includes are actually unneeded.

<asm/mca.h> previously included 436 headers, and now it includes
only 138. I confirmed <asm/mca.h> is still self-contained.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 arch/ia64/include/asm/mca.h | 9 +++------
 arch/ia64/kernel/efi.c      | 1 +
 arch/ia64/kernel/mca.c      | 1 +
 3 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/arch/ia64/include/asm/mca.h b/arch/ia64/include/asm/mca.h
index c92b9c15962c..05805249296c 100644
--- a/arch/ia64/include/asm/mca.h
+++ b/arch/ia64/include/asm/mca.h
@@ -14,13 +14,10 @@
 
 #if !defined(__ASSEMBLY__)
 
-#include <linux/interrupt.h>
+#include <linux/percpu.h>
+#include <linux/threads.h>
 #include <linux/types.h>
-
-#include <asm/param.h>
-#include <asm/sal.h>
-#include <asm/processor.h>
-#include <asm/mca_asm.h>
+#include <asm/ptrace.h>
 
 #define IA64_MCA_RENDEZ_TIMEOUT		(20 * 1000)	/* value in milliseconds - 20 seconds */
 
diff --git a/arch/ia64/kernel/efi.c b/arch/ia64/kernel/efi.c
index f932b25fb817..b6bb718ed1ff 100644
--- a/arch/ia64/kernel/efi.c
+++ b/arch/ia64/kernel/efi.c
@@ -39,6 +39,7 @@
 #include <asm/meminit.h>
 #include <asm/processor.h>
 #include <asm/mca.h>
+#include <asm/sal.h>
 #include <asm/setup.h>
 #include <asm/tlbflush.h>
 
diff --git a/arch/ia64/kernel/mca.c b/arch/ia64/kernel/mca.c
index 17151269d655..3911c561d2bb 100644
--- a/arch/ia64/kernel/mca.c
+++ b/arch/ia64/kernel/mca.c
@@ -96,6 +96,7 @@
 #include <asm/ptrace.h>
 #include <asm/sal.h>
 #include <asm/mca.h>
+#include <asm/mca_asm.h>
 #include <asm/kexec.h>
 
 #include <asm/irq.h>
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH 3/3] ia64: remove generated/nr-irqs.h generation to fix build warning
  2020-08-29  5:15 [PATCH 0/3] ia64: clean-up header dependency and build process, fix build warning Masahiro Yamada
  2020-08-29  5:15 ` [PATCH 1/3] ia64: do not typedef struct pal_min_state_area_s Masahiro Yamada
  2020-08-29  5:15 ` [PATCH 2/3] ia64: remove unneeded header includes from <asm/mca.h> Masahiro Yamada
@ 2020-08-29  5:15 ` Masahiro Yamada
  2020-08-29 18:22   ` Randy Dunlap
  2021-02-07  2:46   ` Masahiro Yamada
  2020-12-04  5:43 ` [PATCH 0/3] ia64: clean-up header dependency and build process, " Masahiro Yamada
  3 siblings, 2 replies; 10+ messages in thread
From: Masahiro Yamada @ 2020-08-29  5:15 UTC (permalink / raw)
  To: Tony Luck, Fenghua Yu, linux-ia64
  Cc: Randy Dunlap, linux-kbuild, Masahiro Yamada, linux-kernel

Randy reports the following warning when building ARCH=ia64 with
CONFIG_IA64_PALINFO=m:

../scripts/Makefile.build:68: 'arch/ia64/kernel/palinfo.ko' will not be built even though obj-m is specified.
../scripts/Makefile.build:69: You cannot use subdir-y/m to visit a module Makefile. Use obj-y/m instead.

This message is actually false-positive, and you can get palinfo.ko
correctly built. It is emitted in the archprepare stage, where Kbuild
descends into arch/ia64/kernel to generate include/generated/nr-irqs.h
instead of any kind of kernel objects.

arch/ia64/kernel/nr-irqs.c was introduced by commit 213060a4d699
("[IA64] pvops: paravirtualize NR_IRQS") to pre-calculate:

   NR_IRQS = max(IA64_NATIVE_NR_IRQS, XEN_NR_IRQS, FOO_NR_IRQS...)

Since commit d52eefb47d4e ("ia64/xen: Remove Xen support for ia64"), this
union contains just one field, making NR_IRQS and IA64_NATIVE_NR_IRQS
always match.

So, the following hard-coding now works:

  #define NR_IRQS                IA64_NATIVE_NR_IRQS

If you need to re-introduce NR_IRQS = max(...) gimmick in the future,
please try to implement it in asm-offsets.c instead of a separate file.
It will be possible because the header inclusion has been consolidated
to make asm-offsets.c independent of <asm/irqs.h>.

Reported-by: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 arch/ia64/Makefile          |  6 ------
 arch/ia64/include/asm/irq.h |  4 +++-
 arch/ia64/kernel/Makefile   |  5 -----
 arch/ia64/kernel/nr-irqs.c  | 22 ----------------------
 4 files changed, 3 insertions(+), 34 deletions(-)
 delete mode 100644 arch/ia64/kernel/nr-irqs.c

diff --git a/arch/ia64/Makefile b/arch/ia64/Makefile
index 2876a7df1b0a..3d54d411fcc4 100644
--- a/arch/ia64/Makefile
+++ b/arch/ia64/Makefile
@@ -87,9 +87,3 @@ define archhelp
   echo '  install	- Install compressed kernel image'
   echo '* unwcheck	- Check vmlinux for invalid unwind info'
 endef
-
-archprepare: make_nr_irqs_h
-PHONY += make_nr_irqs_h
-
-make_nr_irqs_h:
-	$(Q)$(MAKE) $(build)=arch/ia64/kernel include/generated/nr-irqs.h
diff --git a/arch/ia64/include/asm/irq.h b/arch/ia64/include/asm/irq.h
index 5acf52e90872..0eccf33dfe8b 100644
--- a/arch/ia64/include/asm/irq.h
+++ b/arch/ia64/include/asm/irq.h
@@ -14,7 +14,9 @@
 
 #include <linux/types.h>
 #include <linux/cpumask.h>
-#include <generated/nr-irqs.h>
+#include <asm/native/irq.h>
+
+#define NR_IRQS		IA64_NATIVE_NR_IRQS
 
 static __inline__ int
 irq_canonicalize (int irq)
diff --git a/arch/ia64/kernel/Makefile b/arch/ia64/kernel/Makefile
index 1a8df6669eee..7c9354ee4b3e 100644
--- a/arch/ia64/kernel/Makefile
+++ b/arch/ia64/kernel/Makefile
@@ -48,8 +48,3 @@ CFLAGS_traps.o  += -mfixed-range=f2-f5,f16-f31
 
 # The gate DSO image is built using a special linker script.
 include $(src)/Makefile.gate
-
-include/generated/nr-irqs.h: arch/$(SRCARCH)/kernel/nr-irqs.s FORCE
-	$(call filechk,offsets,__ASM_NR_IRQS_H__)
-
-targets += nr-irqs.s
diff --git a/arch/ia64/kernel/nr-irqs.c b/arch/ia64/kernel/nr-irqs.c
deleted file mode 100644
index f2633b22d3be..000000000000
--- a/arch/ia64/kernel/nr-irqs.c
+++ /dev/null
@@ -1,22 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * calculate
- * NR_IRQS = max(IA64_NATIVE_NR_IRQS, XEN_NR_IRQS, FOO_NR_IRQS...)
- * depending on config.
- * This must be calculated before processing asm-offset.c.
- */
-
-#define ASM_OFFSETS_C 1
-
-#include <linux/kbuild.h>
-#include <linux/threads.h>
-#include <asm/native/irq.h>
-
-void foo(void)
-{
-	union paravirt_nr_irqs_max {
-		char ia64_native_nr_irqs[IA64_NATIVE_NR_IRQS];
-	};
-
-	DEFINE(NR_IRQS, sizeof (union paravirt_nr_irqs_max));
-}
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] ia64: remove unneeded header includes from <asm/mca.h>
  2020-08-29  5:15 ` [PATCH 2/3] ia64: remove unneeded header includes from <asm/mca.h> Masahiro Yamada
@ 2020-08-29 18:20   ` Randy Dunlap
  2020-08-31 15:00   ` Ard Biesheuvel
  1 sibling, 0 replies; 10+ messages in thread
From: Randy Dunlap @ 2020-08-29 18:20 UTC (permalink / raw)
  To: Masahiro Yamada, Tony Luck, Fenghua Yu, linux-ia64
  Cc: linux-kbuild, Andrew Morton, Ard Biesheuvel, Dmitry Safonov,
	afzal mohammed, linux-efi, linux-kernel

On 8/28/20 10:15 PM, Masahiro Yamada wrote:
> <asm/mca.h> includes too many unneeded headers.
> 
> This commit cuts off a lot of header includes.
> 
> What we need to include are:
> 
>  - <linux/percpu.h> for DECLARE_PER_CPU(u64, ia64_mca_pal_base)
>  - <linux/threads.h> for NR_CPUS
>  - <linux/types.h> for u8, u64, size_t, etc.
>  - <asm/ptrace.h> for KERNEL_STACK_SIZE
> 
> The other header includes are actually unneeded.
> 
> <asm/mca.h> previously included 436 headers, and now it includes
> only 138. I confirmed <asm/mca.h> is still self-contained.

Nice!

> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>

Reviewed-by: Randy Dunlap <rdunlap@infradead.org>

> ---
> 
>  arch/ia64/include/asm/mca.h | 9 +++------
>  arch/ia64/kernel/efi.c      | 1 +
>  arch/ia64/kernel/mca.c      | 1 +
>  3 files changed, 5 insertions(+), 6 deletions(-)
> 


thanks.
-- 
~Randy


^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] ia64: do not typedef struct pal_min_state_area_s
  2020-08-29  5:15 ` [PATCH 1/3] ia64: do not typedef struct pal_min_state_area_s Masahiro Yamada
@ 2020-08-29 18:21   ` Randy Dunlap
  0 siblings, 0 replies; 10+ messages in thread
From: Randy Dunlap @ 2020-08-29 18:21 UTC (permalink / raw)
  To: Masahiro Yamada, Tony Luck, Fenghua Yu, linux-ia64
  Cc: linux-kbuild, Andrew Morton, Arnd Bergmann, Dmitry Safonov,
	afzal mohammed, linux-kernel

On 8/28/20 10:15 PM, Masahiro Yamada wrote:
> Documentation/process/coding-style.rst says:
> 
>   Please don't use things like ``vps_t``.
>   It's a **mistake** to use typedef for structures and pointers.
> 
> This commit converts as follows:
> 
>   struct pal_min_state_area_s  ->  struct pal_min_state_area
>          pal_min_state_area_t  ->  struct pal_min_state_area
> 
> My main motivation for this is to slim down the include directives
> of <asm/mca.h> in the next commit.
> 
> Currently, <asm/mca.h> is required to include <asm/pal.h> directly
> or indirectly due to (pal_min_state_area_t *). Otherwise, it would
> have no idea what pal_min_state_area_t is.
> 
> Replacing it with (struct pal_min_state_area *) will relax the header
> dependency since it is enough to tell it is a pointer to a structure,
> and to resolve the size of struct pal_min_state_area. It will make
> <asm/mca.h> independent of <asm/pal.h>.
> 
> <asm/pal.h> typedef's a lot of structures, but it is trivial to
> convert the others in the same way.
> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>

Reviewed-by: Randy Dunlap <rdunlap@infradead.org>

> ---
> 
>  arch/ia64/include/asm/mca.h    |  2 +-
>  arch/ia64/include/asm/pal.h    |  4 ++--
>  arch/ia64/include/asm/sal.h    |  2 +-
>  arch/ia64/kernel/asm-offsets.c | 18 +++++++++---------
>  arch/ia64/kernel/mca.c         |  4 ++--
>  arch/ia64/kernel/mca_drv.c     |  2 +-
>  6 files changed, 16 insertions(+), 16 deletions(-)
> 


thanks.
-- 
~Randy

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] ia64: remove generated/nr-irqs.h generation to fix build warning
  2020-08-29  5:15 ` [PATCH 3/3] ia64: remove generated/nr-irqs.h generation to fix build warning Masahiro Yamada
@ 2020-08-29 18:22   ` Randy Dunlap
  2021-02-07  2:46   ` Masahiro Yamada
  1 sibling, 0 replies; 10+ messages in thread
From: Randy Dunlap @ 2020-08-29 18:22 UTC (permalink / raw)
  To: Masahiro Yamada, Tony Luck, Fenghua Yu, linux-ia64
  Cc: linux-kbuild, linux-kernel

On 8/28/20 10:15 PM, Masahiro Yamada wrote:
> Randy reports the following warning when building ARCH=ia64 with
> CONFIG_IA64_PALINFO=m:
> 
> ../scripts/Makefile.build:68: 'arch/ia64/kernel/palinfo.ko' will not be built even though obj-m is specified.
> ../scripts/Makefile.build:69: You cannot use subdir-y/m to visit a module Makefile. Use obj-y/m instead.
> 
> This message is actually false-positive, and you can get palinfo.ko
> correctly built. It is emitted in the archprepare stage, where Kbuild
> descends into arch/ia64/kernel to generate include/generated/nr-irqs.h
> instead of any kind of kernel objects.
> 
> arch/ia64/kernel/nr-irqs.c was introduced by commit 213060a4d699
> ("[IA64] pvops: paravirtualize NR_IRQS") to pre-calculate:
> 
>    NR_IRQS = max(IA64_NATIVE_NR_IRQS, XEN_NR_IRQS, FOO_NR_IRQS...)
> 
> Since commit d52eefb47d4e ("ia64/xen: Remove Xen support for ia64"), this
> union contains just one field, making NR_IRQS and IA64_NATIVE_NR_IRQS
> always match.
> 
> So, the following hard-coding now works:
> 
>   #define NR_IRQS                IA64_NATIVE_NR_IRQS
> 
> If you need to re-introduce NR_IRQS = max(...) gimmick in the future,
> please try to implement it in asm-offsets.c instead of a separate file.
> It will be possible because the header inclusion has been consolidated
> to make asm-offsets.c independent of <asm/irqs.h>.
> 
> Reported-by: Randy Dunlap <rdunlap@infradead.org>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>

Reviewed-by: Randy Dunlap <rdunlap@infradead.org>
Tested-by: Randy Dunlap <rdunlap@infradead.org>

> ---
> 
>  arch/ia64/Makefile          |  6 ------
>  arch/ia64/include/asm/irq.h |  4 +++-
>  arch/ia64/kernel/Makefile   |  5 -----
>  arch/ia64/kernel/nr-irqs.c  | 22 ----------------------
>  4 files changed, 3 insertions(+), 34 deletions(-)
>  delete mode 100644 arch/ia64/kernel/nr-irqs.c
> 


thanks.
-- 
~Randy


^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] ia64: remove unneeded header includes from <asm/mca.h>
  2020-08-29  5:15 ` [PATCH 2/3] ia64: remove unneeded header includes from <asm/mca.h> Masahiro Yamada
  2020-08-29 18:20   ` Randy Dunlap
@ 2020-08-31 15:00   ` Ard Biesheuvel
  1 sibling, 0 replies; 10+ messages in thread
From: Ard Biesheuvel @ 2020-08-31 15:00 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Tony Luck, Fenghua Yu, linux-ia64, Randy Dunlap, linux-kbuild,
	Andrew Morton, Dmitry Safonov, afzal mohammed, linux-efi,
	Linux Kernel Mailing List

On Sat, 29 Aug 2020 at 08:16, Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> <asm/mca.h> includes too many unneeded headers.
>
> This commit cuts off a lot of header includes.
>
> What we need to include are:
>
>  - <linux/percpu.h> for DECLARE_PER_CPU(u64, ia64_mca_pal_base)
>  - <linux/threads.h> for NR_CPUS
>  - <linux/types.h> for u8, u64, size_t, etc.
>  - <asm/ptrace.h> for KERNEL_STACK_SIZE
>
> The other header includes are actually unneeded.
>
> <asm/mca.h> previously included 436 headers, and now it includes
> only 138. I confirmed <asm/mca.h> is still self-contained.
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>

Acked-by: Ard Biesheuvel <ardb@kernel.org>

> ---
>
>  arch/ia64/include/asm/mca.h | 9 +++------
>  arch/ia64/kernel/efi.c      | 1 +
>  arch/ia64/kernel/mca.c      | 1 +
>  3 files changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/arch/ia64/include/asm/mca.h b/arch/ia64/include/asm/mca.h
> index c92b9c15962c..05805249296c 100644
> --- a/arch/ia64/include/asm/mca.h
> +++ b/arch/ia64/include/asm/mca.h
> @@ -14,13 +14,10 @@
>
>  #if !defined(__ASSEMBLY__)
>
> -#include <linux/interrupt.h>
> +#include <linux/percpu.h>
> +#include <linux/threads.h>
>  #include <linux/types.h>
> -
> -#include <asm/param.h>
> -#include <asm/sal.h>
> -#include <asm/processor.h>
> -#include <asm/mca_asm.h>
> +#include <asm/ptrace.h>
>
>  #define IA64_MCA_RENDEZ_TIMEOUT                (20 * 1000)     /* value in milliseconds - 20 seconds */
>
> diff --git a/arch/ia64/kernel/efi.c b/arch/ia64/kernel/efi.c
> index f932b25fb817..b6bb718ed1ff 100644
> --- a/arch/ia64/kernel/efi.c
> +++ b/arch/ia64/kernel/efi.c
> @@ -39,6 +39,7 @@
>  #include <asm/meminit.h>
>  #include <asm/processor.h>
>  #include <asm/mca.h>
> +#include <asm/sal.h>
>  #include <asm/setup.h>
>  #include <asm/tlbflush.h>
>
> diff --git a/arch/ia64/kernel/mca.c b/arch/ia64/kernel/mca.c
> index 17151269d655..3911c561d2bb 100644
> --- a/arch/ia64/kernel/mca.c
> +++ b/arch/ia64/kernel/mca.c
> @@ -96,6 +96,7 @@
>  #include <asm/ptrace.h>
>  #include <asm/sal.h>
>  #include <asm/mca.h>
> +#include <asm/mca_asm.h>
>  #include <asm/kexec.h>
>
>  #include <asm/irq.h>
> --
> 2.25.1
>

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH 0/3] ia64: clean-up header dependency and build process, fix build warning
  2020-08-29  5:15 [PATCH 0/3] ia64: clean-up header dependency and build process, fix build warning Masahiro Yamada
                   ` (2 preceding siblings ...)
  2020-08-29  5:15 ` [PATCH 3/3] ia64: remove generated/nr-irqs.h generation to fix build warning Masahiro Yamada
@ 2020-12-04  5:43 ` Masahiro Yamada
  3 siblings, 0 replies; 10+ messages in thread
From: Masahiro Yamada @ 2020-12-04  5:43 UTC (permalink / raw)
  To: Tony Luck, Fenghua Yu, linux-ia64
  Cc: Randy Dunlap, Linux Kbuild mailing list, Andrew Morton,
	Ard Biesheuvel, Arnd Bergmann, Dmitry Safonov, afzal mohammed,
	linux-efi, Linux Kernel Mailing List

Hi, IA64 maintainers,

Could you check this series, please?
The build warning is still remaining.



On Sat, Aug 29, 2020 at 2:16 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
>
> Randy Dunlap reports the following warning with CONFIG_IA64_PALINFO=m:
>
> ../scripts/Makefile.build:68: 'arch/ia64/kernel/palinfo.ko' will not be built even though obj-m is specified.
> ../scripts/Makefile.build:69: You cannot use subdir-y/m to visit a module Makefile. Use obj-y/m instead.
>
> This comes from the fact Kbuild descends into arch/ia64/kernel/ twice.
>
> First, to generate <generated/nr-irqs.h>,
> Second, to build kernel and module objects.
>
> The warning is emitted in the first descend because it is not the
> intended usage.
>
> I looked into the code closely, and noticed arch/ia64/kernel/nr-irqs.c
> was not needed in the first place.
>
> It was separated out of arch/ia64/kernel/asm-offsets.c just because
> <asm/mca.h> was including too many bogus headers.
>
> IA64 is not actively maintained, and there exists unneeded obsolete code.
>
> The first two patches are the outcome when I played with ARCH=ia64 builds,
> but not prerequisites for 3/3. Anyway I believe they are nice cleanups
> and folded in this patch set.
>
> 3/3 is the important one to fix the false positive warning,
> and it is a nice cleanup too.
>
>
>
> Masahiro Yamada (3):
>   ia64: do not typedef struct pal_min_state_area_s
>   ia64: remove unneeded header includes from <asm/mca.h>
>   ia64: remove generated/nr-irqs.h generation to fix build warning
>
>  arch/ia64/Makefile             |  6 ------
>  arch/ia64/include/asm/irq.h    |  4 +++-
>  arch/ia64/include/asm/mca.h    | 11 ++++-------
>  arch/ia64/include/asm/pal.h    |  4 ++--
>  arch/ia64/include/asm/sal.h    |  2 +-
>  arch/ia64/kernel/Makefile      |  5 -----
>  arch/ia64/kernel/asm-offsets.c | 18 +++++++++---------
>  arch/ia64/kernel/efi.c         |  1 +
>  arch/ia64/kernel/mca.c         |  5 +++--
>  arch/ia64/kernel/mca_drv.c     |  2 +-
>  arch/ia64/kernel/nr-irqs.c     | 22 ----------------------
>  11 files changed, 24 insertions(+), 56 deletions(-)
>  delete mode 100644 arch/ia64/kernel/nr-irqs.c
>
> --
> 2.25.1
>


-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] ia64: remove generated/nr-irqs.h generation to fix build warning
  2020-08-29  5:15 ` [PATCH 3/3] ia64: remove generated/nr-irqs.h generation to fix build warning Masahiro Yamada
  2020-08-29 18:22   ` Randy Dunlap
@ 2021-02-07  2:46   ` Masahiro Yamada
  1 sibling, 0 replies; 10+ messages in thread
From: Masahiro Yamada @ 2021-02-07  2:46 UTC (permalink / raw)
  To: Tony Luck, Fenghua Yu, linux-ia64
  Cc: Randy Dunlap, Linux Kbuild mailing list, Linux Kernel Mailing List

On Sat, Aug 29, 2020 at 2:15 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> Randy reports the following warning when building ARCH=ia64 with
> CONFIG_IA64_PALINFO=m:
>
> ../scripts/Makefile.build:68: 'arch/ia64/kernel/palinfo.ko' will not be built even though obj-m is specified.
> ../scripts/Makefile.build:69: You cannot use subdir-y/m to visit a module Makefile. Use obj-y/m instead.
>
> This message is actually false-positive, and you can get palinfo.ko
> correctly built. It is emitted in the archprepare stage, where Kbuild
> descends into arch/ia64/kernel to generate include/generated/nr-irqs.h
> instead of any kind of kernel objects.
>
> arch/ia64/kernel/nr-irqs.c was introduced by commit 213060a4d699
> ("[IA64] pvops: paravirtualize NR_IRQS") to pre-calculate:
>
>    NR_IRQS = max(IA64_NATIVE_NR_IRQS, XEN_NR_IRQS, FOO_NR_IRQS...)
>
> Since commit d52eefb47d4e ("ia64/xen: Remove Xen support for ia64"), this
> union contains just one field, making NR_IRQS and IA64_NATIVE_NR_IRQS
> always match.
>
> So, the following hard-coding now works:
>
>   #define NR_IRQS                IA64_NATIVE_NR_IRQS
>
> If you need to re-introduce NR_IRQS = max(...) gimmick in the future,
> please try to implement it in asm-offsets.c instead of a separate file.
> It will be possible because the header inclusion has been consolidated
> to make asm-offsets.c independent of <asm/irqs.h>.
>
> Reported-by: Randy Dunlap <rdunlap@infradead.org>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>


This build issue is still remaining.

I will send this series in the next MW
via my kbuild tree.





> ---
>
>  arch/ia64/Makefile          |  6 ------
>  arch/ia64/include/asm/irq.h |  4 +++-
>  arch/ia64/kernel/Makefile   |  5 -----
>  arch/ia64/kernel/nr-irqs.c  | 22 ----------------------
>  4 files changed, 3 insertions(+), 34 deletions(-)
>  delete mode 100644 arch/ia64/kernel/nr-irqs.c
>
> diff --git a/arch/ia64/Makefile b/arch/ia64/Makefile
> index 2876a7df1b0a..3d54d411fcc4 100644
> --- a/arch/ia64/Makefile
> +++ b/arch/ia64/Makefile
> @@ -87,9 +87,3 @@ define archhelp
>    echo '  install      - Install compressed kernel image'
>    echo '* unwcheck     - Check vmlinux for invalid unwind info'
>  endef
> -
> -archprepare: make_nr_irqs_h
> -PHONY += make_nr_irqs_h
> -
> -make_nr_irqs_h:
> -       $(Q)$(MAKE) $(build)=arch/ia64/kernel include/generated/nr-irqs.h
> diff --git a/arch/ia64/include/asm/irq.h b/arch/ia64/include/asm/irq.h
> index 5acf52e90872..0eccf33dfe8b 100644
> --- a/arch/ia64/include/asm/irq.h
> +++ b/arch/ia64/include/asm/irq.h
> @@ -14,7 +14,9 @@
>
>  #include <linux/types.h>
>  #include <linux/cpumask.h>
> -#include <generated/nr-irqs.h>
> +#include <asm/native/irq.h>
> +
> +#define NR_IRQS                IA64_NATIVE_NR_IRQS
>
>  static __inline__ int
>  irq_canonicalize (int irq)
> diff --git a/arch/ia64/kernel/Makefile b/arch/ia64/kernel/Makefile
> index 1a8df6669eee..7c9354ee4b3e 100644
> --- a/arch/ia64/kernel/Makefile
> +++ b/arch/ia64/kernel/Makefile
> @@ -48,8 +48,3 @@ CFLAGS_traps.o  += -mfixed-range=f2-f5,f16-f31
>
>  # The gate DSO image is built using a special linker script.
>  include $(src)/Makefile.gate
> -
> -include/generated/nr-irqs.h: arch/$(SRCARCH)/kernel/nr-irqs.s FORCE
> -       $(call filechk,offsets,__ASM_NR_IRQS_H__)
> -
> -targets += nr-irqs.s
> diff --git a/arch/ia64/kernel/nr-irqs.c b/arch/ia64/kernel/nr-irqs.c
> deleted file mode 100644
> index f2633b22d3be..000000000000
> --- a/arch/ia64/kernel/nr-irqs.c
> +++ /dev/null
> @@ -1,22 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -/*
> - * calculate
> - * NR_IRQS = max(IA64_NATIVE_NR_IRQS, XEN_NR_IRQS, FOO_NR_IRQS...)
> - * depending on config.
> - * This must be calculated before processing asm-offset.c.
> - */
> -
> -#define ASM_OFFSETS_C 1
> -
> -#include <linux/kbuild.h>
> -#include <linux/threads.h>
> -#include <asm/native/irq.h>
> -
> -void foo(void)
> -{
> -       union paravirt_nr_irqs_max {
> -               char ia64_native_nr_irqs[IA64_NATIVE_NR_IRQS];
> -       };
> -
> -       DEFINE(NR_IRQS, sizeof (union paravirt_nr_irqs_max));
> -}
> --
> 2.25.1
>


-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-02-07  2:48 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-29  5:15 [PATCH 0/3] ia64: clean-up header dependency and build process, fix build warning Masahiro Yamada
2020-08-29  5:15 ` [PATCH 1/3] ia64: do not typedef struct pal_min_state_area_s Masahiro Yamada
2020-08-29 18:21   ` Randy Dunlap
2020-08-29  5:15 ` [PATCH 2/3] ia64: remove unneeded header includes from <asm/mca.h> Masahiro Yamada
2020-08-29 18:20   ` Randy Dunlap
2020-08-31 15:00   ` Ard Biesheuvel
2020-08-29  5:15 ` [PATCH 3/3] ia64: remove generated/nr-irqs.h generation to fix build warning Masahiro Yamada
2020-08-29 18:22   ` Randy Dunlap
2021-02-07  2:46   ` Masahiro Yamada
2020-12-04  5:43 ` [PATCH 0/3] ia64: clean-up header dependency and build process, " Masahiro Yamada

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).