Page MenuHomeFreeBSD
Differential D31238

umtx: Split do_unlock_pi on two counterparts.
ClosedPublic
Actions

Authored by dchagin on Jul 20 2021, 2:14 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Oct 22, 3:36 AM
Unknown Object (File)
Wed, Oct 15, 5:34 PM
Unknown Object (File)
Mon, Oct 13, 1:38 AM
Unknown Object (File)
Sat, Oct 11, 4:14 AM
Unknown Object (File)
Fri, Oct 10, 12:34 AM
Unknown Object (File)
Wed, Oct 1, 4:36 PM
Unknown Object (File)
Sat, Sep 27, 6:15 PM
Unknown Object (File)
Sat, Sep 27, 12:52 AM
View All Files
Subscribers
imp

Details

Reviewers
kib
markj
Commits
rGa09a84f84f34: umtx: Split do_unlock_pi on two counterparts.
rG9e32efa79bf2: umtx: Split do_unlock_pi on two counterparts.
Summary

umtx_frop_pi will be used by Linux emulation layer.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 40578
Build 37467: arc lint + arc unit

Event Timeline

dchagin created this revision.Jul 20 2021, 2:14 PM
Herald added a subscriber: imp. · View Herald TranscriptJul 20 2021, 2:14 PM
dchagin requested review of this revision.Jul 20 2021, 2:14 PM
Harbormaster completed remote builds in B40557: Diff 92493.Jul 20 2021, 2:14 PM
dchagin added reviewers: kib, markj.Jul 20 2021, 2:21 PM
dchagin changed the repository for this revision from rS FreeBSD src repository - subversion to rG FreeBSD src repository.
dchagin updated this revision to Diff 92521.Jul 20 2021, 7:31 PM

followup D31248

Harbormaster completed remote builds in B40578: Diff 92521.Jul 20 2021, 7:31 PM
kib added inline comments.Jul 20 2021, 10:20 PM
sys/kern/kern_umtx.c
1720

If exposing such function as the generic KPI, we should assert there that key is locked and busy. For that, you need to add corresponding primitives.

dchagin updated this revision to Diff 92535.Jul 20 2021, 10:56 PM

done, thanks!

Harbormaster completed remote builds in B40589: Diff 92535.Jul 20 2021, 10:56 PM
kib added inline comments.Jul 20 2021, 11:13 PM
sys/kern/kern_umtx.c
1727

Shouldn't #endif go before this line? Otherwise uc is not defined.

It might be better to define some helper e.g. UMTXQ_ASSERT_LOCKED_BUSY(key) and expand it into block as you write for INVARIANTS, otherwise keep it empty.

dchagin updated this revision to Diff 92559.Jul 21 2021, 7:38 AM

done,
sleep_pi may be should refactored to use this macros

Harbormaster completed remote builds in B40592: Diff 92559.Jul 21 2021, 7:38 AM
kib added inline comments.Jul 21 2021, 10:31 AM
sys/kern/kern_umtx.c
1725

No, pass the key to UMTXQ_LOCKED_ASSERT_BUSY(), so that the INVARIANTS block above is not needed.

dchagin updated this revision to Diff 92599.Jul 21 2021, 7:03 PM

done

Harbormaster completed remote builds in B40607: Diff 92599.Jul 21 2021, 7:03 PM
kib added inline comments.Jul 21 2021, 7:24 PM
sys/kern/kern_umtx.c
92

arg should be key and not uc

98

This is better, but please do

#ifdef INVARIANTS
#define UMTXQ_ASSERT_LOCKED_BUSY(key) <your definition>
#else 
#define UMTXQ_ASSERT_LOCKED_BUSY(key) do {} while (0)
#endif

So that uc is not calculated for !INVARIANTS case. Also note a suggested name change

dchagin updated this revision to Diff 92606.Jul 21 2021, 8:03 PM

fixed, sorry(

Harbormaster completed remote builds in B40612: Diff 92606.Jul 21 2021, 8:03 PM
kib added inline comments.Jul 21 2021, 8:24 PM
sys/kern/kern_umtx.c
93

UMTXQ_ASSERT_LOCKED_BUSY

dchagin updated this revision to Diff 92609.Jul 21 2021, 9:21 PM

done,
UMTXQ_LOCKED_ASSERT?

Harbormaster completed remote builds in B40614: Diff 92609.Jul 21 2021, 9:21 PM
kib accepted this revision.Jul 21 2021, 10:10 PM
In D31238#704101, @dchagin wrote:

UMTXQ_LOCKED_ASSERT?

Are you suggesting to call UMTXQ_ASSERT_LOCKED_BUSY() by UMTXQ_LOCKED_ASSERT() instead? The assert does both lock and busy checks, so IMO the longer name is more correct.

This revision is now accepted and ready to land.Jul 21 2021, 10:10 PM
dchagin added a comment.Jul 22 2021, 6:52 AM
In D31238#704112, @kib wrote:
In D31238#704101, @dchagin wrote:

UMTXQ_LOCKED_ASSERT?

Are you suggesting to call UMTXQ_ASSERT_LOCKED_BUSY() by UMTXQ_LOCKED_ASSERT() instead? The assert does both lock and busy checks, so IMO the longer name is more correct.

no, I prefer to follow to the same style in code, naming, etc, that's why i named the macro like UMTXQ_LOCKED_ASSERT_BUSY. Thank you for your patience ))

Closed by commit rG9e32efa79bf2: umtx: Split do_unlock_pi on two counterparts. (authored by dchagin). · Explain WhyJul 29 2021, 9:59 AM
This revision was automatically updated to reflect the committed changes.
dchagin added a commit: rG9e32efa79bf2: umtx: Split do_unlock_pi on two counterparts..
dchagin added a commit: rGa09a84f84f34: umtx: Split do_unlock_pi on two counterparts..Jun 17 2022, 7:38 PM

Revision Contents
Changeset List

PathSize
sys/
kern/
132 lines
sys/
1 line

Diff 92521

View Options

sys/kern/kern_umtx.c

Loading...
View Options

sys/sys/umtxvar.h

Loading...