[PATCH v3] bitfield.h: add FIELD_MAX() and field_max()

[PATCH v3] bitfield.h: add FIELD_MAX() and field_max()

[Alex Elder]

Define FIELD_MAX(), which supplies the maximum value that can be
represented by a field value.  Define field_max() as well, to go
along with the lower-case forms of the field mask functions.

Signed-off-by: Alex Elder <elder@linaro.org>
Acked-by: Jakub Kicinski <kuba@kernel.org>
---
v3: Rebased on latest netdev-next/master.

David, please take this into net-next as soon as possible.  When the
IPA code was merged the other day this prerequisite patch was not
included, and as a result the IPA driver fails to build.  Thank you.

  See: https://lkml.org/lkml/2020/3/10/1839

					-Alex

 include/linux/bitfield.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
index 4bbb5f1c8b5b..48ea093ff04c 100644
--- a/include/linux/bitfield.h
+++ b/include/linux/bitfield.h
@@ -55,6 +55,19 @@
 					      (1ULL << __bf_shf(_mask))); \
 	})
 
+/**
+ * FIELD_MAX() - produce the maximum value representable by a field
+ * @_mask: shifted mask defining the field's length and position
+ *
+ * FIELD_MAX() returns the maximum value that can be held in the field
+ * specified by @_mask.
+ */
+#define FIELD_MAX(_mask)						\
+	({								\
+		__BF_FIELD_CHECK(_mask, 0ULL, 0ULL, "FIELD_MAX: ");	\
+		(typeof(_mask))((_mask) >> __bf_shf(_mask));		\
+	})
+
 /**
  * FIELD_FIT() - check if value fits in the field
  * @_mask: shifted mask defining the field's length and position
@@ -110,6 +123,7 @@ static __always_inline u64 field_mask(u64 field)
 {
 	return field / field_multiplier(field);
 }
+#define field_max(field)	((typeof(field))field_mask(field))
 #define ____MAKE_OP(type,base,to,from)					\
 static __always_inline __##type type##_encode_bits(base v, base field)	\
 {									\
-- 
2.20.1

Re: [PATCH v3] bitfield.h: add FIELD_MAX() and field_max()

[Nick Desaulniers]

> Define FIELD_MAX(), which supplies the maximum value that can be
> represented by a field value.  Define field_max() as well, to go
> along with the lower-case forms of the field mask functions.
> 
> Signed-off-by: Alex Elder <elder@linaro.org>
> Acked-by: Jakub Kicinski <kuba@kernel.org>
> ---
> v3: Rebased on latest netdev-next/master.
> 
> David, please take this into net-next as soon as possible.  When the
> IPA code was merged the other day this prerequisite patch was not
> included, and as a result the IPA driver fails to build.  Thank you.
> 
>   See: https://lkml.org/lkml/2020/3/10/1839
> 
> 					-Alex

In particular, this seems to now have regressed into mainline for the 5.7
merge window as reported by Linaro's ToolChain Working Group's CI.
Link: https://github.com/ClangBuiltLinux/linux/issues/963

Re: [PATCH v3] bitfield.h: add FIELD_MAX() and field_max()

[Alex Elder]

On 4/1/20 12:35 PM, Nick Desaulniers wrote:
>> Define FIELD_MAX(), which supplies the maximum value that can be
>> represented by a field value.  Define field_max() as well, to go
>> along with the lower-case forms of the field mask functions.
>>
>> Signed-off-by: Alex Elder <elder@linaro.org>
>> Acked-by: Jakub Kicinski <kuba@kernel.org>
>> ---
>> v3: Rebased on latest netdev-next/master.
>>
>> David, please take this into net-next as soon as possible.  When the
>> IPA code was merged the other day this prerequisite patch was not
>> included, and as a result the IPA driver fails to build.  Thank you.
>>
>>   See: https://lkml.org/lkml/2020/3/10/1839
>>
>> 					-Alex
> 
> In particular, this seems to now have regressed into mainline for the 5.7
> merge window as reported by Linaro's ToolChain Working Group's CI.
> Link: https://github.com/ClangBuiltLinux/linux/issues/963

Is the problem you're referring to the result of a build done
in the midst of a bisect?

The fix for this build error is currently present in the
torvalds/linux.git master branch:
    6fcd42242ebc soc: qcom: ipa: kill IPA_RX_BUFFER_ORDER

I may be mistaken, but I believe this is the same problem I discussed
with Maxim Kuvyrkov this morning.  A different build problem led to
an automated bisect, which conluded this was the cause because it
landed somewhere between the initial pull of the IPA code and the fix
I reference above.

					-Alex

Re: [PATCH v3] bitfield.h: add FIELD_MAX() and field_max()

[Nick Desaulniers]

On Wed, Apr 1, 2020 at 11:24 AM Alex Elder <elder@linaro.org> wrote:
>
> On 4/1/20 12:35 PM, Nick Desaulniers wrote:
> >> Define FIELD_MAX(), which supplies the maximum value that can be
> >> represented by a field value.  Define field_max() as well, to go
> >> along with the lower-case forms of the field mask functions.
> >>
> >> Signed-off-by: Alex Elder <elder@linaro.org>
> >> Acked-by: Jakub Kicinski <kuba@kernel.org>
> >> ---
> >> v3: Rebased on latest netdev-next/master.
> >>
> >> David, please take this into net-next as soon as possible.  When the
> >> IPA code was merged the other day this prerequisite patch was not
> >> included, and as a result the IPA driver fails to build.  Thank you.
> >>
> >>   See: https://lkml.org/lkml/2020/3/10/1839
> >>
> >>                                      -Alex
> >
> > In particular, this seems to now have regressed into mainline for the 5.7
> > merge window as reported by Linaro's ToolChain Working Group's CI.
> > Link: https://github.com/ClangBuiltLinux/linux/issues/963
>
> Is the problem you're referring to the result of a build done
> in the midst of a bisect?
>
> The fix for this build error is currently present in the
> torvalds/linux.git master branch:
>     6fcd42242ebc soc: qcom: ipa: kill IPA_RX_BUFFER_ORDER

Is that right? That patch is in mainline, but looks unrelated to what
I'm referring to.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6fcd42242ebcc98ebf1a9a03f5e8cb646277fd78
From my github link above, the issue I'm referring to is a
-Wimplicit-function-declaration warning related to field_max.
6fcd42242ebc doesn't look related.

>
> I may be mistaken, but I believe this is the same problem I discussed
> with Maxim Kuvyrkov this morning.  A different build problem led to
> an automated bisect, which conluded this was the cause because it
> landed somewhere between the initial pull of the IPA code and the fix
> I reference above.

Yes, Maxim runs Linaro's ToolChain Working Group (IIUC, but you work
there, so you probably know better than I do), that's the CI I was
referring to.

I'm more concerned when I see reports of regressions *in mainline*.
The whole point of -next is that warnings reported there get fixed
BEFORE the merge window opens, so that we don't regress mainline.  Or
we drop the patches in -next.
-- 
Thanks,
~Nick Desaulniers

Re: [PATCH v3] bitfield.h: add FIELD_MAX() and field_max()

[Alex Elder]

On 4/1/20 2:13 PM, Nick Desaulniers wrote:
> On Wed, Apr 1, 2020 at 11:24 AM Alex Elder <elder@linaro.org> wrote:
>>
>> On 4/1/20 12:35 PM, Nick Desaulniers wrote:
>>>> Define FIELD_MAX(), which supplies the maximum value that can be
>>>> represented by a field value.  Define field_max() as well, to go
>>>> along with the lower-case forms of the field mask functions.
>>>>
>>>> Signed-off-by: Alex Elder <elder@linaro.org>
>>>> Acked-by: Jakub Kicinski <kuba@kernel.org>
>>>> ---
>>>> v3: Rebased on latest netdev-next/master.
>>>>
>>>> David, please take this into net-next as soon as possible.  When the
>>>> IPA code was merged the other day this prerequisite patch was not
>>>> included, and as a result the IPA driver fails to build.  Thank you.
>>>>
>>>>   See: https://lkml.org/lkml/2020/3/10/1839
>>>>
>>>>                                      -Alex
>>>
>>> In particular, this seems to now have regressed into mainline for the 5.7
>>> merge window as reported by Linaro's ToolChain Working Group's CI.
>>> Link: https://github.com/ClangBuiltLinux/linux/issues/963
>>
>> Is the problem you're referring to the result of a build done
>> in the midst of a bisect?
>>
>> The fix for this build error is currently present in the
>> torvalds/linux.git master branch:
>>     6fcd42242ebc soc: qcom: ipa: kill IPA_RX_BUFFER_ORDER
> 
> Is that right? That patch is in mainline, but looks unrelated to what
> I'm referring to.
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6fcd42242ebcc98ebf1a9a03f5e8cb646277fd78
> From my github link above, the issue I'm referring to is a
> -Wimplicit-function-declaration warning related to field_max.
> 6fcd42242ebc doesn't look related.

I'm very sorry, I pointed you at the wrong commit.  This one is
also present in torvalds/linux.git master:

  e31a50162feb bitfield.h: add FIELD_MAX() and field_max()

It defines field_max() as a macro in <linux/bitfield.h>, and
"gsi.c" includes that header file.

This was another commit that got added late, after the initial
IPA code was accepted.

>> I may be mistaken, but I believe this is the same problem I discussed
>> with Maxim Kuvyrkov this morning.  A different build problem led to
>> an automated bisect, which conluded this was the cause because it
>> landed somewhere between the initial pull of the IPA code and the fix
>> I reference above.
> 
> Yes, Maxim runs Linaro's ToolChain Working Group (IIUC, but you work
> there, so you probably know better than I do), that's the CI I was
> referring to.
> 
> I'm more concerned when I see reports of regressions *in mainline*.
> The whole point of -next is that warnings reported there get fixed
> BEFORE the merge window opens, so that we don't regress mainline.  Or
> we drop the patches in -next.

Can you tell me where I can find the commit id of the kernel
that is being built when this error is reported?  I would
like to examine things and build it myself so I can fix it.
But so far haven't found what I need to check out.

Thank you.

					-Alex

Re: [PATCH v3] bitfield.h: add FIELD_MAX() and field_max()

[Nick Desaulniers]

On Wed, Apr 1, 2020 at 12:44 PM Alex Elder <elder@linaro.org> wrote:
>
> On 4/1/20 2:13 PM, Nick Desaulniers wrote:
> > On Wed, Apr 1, 2020 at 11:24 AM Alex Elder <elder@linaro.org> wrote:
> >>
> >> On 4/1/20 12:35 PM, Nick Desaulniers wrote:
> >>>> Define FIELD_MAX(), which supplies the maximum value that can be
> >>>> represented by a field value.  Define field_max() as well, to go
> >>>> along with the lower-case forms of the field mask functions.
> >>>>
> >>>> Signed-off-by: Alex Elder <elder@linaro.org>
> >>>> Acked-by: Jakub Kicinski <kuba@kernel.org>
> >>>> ---
> >>>> v3: Rebased on latest netdev-next/master.
> >>>>
> >>>> David, please take this into net-next as soon as possible.  When the
> >>>> IPA code was merged the other day this prerequisite patch was not
> >>>> included, and as a result the IPA driver fails to build.  Thank you.
> >>>>
> >>>>   See: https://lkml.org/lkml/2020/3/10/1839
> >>>>
> >>>>                                      -Alex
> >>>
> >>> In particular, this seems to now have regressed into mainline for the 5.7
> >>> merge window as reported by Linaro's ToolChain Working Group's CI.
> >>> Link: https://github.com/ClangBuiltLinux/linux/issues/963
> >>
> >> Is the problem you're referring to the result of a build done
> >> in the midst of a bisect?
> >>
> >> The fix for this build error is currently present in the
> >> torvalds/linux.git master branch:
> >>     6fcd42242ebc soc: qcom: ipa: kill IPA_RX_BUFFER_ORDER
> >
> > Is that right? That patch is in mainline, but looks unrelated to what
> > I'm referring to.
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6fcd42242ebcc98ebf1a9a03f5e8cb646277fd78
> > From my github link above, the issue I'm referring to is a
> > -Wimplicit-function-declaration warning related to field_max.
> > 6fcd42242ebc doesn't look related.
>
> I'm very sorry, I pointed you at the wrong commit.  This one is
> also present in torvalds/linux.git master:
>
>   e31a50162feb bitfield.h: add FIELD_MAX() and field_max()
>
> It defines field_max() as a macro in <linux/bitfield.h>, and
> "gsi.c" includes that header file.
>
> This was another commit that got added late, after the initial
> IPA code was accepted.

Yep, that looks better.

>
> >> I may be mistaken, but I believe this is the same problem I discussed
> >> with Maxim Kuvyrkov this morning.  A different build problem led to
> >> an automated bisect, which conluded this was the cause because it
> >> landed somewhere between the initial pull of the IPA code and the fix
> >> I reference above.
> >
> > Yes, Maxim runs Linaro's ToolChain Working Group (IIUC, but you work
> > there, so you probably know better than I do), that's the CI I was
> > referring to.
> >
> > I'm more concerned when I see reports of regressions *in mainline*.
> > The whole point of -next is that warnings reported there get fixed
> > BEFORE the merge window opens, so that we don't regress mainline.  Or
> > we drop the patches in -next.
>
> Can you tell me where I can find the commit id of the kernel
> that is being built when this error is reported?  I would
> like to examine things and build it myself so I can fix it.
> But so far haven't found what I need to check out.

From the report: https://groups.google.com/g/clang-built-linux/c/pX-kr_t5l_A
Configuration details:
rr[llvm_url]="https://github.com/llvm/llvm-project.git"
rr[linux_url]="https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git"
rr[linux_branch]="7111951b8d4973bda27ff663f2cf18b663d15b48"

the linux_branch looks like a SHA of what the latest ToT of mainline
was when the CI ran.

I was suspecting that maybe there was a small window between the
regression, and the fix, and when the bot happened to sync.  But it
seems that: e31a50162feb352147d3fc87b9e036703c8f2636 landed before
7111951b8d4973bda27ff663f2cf18b663d15b48 IIUC.

So I think the bot had your change when it ran, so still seeing a
failure is curious.  Unless I've misunderstood something.
-- 
Thanks,
~Nick Desaulniers

Re: [PATCH v3] bitfield.h: add FIELD_MAX() and field_max()

[Alex Elder]

On 4/1/20 2:54 PM, Nick Desaulniers wrote:
> On Wed, Apr 1, 2020 at 12:44 PM Alex Elder <elder@linaro.org> wrote:
>>
>> On 4/1/20 2:13 PM, Nick Desaulniers wrote:
>>> On Wed, Apr 1, 2020 at 11:24 AM Alex Elder <elder@linaro.org> wrote:
>>>>
>>>> On 4/1/20 12:35 PM, Nick Desaulniers wrote:
>>>>>> Define FIELD_MAX(), which supplies the maximum value that can be
>>>>>> represented by a field value.  Define field_max() as well, to go
>>>>>> along with the lower-case forms of the field mask functions.
>>>>>>
>>>>>> Signed-off-by: Alex Elder <elder@linaro.org>
>>>>>> Acked-by: Jakub Kicinski <kuba@kernel.org>
>>>>>> ---
>>>>>> v3: Rebased on latest netdev-next/master.
>>>>>>
>>>>>> David, please take this into net-next as soon as possible.  When the
>>>>>> IPA code was merged the other day this prerequisite patch was not
>>>>>> included, and as a result the IPA driver fails to build.  Thank you.
>>>>>>
>>>>>>   See: https://lkml.org/lkml/2020/3/10/1839
>>>>>>
>>>>>>                                      -Alex
>>>>>
>>>>> In particular, this seems to now have regressed into mainline for the 5.7
>>>>> merge window as reported by Linaro's ToolChain Working Group's CI.
>>>>> Link: https://github.com/ClangBuiltLinux/linux/issues/963
>>>>
>>>> Is the problem you're referring to the result of a build done
>>>> in the midst of a bisect?
>>>>
>>>> The fix for this build error is currently present in the
>>>> torvalds/linux.git master branch:
>>>>     6fcd42242ebc soc: qcom: ipa: kill IPA_RX_BUFFER_ORDER
>>>
>>> Is that right? That patch is in mainline, but looks unrelated to what
>>> I'm referring to.
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6fcd42242ebcc98ebf1a9a03f5e8cb646277fd78
>>> From my github link above, the issue I'm referring to is a
>>> -Wimplicit-function-declaration warning related to field_max.
>>> 6fcd42242ebc doesn't look related.
>>
>> I'm very sorry, I pointed you at the wrong commit.  This one is
>> also present in torvalds/linux.git master:
>>
>>   e31a50162feb bitfield.h: add FIELD_MAX() and field_max()
>>
>> It defines field_max() as a macro in <linux/bitfield.h>, and
>> "gsi.c" includes that header file.
>>
>> This was another commit that got added late, after the initial
>> IPA code was accepted.
> 
> Yep, that looks better.

Sorry about that.  The two actually are related in a way, because
without the first one I pointed you at, a *different* problem
involving field_max() gets triggered.  But that's irrelevant to
this discussion...

>>>> I may be mistaken, but I believe this is the same problem I discussed
>>>> with Maxim Kuvyrkov this morning.  A different build problem led to
>>>> an automated bisect, which conluded this was the cause because it
>>>> landed somewhere between the initial pull of the IPA code and the fix
>>>> I reference above.
>>>
>>> Yes, Maxim runs Linaro's ToolChain Working Group (IIUC, but you work
>>> there, so you probably know better than I do), that's the CI I was
>>> referring to.
>>>
>>> I'm more concerned when I see reports of regressions *in mainline*.
>>> The whole point of -next is that warnings reported there get fixed
>>> BEFORE the merge window opens, so that we don't regress mainline.  Or
>>> we drop the patches in -next.
>>
>> Can you tell me where I can find the commit id of the kernel
>> that is being built when this error is reported?  I would
>> like to examine things and build it myself so I can fix it.
>> But so far haven't found what I need to check out.
> 
> From the report: https://groups.google.com/g/clang-built-linux/c/pX-kr_t5l_A

That link doesn't work for me.

> Configuration details:
> rr[llvm_url]="https://github.com/llvm/llvm-project.git"
> rr[linux_url]="https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git"
> rr[linux_branch]="7111951b8d4973bda27ff663f2cf18b663d15b48"

That commit is just the one in which Linux v5.6 is tagged.
It doesn't include any of this code (but it's the last
tagged release that current linus/master is built on).

It doesn't answer my question about what commit id was
used for this build, unfortunately.

> the linux_branch looks like a SHA of what the latest ToT of mainline
> was when the CI ran.
> 
> I was suspecting that maybe there was a small window between the
> regression, and the fix, and when the bot happened to sync.  But it
> seems that: e31a50162feb352147d3fc87b9e036703c8f2636 landed before
> 7111951b8d4973bda27ff663f2cf18b663d15b48 IIUC.

Yes, this:
  e31a50162feb bitfield.h: add FIELD_MAX() and field_max()
landed about 200 commits after the code that needed it.

So there's a chance the kernel that got built was somewhere
between those two, and I believe the problem you point out
would happen in that case.  This is why I started by asking
whether it was something built during a bisect.

It's still not clear to me what happened here.  I can explain
how this *could* happen, but I don't believe problem exists
in the latest upstream kernel commit.

Is there something else you think I should do?

					-Alex

> So I think the bot had your change when it ran, so still seeing a
> failure is curious.  Unless I've misunderstood something.

Re: [PATCH v3] bitfield.h: add FIELD_MAX() and field_max()

[Nick Desaulniers]

On Wed, Apr 1, 2020 at 1:21 PM Alex Elder <elder@linaro.org> wrote:
>
> On 4/1/20 2:54 PM, Nick Desaulniers wrote:
> > On Wed, Apr 1, 2020 at 12:44 PM Alex Elder <elder@linaro.org> wrote:
> >>
> >> Can you tell me where I can find the commit id of the kernel
> >> that is being built when this error is reported?  I would
> >> like to examine things and build it myself so I can fix it.
> >> But so far haven't found what I need to check out.
> >
> > From the report: https://groups.google.com/g/clang-built-linux/c/pX-kr_t5l_A
>
> That link doesn't work for me.

Sigh, second internal bug filed against google groups this
week...settings look correct but I too see a 404 when in private
browsing mode.

>
> > Configuration details:
> > rr[llvm_url]="https://github.com/llvm/llvm-project.git"
> > rr[linux_url]="https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git"
> > rr[linux_branch]="7111951b8d4973bda27ff663f2cf18b663d15b48"
>
> That commit is just the one in which Linux v5.6 is tagged.
> It doesn't include any of this code (but it's the last
> tagged release that current linus/master is built on).
>
> It doesn't answer my question about what commit id was
> used for this build, unfortunately.

7111951b8d4973bda27ff663f2cf18b663d15b48 *is* the commit id that was
used for the build.  It sync'd the mainline tree at that commit.

> > the linux_branch looks like a SHA of what the latest ToT of mainline
> > was when the CI ran.
> >
> > I was suspecting that maybe there was a small window between the
> > regression, and the fix, and when the bot happened to sync.  But it
> > seems that: e31a50162feb352147d3fc87b9e036703c8f2636 landed before
> > 7111951b8d4973bda27ff663f2cf18b663d15b48 IIUC.
>
> Yes, this:
>   e31a50162feb bitfield.h: add FIELD_MAX() and field_max()
> landed about 200 commits after the code that needed it.
>
> So there's a chance the kernel that got built was somewhere
> between those two, and I believe the problem you point out
> would happen in that case.  This is why I started by asking
> whether it was something built during a bisect.
>
> It's still not clear to me what happened here.  I can explain
> how this *could* happen, but I don't believe problem exists
> in the latest upstream kernel commit.
>
> Is there something else you think I should do?

mainline is hosed for aarch64 due to some dtc failures.  I'm not sure
how TCWG's CI chooses the bisection starting point, but if mainline
was broken, and it jumped back say 300 commits, then the automated
bisection may have converged on your first patch, but not the second.

I just checked out mainline @ 7111951b8d4973bda27ff663f2cf18b663d15b48
and couldn't reproduce, so I assume the above is what happened.  So
sorry for the noise, I'll go investigate the dtc failure.  Not sure
how that skipped -next coverage.
-- 
Thanks,
~Nick Desaulniers

Re: [PATCH v3] bitfield.h: add FIELD_MAX() and field_max()

[Alex Elder]

On 4/1/20 5:26 PM, Nick Desaulniers wrote:
> On Wed, Apr 1, 2020 at 1:21 PM Alex Elder <elder@linaro.org> wrote:
>>
>> On 4/1/20 2:54 PM, Nick Desaulniers wrote:
>>> On Wed, Apr 1, 2020 at 12:44 PM Alex Elder <elder@linaro.org> wrote:
>>>>
>>>> Can you tell me where I can find the commit id of the kernel
>>>> that is being built when this error is reported?  I would
>>>> like to examine things and build it myself so I can fix it.
>>>> But so far haven't found what I need to check out.
>>>
>>> From the report: https://groups.google.com/g/clang-built-linux/c/pX-kr_t5l_A
>>
>> That link doesn't work for me.
> 
> Sigh, second internal bug filed against google groups this
> week...settings look correct but I too see a 404 when in private
> browsing mode.
> 
>>
>>> Configuration details:
>>> rr[llvm_url]="https://github.com/llvm/llvm-project.git"
>>> rr[linux_url]="https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git"
>>> rr[linux_branch]="7111951b8d4973bda27ff663f2cf18b663d15b48"
>>
>> That commit is just the one in which Linux v5.6 is tagged.
>> It doesn't include any of this code (but it's the last
>> tagged release that current linus/master is built on).
>>
>> It doesn't answer my question about what commit id was
>> used for this build, unfortunately.
> 
> 7111951b8d4973bda27ff663f2cf18b663d15b48 *is* the commit id that was
> used for the build.  It sync'd the mainline tree at that commit.
> 
>>> the linux_branch looks like a SHA of what the latest ToT of mainline
>>> was when the CI ran.
>>>
>>> I was suspecting that maybe there was a small window between the
>>> regression, and the fix, and when the bot happened to sync.  But it
>>> seems that: e31a50162feb352147d3fc87b9e036703c8f2636 landed before
>>> 7111951b8d4973bda27ff663f2cf18b663d15b48 IIUC.
>>
>> Yes, this:
>>   e31a50162feb bitfield.h: add FIELD_MAX() and field_max()
>> landed about 200 commits after the code that needed it.
>>
>> So there's a chance the kernel that got built was somewhere
>> between those two, and I believe the problem you point out
>> would happen in that case.  This is why I started by asking
>> whether it was something built during a bisect.
>>
>> It's still not clear to me what happened here.  I can explain
>> how this *could* happen, but I don't believe problem exists
>> in the latest upstream kernel commit.
>>
>> Is there something else you think I should do?
> 
> mainline is hosed for aarch64 due to some dtc failures.  I'm not sure
> how TCWG's CI chooses the bisection starting point, but if mainline
> was broken, and it jumped back say 300 commits, then the automated
> bisection may have converged on your first patch, but not the second.

This is similar to the situation I discussed with Maxim this
morning.  A different failure (yes, DTC related) led to an
automated bisect process, which landed on my commit. And my
commit unfortunately has the the known issue that was later
corrected.

Maxim said this was what started the automated bisect:
===
+# 00:01:41 make[2]: *** [arch/arm64/boot/dts/ti/k3-am654-base-board.dtb] Error 2
+# 00:01:41 make[2]: *** [arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dtb] Error 2
+# 00:01:41 make[1]: *** [arch/arm64/boot/dts/ti] Error 2
+# 00:01:41 make: *** [dtbs] Error 2
===

> I just checked out mainline @ 7111951b8d4973bda27ff663f2cf18b663d15b48
> and couldn't reproduce, so I assume the above is what happened.  So
> sorry for the noise, I'll go investigate the dtc failure.  Not sure
> how that skipped -next coverage.

Thank you for following up.

					-Alex

Re: [PATCH v3] bitfield.h: add FIELD_MAX() and field_max()

[Nick Desaulniers]

On Wed, Apr 1, 2020 at 4:18 PM Alex Elder <elder@linaro.org> wrote:
>
> On 4/1/20 5:26 PM, Nick Desaulniers wrote:
> >
> > mainline is hosed for aarch64 due to some dtc failures.  I'm not sure
> > how TCWG's CI chooses the bisection starting point, but if mainline
> > was broken, and it jumped back say 300 commits, then the automated
> > bisection may have converged on your first patch, but not the second.
>
> This is similar to the situation I discussed with Maxim this
> morning.  A different failure (yes, DTC related) led to an
> automated bisect process, which landed on my commit. And my
> commit unfortunately has the the known issue that was later
> corrected.
>
> Maxim said this was what started the automated bisect:
> ===
> +# 00:01:41 make[2]: *** [arch/arm64/boot/dts/ti/k3-am654-base-board.dtb] Error 2
> +# 00:01:41 make[2]: *** [arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dtb] Error 2
> +# 00:01:41 make[1]: *** [arch/arm64/boot/dts/ti] Error 2
> +# 00:01:41 make: *** [dtbs] Error 2

DTC thread:
https://lore.kernel.org/linux-arm-kernel/20200401223500.224253-1-ndesaulniers@google.com/

Maxim, can you describe how the last known good sha is chosen?  If you
persist anything between builds, like ccache dir, maybe you could
propagate a sha of the last successful build, updating it if no
regression occurred?  Then that can always be a precise last known
good sha.  Though I don't know if the merge commits complicate this.
-- 
Thanks,
~Nick Desaulniers

Re: [PATCH v3] bitfield.h: add FIELD_MAX() and field_max()

[Maxim Kuvyrkov]

> On 2 Apr 2020, at 03:26, Nick Desaulniers <ndesaulniers@google.com> wrote:
> 
> On Wed, Apr 1, 2020 at 4:18 PM Alex Elder <elder@linaro.org> wrote:
>> 
>> On 4/1/20 5:26 PM, Nick Desaulniers wrote:
>>> 
>>> mainline is hosed for aarch64 due to some dtc failures.  I'm not sure
>>> how TCWG's CI chooses the bisection starting point, but if mainline
>>> was broken, and it jumped back say 300 commits, then the automated
>>> bisection may have converged on your first patch, but not the second.
>> 
>> This is similar to the situation I discussed with Maxim this
>> morning.  A different failure (yes, DTC related) led to an
>> automated bisect process, which landed on my commit. And my
>> commit unfortunately has the the known issue that was later
>> corrected.
>> 
>> Maxim said this was what started the automated bisect:
>> ===
>> +# 00:01:41 make[2]: *** [arch/arm64/boot/dts/ti/k3-am654-base-board.dtb] Error 2
>> +# 00:01:41 make[2]: *** [arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dtb] Error 2
>> +# 00:01:41 make[1]: *** [arch/arm64/boot/dts/ti] Error 2
>> +# 00:01:41 make: *** [dtbs] Error 2
> 
> DTC thread:
> https://lore.kernel.org/linux-arm-kernel/20200401223500.224253-1-ndesaulniers@google.com/
> 
> Maxim, can you describe how the last known good sha is chosen?  If you
> persist anything between builds, like ccache dir, maybe you could
> propagate a sha of the last successful build, updating it if no
> regression occurred?  Then that can always be a precise last known
> good sha.  Though I don't know if the merge commits complicate this.

Well, since you asked, the simplified version is …

Bisection is done between “baseline" commit and “regressed” commit, not between "last known-good” commit and “bad” commit.  Each build has a metric, and regression happens when metric for new build is worse than metric for baseline build.  For tcwg_kernel jobs the metric is the number of .o files produced in the kernel build (i.e., build with 18555 .o files is worse than build with 18560 .o files).

If a new build hasn’t regressed compared to the baseline build, then baseline metric is set to that of the new build, and baseline commit is set to sha1 of the new build.

If the new build has regressed, then bisection between baseline sha1 and new sha1 is triggered.  Once bisection identifies the first bad commit, CI notification is emailed, and baseline is reset to the first bad commit — so that we detect even worse regressions when they occur.

The baseline state is recorded in git repos (with one branch per CI configuration):
- metric / artifacts: https://git.linaro.org/toolchain/ci/base-artifacts.git/refs/heads
- linux kernel: https://git.linaro.org/toolchain/ci/linux.git/refs/heads
- llvm: https://git.linaro.org/toolchain/ci/llvm-project.git/refs/heads

--
Maxim Kuvyrkov
https://www.linaro.org