On 8/11/21 2:38 PM, Luiz Otavio O Souza wrote:
On Wed, Aug 11, 2021 at 3:49 PM John Baldwin <j...@freebsd.org> wrote:
On 8/10/21 3:41 PM, Scott Long wrote:
The branch main has been updated by scottl:
URL:
https://cgit.FreeBSD.org/src/commit/?id=35547df5c78653b2da030f920323c0357056099f
commit 35547df5c78653b2da030f920323c0357056099f
Author: Scott Long <sco...@freebsd.org>
AuthorDate: 2021-08-10 22:36:38 +0000
Commit: Scott Long <sco...@freebsd.org>
CommitDate: 2021-08-10 22:36:38 +0000
Call wakeup() with the lock held to avoid missed wakeup races.
Submitted by: luiz
Sponsored by: Rubicon Communications, LLC ("Netgate")
---
sys/dev/sdhci/sdhci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sys/dev/sdhci/sdhci.c b/sys/dev/sdhci/sdhci.c
index d075c2e05000..573e6949b57e 100644
--- a/sys/dev/sdhci/sdhci.c
+++ b/sys/dev/sdhci/sdhci.c
@@ -2078,8 +2078,8 @@ sdhci_generic_release_host(device_t brdev __unused,
device_t reqdev)
/* Deactivate led. */
WR1(slot, SDHCI_HOST_CONTROL, slot->hostctrl &= ~SDHCI_CTRL_LED);
slot->bus_busy--;
- SDHCI_UNLOCK(slot);
wakeup(slot);
+ SDHCI_UNLOCK(slot);
return (0);
}
Hmm, how does this avoid a race? The sleep is checking bus_busy under
the lock and should never see a stale value and go back to sleep after
the wakeup has occurred:
SDHCI_LOCK(slot);
while (slot->bus_busy)
msleep(slot, &slot->mtx, 0, "sdhciah", 0);
slot->bus_busy++;
/* Activate led. */
WR1(slot, SDHCI_HOST_CONTROL, slot->hostctrl |= SDHCI_CTRL_LED);
SDHCI_UNLOCK(slot);
Dropping the lock before wakeup() is a tiny optimization that avoids
having the second thread wakeup and immediately block on the lock before
it has been released by the first thread.
'race' is probably wrong here. this change will prevent a second
thread from taking the bus before you call wakeup() - poking all other
threads unnecessarily.
This change does not prevent that. The other thread and the thread that
are awakened will race with each other to acquire the lock. wakeup()
doesn't do any sort of explicit lock handoff to the thread being awakened
and it's just as likely for a thread not yet asleep to acquire the lock as
for the thread being awakened to acquire the lock. If you have observed
thundering herd problems with this wakeup() then you might want to change
it to wakeup_one().
--
John Baldwin
_______________________________________________
dev-commits-src-main@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/dev-commits-src-main
To unsubscribe, send any mail to "dev-commits-src-main-unsubscr...@freebsd.org"
