From nobody Fri Apr 15 16:33:31 2022 X-Original-To: dev-commits-src-branches@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 0B9297ED5F2; Fri, 15 Apr 2022 16:33:32 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Kg22M6XLBz3M4h; Fri, 15 Apr 2022 16:33:31 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1650040411; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=qlXiPpPYcvuC4JS6q4+bfW8ZzbhtMDZHbTBqVRh6cRA=; b=Kex/w1SiprGW3DoNR2onBZGNbDlbag2NhVGc5jKaWIPjFwlJIucRakoEPbzZcGr48ctjBT vaWcIaJuY95hIqMrvfDyTfQs2gAc4PqImBKsW26y1iuYfCfbbHFYrhX6RL6JALSrpmxos0 +OJKz7pketGsYs6Mb3kfEiFHfORzuuUanIdipAM0ojNo0qqHYTPHfzNh812ymJQgwRVo/M +wk5QPpMtZwXid/kyXkO83IOK+J2X69H0s7MFWoSE/KbsWYJG6P6YbR+Xh6dVbYepaSqXy TTd+3GhetPBicfeXUAablSLvcg9FEDYfUshQAVcop5Uk9egLAQf4SOQm19pdeg== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id C07F518AA5; Fri, 15 Apr 2022 16:33:31 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 23FGXVYD038629; Fri, 15 Apr 2022 16:33:31 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 23FGXVkx038628; Fri, 15 Apr 2022 16:33:31 GMT (envelope-from git) Date: Fri, 15 Apr 2022 16:33:31 GMT Message-Id: <202204151633.23FGXVkx038628@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Ed Maste Subject: git: 3ec2816ad7a1 - stable/13 - kbd: replace vestigial spl calls with Giant assertions List-Id: Commits to the stable branches of the FreeBSD src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-branches List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-branches@freebsd.org X-BeenThere: dev-commits-src-branches@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: emaste X-Git-Repository: src X-Git-Refname: refs/heads/stable/13 X-Git-Reftype: branch X-Git-Commit: 3ec2816ad7a19dc2ae21ccd2d2d2c236027e4572 Auto-Submitted: auto-generated ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1650040411; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=qlXiPpPYcvuC4JS6q4+bfW8ZzbhtMDZHbTBqVRh6cRA=; b=NRrafQ8o68273M/D9ac4c0xhJwUMiLyyNGpd8YuGpceDCpftmB4/DlWeU2aP8t1SOvGBIN D7q9S5eHQIdwfziHE0kz3Pcs4sBBatxcslP2wBAft7O0xp4wVSHUNdldCA4x/SKjDrPu/e 2ZsPdahjgVkNeHhCLh50z7wyEsKsrba5J4G8BjaYZa+Os95PzOAvt+MDx8kvJ7E9T2S6su X6BF2UuByApgaChI1wVEmtCwykKmfGh0aAl/P+gYrMfoYTlWfsoNjgQIzRhXL4DFXYGZDX 2IozSWyHwrZfokJ6uacHaH0RELj0Lpk5PUitMrcNSiMMBW3VrnosAlDpsObTDA== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1650040411; a=rsa-sha256; cv=none; b=yse9xXO4JnKW3Mrtj5t+O9ba3Vj/azUiccVDWwTq79H+g2/0rC8RYCf7e6jxBfAYbp6+5q mhFyouL15tpfW9r8RNl5zdPX9ldk8ivVGYj4Ubu2cVHONUjpq5B2s0yTvXpgntKwlAnaUi 3wtO5D8ruoj0Db/Wr9fk1YIt2+j3YJeMVeC8snaPtflPsXMuxo4S5zK/oJb4Yr9uVvg77B WA+2zDQ3OEJf6/pcf0uOv987DeteZw9MF34CNf0ihyLh9li/l6KLeD7aYFCXaXTyd1Jwyw cCJTlCg3JiNZ9IXcrpzzvbLl9JpQxZ1wqQnlIieWbq7jReAEdY0adwKlEpo8JA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-ThisMailContainsUnwantedMimeParts: N The branch stable/13 has been updated by emaste: URL: https://cgit.FreeBSD.org/src/commit/?id=3ec2816ad7a19dc2ae21ccd2d2d2c236027e4572 commit 3ec2816ad7a19dc2ae21ccd2d2d2c236027e4572 Author: Ed Maste AuthorDate: 2022-03-22 17:48:43 +0000 Commit: Ed Maste CommitDate: 2022-04-15 16:29:02 +0000 kbd: replace vestigial spl calls with Giant assertions The keyboard driver was initially protected via spl* interrupt priority calls but (as part of a comprehensive effort) migrated to use the Giant lock (mutex). The spl calls left behind became NOPs but they can be confusing as they have no bearing on the actual mutual exclusion that is now present. Remove them from kbd and add assertions that Giant is held. markj notes that there is conflation between the "bus topo" lock (which is Giant under the hood) and Giant. The assertions could either be addressed as a small item along with bus topology locking work or they'll be removed if kbd is decoupled from Giant. PR: 206680 Reviewed by: markj MFC after: 3 weeks Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D34645 (cherry picked from commit a0cd78bf2c148d7ab63454471771e3f4b572522f) --- sys/dev/kbd/kbd.c | 67 ++++++++++--------------------------------------------- 1 file changed, 12 insertions(+), 55 deletions(-) diff --git a/sys/dev/kbd/kbd.c b/sys/dev/kbd/kbd.c index 70c0ef15a56e..deb7672de7fa 100644 --- a/sys/dev/kbd/kbd.c +++ b/sys/dev/kbd/kbd.c @@ -35,7 +35,9 @@ __FBSDID("$FreeBSD$"); #include #include #include +#include #include +#include #include #include #include @@ -97,13 +99,11 @@ kbd_realloc_array(void) { keyboard_t **new_kbd; int newsize; - int s; - s = spltty(); + GIANT_REQUIRED; newsize = rounddown(keyboards + ARRAY_DELTA, ARRAY_DELTA); new_kbd = malloc(sizeof(*new_kbd)*newsize, M_DEVBUF, M_NOWAIT|M_ZERO); if (new_kbd == NULL) { - splx(s); return (ENOMEM); } bcopy(keyboard, new_kbd, sizeof(*keyboard)*keyboards); @@ -111,7 +111,6 @@ kbd_realloc_array(void) free(keyboard, M_DEVBUF); keyboard = new_kbd; keyboards = newsize; - splx(s); if (bootverbose) printf("kbd: new array size %d\n", keyboards); @@ -245,30 +244,26 @@ int kbd_unregister(keyboard_t *kbd) { int error; - int s; + GIANT_REQUIRED; if ((kbd->kb_index < 0) || (kbd->kb_index >= keyboards)) return (ENOENT); if (keyboard[kbd->kb_index] != kbd) return (ENOENT); - s = spltty(); if (KBD_IS_BUSY(kbd)) { error = (*kbd->kb_callback.kc_func)(kbd, KBDIO_UNLOADING, kbd->kb_callback.kc_arg); if (error) { - splx(s); return (error); } if (KBD_IS_BUSY(kbd)) { - splx(s); return (EBUSY); } } KBD_INVALID(kbd); keyboard[kbd->kb_index] = NULL; - splx(s); return (0); } @@ -333,16 +328,14 @@ kbd_allocate(char *driver, int unit, void *id, kbd_callback_func_t *func, void *arg) { int index; - int s; + GIANT_REQUIRED; if (func == NULL) return (-1); - s = spltty(); index = kbd_find_keyboard(driver, unit); if (index >= 0) { if (KBD_IS_BUSY(keyboard[index])) { - splx(s); return (-1); } keyboard[index]->kb_token = id; @@ -351,7 +344,6 @@ kbd_allocate(char *driver, int unit, void *id, kbd_callback_func_t *func, keyboard[index]->kb_callback.kc_arg = arg; kbdd_clear_state(keyboard[index]); } - splx(s); return (index); } @@ -359,9 +351,8 @@ int kbd_release(keyboard_t *kbd, void *id) { int error; - int s; - s = spltty(); + GIANT_REQUIRED; if (!KBD_IS_VALID(kbd) || !KBD_IS_BUSY(kbd)) { error = EINVAL; } else if (kbd->kb_token != id) { @@ -374,7 +365,6 @@ kbd_release(keyboard_t *kbd, void *id) kbdd_clear_state(kbd); error = 0; } - splx(s); return (error); } @@ -383,9 +373,8 @@ kbd_change_callback(keyboard_t *kbd, void *id, kbd_callback_func_t *func, void *arg) { int error; - int s; - s = spltty(); + GIANT_REQUIRED; if (!KBD_IS_VALID(kbd) || !KBD_IS_BUSY(kbd)) { error = EINVAL; } else if (kbd->kb_token != id) { @@ -397,7 +386,6 @@ kbd_change_callback(keyboard_t *kbd, void *id, kbd_callback_func_t *func, kbd->kb_callback.kc_arg = arg; error = 0; } - splx(s); return (error); } @@ -542,20 +530,17 @@ genkbdopen(struct cdev *dev, int mode, int flag, struct thread *td) { keyboard_t *kbd; genkbd_softc_t *sc; - int s; int i; - s = spltty(); + GIANT_REQUIRED; sc = dev->si_drv1; kbd = kbd_get_keyboard(KBD_INDEX(dev)); if ((sc == NULL) || (kbd == NULL) || !KBD_IS_VALID(kbd)) { - splx(s); return (ENXIO); } i = kbd_allocate(kbd->kb_name, kbd->kb_unit, sc, genkbd_event, (void *)sc); if (i < 0) { - splx(s); return (EBUSY); } /* assert(i == kbd->kb_index) */ @@ -567,7 +552,6 @@ genkbdopen(struct cdev *dev, int mode, int flag, struct thread *td) */ sc->gkb_q_length = 0; - splx(s); return (0); } @@ -577,13 +561,12 @@ genkbdclose(struct cdev *dev, int mode, int flag, struct thread *td) { keyboard_t *kbd; genkbd_softc_t *sc; - int s; + GIANT_REQUIRED; /* * NOTE: the device may have already become invalid. * kbd == NULL || !KBD_IS_VALID(kbd) */ - s = spltty(); sc = dev->si_drv1; kbd = kbd_get_keyboard(KBD_INDEX(dev)); if ((sc == NULL) || (kbd == NULL) || !KBD_IS_VALID(kbd)) { @@ -591,7 +574,6 @@ genkbdclose(struct cdev *dev, int mode, int flag, struct thread *td) } else { kbd_release(kbd, (void *)sc); } - splx(s); return (0); } @@ -603,35 +585,29 @@ genkbdread(struct cdev *dev, struct uio *uio, int flag) u_char buffer[KB_BUFSIZE]; int len; int error; - int s; + GIANT_REQUIRED; /* wait for input */ - s = spltty(); sc = dev->si_drv1; kbd = kbd_get_keyboard(KBD_INDEX(dev)); if ((sc == NULL) || (kbd == NULL) || !KBD_IS_VALID(kbd)) { - splx(s); return (ENXIO); } while (sc->gkb_q_length == 0) { if (flag & O_NONBLOCK) { - splx(s); return (EWOULDBLOCK); } sc->gkb_flags |= KB_ASLEEP; error = tsleep(sc, PZERO | PCATCH, "kbdrea", 0); kbd = kbd_get_keyboard(KBD_INDEX(dev)); if ((kbd == NULL) || !KBD_IS_VALID(kbd)) { - splx(s); return (ENXIO); /* our keyboard has gone... */ } if (error) { sc->gkb_flags &= ~KB_ASLEEP; - splx(s); return (error); } } - splx(s); /* copy as much input as possible */ error = 0; @@ -680,10 +656,9 @@ genkbdpoll(struct cdev *dev, int events, struct thread *td) keyboard_t *kbd; genkbd_softc_t *sc; int revents; - int s; + GIANT_REQUIRED; revents = 0; - s = spltty(); sc = dev->si_drv1; kbd = kbd_get_keyboard(KBD_INDEX(dev)); if ((sc == NULL) || (kbd == NULL) || !KBD_IS_VALID(kbd)) { @@ -694,7 +669,6 @@ genkbdpoll(struct cdev *dev, int events, struct thread *td) else selrecord(td, &sc->gkb_rsel); } - splx(s); return (revents); } @@ -818,11 +792,10 @@ genkbd_commonioctl(keyboard_t *kbd, u_long cmd, caddr_t arg) okeymap_t *omapp; keyarg_t *keyp; fkeyarg_t *fkeyp; - int s; int i, j; int error; - s = spltty(); + GIANT_REQUIRED; switch (cmd) { case KDGKBINFO: /* get keyboard information */ @@ -848,7 +821,6 @@ genkbd_commonioctl(keyboard_t *kbd, u_long cmd, caddr_t arg) case GIO_KEYMAP: /* get keyboard translation table */ error = copyout(kbd->kb_keymap, *(void **)arg, sizeof(keymap_t)); - splx(s); return (error); case OGIO_KEYMAP: /* get keyboard translation table (compat) */ mapp = kbd->kb_keymap; @@ -879,7 +851,6 @@ genkbd_commonioctl(keyboard_t *kbd, u_long cmd, caddr_t arg) } else { error = copyin(*(void **)arg, mapp, sizeof *mapp); if (error != 0) { - splx(s); free(mapp, M_TEMP); return (error); } @@ -887,7 +858,6 @@ genkbd_commonioctl(keyboard_t *kbd, u_long cmd, caddr_t arg) error = keymap_change_ok(kbd->kb_keymap, mapp, curthread); if (error != 0) { - splx(s); free(mapp, M_TEMP); return (error); } @@ -896,7 +866,6 @@ genkbd_commonioctl(keyboard_t *kbd, u_long cmd, caddr_t arg) free(mapp, M_TEMP); break; #else - splx(s); return (ENODEV); #endif @@ -904,7 +873,6 @@ genkbd_commonioctl(keyboard_t *kbd, u_long cmd, caddr_t arg) keyp = (keyarg_t *)arg; if (keyp->keynum >= sizeof(kbd->kb_keymap->key) / sizeof(kbd->kb_keymap->key[0])) { - splx(s); return (EINVAL); } bcopy(&kbd->kb_keymap->key[keyp->keynum], &keyp->key, @@ -915,20 +883,17 @@ genkbd_commonioctl(keyboard_t *kbd, u_long cmd, caddr_t arg) keyp = (keyarg_t *)arg; if (keyp->keynum >= sizeof(kbd->kb_keymap->key) / sizeof(kbd->kb_keymap->key[0])) { - splx(s); return (EINVAL); } error = key_change_ok(&kbd->kb_keymap->key[keyp->keynum], &keyp->key, curthread); if (error != 0) { - splx(s); return (error); } bcopy(&keyp->key, &kbd->kb_keymap->key[keyp->keynum], sizeof(keyp->key)); break; #else - splx(s); return (ENODEV); #endif @@ -940,20 +905,17 @@ genkbd_commonioctl(keyboard_t *kbd, u_long cmd, caddr_t arg) error = accent_change_ok(kbd->kb_accentmap, (accentmap_t *)arg, curthread); if (error != 0) { - splx(s); return (error); } bcopy(arg, kbd->kb_accentmap, sizeof(*kbd->kb_accentmap)); break; #else - splx(s); return (ENODEV); #endif case GETFKEY: /* get functionkey string */ fkeyp = (fkeyarg_t *)arg; if (fkeyp->keynum >= kbd->kb_fkeytab_size) { - splx(s); return (EINVAL); } bcopy(kbd->kb_fkeytab[fkeyp->keynum].str, fkeyp->keydef, @@ -964,13 +926,11 @@ genkbd_commonioctl(keyboard_t *kbd, u_long cmd, caddr_t arg) #ifndef KBD_DISABLE_KEYMAP_LOAD fkeyp = (fkeyarg_t *)arg; if (fkeyp->keynum >= kbd->kb_fkeytab_size) { - splx(s); return (EINVAL); } error = fkey_change_ok(&kbd->kb_fkeytab[fkeyp->keynum], fkeyp, curthread); if (error != 0) { - splx(s); return (error); } kbd->kb_fkeytab[fkeyp->keynum].len = min(fkeyp->flen, MAXFK); @@ -978,16 +938,13 @@ genkbd_commonioctl(keyboard_t *kbd, u_long cmd, caddr_t arg) kbd->kb_fkeytab[fkeyp->keynum].len); break; #else - splx(s); return (ENODEV); #endif default: - splx(s); return (ENOIOCTL); } - splx(s); return (0); }