Don't use sleeping allocations for ufs dirhash blocks when holding directory vnode
ClosedPublic
Actions

Authored by dgmorris_earthlink.net on Mar 3 2021, 6:37 PM.

Details

Reviewers

vangyzen
markj
mckusick
kib

Commits

rGf17a5900850b: ufs: Avoid M_WAITOK allocations when building a dirhash

Summary

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=253992

System with a based-on-FreeBSD-12 kernel but with a timeout panic for the UFS root vnode lock paniced due to a ufsdirhash_build() call with M_WAITOK, UMA_ZONE_FIRSTTOUCH (the default) and a domain which was OOM. The ufsdirhash_build() code is invoked with the directory vnode exclusively locked (per ufs_lookup.c / the caller) and in all other places takes care to allocate M_NOWAIT to avoid such issues.

So it seems reasonable to make this one allocation also M_NOWAIT instead of M_WAITOK, especially given the FIRSTTOUCH default behavior where only the calling cpu's domain can satisfy the request.

Test Plan

Organizational stress testing, no unit test planned -- change is pretty straightforward.

Diff Detail

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

dgmorris_earthlink.net requested review of this revision.Mar 3 2021, 6:37 PM

dgmorris_earthlink.net created this revision.

vangyzen accepted this revision.Mar 3 2021, 7:50 PM

This revision is now accepted and ready to land.Mar 3 2021, 7:50 PM

markj accepted this revision.Mar 3 2021, 7:53 PM

markj added reviewers: mckusick, kib.

markj added inline comments.

sys/ufs/ufs/ufs_dirhash.c
111	I would just drop the NOWAIT suffix, or get rid of the UMA wrappers entirely.

Took Mark's suggestion on dropping _NOWAIT suffix on allocation macro.
Left the macro for symmetry with the FREE macros in the file.

This revision now requires review to proceed.Mar 3 2021, 8:09 PM

dgmorris_earthlink.net marked an inline comment as done.Mar 3 2021, 8:09 PM

This seems like a reasonable solution to the problem. The allocation will fail in a few cases where it previously would have succeeded, but hopefully those will be rare. The effect of failing will simply be slower lookups rather than unexpected errors to applications.

This revision is now accepted and ready to land.Mar 3 2021, 9:19 PM

Closed by commit rGf17a5900850b: ufs: Avoid M_WAITOK allocations when building a dirhash (authored by dgmorris_earthlink.net, committed by markj). · Explain WhyMay 20 2021, 3:28 PM

This revision was automatically updated to reflect the committed changes.

markj added a commit: rGf17a5900850b: ufs: Avoid M_WAITOK allocations when building a dirhash.