]> CyberLeo.Net >> Repos - FreeBSD/FreeBSD.git/commit
Fix a hang introduced in r351599.
authorken <ken@FreeBSD.org>
Mon, 2 Dec 2019 19:57:39 +0000 (19:57 +0000)
committerken <ken@FreeBSD.org>
Mon, 2 Dec 2019 19:57:39 +0000 (19:57 +0000)
commit3d73341d003e961e5ed6c5a7e70a1a0cfd0d3628
treee8f32aa6aa7c34155b41abca3d01fbed1e340beb
parent16d1bb252412a79b82e8403d76e984adf5c3fea6
Fix a hang introduced in r351599.

My changes in 351599 (kindly committed by avg) made the cd(4) media check
asynchronous to avoid a sleep while holding a mutex.

There was a difficult to reproduce bug with those changes that caused a
hang on boot on some single processor machines/VMs.  Leandro Lupori
managed to reproduce the bug, diagnose it, and supplied a patch!  Here is
his analysis, from the PR:

======
I was able to reproduce the problem described in comment#14.

Actually, I wasn't trying to reproduce it, I just started seeing it a few
weeks ago, in CURRENT.

I can reproduce it consistently, by using QEMU to run a PowerPC64 VM with a
single core/thread (-smp 1).

It happens only when there is no media in the emulated CD-ROM, a device
that QEMU adds by default, unless -nodefaults is specified in command line.

I've debugged it and this is what I've found:

1- After the CD probe is successful, GEOM will try to open the device,
which will end up calling cdcheckmedia(), that sets CD state to
CD_STATE_MEDIA_PREVENT.
2- Next, scsi_prevent() is executed and succeeds, the CD_FLAG_DISC_LOCKED
flag is set and CD state moves to CD_STATE_MEDIA_SIZE.
3- Next, scsi_read_capacity() is executed and fails, state is set to
CD_STATE_MEDIA_ALLOW, cdmediaprobedone() is called and wakes up
cdcheckmedia().
4- Then, when cdstart() is invoked to process CD_STATE_MEDIA_ALLOW, it
first checks if CD_FLAG_DISC_LOCKED is set, and if so skips directly to
CD_STATE_MEDIA_SIZE state. This will repeat the steps of bullet 3, entering
an infinite MEDIA_SIZE command loop.

When there is a least another core/thread, the GEOM thread that performed
the initial cdopen() will get scheduled again, closing the CD device, that
will call cdprevent(PR_ALLOW) that clears the CD_FLAG_DISC_LOCKED flag and
breaks the loop.

So, apparently, the problem is CD_STATE_MEDIA_ALLOW being skipped when
CD_FLAG_DISC_LOCKED is set. If I understand correctly, in this case, the
state should be advanced to CD_STATE_MEDIA size only when the current state
is CD_STATE_MEDIA_PREVENT.
=====

PR: kern/219857
Submitted by: Leandro Lupori <leandro.lupori@gmail.com>
MFC after: 1 week
sys/cam/scsi/scsi_cd.c