[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] core-parking: fix build with gcc12 and NR_CPUS=1


  • To: Julien Grall <julien@xxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Fri, 9 Sep 2022 16:21:47 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=FRke+5frwSExLZEbhuphMT4E8VuOaqwzkQoVriMaJQU=; b=mF630snR9VqlVg+/KOsCMj+LE6I1QoFKB/VaNyQhm5WqPnB2Y3rsJIesfF+PD8IT4p+NOQHAtPRMSzhK3qLNkLGErv/3U8FZs7j3v9gvPmMY9fai/ubPhGgu0Ix8w+eakT8umJqAC+ig3RoCjZayWjXkmeZ4+wfDChf++0MGtwcKpDhm5/ySCYlVZwsxAcJ4voKSk9MddVbMXV/WbF4R/VdQ5eVQAbKiCBMZeOFo/yblcuB4rXnxqQrHLOHnlx8L52It/wOQTPSAftW0WJ3CvPu6z3gXokJKbXt1ywAk2WVRRvDmcMUyoV01PMt1fWAbihymEeiddB5KmHKyxUkadg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=I0kbDQ4bb2W7oEF3aXRmd/GGSAEnNyYnlvS11vGvZ8CPFU9oj9Bu8mUM2OV08TgVv8GEQnLgO9YtNiHrwwdwxEkP6vGsJOMy4MICuRbuWgbfsm6v3tBrL2Q/IFZlYwktf7IsEpgpBpPFrP6xhzYbw7vsdx+HsQALFuL4Y7jr02EU/P825hn6sKLMSWEE8aiGZpC/ntNoE6lqxIi5Wi5+3iTzI6mxN+7nlOjHU4/eohQEpCcKEYig7tRzXQ6b0YGAITgPWUbPVjYbNkYhn+vkMfzbcs6EIwm/tyrqTINaPxO5QH8jizH1WjjRaWz5lFCf1cWhd1bs+piE+WHa7VC6FA==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Fri, 09 Sep 2022 14:21:58 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 09.09.2022 14:27, Julien Grall wrote:
> Hi,
> 
> On 09/09/2022 13:14, Jan Beulich wrote:
>> On 09.09.2022 13:00, Julien Grall wrote:
>>> On 09/09/2022 11:18, Jan Beulich wrote:
>>>> Gcc12 takes issue with core_parking_remove()'s
>>>>
>>>>       for ( ; i < cur_idle_nums; ++i )
>>>>           core_parking_cpunum[i] = core_parking_cpunum[i + 1];
>>>>
>>>> complaining that the right hand side array access is past the bounds of
>>>> 1. Clearly the compiler can't know that cur_idle_nums can only ever be
>>>> zero in this case (as the sole CPU cannot be parked).
>>>>
>>>> Beyond addressing the immediate issue also adjust core_parking_init():
>>>> There's no point registering any policy when there's no CPU to park.
>>>> Since this still doesn't result in the compiler spotting that
>>>> core_parking_policy is never written (and hence is continuously NULL),
>>>> also amend core_parking_helper() to avoid eventual similar issues there
>>>> (minimizing generated code at the same time).
>>>
>>> Given that CONFIG_NR_CPUS is a build time option. Wouldn't it be better
>>> to set CONFIG_CORE_PARKING=n and provide dummy helper for any function
>>> that may be called unconditionally?
>>
>> That might be an option, yes; not sure whether that's really better. It's
>> likely a more intrusive change ...
> 
> I quickly try to implement it (see below) and the result is IHMO a lot 
> nicer and make clear the code is not necessary on uni-processor.

Hmm, we can do something like this, but ...

> This is only compile tested.
> 
> diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
> index 6a7825f4ba3c..f9a3daccdc92 100644
> --- a/xen/arch/x86/Kconfig
> +++ b/xen/arch/x86/Kconfig
> @@ -10,7 +10,7 @@ config X86
>          select ALTERNATIVE_CALL
>          select ARCH_MAP_DOMAIN_PAGE
>          select ARCH_SUPPORTS_INT128
> -       select CORE_PARKING
> +       select CORE_PARKING if NR_CPUS > 1
>          select HAS_ALTERNATIVE
>          select HAS_COMPAT
>          select HAS_CPUFREQ
> diff --git a/xen/arch/x86/include/asm/smp.h b/xen/arch/x86/include/asm/smp.h
> index 41a3b6a0dadf..7baca00be182 100644
> --- a/xen/arch/x86/include/asm/smp.h
> +++ b/xen/arch/x86/include/asm/smp.h
> @@ -61,7 +61,15 @@ long cf_check cpu_up_helper(void *data);
>   long cf_check cpu_down_helper(void *data);
> 
>   long cf_check core_parking_helper(void *data);
> +
> +#ifdef CONFIG_CORE_PARKING
>   bool core_parking_remove(unsigned int cpu);
> +#else
> +static inline bool core_parking_remove(unsigned int cpu)
> +{
> +    return false;
> +}
> +#endif
>   uint32_t get_cur_idle_nums(void);
> 
>   /*
> diff --git a/xen/arch/x86/platform_hypercall.c 
> b/xen/arch/x86/platform_hypercall.c
> index a7341dc3d7d3..5d13fac41bd4 100644
> --- a/xen/arch/x86/platform_hypercall.c
> +++ b/xen/arch/x86/platform_hypercall.c
> @@ -718,6 +718,7 @@ ret_t do_platform_op(
>                         op->u.mem_add.pxm);
>           break;
> 
> +#ifdef CONFIG_CORE_PARKING
>       case XENPF_core_parking:
>       {
>           uint32_t idle_nums;
> @@ -743,6 +744,7 @@ ret_t do_platform_op(
>           }
>       }
>       break;
> +#endif

... this needs doing differently to prevent the hypercall changing
behavior. Will send a v2 in a minute.

Jan



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.