From nobody Thu Mar 24 16:56:40 2022 X-Original-To: dev-commits-src-main@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 99BD61A2D3B6; Thu, 24 Mar 2022 16:56:40 +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 4KPWbD3tsGz4smJ; Thu, 24 Mar 2022 16:56:40 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1648141000; 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=YYJFS/jO4aDO57LfGi5NN7X0h10tkIeja540ET45fv8=; b=q7CxOqgCSRghfbN5pzrWUPE1Cm/NUIF2rpBNOqFxWI0BiAgpyygjonUvThbj33DJlDNGUZ b+rflWLrgKXfO37Wrct1zlrSgZrGrzp6K0ELgsKLhtvMmfxHRCGWCopyylQ6ATB+mnNEou JjX8nnA0vFDxzA80sUuOgQOLM8cF9EUQAD5b+mbdaKT8PUqFnLvMliuCFFEbHWB06k8USF 8kwUXbDTcPp7p0w+oJzhtJ0XmPJiDDC0bIawVNmHu8pBVruu58p4wtJbFpZ/iokXpf+4VE sE6148nSwmyaZ/FUN4xD6L0mEAoailAHmpDYhWuVR7O8i94ZAWLK6wZ9/hFzew== 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 658561B86B; Thu, 24 Mar 2022 16:56:40 +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 22OGue5A031389; Thu, 24 Mar 2022 16:56:40 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 22OGuegV031388; Thu, 24 Mar 2022 16:56:40 GMT (envelope-from git) Date: Thu, 24 Mar 2022 16:56:40 GMT Message-Id: <202203241656.22OGuegV031388@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Ed Maste Subject: git: a0cd78bf2c14 - main - kbd: replace vestigial spl calls with Giant assertions List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-main@freebsd.org X-BeenThere: dev-commits-src-main@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/main X-Git-Reftype: branch X-Git-Commit: a0cd78bf2c148d7ab63454471771e3f4b572522f Auto-Submitted: auto-generated ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1648141000; 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=YYJFS/jO4aDO57LfGi5NN7X0h10tkIeja540ET45fv8=; b=AgpZiDWMHpXwN6crcGtSInAvSms3KlzCDTy94Xp/+h09n5+yVaME5eXuYZE3fk98li0574 wcBQKGDhDgLM3Sf++kjbPfx90uglhk6v4hl5xHUnUMbtB/JkOFtUtFp4W948TOvSCB698k hr8sB1v9GvDmJVnQlCMQdIR0OJrbubMjS3aF19bTixFwEQAYtELA85HvyNya+n6XhyX3iT qr6KYpY4fhiYUM8mOEw2T7vix0UFl+KSvUxlSCuEWnihKsmgYO3CMDaW+Zbf7kPOS1556M PibJtipG0Ug946xbRca2wJBiZAY8EnR+W6TOxUbE5Xdm8SZ8C/LTGJMWC1Xchg== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1648141000; a=rsa-sha256; cv=none; b=fujqVJhzpzK3tV4iusKHEbSYRmw7u8W2ZZvdxSWNJkGGdjPj6LEgK1H9jNUtZJaL/F85sF um4l3qN+LoBfJC5+vLr5XqKTUsRR0H6JUabpDDTX9ZOt010LHUvCyFBwhq5dv+OeYYoDHy xsOrEPjzV9qBElR75ysFAhICfSN0uiWtmReeioSDbqcKc3xz8t9evTi5q5LEhXCO6JcoZA TgloLHjSdFR/pU8fuopba3p13V+aHWCCTWbBbvzKdUWhJ0xclw/M/+l4tGH1YpcAsuIilm I8x9KdhLqCqJUyRjK0CJ+aF7QM4bT/qhTRtLTih0Mfca38D6vHAW2ZlJfQBckQ== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-ThisMailContainsUnwantedMimeParts: N The branch main has been updated by emaste: URL: https://cgit.FreeBSD.org/src/commit/?id=a0cd78bf2c148d7ab63454471771e3f4b572522f commit a0cd78bf2c148d7ab63454471771e3f4b572522f Author: Ed Maste AuthorDate: 2022-03-22 17:48:43 +0000 Commit: Ed Maste CommitDate: 2022-03-24 16:56:29 +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 --- 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); }