Page MenuHomeFreeBSD

Move td_softdep_cleanup() from userret() to ast()
ClosedPublic

Authored by trasz on Sep 9 2020, 3:24 PM.
Tags
None
Referenced Files
F132351130: D26375.id76999.diff
Thu, Oct 16, 3:52 AM
F132351102: D26375.id76814.diff
Thu, Oct 16, 3:51 AM
F132351081: D26375.id.diff
Thu, Oct 16, 3:51 AM
Unknown Object (File)
Wed, Oct 15, 4:50 PM
Unknown Object (File)
Mon, Sep 22, 1:55 AM
Unknown Object (File)
Aug 25 2025, 12:24 AM
Unknown Object (File)
Aug 7 2025, 5:04 PM
Unknown Object (File)
Jun 29 2025, 3:41 AM
Subscribers

Details

Summary

Move td_softdep_cleanup() from userret() to ast(); it's infrequent
at best. The schedule_cleanup() function already sets TDF_ASTPENDING.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

trasz requested review of this revision.Sep 9 2020, 3:24 PM
sys/kern/subr_trap.c
198 ↗(On Diff #76814)

Leave the assert there.

sys/kern/subr_trap.c
198 ↗(On Diff #76814)

Wouldn't it trip at the first possibility due to ast() being called from the syscall enter assembly code some time after userret() returns?

sys/kern/subr_trap.c
198 ↗(On Diff #76814)

I do not understand what are you saying there.

The assert can be triggered if userret() is executed before ast is handled, or if ast() 'forgot' to handle requested su cleanup. I do not believe that the first case is possible.

Or rather, if userret() triggers that, there is a bug in your patch.

sys/kern/subr_trap.c
198 ↗(On Diff #76814)

Erm, are you sure about it? My understanding is that (eg) amd64_syscall() calls syscallret(), syscallret() calls userret(), and then amd64_syscall() returns; ast() gets only called afterwards.

kib added inline comments.
sys/kern/subr_trap.c
198 ↗(On Diff #76814)

Yes you are right. I would say that this should be fixed, but actually your series of the patches makes it not matter at all.

This revision is now accepted and ready to land.Sep 10 2020, 10:55 PM

This change looks reasonable to me as at least for amd64 every path out of the kernel appears to check TDF_ASTPENDING and calls ast() if set.