Page MenuHomeFreeBSD
Differential D25914

Unlocked getblk: Fix new false-positive assertion
ClosedPublic
Actions

Authored by cem on Aug 1 2020, 5:52 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Dec 12, 5:52 PM
Unknown Object (File)
Nov 17 2024, 6:46 PM
Unknown Object (File)
Nov 13 2024, 10:30 AM
Unknown Object (File)
Oct 3 2024, 1:51 AM
Unknown Object (File)
Oct 3 2024, 1:50 AM
Unknown Object (File)
Oct 3 2024, 1:49 AM
Unknown Object (File)
Sep 28 2024, 2:55 PM
Unknown Object (File)
Sep 23 2024, 10:00 PM
View All 42 Files
Subscribers
imp
pho

Details

Reviewers
gallatin
markj
kib
Commits
rS363770: Unlocked getblk: Fix new false-positive assertion
Summary

A free buf's lock may be held (temporarily) due to unlocked lookup, so
buf_alloc() must acquire it without LK_NOWAIT. The unlocked getblk path
should unlock it promptly once it realizes the identity does not match
the buffer it was searching for.

Report by: gallatin

Test Plan

Report from Drew:

getnewbuf_empty: Locked buf 0xfffffe00cf5b52e0 on free queue

I audited the tree for other BUF_LOCK / BUF_TIMELOCK invocations with LK_NOWAIT or LK_SLEEPFAIL; no others seemed problematic.

Diff Detail

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

Event Timeline

cem requested review of this revision.Aug 1 2020, 5:52 PM
cem created this revision.
Harbormaster completed remote builds in B32696: Diff 75254.Aug 1 2020, 5:52 PM
cem edited the test plan for this revision. (Show Details)Aug 1 2020, 6:08 PM
kib accepted this revision.Aug 1 2020, 8:23 PM
This revision is now accepted and ready to land.Aug 1 2020, 8:23 PM
pho added a comment.Aug 1 2020, 8:28 PM

I am running tests with the scenario that triggered the panic multiple times.

pho added a comment.Aug 2 2020, 5:51 AM
In D25914#574158, @pho wrote:

I am running tests with the scenario that triggered the panic multiple times.

No problems seen with this test for a 10 hour run.

Closed by commit rS363770: Unlocked getblk: Fix new false-positive assertion (authored by cem). · Explain WhyAug 2 2020, 4:34 PM
This revision was automatically updated to reflect the committed changes.
cem added a commit: rS363770: Unlocked getblk: Fix new false-positive assertion.
Herald added a subscriber: imp. · View Herald TranscriptAug 2 2020, 4:34 PM
cem added a comment.Aug 2 2020, 4:43 PM

Should we also null out bufobj at buf_free time to ensure the identity check doesn’t match freed bufs, or is that guaranteed somewhere else? It looks like instead, bufobj is nulled on alloc today.

Revision Contents
Changeset List

PathSize
head/
sys/
kern/
8 lines

Diff 75273

View Options

head/sys/kern/vfs_bio.c

Loading...