Page MenuHomeFreeBSD

Record part of the owner struct thread pointer into busy_lock.
ClosedPublic

Authored by kib on Nov 9 2019, 8:05 PM.

Details

Summary

Record as much bits from curthread into busy_lock. Low bits for struct thread * representation are zero due to struct and zone alignment, and they leave space for busy flags (perhaps except statically allocated thread0). Upper bits are not very interesting for assert, and in most practical situations recorded value should allow to identify the owner with certainty.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

I used 'tid' for debugging in the past. You can get the whole tid in. We can do somewhat more meaningful asserts with that but we'd have to deal with I/O ala BUF_KERNPROC

I don't know what the significance of the R postfix on "EXCLUSIVER" means. If you are going to change these macros I would suggest we rename it to EXCLUSIVE.

Is there a bug you are chasing?

In D22298#487577, @jeff wrote:

I used 'tid' for debugging in the past. You can get the whole tid in. We can do somewhat more meaningful asserts with that but we'd have to deal with I/O ala BUF_KERNPROC

I considered using tid, and I did not like it. The bit pattern for curthread allows to immediately recognize the value, while tid would need to be extracted. Also, tid could be reused, while struct thread is type-stable.

I don't know what the significance of the R postfix on "EXCLUSIVER" means. If you are going to change these macros I would suggest we rename it to EXCLUSIVE.

Ok.

Is there a bug you are chasing?

I principle yes, but I am not sure that this would directly points the finger. Still it should be useful.

sys/vm/vm_page.h
692 ↗(On Diff #64125)

This assert may not work in all cases as written due to busy being released in a different thread than the one that acquired it.

sys/vm/vm_page.h
692 ↗(On Diff #64125)

I do not remember such places.

I think I will introduce weaker asserts in addition to this stronger kind.

sys/vm/vm_page.h
692 ↗(On Diff #64125)

I only know of specific examples with sbusy. I'm not 100% sure it does happen with xbusy. If it passes stress tests it's probably ok.

I'd find the name VPB_CURTHREAD_EXCLUSIVE more intuitive.

This revision is now accepted and ready to land.Nov 11 2019, 2:05 PM

Weaken asserts when pages are unbusy in context of io completion handlers.

This variant passed stress2 testing.

This revision now requires review to proceed.Nov 17 2019, 6:02 PM
This revision was not accepted when it landed; it landed in state Needs Review.Nov 24 2019, 7:12 PM
This revision was automatically updated to reflect the committed changes.