From nobody Wed Jan 05 17:26:13 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 0778A19244E5 for ; Wed, 5 Jan 2022 17:26:32 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: from mail-ua1-x930.google.com (mail-ua1-x930.google.com [IPv6:2607:f8b0:4864:20::930]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1D4" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4JTbxg4FJFz3JgY for ; Wed, 5 Jan 2022 17:26:31 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: by mail-ua1-x930.google.com with SMTP id o20so1704181uat.2 for ; Wed, 05 Jan 2022 09:26:31 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bsdimp-com.20210112.gappssmtp.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=oJdztgPzMuee8H9WVjV6dvLMXmfkDFRwKRD30Tz1TO0=; b=hi0MR26XJH08SOJMKVsosYgnLs5eqY1ra+KDf8aLjrdWVSSk5YCtNZgZd11HkSKkCU 8YODB+pxpuo+homcizMGfNKRQ+Uts1Xc5edEkXFj/52S/rboC0p3n3qcjQXEoioPSxzj zm0lZD3RQkseKkp2Ktf1vOftz7BkYvjcBy0mm8/yKs07ylr3xSz2SECI1ykXU0h2Psg2 qKnQJDHXl+STJl4B3KFhILDAW3EaV5+48q5PiHgjVTMgAUK1H3ZJsvem7Z/cts1tm6FQ CbqqFJ19mxi3rGKhEe8aRzD9ShpVYwn/bKKiq8yxTrGLgqo6vdciGfvXouAle+nzaxd0 ay3g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=oJdztgPzMuee8H9WVjV6dvLMXmfkDFRwKRD30Tz1TO0=; b=gidlE/WzKTVq93uVYX3VsEHM/omBL9KZZSIjRlQJ+6NMSW9DU3C5hHLF6qSdq7AyF6 kNm73tzBjCyfR13bWnPYihP+r3+Hd2OarNSMj66JCZUwFw9OwTUHnepOt9p0mz3zc5gA N+mSgs24D2bjFssZnDlABjFXkEySa3STqZRty9i15/+tFW2FuvzwcLz92QFacrDhhZKM 10xw7ehYuE+mv73fB0ACjuWBxK8K8IJAmnI4/gihZVlc39ehB25AaPafF8ilYrI0n0wW eWlHFVKtCvOVVSS5/8d/Q3bv6AntcDZlubDjp3buGwUjXGttxfprS7gVTewuEDhru6E+ n8xg== X-Gm-Message-State: AOAM5306KKNcHCjorWMQwEpP9RFgPK71E4CfA1pzzNj6EjM9w/DMgWkM 4fbhFAHjId/uGUOB2nZI0gSuhvR0vP8jpYFkpTTSFpodXIM= X-Google-Smtp-Source: ABdhPJz73YOhf6v/rSEdMXL0W5i8a32n0gb0SH2fVVQZ3MS2vyj75T97mZsr4/x6GRPOuYBe5UgwfF49gCc0WQFCCoY= X-Received: by 2002:a05:6102:ec2:: with SMTP id m2mr17803055vst.6.1641403584890; Wed, 05 Jan 2022 09:26:24 -0800 (PST) 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 References: <202201051641.205GfWZV041757@gitrepo.freebsd.org> In-Reply-To: <202201051641.205GfWZV041757@gitrepo.freebsd.org> From: Warner Losh Date: Wed, 5 Jan 2022 10:26:13 -0700 Message-ID: Subject: Re: git: 9e007a88d65b - main - atkbd: Reduce polling rate from 10Hz to ~1Hz. To: Alexander Motin Cc: src-committers , "" , dev-commits-src-main@freebsd.org Content-Type: multipart/alternative; boundary="0000000000009c5c2905d4d90db6" X-Rspamd-Queue-Id: 4JTbxg4FJFz3JgY X-Spamd-Bar: ---- Authentication-Results: mx1.freebsd.org; none X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[] X-ThisMailContainsUnwantedMimeParts: N --0000000000009c5c2905d4d90db6 Content-Type: text/plain; charset="UTF-8" Maybe we default hw.atkbd.hz to 0. That will give us more info if this is even a thing still. And it would give users hit by this a no recompile fix and give us feedback as to how often this happens. We used to miss ISA interrupts in the early SMPNG days, and that's the time this change was introduced. The PIC does a good job of latching the state and we have no other drivers that have this workaround absent issues with the device itself. Warner On Wed, Jan 5, 2022, 9:41 AM Alexander Motin wrote: > The branch main has been updated by mav: > > URL: > https://cgit.FreeBSD.org/src/commit/?id=9e007a88d65ba0d23e73c3c052d474a78260d503 > > commit 9e007a88d65ba0d23e73c3c052d474a78260d503 > Author: Alexander Motin > AuthorDate: 2022-01-05 16:32:44 +0000 > Commit: Alexander Motin > CommitDate: 2022-01-05 16:41:26 +0000 > > atkbd: Reduce polling rate from 10Hz to ~1Hz. > > In my understanding this is only needed to workaround lost interrupts. > I was thinking to remove it completely, but the comment about edge- > triggered interrupt may be true and needs deeper investigation. ~1Hz > should be often enough to handle the supposedly rare loss cases, but > rare enough to not appear in top. Add sysctl hw.atkbd.hz to tune it. > > MFC after: 1 month > --- > sys/dev/atkbdc/atkbd.c | 15 +++++++++++++-- > 1 file changed, 13 insertions(+), 2 deletions(-) > > diff --git a/sys/dev/atkbdc/atkbd.c b/sys/dev/atkbdc/atkbd.c > index 40dd698984e3..cee1207df973 100644 > --- a/sys/dev/atkbdc/atkbd.c > +++ b/sys/dev/atkbdc/atkbd.c > @@ -42,6 +42,7 @@ __FBSDID("$FreeBSD$"); > #include > #include > #include > +#include > > #include > #include > @@ -73,6 +74,13 @@ typedef struct atkbd_state { > #endif > } atkbd_state_t; > > +static SYSCTL_NODE(_hw, OID_AUTO, atkbd, CTLFLAG_RD | CTLFLAG_MPSAFE, 0, > + "AT keyboard"); > + > +static int atkbdhz = 1; > +SYSCTL_INT(_hw_atkbd, OID_AUTO, hz, CTLFLAG_RWTUN, &atkbdhz, 0, > + "Polling frequency (in hz)"); > + > static void atkbd_timeout(void *arg); > static int atkbd_reset(KBDC kbdc, int flags, int c); > > @@ -198,8 +206,11 @@ atkbd_timeout(void *arg) > kbdd_intr(kbd, NULL); > } > splx(s); > - state = (atkbd_state_t *)kbd->kb_data; > - callout_reset(&state->ks_timer, hz / 10, atkbd_timeout, arg); > + if (atkbdhz > 0) { > + state = (atkbd_state_t *)kbd->kb_data; > + callout_reset_sbt(&state->ks_timer, SBT_1S / atkbdhz, 0, > + atkbd_timeout, arg, C_PREL(1)); > + } > } > > /* LOW-LEVEL */ > --0000000000009c5c2905d4d90db6 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Maybe we default hw.atkbd.hz to 0. That will give us= more info if this is even a thing still. And it would give users hit by th= is a no recompile fix and give us feedback as to how often this happens.

We used to miss ISA interr= upts in the early SMPNG days, and that's the time this change was intro= duced. The PIC does a good job of latching the state and we have no other d= rivers that have this workaround absent issues with the device itself.

Warner

On Wed, Jan= 5, 2022, 9:41 AM Alexander Motin <ma= v@freebsd.org> wrote:
The br= anch main has been updated by mav:

URL: ht= tps://cgit.FreeBSD.org/src/commit/?id=3D9e007a88d65ba0d23e73c3c052d474a7826= 0d503

commit 9e007a88d65ba0d23e73c3c052d474a78260d503
Author:=C2=A0 =C2=A0 =C2=A0Alexander Motin <mav@FreeBSD.org>
AuthorDate: 2022-01-05 16:32:44 +0000
Commit:=C2=A0 =C2=A0 =C2=A0Alexander Motin <mav@FreeBSD.org>
CommitDate: 2022-01-05 16:41:26 +0000

=C2=A0 =C2=A0 atkbd: Reduce polling rate from 10Hz to ~1Hz.

=C2=A0 =C2=A0 In my understanding this is only needed to workaround lost in= terrupts.
=C2=A0 =C2=A0 I was thinking to remove it completely, but the comment about= edge-
=C2=A0 =C2=A0 triggered interrupt may be true and needs deeper investigatio= n.=C2=A0 ~1Hz
=C2=A0 =C2=A0 should be often enough to handle the supposedly rare loss cas= es, but
=C2=A0 =C2=A0 rare enough to not appear in top.=C2=A0 Add sysctl hw.atkbd.h= z to tune it.

=C2=A0 =C2=A0 MFC after:=C2=A0 =C2=A0 =C2=A0 1 month
---
=C2=A0sys/dev/atkbdc/atkbd.c | 15 +++++++++++++--
=C2=A01 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/sys/dev/atkbdc/atkbd.c b/sys/dev/atkbdc/atkbd.c
index 40dd698984e3..cee1207df973 100644
--- a/sys/dev/atkbdc/atkbd.c
+++ b/sys/dev/atkbdc/atkbd.c
@@ -42,6 +42,7 @@ __FBSDID("$FreeBSD$");
=C2=A0#include <sys/proc.h>
=C2=A0#include <sys/limits.h>
=C2=A0#include <sys/malloc.h>
+#include <sys/sysctl.h>

=C2=A0#include <machine/bus.h>
=C2=A0#include <machine/resource.h>
@@ -73,6 +74,13 @@ typedef struct atkbd_state {
=C2=A0#endif
=C2=A0} atkbd_state_t;

+static SYSCTL_NODE(_hw, OID_AUTO, atkbd, CTLFLAG_RD | CTLFLAG_MPSAFE, 0, +=C2=A0 =C2=A0 "AT keyboard");
+
+static int atkbdhz =3D 1;
+SYSCTL_INT(_hw_atkbd, OID_AUTO, hz, CTLFLAG_RWTUN, &atkbdhz, 0,
+=C2=A0 =C2=A0 "Polling frequency (in hz)");
+
=C2=A0static void=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 atkbd_timeout(vo= id *arg);
=C2=A0static int=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0atkbd_reset= (KBDC kbdc, int flags, int c);

@@ -198,8 +206,11 @@ atkbd_timeout(void *arg)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 kbdd_intr(kbd, NULL);
=C2=A0 =C2=A0 =C2=A0 =C2=A0 }
=C2=A0 =C2=A0 =C2=A0 =C2=A0 splx(s);
-=C2=A0 =C2=A0 =C2=A0 =C2=A0state =3D (atkbd_state_t *)kbd->kb_data;
-=C2=A0 =C2=A0 =C2=A0 =C2=A0callout_reset(&state->ks_timer, hz / 10,= atkbd_timeout, arg);
+=C2=A0 =C2=A0 =C2=A0 =C2=A0if (atkbdhz > 0) {
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0state =3D (atkbd_st= ate_t *)kbd->kb_data;
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0callout_reset_sbt(&= amp;state->ks_timer, SBT_1S / atkbdhz, 0,
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0atkbd= _timeout, arg, C_PREL(1));
+=C2=A0 =C2=A0 =C2=A0 =C2=A0}
=C2=A0}

=C2=A0/* LOW-LEVEL */
--0000000000009c5c2905d4d90db6--