Re: /usr/src/sys/net/if_epair.c:181:6: error: ...

On 2022-Mar-23, at 03:51, Kristof Provost <[email protected]> wrote:

> Hi Mark,

Hello.

> On 23 Mar 2022, at 4:23, Mark Millard wrote:
>> On 2022-Mar-22, at 20:16, Mark Millard <[email protected]> wrote:
>> 
>>> [Trying again after getting material from the wrong
>>> commit the first time.]
>>> 
>>> On 2022-Mar-22, at 18:26, bob prohaska <[email protected]> wrote:
>>> 
>>>> A Pi2 running
>>>> FreeBSD www.zefox.net 12.3-STABLE FreeBSD 12.3-STABLE r371495 GENERIC  arm
>>>> 
>>>> stops buildkernel with:
>>>> --- if_epair.o ---
>>>> /usr/src/sys/net/if_epair.c:181:6: error: implicit declaration of function 'atomic_testandclear_long' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
>>>>      if (atomic_testandclear_long(&q->state, BIT_MBUF_QUEUED))
>>>>          ^
>>>> 
>>>> Not sure if this is specific to the Raspberry Pi 2, it didn't show up on a pair of Pi3's
>>>> and a single Pi4. The system is still using svnlite, info reports
>>>> root@www:/usr/src # svnlite info
>>>> Path: .
>>>> Working Copy Root Path: /usr/src
>>>> URL: svn://svn.freebsd.org/base/stable/12
>>>> Relative URL: ^/stable/12
>>>> Repository Root: svn://svn.freebsd.org/base
>>>> Repository UUID: ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f
>>>> Revision: 371771
>>>> Node Kind: directory
>>>> Schedule: normal
>>>> Last Changed Author: 0mp
>>>> Last Changed Rev: 371771
>>>> Last Changed Date: 2022-03-22 15:28:40 -0700 (Tue, 22 Mar 2022)
>>>> 
>>>> 
>>>> Didn't see anything similar on bugs.freebsd.org, if it's worth a bug report or
>>>> there's a workaround please post. It was built using WITH_META_MODE if that
>>>> matters.
>>> 
>>> QUOTE
>>> author	Kristof Provost <[email protected]>	2022-03-17 02:35:13 +0000
>>> committer	Kristof Provost <[email protected]>	2022-03-20 00:25:06 +0000
>>> commit	b1a3f8dccb6203036b7ee81201fd5b5a8de36f0d (patch)
>>> . . .
>>> if_epair: build fix
>>> 66acf7685b failed to build on riscv (and mips). This is because the
>>> atomic_testandset_int() (and friends) functions do not exist there.
>>> Happily those platforms do have the long variant, so switch to that.
>>> END QUOTE
>>> 
>>> broke things for stable/12 by adding the atomic_testandclear_long
>>> usage without defining it as well.
>>> 
>>> It goes like this:
>>> 
>>> path: root/sys/arm/include/atomic.h
>>> Commit message (Expand)	Author	Age	Files	Lines
>>> * 	MFC r341787 by hselasky: Implement atomic_swap_xxx() for all platforms.	Andriy Gapon	2019-10-24	1	-0/+7
>>> * 	Remove arm-specific implementations of atomic_load/store_xxx() now that	Ian Lepore	2017-12-20	1	-27/+0
>>> . . .
>>> 
>>> So not updated in a long time. But for armv7 and the like, it includes:
>>> 
>>> path: root/sys/arm/include/atomic-v6.h
>>> Commit message (Expand)	Author	Age	Files	Lines
>>> * 	MFC r352938:	Ian Lepore	2019-12-07	1	-100/+256
>>> * 	MFC r341679:	Michal Meloun	2018-12-14	1	-1/+1
>>> . . .
>>> 
>>> Also not updated in a long time.
>>> 
>>> sys/arm/include/atomic-v6.h has various "atomic_testand"
>>> examples ( sys/arm/include/atomic.h does not ):
>>> 
>>> atomic_testandset_32
>>> atomic_testandset_int
>>> atomic_testandset_long
>>> atomic_testandset_64
>>> 
>>> But no examples of "atomic_testandclear"
>>> 
>>> 
>> 
>> The slightly older commit:
>> 
>> QUOTE
>> author	Michael Gmelin <[email protected]>	2022-03-16 22:08:55 +0000
>> committer	Kristof Provost <[email protected]>	2022-03-20 00:24:51 +0000
>> commit	fb3644ab2afe777fdd2539bc996a390443f052f1 (patch)
>> . . .
>> if_epair: fix race condition on multi-core systems
>> END QUOTE
>> 
>> has a use of "atomic_testandclear_int" as well.
>> 
>> 
> Thanks for the report.
> 
> Can you try the attached patch? I’m not going to argue with the MI code about the atomic_testandclear_int, but instead revert the new if_epair code (in stable/12 only, of course).

We will see if Bob P. can try it. (But his builds
take time, given the machine type involved.)

I'm not set up to build anything for 12. I just looked
up what was going on in Bob's report and commented.

Ultimately, if needed, I could deal with setting up a
temporary context for building stable/12 on aarch64.

===
Mark Millard
marklmi at yahoo.com