Re: [PATCH 07/23] hw/block/nvme: Use definition to avoid dynamic stack allocation

From: Keith Busch <kbusch@kernel.org>
To: "Philippe Mathieu-Daudé" <philmd@redhat.com>
Cc: "Kevin Wolf" <kwolf@redhat.com>,
	"Daniel P. Berrangé" <berrange@redhat.com>,
	qemu-block@nongnu.org,
	"Richard Henderson" <richard.henderson@linaro.org>,
	qemu-devel@nongnu.org, "Max Reitz" <mreitz@redhat.com>,
	qemu-ppc@nongnu.org, "Gerd Hoffmann" <kraxel@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Klaus Jensen" <its@irrelevant.dk>,
	"Marc-André Lureau" <marcandre.lureau@redhat.com>
Subject: Re: [PATCH 07/23] hw/block/nvme: Use definition to avoid dynamic stack allocation
Date: Wed, 5 May 2021 14:22:38 -0700	[thread overview]
Message-ID: <20210505212238.GA1186879@dhcp-10-100-145-180.wdc.com> (raw)
In-Reply-To: <20210505211047.1496765-8-philmd@redhat.com>

On Wed, May 05, 2021 at 11:10:31PM +0200, Philippe Mathieu-Daudé wrote:
> The compiler isn't clever enough to figure 'SEG_CHUNK_SIZE' is
> a constant! Help it by using a definitions instead.

I don't understand. It's labeled 'const', so any reasonable compiler
will place it in the 'text' segment of the executable rather than on the
stack. While that's compiler specific, is there really a compiler doing
something bad with this? If not, I do prefer the 'const' here if only
because it limits the symbol scope ('static const' is also preferred if
that helps).

> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/block/nvme.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 5fe082ec34c..2f6d4925826 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -812,7 +812,7 @@ static uint16_t nvme_map_sgl(NvmeCtrl *n, NvmeSg *sg, NvmeSglDescriptor sgl,
>       * descriptors and segment chain) than the command transfer size, so it is
>       * not bounded by MDTS.
>       */
> -    const int SEG_CHUNK_SIZE = 256;
> +#define SEG_CHUNK_SIZE 256
>  
>      NvmeSglDescriptor segment[SEG_CHUNK_SIZE], *sgld, *last_sgld;
>      uint64_t nsgld;
> -- 
> 2.26.3
> 