Thread (11 messages) 11 messages, 3 authors, 2020-04-02

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

From: Alex Elder <hidden>
Date: 2020-04-01 20:21:39
Also in: lkml

On 4/1/20 2:54 PM, Nick Desaulniers wrote:
On Wed, Apr 1, 2020 at 12:44 PM Alex Elder [off-list ref] wrote:
quoted
On 4/1/20 2:13 PM, Nick Desaulniers wrote:
quoted
On Wed, Apr 1, 2020 at 11:24 AM Alex Elder [off-list ref] wrote:
quoted
On 4/1/20 12:35 PM, Nick Desaulniers wrote:
quoted
quoted
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 <redacted>
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...
quoted
quoted
quoted
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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help