Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/dev/scsipi scsi(4): Take kernel lock around entry into a...



details:   https://anonhg.NetBSD.org/src/rev/6e8ae674676f
branches:  trunk
changeset: 1029113:6e8ae674676f
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Tue Dec 21 22:53:21 2021 +0000

description:
scsi(4): Take kernel lock around entry into autoconf.

This code paths is entered by kthreads marked MP-safe, not just from
autoconf.

I'm not sure this is sufficient -- it's not clear to me whether
anything prevents concurrently scanning the same target.  Someone with
a better understanding of scsi(4) locking will have to audit this.

(For example, maybe it is guaranteed only to happen only either (a)
in autoconf, or (b) in a thread that doesn't start until autoconf is
done.  But I don't know -- and if it is this, it should be asserted
so we can verify it.)

diffstat:

 sys/dev/scsipi/scsiconf.c |  7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diffs (38 lines):

diff -r 73d2491b5b28 -r 6e8ae674676f sys/dev/scsipi/scsiconf.c
--- a/sys/dev/scsipi/scsiconf.c Tue Dec 21 22:21:11 2021 +0000
+++ b/sys/dev/scsipi/scsiconf.c Tue Dec 21 22:53:21 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: scsiconf.c,v 1.292 2021/08/07 16:19:16 thorpej Exp $   */
+/*     $NetBSD: scsiconf.c,v 1.293 2021/12/21 22:53:21 riastradh Exp $ */
 
 /*-
  * Copyright (c) 1998, 1999, 2004 The NetBSD Foundation, Inc.
@@ -48,7 +48,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: scsiconf.c,v 1.292 2021/08/07 16:19:16 thorpej Exp $");
+__KERNEL_RCSID(0, "$NetBSD: scsiconf.c,v 1.293 2021/12/21 22:53:21 riastradh Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -1012,6 +1012,7 @@
        locs[SCSIBUSCF_TARGET] = target;
        locs[SCSIBUSCF_LUN] = lun;
 
+       KERNEL_LOCK(1, NULL);
        if ((cf = config_search(sc->sc_dev, &sa,
                                CFARGS(.submatch = config_stdsubmatch,
                                       .locators = locs))) != NULL) {
@@ -1034,9 +1035,11 @@
                 */
                config_attach(sc->sc_dev, cf, &sa, scsibusprint,
                    CFARGS(.locators = locs));
+               KERNEL_UNLOCK_ONE(NULL);
        } else {
                scsibusprint(&sa, device_xname(sc->sc_dev));
                aprint_normal(" not configured\n");
+               KERNEL_UNLOCK_ONE(NULL);
                goto bad;
        }
 



Home | Main Index | Thread Index | Old Index