Messages in this thread | | | Date | Fri, 20 Jan 2023 10:43:10 +0100 | Subject | Re: Internal vs. external barriers (was: Re: Interesting LKMM litmus test) | From | Jonas Oberhauser <> |
| |
On 1/19/2023 10:53 PM, Paul E. McKenney wrote: > On Thu, Jan 19, 2023 at 02:51:53PM -0500, Alan Stern wrote: >> On Thu, Jan 19, 2023 at 10:41:07AM -0800, Paul E. McKenney wrote: >>> In contrast, this actually needs srcu_down_read() and srcu_up_read(): >>> >>> ------------------------------------------------------------------------ >>> >>> C C-srcu-nest-6 >>> >>> (* >>> * Result: Never >>> * >>> * Flag unbalanced-srcu-locking >>> * This would be valid for srcu_down_read() and srcu_up_read(). >>> *) >>> >>> {} >>> >>> P0(int *x, int *y, struct srcu_struct *s1, int *idx) >>> { >>> int r2; >>> int r3; >>> >>> r3 = srcu_down_read(s1); >>> WRITE_ONCE(*idx, r3); >>> r2 = READ_ONCE(*y); >>> } >>> >>> P1(int *x, int *y, struct srcu_struct *s1, int *idx) >>> { >>> int r1; >>> int r3; >>> >>> r1 = READ_ONCE(*x); >>> r3 = READ_ONCE(*idx); >>> srcu_up_read(s1, r3); >>> } >>> >>> P2(int *x, int *y, struct srcu_struct *s1) >>> { >>> WRITE_ONCE(*y, 1); >>> synchronize_srcu(s1); >>> WRITE_ONCE(*x, 1); >>> } >>> >>> locations [0:r1] >>> exists (1:r1=1 /\ 0:r2=0) >> I modified this litmus test by adding a flag variable with an >> smp_store_release in P0, an smp_load_acquire in P1, and a filter clause >> to ensure that P1 reads the flag and idx from P1.
This sounds like good style. I suppose this is already flagged as mismatched srcu_unlock(), in case you accidentally read from the initial write?
>> It turns out that the idea of removing rf edges from Srcu-unlock events >> doesn't work well. The missing edges mess up herd's calculation of the >> fr relation and the coherence axiom. So I've gone back to filtering >> those edges out of carry-dep. >> >> Also, Boqun's suggestion for flagging ordinary accesses to SRCU >> structures no longer works, because the lock and unlock operations now >> _are_ normal accesses. I removed that check too, but it shouldn't hurt >> much because I don't expect to encounter litmus tests that try to read >> or write srcu_structs directly. > Agreed. I for one would definitely have something to say about an > SRCU-usage patch that directly manipulated a srcu_struct structure! ;-)
Wouldn't the point of having it being flagged be that herd (or similar tools) would be having something to say long before it has to reach your pair of eyes? I don't think Boqun's patch is hard to repair. Besides the issue you mention, I think it's also missing Sync-srcu, which seems to be linked by loc based on its first argument.
How about something like this?
let ALL-LOCKS = LKR | LKW | UL | LF | RU | Srcu-lock | Srcu-unlock | Sync-srcu flag ~empty ~[ALL_LOCKS | IW] ; loc ; [ALL-LOCKS] as mixed-lock-accesses
If you're using something that isn't a lock or intial write on the same location as a lock, you get the flag.
Best wishes, jonas
| |