Re: [PATCH] blk-iocost: fix seq_printf compile type mismatch error

Re: [PATCH] blk-iocost: fix seq_printf compile type mismatch error

[Carlos Bilbao]

On 7/17/23 09:18, Carlos Bilbao wrote:
> From: amd <amd-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>

Please disregard this "From" label.

> 
> Fix two type mismatch errors encountered while compiling blk-iocost.c with
> GCC version 13.1.1 that involved constant operator WEIGHT_ONE. Cast the
> result of the division operation to (unsigned int) to match the expected
> format specifier %u in two seq_printf invocations.
> 
> Reviewed-by: Carlos Bilbao <carlos.bilbao-5C7GfCeVMHo@public.gmane.org>
> ---
>   block/blk-iocost.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/block/blk-iocost.c b/block/blk-iocost.c
> index 495396425bad..4721009a3f03 100644
> --- a/block/blk-iocost.c
> +++ b/block/blk-iocost.c
> @@ -3032,7 +3032,7 @@ static u64 ioc_weight_prfill(struct seq_file *sf, struct blkg_policy_data *pd,
>   	struct ioc_gq *iocg = pd_to_iocg(pd);
>   
>   	if (dname && iocg->cfg_weight)
> -		seq_printf(sf, "%s %u\n", dname, iocg->cfg_weight / WEIGHT_ONE);
> +		seq_printf(sf, "%s %u\n", dname, (unsigned int)(iocg->cfg_weight / (unsigned int)WEIGHT_ONE));
>   	return 0;
>   }
>   
> @@ -3042,7 +3042,7 @@ static int ioc_weight_show(struct seq_file *sf, void *v)
>   	struct blkcg *blkcg = css_to_blkcg(seq_css(sf));
>   	struct ioc_cgrp *iocc = blkcg_to_iocc(blkcg);
>   
> -	seq_printf(sf, "default %u\n", iocc->dfl_weight / WEIGHT_ONE);
> +	seq_printf(sf, "default %u\n", (unsigned int) (iocc->dfl_weight / (unsigned int)WEIGHT_ONE));
>   	blkcg_print_blkgs(sf, blkcg, ioc_weight_prfill,
>   			  &blkcg_policy_iocost, seq_cft(sf)->private, false);
>   	return 0;

Thanks,
Carlos

Re: [PATCH] blk-iocost: fix seq_printf compile type mismatch error

[Tejun Heo]

On Mon, Jul 17, 2023 at 09:18:52AM -0500, Carlos Bilbao wrote:
> From: amd <amd-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
> 
> Fix two type mismatch errors encountered while compiling blk-iocost.c with
> GCC version 13.1.1 that involved constant operator WEIGHT_ONE. Cast the
> result of the division operation to (unsigned int) to match the expected
> format specifier %u in two seq_printf invocations.

Can you detail the warnings? Was that on 32bit compiles?

Thanks.

-- 
tejun

Re: [PATCH] blk-iocost: fix seq_printf compile type mismatch error

[Carlos Bilbao]

On 7/17/23 13:49, Tejun Heo wrote:
> On Mon, Jul 17, 2023 at 09:18:52AM -0500, Carlos Bilbao wrote:
>> From: amd <amd-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
>>
>> Fix two type mismatch errors encountered while compiling blk-iocost.c with
>> GCC version 13.1.1 that involved constant operator WEIGHT_ONE. Cast the
>> result of the division operation to (unsigned int) to match the expected
>> format specifier %u in two seq_printf invocations.
> 
> Can you detail the warnings? Was that on 32bit compiles?

The concrete error was: "format ‘%u’ expects argument of type ‘unsigned
int’, but argument 3 has type ‘long unsigned int’". If I run:

$ echo | cpp -dM | grep __LP64__
#define __LP64__ 1

which makes me believe it is not 32 bits.

> 
> Thanks.
> 

Thanks,
Carlos

Re: [PATCH] blk-iocost: fix seq_printf compile type mismatch error

[Tejun Heo]

On Tue, Jul 18, 2023 at 10:37:23AM -0500, Carlos Bilbao wrote:
> On 7/17/23 13:49, Tejun Heo wrote:
> > On Mon, Jul 17, 2023 at 09:18:52AM -0500, Carlos Bilbao wrote:
> > > From: amd <amd@localhost.localdomain>
> > > 
> > > Fix two type mismatch errors encountered while compiling blk-iocost.c with
> > > GCC version 13.1.1 that involved constant operator WEIGHT_ONE. Cast the
> > > result of the division operation to (unsigned int) to match the expected
> > > format specifier %u in two seq_printf invocations.
> > 
> > Can you detail the warnings? Was that on 32bit compiles?
> 
> The concrete error was: "format ‘%u’ expects argument of type ‘unsigned
> int’, but argument 3 has type ‘long unsigned int’". If I run:
> 
> $ echo | cpp -dM | grep __LP64__
> #define __LP64__ 1
> 
> which makes me believe it is not 32 bits.

So, we broke up the enum definitions so that WEIGHT_ONE doesn't end up being
a ulong. Which kernel are you building? Can you plesae try the current
linus#master?

Thanks.

-- 
tejun

RE: [PATCH] blk-iocost: fix seq_printf compile type mismatch error

[David Laight]

From: Tejun Heo
> Sent: 17 July 2023 19:49
> 
> On Mon, Jul 17, 2023 at 09:18:52AM -0500, Carlos Bilbao wrote:
> > From: amd <amd@localhost.localdomain>
> >
> > Fix two type mismatch errors encountered while compiling blk-iocost.c with
> > GCC version 13.1.1 that involved constant operator WEIGHT_ONE. Cast the
> > result of the division operation to (unsigned int) to match the expected
> > format specifier %u in two seq_printf invocations.
> 
> Can you detail the warnings? Was that on 32bit compiles?

The problem is caused by gcc 13 changing the types of the
constants inside an enum to be all the same.

The best fix is (probably) to replace all the enum used to
define unrelated constants with #defines.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH] blk-iocost: fix seq_printf compile type mismatch error

['Tejun Heo']

On Wed, Jul 19, 2023 at 08:57:32AM +0000, David Laight wrote:
> From: Tejun Heo
> > Sent: 17 July 2023 19:49
> > 
> > On Mon, Jul 17, 2023 at 09:18:52AM -0500, Carlos Bilbao wrote:
> > > From: amd <amd-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
> > >
> > > Fix two type mismatch errors encountered while compiling blk-iocost.c with
> > > GCC version 13.1.1 that involved constant operator WEIGHT_ONE. Cast the
> > > result of the division operation to (unsigned int) to match the expected
> > > format specifier %u in two seq_printf invocations.
> > 
> > Can you detail the warnings? Was that on 32bit compiles?
> 
> The problem is caused by gcc 13 changing the types of the
> constants inside an enum to be all the same.
> 
> The best fix is (probably) to replace all the enum used to
> define unrelated constants with #defines.

Yeah, but then you end up without any way to read that value from outside
the kernel for BPF, drgn or any other tools which use debug info. That
actually matters.

Thanks.

-- 
tejun

RE: [PATCH] blk-iocost: fix seq_printf compile type mismatch error

[David Laight]

From: Tejun Heo
> Sent: 20 July 2023 21:07
> 
> On Wed, Jul 19, 2023 at 08:57:32AM +0000, David Laight wrote:
> > From: Tejun Heo
> > > Sent: 17 July 2023 19:49
> > >
> > > On Mon, Jul 17, 2023 at 09:18:52AM -0500, Carlos Bilbao wrote:
> > > > From: amd <amd@localhost.localdomain>
> > > >
> > > > Fix two type mismatch errors encountered while compiling blk-iocost.c with
> > > > GCC version 13.1.1 that involved constant operator WEIGHT_ONE. Cast the
> > > > result of the division operation to (unsigned int) to match the expected
> > > > format specifier %u in two seq_printf invocations.
> > >
> > > Can you detail the warnings? Was that on 32bit compiles?
> >
> > The problem is caused by gcc 13 changing the types of the
> > constants inside an enum to be all the same.
> >
> > The best fix is (probably) to replace all the enum used to
> > define unrelated constants with #defines.
> 
> Yeah, but then you end up without any way to read that value from outside
> the kernel for BPF, drgn or any other tools which use debug info. That
> actually matters.

Some of those constants (probably including the one that forces
the enum to 'long' are very boring.
I don't remember which one caused the change, but some were
similar to 'microseconds in a second'.

In any case it is enough to split the enum.
If you really need unrelated constants to be defined in an enum
them maybe use a separate enum for each.
Using (on one line):
	enum { name = constant };
may work best.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH] blk-iocost: fix seq_printf compile type mismatch error

['Tejun Heo']

Hello,

On Fri, Jul 21, 2023 at 08:05:59AM +0000, David Laight wrote:
> In any case it is enough to split the enum.
> If you really need unrelated constants to be defined in an enum
> them maybe use a separate enum for each.
> Using (on one line):
> 	enum { name = constant };

Yeah, I'm hoping it won't come down to that. Hopefully, we can limp along
like this until we can always assume the new behavior. Right now, the
problem is that both gcc<13 and gcc=13 have to supported when the two assign
different types to the same enum definitions.

Thanks.

-- 
tejun

Re: [PATCH] blk-iocost: fix seq_printf compile type mismatch error

[Carlos Bilbao]

On 7/17/23 9:18 AM, Carlos Bilbao wrote:
> From: amd <amd@localhost.localdomain>
> 
> Fix two type mismatch errors encountered while compiling blk-iocost.c with
> GCC version 13.1.1 that involved constant operator WEIGHT_ONE. Cast the
> result of the division operation to (unsigned int) to match the expected
> format specifier %u in two seq_printf invocations.
> 
> Reviewed-by: Carlos Bilbao <carlos.bilbao@amd.com>
This should have been:

Signed-off-by: Carlos Bilbao <carlos.bilbao@amd.com>

> ---
>  block/blk-iocost.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/block/blk-iocost.c b/block/blk-iocost.c
> index 495396425bad..4721009a3f03 100644
> --- a/block/blk-iocost.c
> +++ b/block/blk-iocost.c
> @@ -3032,7 +3032,7 @@ static u64 ioc_weight_prfill(struct seq_file *sf, struct blkg_policy_data *pd,
>  	struct ioc_gq *iocg = pd_to_iocg(pd);
>  
>  	if (dname && iocg->cfg_weight)
> -		seq_printf(sf, "%s %u\n", dname, iocg->cfg_weight / WEIGHT_ONE);
> +		seq_printf(sf, "%s %u\n", dname, (unsigned int)(iocg->cfg_weight / (unsigned int)WEIGHT_ONE));
>  	return 0;
>  }
>  
> @@ -3042,7 +3042,7 @@ static int ioc_weight_show(struct seq_file *sf, void *v)
>  	struct blkcg *blkcg = css_to_blkcg(seq_css(sf));
>  	struct ioc_cgrp *iocc = blkcg_to_iocc(blkcg);
>  
> -	seq_printf(sf, "default %u\n", iocc->dfl_weight / WEIGHT_ONE);
> +	seq_printf(sf, "default %u\n", (unsigned int) (iocc->dfl_weight / (unsigned int)WEIGHT_ONE));
>  	blkcg_print_blkgs(sf, blkcg, ioc_weight_prfill,
>  			  &blkcg_policy_iocost, seq_cft(sf)->private, false);
>  	return 0;

Thanks,
Carlos

Re: [PATCH] blk-iocost: fix seq_printf compile type mismatch error

[Tejun Heo]

On Fri, Jul 21, 2023 at 05:32:04PM -0500, Carlos Bilbao wrote:
> On 7/17/23 9:18 AM, Carlos Bilbao wrote:
> > From: amd <amd-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
> > 
> > Fix two type mismatch errors encountered while compiling blk-iocost.c with
> > GCC version 13.1.1 that involved constant operator WEIGHT_ONE. Cast the
> > result of the division operation to (unsigned int) to match the expected
> > format specifier %u in two seq_printf invocations.
> > 
> > Reviewed-by: Carlos Bilbao <carlos.bilbao-5C7GfCeVMHo@public.gmane.org>
> This should have been:
> 
> Signed-off-by: Carlos Bilbao <carlos.bilbao-5C7GfCeVMHo@public.gmane.org>

Carlos, can you please retest the current linus#master?

Thanks.

-- 
tejun

Re: [PATCH] blk-iocost: fix seq_printf compile type mismatch error

[Carlos Bilbao]

On 7/21/23 20:14, Tejun Heo wrote:
> On Fri, Jul 21, 2023 at 05:32:04PM -0500, Carlos Bilbao wrote:
>> On 7/17/23 9:18 AM, Carlos Bilbao wrote:
>>> From: amd <amd-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
>>>
>>> Fix two type mismatch errors encountered while compiling blk-iocost.c with
>>> GCC version 13.1.1 that involved constant operator WEIGHT_ONE. Cast the
>>> result of the division operation to (unsigned int) to match the expected
>>> format specifier %u in two seq_printf invocations.
>>>
>>> Reviewed-by: Carlos Bilbao <carlos.bilbao-5C7GfCeVMHo@public.gmane.org>
>> This should have been:
>>
>> Signed-off-by: Carlos Bilbao <carlos.bilbao-5C7GfCeVMHo@public.gmane.org>
> 
> Carlos, can you please retest the current linus#master?
> 

Yes, that version compiled successfully without changes in my compiler. The
kernel that gave me said error was a custom v6.1.0-rc4 (Obtained from:
https://github.com/coconut-svsm/linux/tree/svsm-host)

> Thanks.
> 

Thanks,
Carlos