Subject: Re: [PATCH 6/7] powerpc/entry: factout irqentry-state

From: Thomas Gleixner <[email-masked]>
Date: Tue, 15 Oct 2024 17:42:45 +0200

On Sat, Oct 12 2024 at 11:56, Luming Yu wrote:

> To have lowlevel paca.h include high level entry-common.h cause
> include file dependency mess. Split irqentry-state.h to have
> the irqentry_state.h can be included in low level paca.h

What's the rationale for this?

> Signed-off-by: Luming Yu <[email-masked]>
> ---
>  arch/powerpc/include/asm/paca.h |  2 ++
>  arch/powerpc/kernel/interrupt.c |  2 ++
>  include/linux/entry-common.h    | 24 ------------------------
>  include/linux/irqentry-state.h  | 28 ++++++++++++++++++++++++++++

This is not how it works. Split the include file in a first step and
then make changes to the PPC side.

> diff --git a/include/linux/entry-common.h b/include/linux/entry-common.h
> index d95ab85f96ba..6521171469f2 100644
> --- a/include/linux/entry-common.h
> +++ b/include/linux/entry-common.h
> @@ -352,30 +352,6 @@ void irqentry_enter_from_user_mode(struct pt_regs *regs);
>   */
>  void irqentry_exit_to_user_mode(struct pt_regs *regs);
>  
> -#ifndef irqentry_state
> -/**
> - * struct irqentry_state - Opaque object for exception state storage
> - * @exit_rcu: Used exclusively in the irqentry_*() calls; signals whether the
> - *            exit path has to invoke ct_irq_exit().
> - * @lockdep: Used exclusively in the irqentry_nmi_*() calls; ensures that
> - *           lockdep state is restored correctly on exit from nmi.
> - *
> - * This opaque object is filled in by the irqentry_*_enter() functions and
> - * must be passed back into the corresponding irqentry_*_exit() functions
> - * when the exception is complete.
> - *
> - * Callers of irqentry_*_[enter|exit]() must consider this structure opaque
> - * and all members private.  Descriptions of the members are provided to aid in
> - * the maintenance of the irqentry_*() functions.
> - */
> -typedef struct irqentry_state {
> -	union {
> -		bool	exit_rcu;
> -		bool	lockdep;
> -	};
> -} irqentry_state_t;
> -#endif
> -
>  /**
>   * irqentry_enter - Handle state tracking on ordinary interrupt entries
>   * @regs:	Pointer to pt_regs of interrupted context
> diff --git a/include/linux/irqentry-state.h b/include/linux/irqentry-state.h
> new file mode 100644
> index 000000000000..d4ddeb1c6ab6
> --- /dev/null
> +++ b/include/linux/irqentry-state.h
> @@ -0,0 +1,28 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __LINUX_IRQENTRYCOMMON_H
> +#define __LINUX_IRQENTRYCOMMON_H

The guards reflect the header file name and are not randomly chosen strings.

But aside of that. How is any of this supposed to compile?

You move the typedef into a separate header and then nothing (except
powerpc) includes it. Oh well.

Thanks,

        tglx