syscall, internal/poll: accept4-to-accept fallback removal broke Go code on Synology DSM 6.2 ARM devices

[bradfitz]

We've started getting bug reports that our application is broken on older ARM-based Synology devices. SyncThing is also hitting it in syncthing/syncthing#8325.

In #45964 it was decided to require Linux 2.6.32 at minimum because that seemed like a popular cut-off as seen in the wild (due to Synology DSM 6.2) as seen by data I'd earlier collected.

Then some Go patches landed to simplify things based on that new minimum:

• https://go-review.googlesource.com/c/go/+/346849 (6705191) for Go 1.18
• ... (something else? still digging) ....
  *b8b9e83 for Go 1.20rc1

Because packages in the Synology Package Center don't update often (it's like quarters to a year, not days like in the iOS/Google app stores), people are just hitting this now.

As @sfanous says in syncthing/syncthing#8325 (comment):

Did some more digging, it seems accept4 for ARM was added in 2.6.36.
....

So it seems like maybe we should revert that 6705191 accept4->accept fallback if we truly want to say that 2.6.32 is our Linux minimum?

Oh, but I see at https://github.com/golang/go/wiki/MinimumRequirements that it says:

Kernel version 2.6.32 or later. [This depends on architecture though, we need to have specific builder for this.] Linux/ARMv5 requires much newer kernels, at least v3.1 (for __kuser_cmpxchg64).

... and sure enough, some of these older Synology boxes experiencing problems are ARMv5 (e.g. Linux ds212j 2.6.32.12 #25556 Thu Jul 1 14:25:48 CST 2021 armv5tel GNU/Linux synology_88f6281_212j) so maybe accept4 would just be the first of many problems they'd hit. I suppose I could try to run the Go std tests on one of these old boxes, but it's a bespoke userspace so would require some machinery running elsewhere.

/cc @tklauser @ianlancetaylor @knyar (who dig much of this digging)

[bradfitz]

Does Go actually use __kuser_cmpxchg64? I see:

// Linux/ARM atomic operations.

// Because there is so much variation in ARM devices,
// the Linux kernel provides an appropriate compare-and-swap
// implementation at address 0xffff0fc0.  Caller sets:
//      R0 = old value
//      R1 = new value
//      R2 = addr
//      LR = return address
// The function returns with CS true if the swap happened.
// http://lxr.linux.no/linux+v2.6.37.2/arch/arm/kernel/entry-armv.S#L850
// On older kernels (before 2.6.24) the function can incorrectly
// report a conflict, so we have to double-check the compare ourselves
// and retry if necessary.
//
// https://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=b49c0f24cf6744a3f4fd09289fe7cade349dead5
//
TEXT cas<>(SB),NOSPLIT,$0
        MOVW    $0xffff0fc0, R15 // R15 is hardware PC.

TEXT ·Cas(SB),NOSPLIT|NOFRAME,$0
        MOVB    runtime·goarm(SB), R11
        CMP     $7, R11
        BLT     2(PC)
        JMP     ·armcas(SB)
        JMP     kernelcas<>(SB)

TEXT kernelcas<>(SB),NOSPLIT,$0
        MOVW    ptr+0(FP), R2
        // trigger potential paging fault here,
        // because we don't know how to traceback through __kuser_cmpxchg
        MOVW    (R2), R0
        MOVW    old+4(FP), R0

[ianlancetaylor]

Looks like https://go.dev/cl/93637 (from 2018) removed the use of __kuser_cmpxchg64, saying "On linux/arm, 64-bit CAS kernel helper is dropped, as we're trying to move away from kernel helpers."

[ianlancetaylor]

The Linux man page says that accept4 was added in kernel version 2.6.28. However, I do see https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=21d93e2e29722d7832f61cc56d73fb953ee6578e which says "sys_accept4() was added in kernel 2.6.28, but ARM was not updated to include it." So, yeah. I think we need to revert CL 346849. CC @tklauser .

[ianlancetaylor]

@gopherbot Please open backport issues.

We thought that we could require support for accept4 on Linux when we updated the minimum kernel version to 2.6.32, but it turns out that on 32-bit ARM accept4 was only added in kernel version 2.6.36.

[gopherbot]

Backport issue(s) opened: #57338 (for 1.18), #57339 (for 1.19).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases.

[tklauser]

The Linux man page says that accept4 was added in kernel version 2.6.28. However, I do see https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=21d93e2e29722d7832f61cc56d73fb953ee6578e which says "sys_accept4() was added in kernel 2.6.28, but ARM was not updated to include it." So, yeah. I think we need to revert CL 346849. CC @tklauser .

I agree and I think we'd need to revert https://go.dev/cl/422375 (dropping the accept fallback from internal/poll), https://go.dev/cl/410737 (removing syscall.accept for linux/loong64), https://go.dev/cl/386415 (removing syscall.accept for all Linux platforms except loong64) in addition.

[ianlancetaylor]

I'm working on a patch, rather than a set of reverts.

[gopherbot]

Change https://go.dev/cl/457995 mentions this issue: syscall, internal/poll: fall back to accept on linux-arm

[gopherbot]

Change https://go.dev/cl/457996 mentions this issue: [release-branch.go1.19] syscall, internal/poll: fall back to accept on linux-arm

[gopherbot]

Change https://go.dev/cl/457997 mentions this issue: [release-branch.go1.18] syscall, internal/poll: fall back to accept on linux-arm