linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mike Travis <mike.travis@hpe.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Borislav Petkov <bp@alien8.de>,
	Dimitri Sivanich <dimitri.sivanich@hpe.com>,
	Russ Anderson <russ.anderson@hpe.com>,
	Hedi Berriche <hedi.berriche@hpe.com>,
	Steve Wahl <steve.wahl@hpe.com>,
	x86@kernel.org, linux-kernel@vger.kernel.org,
	stable@vger.kernel.org
Subject: Re: [PATCH 8/8] x86/platform/uv: Account for UV Hubless in is_uvX_hub Ops
Date: Wed, 4 Sep 2019 03:18:04 -0700	[thread overview]
Message-ID: <9a5a57a1-62ef-d1ad-55c1-30873ecb7ba6@hpe.com> (raw)
In-Reply-To: <20190904065246.GB18003@infradead.org>

Hi Christoph,

I can do these and leave some comments as markers to find the code to 
remove later.  The real problem is uv_mmrs.h is a generated file and the 
scripts cannot be easily redone.  Basically they copy in the verilog 
definition files from the UV HUB design files and create kernel 
compatible files that also then generate run-time checking of hardware 
specifics like MMR addresses and field definitions.  The change to 
remove those "is supported defines" takes a higher level process that 
cannot be done within this short release cycle.  We are racing to 
include this support in the next linux release for pending new hardware 
introductions.

Also that script is in a state of flux for the next UV arch and 
processor updates so changing anything at this moment is highly 
disruptive.  Is there a specific reason why this UV only code that does 
not touch anything else in the kernel and only affects our productivity 
and system performance appears to be such a problem with you?  And why 
now, at this moment, and after 10 years does it suddenly seem so 
important?  I'm all for clean up but not disruption.

Bottom line, I agree to all these changes and I promise to tend to them 
in the next release cycle.  But please let us get through this cycle 
without this much chaos?

Thanks,
Mike

On 9/3/2019 11:52 PM, Christoph Hellwig wrote:
> On Tue, Sep 03, 2019 at 11:58:49AM -0700, Mike Travis wrote:
>> Those ifdefs are not dead code and are being actively used.  Plus UV1
>> support is dead and I think the last running system died about a year ago
>> and no support or parts are available.  So undef'ing these macros will
>> simplify and reduce the size of the object code.
> 
> I'm not complaining about removing some ifdefs, that is always good.
> I complain about keeping the others that are dead.  And if Hub 1 is
> dead please drop all the checks and support code for it.
> 
> A patch against current mainline to show what I mean is below.
> 
> ---
>  From e84506399fa9436d47b33491d3e38e9dc3c718c7 Mon Sep 17 00:00:00 2001
> From: Christoph Hellwig <hch@lst.de>
> Date: Tue, 3 Sep 2019 18:05:37 +0200
> Subject: x86/uv: Remove the dead UV?_HUB_IS_SUPPORTED defines
> 
> These are always set, so remove them and the dead code for the case
> where they are not defined.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   arch/x86/include/asm/uv/uv_hub.h  | 38 -------------------------------
>   arch/x86/include/asm/uv/uv_mmrs.h |  7 ------
>   2 files changed, 45 deletions(-)
> 
> diff --git a/arch/x86/include/asm/uv/uv_hub.h b/arch/x86/include/asm/uv/uv_hub.h
> index 6eed0b379412..f71eb659f0de 100644
> --- a/arch/x86/include/asm/uv/uv_hub.h
> +++ b/arch/x86/include/asm/uv/uv_hub.h
> @@ -229,68 +229,33 @@ static inline struct uv_hub_info_s *uv_cpu_hub_info(int cpu)
>   #define UV4_HUB_REVISION_BASE		7
>   #define UV4A_HUB_REVISION_BASE		8	/* UV4 (fixed) rev 2 */
>   
> -#ifdef	UV1_HUB_IS_SUPPORTED
>   static inline int is_uv1_hub(void)
>   {
>   	return uv_hub_info->hub_revision < UV2_HUB_REVISION_BASE;
>   }
> -#else
> -static inline int is_uv1_hub(void)
> -{
> -	return 0;
> -}
> -#endif
>   
> -#ifdef	UV2_HUB_IS_SUPPORTED
>   static inline int is_uv2_hub(void)
>   {
>   	return ((uv_hub_info->hub_revision >= UV2_HUB_REVISION_BASE) &&
>   		(uv_hub_info->hub_revision < UV3_HUB_REVISION_BASE));
>   }
> -#else
> -static inline int is_uv2_hub(void)
> -{
> -	return 0;
> -}
> -#endif
>   
> -#ifdef	UV3_HUB_IS_SUPPORTED
>   static inline int is_uv3_hub(void)
>   {
>   	return ((uv_hub_info->hub_revision >= UV3_HUB_REVISION_BASE) &&
>   		(uv_hub_info->hub_revision < UV4_HUB_REVISION_BASE));
>   }
> -#else
> -static inline int is_uv3_hub(void)
> -{
> -	return 0;
> -}
> -#endif
>   
>   /* First test "is UV4A", then "is UV4" */
> -#ifdef	UV4A_HUB_IS_SUPPORTED
>   static inline int is_uv4a_hub(void)
>   {
>   	return (uv_hub_info->hub_revision >= UV4A_HUB_REVISION_BASE);
>   }
> -#else
> -static inline int is_uv4a_hub(void)
> -{
> -	return 0;
> -}
> -#endif
>   
> -#ifdef	UV4_HUB_IS_SUPPORTED
>   static inline int is_uv4_hub(void)
>   {
>   	return uv_hub_info->hub_revision >= UV4_HUB_REVISION_BASE;
>   }
> -#else
> -static inline int is_uv4_hub(void)
> -{
> -	return 0;
> -}
> -#endif
>   
>   static inline int is_uvx_hub(void)
>   {
> @@ -302,10 +267,7 @@ static inline int is_uvx_hub(void)
>   
>   static inline int is_uv_hub(void)
>   {
> -#ifdef	UV1_HUB_IS_SUPPORTED
>   	return uv_hub_info->hub_revision;
> -#endif
> -	return is_uvx_hub();
>   }
>   
>   union uvh_apicid {
> diff --git a/arch/x86/include/asm/uv/uv_mmrs.h b/arch/x86/include/asm/uv/uv_mmrs.h
> index 62c79e26a59a..9ee5ed6e8b34 100644
> --- a/arch/x86/include/asm/uv/uv_mmrs.h
> +++ b/arch/x86/include/asm/uv/uv_mmrs.h
> @@ -99,13 +99,6 @@
>   #define UV3_HUB_PART_NUMBER_X	0x4321
>   #define UV4_HUB_PART_NUMBER	0x99a1
>   
> -/* Compat: Indicate which UV Hubs are supported. */
> -#define UV1_HUB_IS_SUPPORTED	1
> -#define UV2_HUB_IS_SUPPORTED	1
> -#define UV3_HUB_IS_SUPPORTED	1
> -#define UV4_HUB_IS_SUPPORTED	1
> -#define UV4A_HUB_IS_SUPPORTED	1
> -
>   /* Error function to catch undefined references */
>   extern unsigned long uv_undefined(char *str);
>   
> 

  reply	other threads:[~2019-09-04 10:18 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-03  0:18 [PATCH 0/8] x86/platform/UV: Update UV Hubless System Support Mike Travis
2019-09-03  0:18 ` [PATCH 1/8] x86/platform/uv: Save OEM_ID from ACPI MADT probe Mike Travis
2019-09-03  0:18 ` [PATCH 2/8] x86/platform/uv: Return UV Hubless System Type Mike Travis
2019-09-03  6:49   ` Christoph Hellwig
2019-09-03 14:12     ` Mike Travis
2019-09-03 15:41       ` Christoph Hellwig
2019-09-03 18:49         ` Mike Travis
2019-09-04  6:50           ` Christoph Hellwig
2019-09-03  0:18 ` [PATCH 3/8] x86/platform/uv: Add return code to UV BIOS Init function Mike Travis
2019-09-03  0:18 ` [PATCH 4/8] x86/platform/uv: Setup UV functions for Hubless UV Systems Mike Travis
2019-09-03  0:18 ` [PATCH 5/8] x86/platform/uv: Add UV Hubbed/Hubless Proc FS Files Mike Travis
2019-09-03  6:50   ` Christoph Hellwig
2019-09-03  0:18 ` [PATCH 6/8] x86/platform/uv: Decode UVsystab Info Mike Travis
2019-09-03  6:22   ` Greg KH
2019-09-03  0:18 ` [PATCH 7/8] x86/platform/uv: Check EFI Boot to set reboot type Mike Travis
2019-09-03  0:18 ` [PATCH 8/8] x86/platform/uv: Account for UV Hubless in is_uvX_hub Ops Mike Travis
2019-09-03 16:19   ` Christoph Hellwig
2019-09-03 18:58     ` Mike Travis
2019-09-03 19:03       ` Mike Travis
2019-09-04  6:52       ` Christoph Hellwig
2019-09-04 10:18         ` Mike Travis [this message]
2019-09-03  7:47 ` [PATCH 0/8] x86/platform/UV: Update UV Hubless System Support Ingo Molnar
2019-09-03 14:17   ` Mike Travis
2019-09-05  8:19     ` Ingo Molnar
2019-09-05 13:02 Mike Travis
2019-09-05 13:03 ` [PATCH 8/8] x86/platform/uv: Account for UV Hubless in is_uvX_hub Ops Mike Travis
2019-09-05 18:47 [PATCH 0/8] x86/platform/UV: Update UV Hubless System Support Mike Travis
2019-09-05 18:47 ` [PATCH 8/8] x86/platform/uv: Account for UV Hubless in is_uvX_hub Ops Mike Travis

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=9a5a57a1-62ef-d1ad-55c1-30873ecb7ba6@hpe.com \
    --to=mike.travis@hpe.com \
    --cc=akpm@linux-foundation.org \
    --cc=bp@alien8.de \
    --cc=dimitri.sivanich@hpe.com \
    --cc=hch@infradead.org \
    --cc=hedi.berriche@hpe.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=russ.anderson@hpe.com \
    --cc=stable@vger.kernel.org \
    --cc=steve.wahl@hpe.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    --subject='Re: [PATCH 8/8] x86/platform/uv: Account for UV Hubless in is_uvX_hub Ops' \
    /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

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).