Re: xen | Failed pipeline for staging | 221f2748

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Thu, Sep 12, 2024 at 04:30:29PM +0200, Jan Beulich wrote:
> On 12.09.2024 14:52, GitLab wrote:
> > 
> > 
> > Pipeline #1450753094 has failed!
> > 
> > Project: xen ( https://gitlab.com/xen-project/hardware/xen )
> > Branch: staging ( 
> > https://gitlab.com/xen-project/hardware/xen/-/commits/staging )
> > 
> > Commit: 221f2748 ( 
> > https://gitlab.com/xen-project/hardware/xen/-/commit/221f2748e8dabe8361b8cdfcffbeab9102c4c899
> >  )
> > Commit Message: blkif: reconcile protocol specification with in...
> > Commit Author: Roger Pau Monné
> > Committed by: Jan Beulich ( https://gitlab.com/jbeulich )
> > 
> > 
> > Pipeline #1450753094 ( 
> > https://gitlab.com/xen-project/hardware/xen/-/pipelines/1450753094 ) 
> > triggered by Jan Beulich ( https://gitlab.com/jbeulich )
> > had 13 failed jobs.
> > 
> > Job #7809027717 ( 
> > https://gitlab.com/xen-project/hardware/xen/-/jobs/7809027717/raw )
> > 
> > Stage: build
> > Name: ubuntu-24.04-x86_64-clang
> > Job #7809027747 ( 
> > https://gitlab.com/xen-project/hardware/xen/-/jobs/7809027747/raw )
> > 
> > Stage: build
> > Name: opensuse-tumbleweed-clang
> > Job #7809027539 ( 
> > https://gitlab.com/xen-project/hardware/xen/-/jobs/7809027539/raw )
> > 
> > Stage: build
> > Name: debian-bookworm-clang-debug
> 
> I picked this one as example - Clang dislikes the switch to bool in
> 
> --- a/xen/arch/x86/include/asm/mm.h
> +++ b/xen/arch/x86/include/asm/mm.h
> @@ -286,8 +286,8 @@ struct page_info
>          struct {
>              u16 nr_validated_ptes:PAGETABLE_ORDER + 1;
>              u16 :16 - PAGETABLE_ORDER - 1 - 1;
> -            u16 partial_flags:1;
> -            s16 linear_pt_count;
> +            bool partial_flags:1;
> +            int16_t linear_pt_count;
>          };
>  
>          /*
> 
> for places where that field's set using PTF_partial_set:
> 
> arch/x86/mm.c:1582:35: error: converting the result of '<<' to a boolean 
> always evaluates to true [-Werror,-Wtautological-constant-compare]
>             page->partial_flags = PTF_partial_set;
>                                   ^
> I wonder why we're not using plain "true" there. Alternatively the change to
> bool would need undoing.

I'm hitting this locally when building with clang.

I find it confusing that the partial_flags field cannot possibly be a
flags field, for it having a width of 1 bit.

I think my proposal would be to rename to partially_validated (or
similar) and set it using `true`, but that would also imply re-writing
part of the comment in struct page_info definition.

I can prepare a patch if this is deemed appropriate.

Thanks, Roger.