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 -AlexIn 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/963Is 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_ORDERIs 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.