Page MenuHomeFreeBSD

getblk: Avoid sleeping on wrong buf in lockless path
ClosedPublic

Authored by cem on Jul 30 2020, 9:57 PM.
Tags
None
Referenced Files
F132394756: D25898.id75189.diff
Thu, Oct 16, 1:26 PM
F132394750: D25898.id75191.diff
Thu, Oct 16, 1:26 PM
F132394749: D25898.id75188.diff
Thu, Oct 16, 1:26 PM
F132394744: D25898.id.diff
Thu, Oct 16, 1:26 PM
F132341110: D25898.diff
Thu, Oct 16, 1:43 AM
Unknown Object (File)
Mon, Oct 13, 8:32 AM
Unknown Object (File)
Sun, Oct 5, 3:48 AM
Unknown Object (File)
Sun, Sep 21, 3:07 PM
Subscribers

Details

Summary

If the buffer identity changed during lookup, sleeping could introduce a
lock order reversal. Since we do not know if the identity changed until we
get the lock, we must try-lock (LK_NOWAIT) only. EINTR and ERESTART error
handling becomes irrelevant, as we no longer sleep.

Reported by: kib
X-MFC-With: r363482

Diff Detail

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

Event Timeline

cem requested review of this revision.Jul 30 2020, 9:57 PM
cem created this revision.
sys/kern/vfs_bio.c
3868 ↗(On Diff #75188)

SLEEPFAIL does not make sense together with NOWAIT.

sys/kern/vfs_bio.c
3888–3891 ↗(On Diff #75188)

Should this logic be changed to avoid LK_SLEEPFAIL | LK_NOWAIT?

lockflags = LK_EXCLUSIVE | LK_INTERLOCK;

if ((flags & GB_LOCK_NOWAIT) != 0)
    lockflags |= LK_NOWAIT;
else
    lockflags |= LK_SLEEPFAIL;

Drop LK_SLEEPFAIL for LK_NOWAIT lockless lookup path. No functional change.

kib added inline comments.
sys/kern/vfs_bio.c
3868 ↗(On Diff #75188)

Drop lockflags use there, pass LK_EXCLUSIVE | LK_NOWAIT directly to BUF_TIMELOCK().

3888–3891 ↗(On Diff #75188)

I think it would clarify the code.

This revision is now accepted and ready to land.Jul 30 2020, 11:46 PM
cem marked an inline comment as done.Jul 31 2020, 12:03 AM
cem added inline comments.
sys/kern/vfs_bio.c
3868 ↗(On Diff #75188)

will do.

3888–3891 ↗(On Diff #75188)

will do as a follow-up

This revision was automatically updated to reflect the committed changes.