Page MenuHomeFreeBSD

umtx: convert hand-rolled busy lock to sx lock
AbandonedPublic

Authored by mjg on Aug 8 2022, 7:24 PM.
Tags
None
Referenced Files
F82914141: D36083.id.diff
Fri, May 3, 10:33 PM
F82900315: D36083.id109020.diff
Fri, May 3, 6:12 PM
Unknown Object (File)
Sat, Apr 27, 9:14 AM
Unknown Object (File)
Sat, Apr 13, 3:12 PM
Unknown Object (File)
Dec 25 2023, 8:07 PM
Unknown Object (File)
Nov 30 2023, 4:54 AM
Unknown Object (File)
Apr 8 2023, 11:03 AM
Unknown Object (File)
Mar 22 2023, 6:35 PM
Subscribers
None

Details

Reviewers
kib
markj
Summary

The current implementation spins for an arbitrary time and goes off cpu, which I can see on dtrace when doing -j 104 tinderbox on a kernel with other patches. Use an sx lock instead.

commit 1d17a05b7ecad6028d2ec9ee1e65d840dc4fafb1
Author: Mateusz Guzik <mjg@FreeBSD.org>
Date:   Fri Aug 12 16:35:23 2022 +0000

    linux: employ umtxq_busy_unlocked
    
    Reviewed by:
    Differential Revision:

commit 2bdd0429def9352ce0820285a020b92eed709653
Author: Mateusz Guzik <mjg@FreeBSD.org>
Date:   Thu Aug 4 16:08:32 2022 +0000

    umtx: employ umtxq_busy_unlocked

commit 05ed23714fbf385e299f8ba2a6702317b5014130
Author: Mateusz Guzik <mjg@FreeBSD.org>
Date:   Thu Aug 4 16:04:02 2022 +0000

    umtx: convert busy to sx lock
    
    This avoids the highly pessimal scheme of arbitrarily spinning
    BUSY_SPINS times. In face of contention this almost always
    ends in going off cpu anyway.

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

mjg requested review of this revision.Aug 8 2022, 7:24 PM
mjg created this revision.

Does the linuxulator futex code need some changes as well? Looks like it is still using the umtxq_lock(); umtxq_busy(); pattern in some places.

If the busy lock becomes a proper mutex, then why exactly do we need both uc_busy and uc_lock?

The to linux emul will be made later, they are not *necessary* to keep things rolling.

To my reading umtx lock and busy lock have different scopes, despite most of the time being used together. The patch at hand is supposed to be the minimum which gets the work done.

  • patch futexes
  • macroify busy unlocked
mjg edited the summary of this revision. (Show Details)
In D36083#820643, @mjg wrote:

To my reading umtx lock and busy lock have different scopes, despite most of the time being used together. The patch at hand is supposed to be the minimum which gets the work done.

I agree with Mark, it seems that you can get away with the single (sleepable) lock there. If umtxq_sleep() is changed to sxsleep(), I do not see much need in the separate mutex. This would half the number of atomics used, if worked, typically.