Re: [PATCH 0/3] selftests/bpf: Fix a few issues in arena_spin_lock

Re: [PATCH 0/3] selftests/bpf: Fix a few issues in arena_spin_lock

[Alexei Starovoitov]

On Thu, Apr 24, 2025 at 11:32 AM <patchwork-bot+netdevbpf@kernel.org> wrote:
>
> Hello:
>
> This series was applied to netdev/net-next.git (main)
> by Jakub Kicinski <kuba@kernel.org>:
>
> On Thu, 24 Apr 2025 18:41:24 +0200 you wrote:
> > Hi,
> >
> > I tried running the arena_spin_lock test on s390x and ran into the
> > following issues:
> >
> > * Changing the header file does not lead to rebuilding the test.
> > * The checked for number of CPUs and the actually required number of
> >   CPUs are different.
> > * Endianness issue in spinlock definition.
> >
> > [...]
>
> Here is the summary with links:
>   - [1/3] selftests/bpf: Fix arena_spin_lock.c build dependency
>     https://git.kernel.org/netdev/net-next/c/4fe09ff1a54a
>   - [2/3] selftests/bpf: Fix arena_spin_lock on systems with less than 16 CPUs
>     (no matching commit)
>   - [3/3] selftests/bpf: Fix endianness issue in __qspinlock declaration
>     (no matching commit)

Hmm. Looks like pw-bot had too much influence from AI bots
and started hallucinating itself :)


Ilya,

no worries. Your patches are still under review.
I unmarked it in patchwork.

Re: [PATCH 0/3] selftests/bpf: Fix a few issues in arena_spin_lock

[Konstantin Ryabitsev]

On Thu, Apr 24, 2025 at 11:41:16AM -0700, Alexei Starovoitov wrote:
> On Thu, Apr 24, 2025 at 11:32 AM <patchwork-bot+netdevbpf@kernel.org> wrote:
> >
> > Hello:
> >
> > This series was applied to netdev/net-next.git (main)
> > by Jakub Kicinski <kuba@kernel.org>:
> >
> > On Thu, 24 Apr 2025 18:41:24 +0200 you wrote:
> > > Hi,
> > >
> > > I tried running the arena_spin_lock test on s390x and ran into the
> > > following issues:
> > >
> > > * Changing the header file does not lead to rebuilding the test.
> > > * The checked for number of CPUs and the actually required number of
> > >   CPUs are different.
> > > * Endianness issue in spinlock definition.
> > >
> > > [...]
> >
> > Here is the summary with links:
> >   - [1/3] selftests/bpf: Fix arena_spin_lock.c build dependency
> >     https://git.kernel.org/netdev/net-next/c/4fe09ff1a54a
> >   - [2/3] selftests/bpf: Fix arena_spin_lock on systems with less than 16 CPUs
> >     (no matching commit)
> >   - [3/3] selftests/bpf: Fix endianness issue in __qspinlock declaration
> >     (no matching commit)
> 
> Hmm. Looks like pw-bot had too much influence from AI bots
> and started hallucinating itself :)

I'll look into what happened here.

-K

Re: [PATCH 0/3] selftests/bpf: Fix a few issues in arena_spin_lock

[Jakub Kicinski]

On Thu, 24 Apr 2025 14:51:51 -0400 Konstantin Ryabitsev wrote:
> > Hmm. Looks like pw-bot had too much influence from AI bots
> > and started hallucinating itself :)  
> 
> I'll look into what happened here.

Alexei mentioned that the bot was stopped, I presume to avoid further
mistakes. I'm 100% sure I've seen the bot be confused by merge commits
before. It happens occasionally, IMHO there is no need to take the bot
offline. Is there an ETA on it coming back?

Re: [PATCH 0/3] selftests/bpf: Fix a few issues in arena_spin_lock

[Konstantin Ryabitsev]

On Fri, Apr 25, 2025 at 09:25:51AM -0700, Jakub Kicinski wrote:
> On Thu, 24 Apr 2025 14:51:51 -0400 Konstantin Ryabitsev wrote:
> > > Hmm. Looks like pw-bot had too much influence from AI bots
> > > and started hallucinating itself :)  
> > 
> > I'll look into what happened here.
> 
> Alexei mentioned that the bot was stopped, I presume to avoid further
> mistakes. I'm 100% sure I've seen the bot be confused by merge commits
> before. It happens occasionally, IMHO there is no need to take the bot
> offline. Is there an ETA on it coming back?

Yes, I'm poking at it right now and I'm hoping to bring it back up soon.

-K

Re: [PATCH 0/3] selftests/bpf: Fix a few issues in arena_spin_lock

[Konstantin Ryabitsev]

On Thu, Apr 24, 2025 at 11:41:16AM -0700, Alexei Starovoitov wrote:
> > On Thu, 24 Apr 2025 18:41:24 +0200 you wrote:
> > > Hi,
> > >
> > > I tried running the arena_spin_lock test on s390x and ran into the
> > > following issues:
> > >
> > > * Changing the header file does not lead to rebuilding the test.
> > > * The checked for number of CPUs and the actually required number of
> > >   CPUs are different.
> > > * Endianness issue in spinlock definition.
> > >
> > > [...]
> >
> > Here is the summary with links:
> >   - [1/3] selftests/bpf: Fix arena_spin_lock.c build dependency
> >     https://git.kernel.org/netdev/net-next/c/4fe09ff1a54a
> >   - [2/3] selftests/bpf: Fix arena_spin_lock on systems with less than 16 CPUs
> >     (no matching commit)
> >   - [3/3] selftests/bpf: Fix endianness issue in __qspinlock declaration
> >     (no matching commit)
> 
> Hmm. Looks like pw-bot had too much influence from AI bots
> and started hallucinating itself :)

Looks like it's a mix of bad assumptions and the usual difficulty of
recognizing fast-forward merges that came in through a different tree.

If you look at the commit mentioned above, it has:

| Note that the first patch in this series is a leftover from an
| earlier patchset that was abandoned:
| Link: https://lore.kernel.org/netdev/20250129004337.36898-2-shannon.nelson@amd.com/

This confuses the bot into thinking that the linked message is the source of
the patch (which is why we started using patch.msgid.link to disambiguate
links aimed at cross-referencing and links aimed at indicating commit
provenance -- but we aren't relying on this disambiguation in the bot itself
yet).

The other replies are the usual mess when fast-forward tree updates confuse
things. It's a long-standing hard bug to fix.

I am going to re-enable the bot for now -- in general it's not any more wrong
than usual. I'm scheduling some time next week to try to tackle the
fast-forwards problem.

-K

Re: [PATCH 0/3] selftests/bpf: Fix a few issues in arena_spin_lock

[Alexei Starovoitov]

On Fri, Apr 25, 2025 at 11:58 AM Konstantin Ryabitsev
<konstantin@linuxfoundation.org> wrote:
>
> On Thu, Apr 24, 2025 at 11:41:16AM -0700, Alexei Starovoitov wrote:
> > > On Thu, 24 Apr 2025 18:41:24 +0200 you wrote:
> > > > Hi,
> > > >
> > > > I tried running the arena_spin_lock test on s390x and ran into the
> > > > following issues:
> > > >
> > > > * Changing the header file does not lead to rebuilding the test.
> > > > * The checked for number of CPUs and the actually required number of
> > > >   CPUs are different.
> > > > * Endianness issue in spinlock definition.
> > > >
> > > > [...]
> > >
> > > Here is the summary with links:
> > >   - [1/3] selftests/bpf: Fix arena_spin_lock.c build dependency
> > >     https://git.kernel.org/netdev/net-next/c/4fe09ff1a54a
> > >   - [2/3] selftests/bpf: Fix arena_spin_lock on systems with less than 16 CPUs
> > >     (no matching commit)
> > >   - [3/3] selftests/bpf: Fix endianness issue in __qspinlock declaration
> > >     (no matching commit)
> >
> > Hmm. Looks like pw-bot had too much influence from AI bots
> > and started hallucinating itself :)
>
> Looks like it's a mix of bad assumptions and the usual difficulty of
> recognizing fast-forward merges that came in through a different tree.
>
> If you look at the commit mentioned above, it has:
>
> | Note that the first patch in this series is a leftover from an
> | earlier patchset that was abandoned:
> | Link: https://lore.kernel.org/netdev/20250129004337.36898-2-shannon.nelson@amd.com/
>
> This confuses the bot into thinking that the linked message is the source of
> the patch (which is why we started using patch.msgid.link to disambiguate
> links aimed at cross-referencing and links aimed at indicating commit
> provenance -- but we aren't relying on this disambiguation in the bot itself
> yet).

Thanks for investigating. The above part is clear,
but I still don't understand what was so special about Ilya's
patch that only his first patch in the series became a victim.
msgid-s are completely different.

> The other replies are the usual mess when fast-forward tree updates confuse
> things. It's a long-standing hard bug to fix.
>
> I am going to re-enable the bot for now -- in general it's not any more wrong
> than usual.

Makes sense. Better to have it flaky than none at all.

> I'm scheduling some time next week to try to tackle the
> fast-forwards problem.

Thanks. That would be great.