cam: Drop periph lock when completing I/O with ENOMEM status
ClosedPublic
Actions

Authored by imp on Wed, May 22, 11:08 PM.

Details

Reviewers

ken
mav
gallatin

Group Reviewers

cam

Commits

rG99c14fb99ffc: cam: Drop periph lock when completing I/O with ENOMEM status

Summary

When biofinish calls g_io_deliver with an error of ENOMEM, that kicks
off the slowdown protocol, forcing I/O to go through g_down rather than
be directly dispatch. One of the side effects is that the I/O is
resubmitted, so the start routines get called recursively, leading to a
recursive lock panic. Rather than make the periph lock recursive, drop
and reacquire the lock around such calls to biofinish.

For nda, this happens only when we can't allocate space to construct a
TRIM. For ada and da, this is only for certain ZONE operations.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

imp created this revision.Wed, May 22, 11:08 PM

Herald added a reviewer: cam. · View Herald TranscriptWed, May 22, 11:08 PM

Herald added a subscriber: dab. · View Herald Transcript

imp requested review of this revision.Wed, May 22, 11:08 PM

Harbormaster completed remote builds in B57857: Diff 138951.Wed, May 22, 11:08 PM

imp added a parent revision: D45309: goem: Add counts for enomem and pausing.Wed, May 22, 11:08 PM

imp added a child revision: D45311: nvme: Count number of alginment splits.

imp added reviewers: ken, mav.Wed, May 22, 11:09 PM

gallatin added a subscriber: gallatin.Thu, May 23, 1:40 AM

gallatin added inline comments.

sys/cam/nvme/nvme_da.c
1087	I don't know the code well, but dropping a lock can introduce races.. I've spent a too long chasing my tail on that in the network stack where I suspect a race around dropping lock in a bug i'm working on. I assumed you considered making the periph lock a recursive lock (eg, MTX_RECURSE) ?

imp added inline comments.Thu, May 23, 2:07 AM

sys/cam/nvme/nvme_da.c
1087	We already drop the lock around the call to the SIM action routine, which is the normal exit path. And the periph lock is used all over CAM... so I'm hesitant to contemplate that...