Re: expose host capabilities relative to usb isochronous xfers
On Wed, Jun 12, 2019 at 03:55:59PM +0200, Alexandre Ratchov wrote: > Currenty USB device driver code has no way to obtain how many frames > can be scheduled on the HC. If it attempts to schedule too many > frames, usbd_transfer() fails or silently misbehaves. > > For audio this is a big problem because the max frames count > determines the block size which must be reported to upper layers > before any transfer starts. This makes impossible to implement uaudio > properly. Currently there's a temporary hack to workaround this, which > limits the block size. Now that uaudio is in and works, it's time to > start removing such hacks. > > Similarly, driver code needs to know the minimum number of frames per > transfer to get an interrupt (ie the completion callback). Indeed, if > the transfer frame count is not rounded to it, we silently miss the > interrupt, the completion call-back is not called and playback > stutters. > > The diff below adds a new usbd_bus_info() function which reports the > maximum frames that can be scheduled and the minimum frames per > transfer to get an interrupt. > > [snip] > Index: usbdi.c > === > RCS file: /cvs/src/sys/dev/usb/usbdi.c,v > retrieving revision 1.100 > diff -u -p -u -p -r1.100 usbdi.c > --- usbdi.c 18 Nov 2018 16:33:26 - 1.100 > +++ usbdi.c 12 Jun 2019 13:33:41 - > @@ -275,6 +275,12 @@ usbd_close_pipe(struct usbd_pipe *pipe) > return (USBD_NORMAL_COMPLETION); > } > > +void > +usbd_bus_info(struct usbd_device *dev, struct usbd_bus_info *info) > +{ > + dev->bus->methods->info(dev, info); > +} > + > usbd_status > usbd_transfer(struct usbd_xfer *xfer) > { > [snip] i think above is not good enough with only ehci, ohci, uhci and xhci being taken care of in [snip]'ed parts of the diff unfortunately:] just thinking about dwc2. -Artturi
new env defaults for doas
This has come up a few times before. For background, the default rule for doas is to copy a few environment settings from the user and omit the rest. This is to prevent confusion, and also supposedly for security. However, some of the alleged safe variables like PATH probably aren't that safe. And things like USER are confusing? And why even bother with MAIL? The list is kinda adhoc and mostly copied from what I understood sudo to do at the time, but I believe sudo has changed defaults as well. So here's a new model which I think is safer and more consistent. 1. Always add a DOAS_USER with the invoking user's name. 2. When doing keepenv, nothing changes, except addition of above. 3. When starting with a new environment, fill in USER and HOME and such with the values of the target user, instead of copying from the invoking user. You run a command as a user, you should have that user's environment. 4. DISPLAY and TERM are still copied, since they represent a description of the actual environment in which we are running. (Not sure how useful display is without xauth cookies, but not my problem.) 5. As before, anything can be overridden with setenv in the config. This is a kinda breaking change, but probably in a good way. For simple configurations, I expect nothing changes. For more complicated setups, this new handling is probably closer to what the admin expects or desires. Index: doas.c === RCS file: /home/cvs/src/usr.bin/doas/doas.c,v retrieving revision 1.76 diff -u -p -r1.76 doas.c --- doas.c 12 Jun 2019 02:50:29 - 1.76 +++ doas.c 13 Jun 2019 02:11:07 - @@ -421,6 +421,7 @@ main(int argc, char **argv) errx(1, "no passwd entry for target"); if (setusercontext(NULL, targpw, target, LOGIN_SETGROUP | + LOGIN_SETPATH | LOGIN_SETPRIORITY | LOGIN_SETRESOURCES | LOGIN_SETUMASK | LOGIN_SETUSER) != 0) errx(1, "failed to set user context for target"); @@ -439,8 +440,13 @@ main(int argc, char **argv) syslog(LOG_AUTHPRIV | LOG_INFO, "%s ran command %s as %s from %s", mypw->pw_name, cmdline, targpw->pw_name, cwd); - envp = prepenv(rule); + envp = prepenv(rule, mypw, targpw); + if (rule->cmd) { + /* do this again after setusercontext reset it */ + if (setenv("PATH", safepath, 1) == -1) + err(1, "failed to set PATH '%s'", safepath); + } execvpe(cmd, argv, envp); fail: if (errno == ENOENT) Index: doas.conf.5 === RCS file: /home/cvs/src/usr.bin/doas/doas.conf.5,v retrieving revision 1.35 diff -u -p -r1.35 doas.conf.5 --- doas.conf.5 7 Feb 2018 05:13:57 - 1.35 +++ doas.conf.5 13 Jun 2019 01:41:18 - @@ -51,15 +51,9 @@ again for some time. .It Ic keepenv The user's environment is maintained. The default is to reset the environment, except for the variables -.Ev DISPLAY , -.Ev HOME , -.Ev LOGNAME , -.Ev MAIL , -.Ev PATH , -.Ev TERM , -.Ev USER +.Ev DISPLAY and -.Ev USERNAME . +.Ev TERM . .It Ic setenv { Oo Ar variable ... Oc Oo Ar variable=value ... Oc Ic } In addition to the variables mentioned above, keep the space-separated specified variables. Index: doas.h === RCS file: /home/cvs/src/usr.bin/doas/doas.h,v retrieving revision 1.13 diff -u -p -r1.13 doas.h --- doas.h 6 Apr 2017 21:12:06 - 1.13 +++ doas.h 13 Jun 2019 01:10:29 - @@ -29,7 +29,10 @@ extern struct rule **rules; extern int nrules; extern int parse_errors; -char **prepenv(const struct rule *); +struct passwd; + +char **prepenv(const struct rule *, const struct passwd *, +const struct passwd *); #define PERMIT 1 #define DENY 2 Index: env.c === RCS file: /home/cvs/src/usr.bin/doas/env.c,v retrieving revision 1.6 diff -u -p -r1.6 env.c --- env.c 6 Apr 2017 21:12:06 - 1.6 +++ env.c 13 Jun 2019 02:12:57 - @@ -24,6 +24,7 @@ #include #include #include +#include #include "doas.h" @@ -38,6 +39,8 @@ struct env { u_int count; }; +static void fillenv(struct env *env, const char **envlist); + static int envcmp(struct envnode *a, struct envnode *b) { @@ -68,8 +71,19 @@ freenode(struct envnode *node) free(node); } +static void +addnode(struct env *env, const char *key, const char *value) +{ + struct envnode *node; + + node = createnode(key, value); + RB_INSERT(envtree, &env->root, node); + env->count++; +} + static struct env * -createenv(const struct rule *rule) +createenv(const struct rule *rule, const struct passwd *mypw, +const struct passwd *targpw) { struct env *env; u_int i; @@ -80,6 +94,8 @@ createenv(const struct rule *rule) R
Re: drm(4), multi-threaded task queues and barriers
> From: "Sven M. Hallberg" > Date: Wed, 12 Jun 2019 23:18:24 +0200 > > Mark Kettenis on Tue, Jun 11 2019: > > The drm(4) codebase really needs multi-threaded task queues [...] > > > > The diff also starts 4 threads for each workqueue that gets created by > > the drm(4) layer. The number 4 is a bit arbitrary but it is the > > number of threads that Linux creates per CPU for a so-called "unbound" > > workqueue which hopefully is enough to always make progress. > > > > Please test. > > Looks good and appears to work fine with two displays (one internal, one > external). Will test with three at work tomorrow. > > > > - dev_priv->hotplug.poll_init_work.tq = systq; > > Intentional? Yes. It removes a local modification that should no longer be necessary. Unfortunately the diff doesn't work with amdgpu. Some more thinking needed... > > Index: dev/pci/drm/drm_linux.c > > === > > RCS file: /cvs/src/sys/dev/pci/drm/drm_linux.c,v > > retrieving revision 1.38 > > diff -u -p -r1.38 drm_linux.c > > --- dev/pci/drm/drm_linux.c 9 Jun 2019 12:58:30 - 1.38 > > +++ dev/pci/drm/drm_linux.c 11 Jun 2019 18:54:38 - > > @@ -1399,15 +1399,15 @@ drm_linux_init(void) > > { > > if (system_wq == NULL) { > > system_wq = (struct workqueue_struct *) > > - taskq_create("drmwq", 1, IPL_HIGH, 0); > > + taskq_create("drmwq", 4, IPL_HIGH, 0); > > } > > if (system_unbound_wq == NULL) { > > system_unbound_wq = (struct workqueue_struct *) > > - taskq_create("drmubwq", 1, IPL_HIGH, 0); > > + taskq_create("drmubwq", 4, IPL_HIGH, 0); > > } > > if (system_long_wq == NULL) { > > system_long_wq = (struct workqueue_struct *) > > - taskq_create("drmlwq", 1, IPL_HIGH, 0); > > + taskq_create("drmlwq", 4, IPL_HIGH, 0); > > } > > > > if (taskletq == NULL) > > Index: dev/pci/drm/i915/intel_hotplug.c > > === > > RCS file: /cvs/src/sys/dev/pci/drm/i915/intel_hotplug.c,v > > retrieving revision 1.2 > > diff -u -p -r1.2 intel_hotplug.c > > --- dev/pci/drm/i915/intel_hotplug.c14 Apr 2019 10:14:52 - > > 1.2 > > +++ dev/pci/drm/i915/intel_hotplug.c11 Jun 2019 18:54:38 - > > @@ -619,7 +619,6 @@ void intel_hpd_init_work(struct drm_i915 > > INIT_WORK(&dev_priv->hotplug.hotplug_work, i915_hotplug_work_func); > > INIT_WORK(&dev_priv->hotplug.dig_port_work, i915_digport_work_func); > > INIT_WORK(&dev_priv->hotplug.poll_init_work, i915_hpd_poll_init_work); > > - dev_priv->hotplug.poll_init_work.tq = systq; > > INIT_DELAYED_WORK(&dev_priv->hotplug.reenable_work, > > intel_hpd_irq_storm_reenable_work); > > } > > Index: kern/kern_task.c > > === > > RCS file: /cvs/src/sys/kern/kern_task.c,v > > retrieving revision 1.25 > > diff -u -p -r1.25 kern_task.c > > --- kern/kern_task.c28 Apr 2019 04:20:40 - 1.25 > > +++ kern/kern_task.c11 Jun 2019 18:54:39 - > > @@ -43,6 +43,7 @@ struct taskq { > > TQ_S_DESTROYED > > }tq_state; > > unsigned int tq_running; > > + unsigned int tq_barrier; > > unsigned int tq_nthreads; > > unsigned int tq_flags; > > const char *tq_name; > > @@ -59,6 +60,7 @@ static const char taskq_sys_name[] = "sy > > struct taskq taskq_sys = { > > TQ_S_CREATED, > > 0, > > + 0, > > 1, > > 0, > > taskq_sys_name, > > @@ -77,6 +79,7 @@ static const char taskq_sys_mp_name[] = > > struct taskq taskq_sys_mp = { > > TQ_S_CREATED, > > 0, > > + 0, > > 1, > > TASKQ_MPSAFE, > > taskq_sys_mp_name, > > @@ -122,6 +125,7 @@ taskq_create(const char *name, unsigned > > > > tq->tq_state = TQ_S_CREATED; > > tq->tq_running = 0; > > + tq->tq_barrier = 0; > > tq->tq_nthreads = nthreads; > > tq->tq_name = name; > > tq->tq_flags = flags; > > @@ -161,6 +165,7 @@ taskq_destroy(struct taskq *tq) > > panic("unexpected %s tq state %u", tq->tq_name, tq->tq_state); > > } > > > > + tq->tq_barrier = 0; > > while (tq->tq_running > 0) { > > wakeup(tq); > > msleep(&tq->tq_running, &tq->tq_mtx, PWAIT, "tqdestroy", 0); > > @@ -223,6 +228,7 @@ taskq_barrier(struct taskq *tq) > > > > WITNESS_CHECKORDER(&tq->tq_lock_object, LOP_NEWORDER, NULL); > > > > + SET(t.t_flags, TASK_BARRIER); > > task_add(tq, &t); > > cond_wait(&c, "tqbar"); > > } > > @@ -238,6 +244,7 @@ taskq_del_barrier(struct taskq *tq, stru > > if (task_del(tq, del)) > > return; > > > > + SET(t.t_flags, TASK_BARRIER); > > task_add(tq, &t); > > cond_wait(&c, "tqbar"); > > } > > @@ -304,6 +311,7 @@ taskq_ne
Re: drm(4), multi-threaded task queues and barriers
Mark Kettenis on Tue, Jun 11 2019: > The drm(4) codebase really needs multi-threaded task queues [...] > > The diff also starts 4 threads for each workqueue that gets created by > the drm(4) layer. The number 4 is a bit arbitrary but it is the > number of threads that Linux creates per CPU for a so-called "unbound" > workqueue which hopefully is enough to always make progress. > > Please test. Looks good and appears to work fine with two displays (one internal, one external). Will test with three at work tomorrow. > - dev_priv->hotplug.poll_init_work.tq = systq; Intentional? -p > Index: dev/pci/drm/drm_linux.c > === > RCS file: /cvs/src/sys/dev/pci/drm/drm_linux.c,v > retrieving revision 1.38 > diff -u -p -r1.38 drm_linux.c > --- dev/pci/drm/drm_linux.c 9 Jun 2019 12:58:30 - 1.38 > +++ dev/pci/drm/drm_linux.c 11 Jun 2019 18:54:38 - > @@ -1399,15 +1399,15 @@ drm_linux_init(void) > { > if (system_wq == NULL) { > system_wq = (struct workqueue_struct *) > - taskq_create("drmwq", 1, IPL_HIGH, 0); > + taskq_create("drmwq", 4, IPL_HIGH, 0); > } > if (system_unbound_wq == NULL) { > system_unbound_wq = (struct workqueue_struct *) > - taskq_create("drmubwq", 1, IPL_HIGH, 0); > + taskq_create("drmubwq", 4, IPL_HIGH, 0); > } > if (system_long_wq == NULL) { > system_long_wq = (struct workqueue_struct *) > - taskq_create("drmlwq", 1, IPL_HIGH, 0); > + taskq_create("drmlwq", 4, IPL_HIGH, 0); > } > > if (taskletq == NULL) > Index: dev/pci/drm/i915/intel_hotplug.c > === > RCS file: /cvs/src/sys/dev/pci/drm/i915/intel_hotplug.c,v > retrieving revision 1.2 > diff -u -p -r1.2 intel_hotplug.c > --- dev/pci/drm/i915/intel_hotplug.c 14 Apr 2019 10:14:52 - 1.2 > +++ dev/pci/drm/i915/intel_hotplug.c 11 Jun 2019 18:54:38 - > @@ -619,7 +619,6 @@ void intel_hpd_init_work(struct drm_i915 > INIT_WORK(&dev_priv->hotplug.hotplug_work, i915_hotplug_work_func); > INIT_WORK(&dev_priv->hotplug.dig_port_work, i915_digport_work_func); > INIT_WORK(&dev_priv->hotplug.poll_init_work, i915_hpd_poll_init_work); > - dev_priv->hotplug.poll_init_work.tq = systq; > INIT_DELAYED_WORK(&dev_priv->hotplug.reenable_work, > intel_hpd_irq_storm_reenable_work); > } > Index: kern/kern_task.c > === > RCS file: /cvs/src/sys/kern/kern_task.c,v > retrieving revision 1.25 > diff -u -p -r1.25 kern_task.c > --- kern/kern_task.c 28 Apr 2019 04:20:40 - 1.25 > +++ kern/kern_task.c 11 Jun 2019 18:54:39 - > @@ -43,6 +43,7 @@ struct taskq { > TQ_S_DESTROYED > }tq_state; > unsigned int tq_running; > + unsigned int tq_barrier; > unsigned int tq_nthreads; > unsigned int tq_flags; > const char *tq_name; > @@ -59,6 +60,7 @@ static const char taskq_sys_name[] = "sy > struct taskq taskq_sys = { > TQ_S_CREATED, > 0, > + 0, > 1, > 0, > taskq_sys_name, > @@ -77,6 +79,7 @@ static const char taskq_sys_mp_name[] = > struct taskq taskq_sys_mp = { > TQ_S_CREATED, > 0, > + 0, > 1, > TASKQ_MPSAFE, > taskq_sys_mp_name, > @@ -122,6 +125,7 @@ taskq_create(const char *name, unsigned > > tq->tq_state = TQ_S_CREATED; > tq->tq_running = 0; > + tq->tq_barrier = 0; > tq->tq_nthreads = nthreads; > tq->tq_name = name; > tq->tq_flags = flags; > @@ -161,6 +165,7 @@ taskq_destroy(struct taskq *tq) > panic("unexpected %s tq state %u", tq->tq_name, tq->tq_state); > } > > + tq->tq_barrier = 0; > while (tq->tq_running > 0) { > wakeup(tq); > msleep(&tq->tq_running, &tq->tq_mtx, PWAIT, "tqdestroy", 0); > @@ -223,6 +228,7 @@ taskq_barrier(struct taskq *tq) > > WITNESS_CHECKORDER(&tq->tq_lock_object, LOP_NEWORDER, NULL); > > + SET(t.t_flags, TASK_BARRIER); > task_add(tq, &t); > cond_wait(&c, "tqbar"); > } > @@ -238,6 +244,7 @@ taskq_del_barrier(struct taskq *tq, stru > if (task_del(tq, del)) > return; > > + SET(t.t_flags, TASK_BARRIER); > task_add(tq, &t); > cond_wait(&c, "tqbar"); > } > @@ -304,6 +311,7 @@ taskq_next_work(struct taskq *tq, struct > struct task *next; > > mtx_enter(&tq->tq_mtx); > +retry: > while ((next = TAILQ_FIRST(&tq->tq_worklist)) == NULL) { > if (tq->tq_state != TQ_S_RUNNING) { > mtx_leave(&tq->tq_mtx); > @@ -311,6 +319,17 @@ taskq_next_work(struct taskq *tq, struct > } > >
Re: [patch] rsync: fix free() on uninitialized pointer with -rx and same device
On Thu, Jun 06, 2019 at 02:14:05PM +0200, Christian Weisgerber wrote: > Björn Ketelaars: > > > Diff below is based on the latest diff from naddy@. Changes: > > - reallocarray likes type_t, as such changes type of nxdev and i; > > - use reallocarray instead of malloc as xdev is initialised as NULL. > > ok naddy@ > > -- > Christian "naddy" Weisgerber na...@mips.inka.de > Looks good to me. I have 2 or 3 more patches locally. It would be nice if the above patch can get committed so I can rebase and send a clean patch. Thanks, -- Kind regards, Hiltjo
expose host capabilities relative to usb isochronous xfers
Currenty USB device driver code has no way to obtain how many frames can be scheduled on the HC. If it attempts to schedule too many frames, usbd_transfer() fails or silently misbehaves. For audio this is a big problem because the max frames count determines the block size which must be reported to upper layers before any transfer starts. This makes impossible to implement uaudio properly. Currently there's a temporary hack to workaround this, which limits the block size. Now that uaudio is in and works, it's time to start removing such hacks. Similarly, driver code needs to know the minimum number of frames per transfer to get an interrupt (ie the completion callback). Indeed, if the transfer frame count is not rounded to it, we silently miss the interrupt, the completion call-back is not called and playback stutters. The diff below adds a new usbd_bus_info() function which reports the maximum frames that can be scheduled and the minimum frames per transfer to get an interrupt. Index: ehci.c === RCS file: /cvs/src/sys/dev/usb/ehci.c,v retrieving revision 1.204 diff -u -p -u -p -r1.204 ehci.c --- ehci.c 31 Mar 2019 06:16:38 - 1.204 +++ ehci.c 12 Jun 2019 13:33:38 - @@ -113,6 +113,7 @@ struct ehci_pipe { u_int8_t ehci_reverse_bits(u_int8_t, int); usbd_statusehci_open(struct usbd_pipe *); +void ehci_info(struct usbd_device *, struct usbd_bus_info *); intehci_setaddr(struct usbd_device *, int); void ehci_poll(struct usbd_bus *); void ehci_softintr(void *); @@ -225,6 +226,7 @@ struct usbd_bus_methods ehci_bus_methods .do_poll = ehci_poll, .allocx = ehci_allocx, .freex = ehci_freex, + .info = ehci_info, }; struct usbd_pipe_methods ehci_root_ctrl_methods = { @@ -493,6 +498,21 @@ ehci_init(struct ehci_softc *sc) return (err); } +void +ehci_info(struct usbd_device *dev, struct usbd_bus_info *info) +{ + /* +* XXX: even if most hosts have 1024 frame lists, only 256 +* frame transfers work reliably. As soon as 256 is exceeded, +* we start getting zeroed frames if multiple device are +* running simultaneously. Set this to sc->sc_fl_size, once +* ehci is fixed. Interrups occur every 1m, despite the +* EHCI_CMD_ITC_2 setting. +*/ + info->nframes_max = 256; + info->intr_thres = (dev->speed >= USB_SPEED_HIGH) ? 8 : 1; +} + int ehci_intr(void *v) { Index: ohci.c === RCS file: /cvs/src/sys/dev/usb/ohci.c,v retrieving revision 1.156 diff -u -p -u -p -r1.156 ohci.c --- ohci.c 11 Mar 2019 17:50:08 - 1.156 +++ ohci.c 12 Jun 2019 13:33:40 - @@ -87,6 +87,7 @@ usbd_status ohci_alloc_std_chain(struct struct ohci_soft_td **); usbd_statusohci_open(struct usbd_pipe *); +void ohci_info(struct usbd_device *, struct usbd_bus_info *); intohci_setaddr(struct usbd_device *, int); void ohci_poll(struct usbd_bus *); void ohci_softintr(void *); @@ -236,6 +237,7 @@ struct usbd_bus_methods ohci_bus_methods .do_poll = ohci_poll, .allocx = ohci_allocx, .freex = ohci_freex, + .info = ohci_info, }; struct usbd_pipe_methods ohci_root_ctrl_methods = { @@ -931,6 +933,13 @@ ohci_init(struct ohci_softc *sc) bad1: usb_freemem(&sc->sc_bus, &sc->sc_hccadma); return (err); +} + +void +ohci_info(struct usbd_device *dev, struct usbd_bus_info *info) +{ + info->nframes_max = OHCI_SITD_CHUNK; + info->intr_thres = 1; } struct usbd_xfer * Index: uaudio.c === RCS file: /cvs/src/sys/dev/usb/uaudio.c,v retrieving revision 1.144 diff -u -p -u -p -r1.144 uaudio.c --- uaudio.c9 May 2019 07:09:04 - 1.144 +++ uaudio.c12 Jun 2019 13:33:41 - @@ -382,6 +382,7 @@ struct uaudio_softc { unsigned int ufps; /* USB frames per second */ unsigned int sync_pktsz;/* size of sync packet */ unsigned int host_nframes; /* max frames we can schedule */ + unsigned int host_intr_thres; /* min frames to get an interrupt */ int diff_nsamp; /* samples play is ahead of rec */ int diff_nframes; /* frames play is ahead of rec */ @@ -2820,12 +2821,15 @@ uaudio_stream_open(struct uaudio_softc * s->data_pipe = NULL; s->sync_pipe = NULL; - s->nframes_mask = 0; - i = a->fps; - while (i > 1000) { - s->nframes_mask = (s->nframes_mask << 1) | 1; - i >>= 1; - } + /* +* Find the smallest transfer size (power of two), larger than +* the poll interval and the interrupt threshold. +*/ + i = 0; + while (a->fps < (
Re: [patch] use acme-client to sign certificated with ecdsa keys
On 6/11/19 2:36 PM, Sebastian Benoit wrote: Hi, some feedback below. Renaud: maybe wait for feedback from florian or gilles until acting on my comments, sometimes sending diffs to fast creates more work ;) /Benno As suggested by benno@ removal of the global variable removal of KEYTYPE which was not used and was a leftover of a former patch define ECDSA_KEY to be more readable Index: extern.h === RCS file: /cvs/src/usr.sbin/acme-client/extern.h,v retrieving revision 1.13 diff -u -p -r1.13 extern.h --- extern.h 12 Jun 2019 11:09:25 - 1.13 +++ extern.h 12 Jun 2019 12:27:03 - @@ -207,7 +207,7 @@ int revokeproc(int, const char *, cons int, int, const char *const *, size_t); int fileproc(int, const char *, const char *, const char *, const char *); -int keyproc(int, const char *, const char **, size_t); +int keyproc(int, const char *, const char **, size_t, int); int netproc(int, int, int, int, int, int, int, struct authority_c *, const char *const *, size_t); @@ -274,11 +274,6 @@ char *json_fmt_signed(const char *, con * Should we print debugging messages? */ int verbose; - -/* - * Should we switch to ecdsa? - */ -int ecdsa; /* * What component is the process within (COMP__MAX for none)? Index: keyproc.c === RCS file: /cvs/src/usr.sbin/acme-client/keyproc.c,v retrieving revision 1.13 diff -u -p -r1.13 keyproc.c --- keyproc.c 12 Jun 2019 11:09:25 - 1.13 +++ keyproc.c 12 Jun 2019 12:27:03 - @@ -75,7 +75,8 @@ add_ext(STACK_OF(X509_EXTENSION) *sk, in */ int keyproc(int netsock, const char *keyfile, -const char **alts, size_t altsz) +const char **alts, size_t altsz, +const int keytype) { char *der64 = NULL, *der = NULL, *dercp; char *sans = NULL, *san = NULL; @@ -117,7 +118,7 @@ keyproc(int netsock, const char *keyfile } if (newkey) { - if (ecdsa) { + if (keytype == ECDSA_KEY) { if ((pkey = ec_key_create(f, keyfile)) == NULL) goto out; dodbg("%s: generated ECDSA domain key", keyfile); Index: main.c === RCS file: /cvs/src/usr.sbin/acme-client/main.c,v retrieving revision 1.48 diff -u -p -r1.48 main.c --- main.c 12 Jun 2019 11:09:25 - 1.48 +++ main.c 12 Jun 2019 12:27:04 - @@ -49,7 +49,6 @@ main(int argc, char *argv[]) int popts = 0; pid_t pids[COMP__MAX]; extern int verbose; - extern int ecdsa; extern enum comp proccomp; size_t i, altsz, ne; @@ -148,10 +147,6 @@ main(int argc, char *argv[]) errx(EXIT_FAILURE, "authority %s not found", auth); } - if (domain->keytype == 1) { - ecdsa = 1; - } - acctkey = authority->account; if ((chngdir = domain->challengedir) == NULL) @@ -258,7 +253,8 @@ main(int argc, char *argv[]) close(file_fds[0]); close(file_fds[1]); c = keyproc(key_fds[0], domain->key, - (const char **)alts, altsz); + (const char **)alts, altsz, + domain->keytype); exit(c ? EXIT_SUCCESS : EXIT_FAILURE); } Index: parse.h === RCS file: /cvs/src/usr.sbin/acme-client/parse.h,v retrieving revision 1.11 diff -u -p -r1.11 parse.h --- parse.h 12 Jun 2019 11:09:25 - 1.11 +++ parse.h 12 Jun 2019 12:27:04 - @@ -21,6 +21,7 @@ #define AUTH_MAXLEN 120 /* max length of an authority_c name */ #define DOMAIN_MAXLEN 255 /* max len of a domain name (rfc2181) */ +#define ECDSA_KEY 1 /* * XXX other size limits needed? Index: parse.y === RCS file: /cvs/src/usr.sbin/acme-client/parse.y,v retrieving revision 1.35 diff -u -p -r1.35 parse.y --- parse.y 12 Jun 2019 11:09:25 - 1.35 +++ parse.y 12 Jun 2019 12:27:04 - @@ -100,7 +100,7 @@ typedef struct { %} %token AUTHORITY URL API ACCOUNT -%token DOMAIN ALTERNATIVE NAMES CERT FULL CHAIN KEY SIGN WITH CHALLENGEDIR KEYTYPE +%token DOMAIN ALTERNATIVE NAMES CERT FULL CHAIN KEY SIGN WITH CHALLENGEDIR %token YES NO %token INCLUDE %token ERROR @@ -261,10 +261,9 @@ domain : DOMAIN STRING { ; keytype : RSA { - domain->keytype = 0; } | ECDSA { - domain->keytype = 1; + domain->keytype = ECDSA_KEY; } | /* nothing */ ; smime.p7s Description: S/MIME Cryptographic Signature
Re: ssh_config.5: refer consistently to HostName, not Hostname
On Wed, Jun 12, 2019 at 12:55:01PM +0300, Lauri Tirkkonen wrote: > On Tue, Jun 11 2019 14:40:03 +0100, Jason McIntyre wrote: > > On Tue, Jun 11, 2019 at 03:11:03PM +0300, Lauri Tirkkonen wrote: > > > Hi, trivial manpage diff. > > > > > > > fixed, thanks. > > jmc > > I noticed the followup commit that changes the canonical option name to > 'Hostname'. I also like that capitalization better, but your diff seems > to have been a bit incomplete; in particular the option should be called > oHostname in code also, and a couple manpage mentions of HostName still > remain. X11UseLocalhost is used consistently in comparison. > ouch. thanks for following this up. i've just committed your diff. jmc > diff --git a/usr.bin/ssh/clientloop.c b/usr.bin/ssh/clientloop.c > index fdcca0e25ed..fd43ee4cbb4 100644 > --- a/usr.bin/ssh/clientloop.c > +++ b/usr.bin/ssh/clientloop.c > @@ -121,7 +121,7 @@ extern int muxserver_sock; /* XXX use > mux_client_cleanup() instead */ > > /* > * Name of the host we are connecting to. This is the name given on the > - * command line, or the HostName specified for the user-supplied name in a > + * command line, or the Hostname specified for the user-supplied name in a > * configuration file. > */ > extern char *host; > diff --git a/usr.bin/ssh/readconf.c b/usr.bin/ssh/readconf.c > index 27638663b78..5b9c97b7c8d 100644 > --- a/usr.bin/ssh/readconf.c > +++ b/usr.bin/ssh/readconf.c > @@ -71,7 +71,7 @@ > User foo > > Host fake.com > - HostName another.host.name.real.org > + Hostname another.host.name.real.org > User blaah > Port 34289 > ForwardX11 no > @@ -133,7 +133,7 @@ typedef enum { > oGatewayPorts, oExitOnForwardFailure, > oPasswordAuthentication, oRSAAuthentication, > oChallengeResponseAuthentication, oXAuthLocation, > - oIdentityFile, oHostName, oPort, oCipher, oRemoteForward, oLocalForward, > + oIdentityFile, oHostname, oPort, oCipher, oRemoteForward, oLocalForward, > oCertificateFile, oAddKeysToAgent, oIdentityAgent, > oUser, oEscapeChar, oRhostsRSAAuthentication, oProxyCommand, > oGlobalKnownHostsFile, oUserKnownHostsFile, oConnectionAttempts, > @@ -225,7 +225,7 @@ static struct { > { "certificatefile", oCertificateFile }, > { "addkeystoagent", oAddKeysToAgent }, > { "identityagent", oIdentityAgent }, > - { "hostname", oHostName }, > + { "hostname", oHostname }, > { "hostkeyalias", oHostKeyAlias }, > { "proxycommand", oProxyCommand }, > { "port", oPort }, > @@ -1102,7 +1102,7 @@ parse_char_array: > max_entries = SSH_MAX_HOSTS_FILES; > goto parse_char_array; > > - case oHostName: > + case oHostname: > charptr = &options->hostname; > goto parse_string; > > @@ -2576,7 +2576,7 @@ dump_client_config(Options *o, const char *host) > > /* Most interesting options first: user, host, port */ > dump_cfg_string(oUser, o->user); > - dump_cfg_string(oHostName, host); > + dump_cfg_string(oHostname, host); > dump_cfg_int(oPort, o->port); > > /* Flag options */ > diff --git a/usr.bin/ssh/scp.1 b/usr.bin/ssh/scp.1 > index a2833dab041..29e97808ed1 100644 > --- a/usr.bin/ssh/scp.1 > +++ b/usr.bin/ssh/scp.1 > @@ -164,7 +164,7 @@ For full details of the options listed below, and their > possible values, see > .It HostbasedKeyTypes > .It HostKeyAlgorithms > .It HostKeyAlias > -.It HostName > +.It Hostname > .It IdentitiesOnly > .It IdentityAgent > .It IdentityFile > diff --git a/usr.bin/ssh/sftp.1 b/usr.bin/ssh/sftp.1 > index 259095885a7..a6757705941 100644 > --- a/usr.bin/ssh/sftp.1 > +++ b/usr.bin/ssh/sftp.1 > @@ -241,7 +241,7 @@ For full details of the options listed below, and their > possible values, see > .It HostbasedKeyTypes > .It HostKeyAlgorithms > .It HostKeyAlias > -.It HostName > +.It Hostname > .It IdentitiesOnly > .It IdentityAgent > .It IdentityFile > diff --git a/usr.bin/ssh/ssh.1 b/usr.bin/ssh/ssh.1 > index 9480eba8d3e..4c2c101d765 100644 > --- a/usr.bin/ssh/ssh.1 > +++ b/usr.bin/ssh/ssh.1 > @@ -504,7 +504,7 @@ For full details of the options listed below, and their > possible values, see > .It HostbasedKeyTypes > .It HostKeyAlgorithms > .It HostKeyAlias > -.It HostName > +.It Hostname > .It IdentitiesOnly > .It IdentityAgent > .It IdentityFile > diff --git a/usr.bin/ssh/ssh.c b/usr.bin/ssh/ssh.c > index defa3c1c337..aeefbe4f91c 100644 > --- a/usr.bin/ssh/ssh.c > +++ b/usr.bin/ssh/ssh.c > @@ -146,7 +146,7 @@ char *config = NULL; > > /* > * Name of the host we are connecting to. This is the name given on the > - * command line, or the HostName specified for the user-supplied name in a > + * command line, or the Hostname specified for the user-supplied name in a > * configuration file. > */ > char *host; > diff --git a/usr.bin/ssh/ssh_config.5 b/usr.bin/ssh/ssh_config.5 > index 7bb6cde7ae1..719288b03a5 100644 > --- a/us
net80211: more steady Tx rate with MiRa (please test)
While working on Tx aggregation I noticed that TCP streams will start stalling whenever MiRa decides to start sending frames at Tx rates which the AP tends to fail to receive. This means we're dropping far too many frames while trying to find an optimal Tx rate to use. The problem can be observed with tcpbench falling into 0.0 Mbps occasionally and taking lots of time to recover. This problem disappeared when I hacked a fixed working Tx rate into the driver and disabled MiRa. So I took a closer look at MiRa again and measured percentage of good/bad MCS used for transmission. The diff below eliminates a lot of frames being sent at bad rates and results in more steady overall Tx performance while testing my WIP Tx aggregation code. I can my numbers by recording outgoing frame headers in pcap file while running tcpbench: tcpdump -n -i iwn0 -y IEEE802_11_RADIO -w /tmp/iwn.pcap Now I filter this pcap file by MCS index in wireshark, with an expression such as 'radiotap.mcs.index == 7', and look at the 'Displayed:' percentage in the bottom right corner of wireshark's window. Since I am knee-deep in Tx aggregation right now, I would like to delegate testing of the diff below against plain -current to the community. If some of you could test the diff below and report back to me I would appreciate it. You don't need to get numbers from wireshark for this if you don't want to. Letting me know if Tx is faster or not and whether there are any perceived regressions is sufficient. The drivers affected by this change are athn(4), iwn(4), and iwm(4). Don't bother testing with any other drivers. I have tested this diff with iwn(4) only so far. diff refs/heads/master refs/heads/txagg blob - 0d14daff4d1c8973b1d1180a9885a25c6754940b blob + 8a0f3b90198e9707ae08ef53aa4e68706ce2fa15 --- sys/net80211/ieee80211_mira.c +++ sys/net80211/ieee80211_mira.c @@ -510,6 +510,12 @@ ieee80211_mira_reset_driver_stats(struct ieee80211_mir /* Number of bytes which, alternatively, render a probe valid. */ #define IEEE80211_MIRA_MIN_PROBE_BYTES IEEE80211_MAX_LEN +/* Number of Tx failures which, alternatively, render a probe valid. */ +#define IEEE80211_MIRA_MAX_PROBE_TXFAIL 1 + +/* Number of Tx retries which, alternatively, render a probe valid. */ +#define IEEE80211_MIRA_MAX_PROBE_RETRIES 4 + int ieee80211_mira_next_lower_intra_rate(struct ieee80211_mira_node *mn, struct ieee80211_node *ni) @@ -719,7 +725,9 @@ ieee80211_mira_probe_valid(struct ieee80211_mira_node struct ieee80211_mira_goodput_stats *g = &mn->g[ni->ni_txmcs]; return (g->nprobes >= IEEE80211_MIRA_MIN_PROBE_FRAMES || - g->nprobe_bytes >= IEEE80211_MIRA_MIN_PROBE_BYTES); + g->nprobe_bytes >= IEEE80211_MIRA_MIN_PROBE_BYTES || + mn->txfail >= IEEE80211_MIRA_MAX_PROBE_TXFAIL || + mn->retries >= IEEE80211_MIRA_MAX_PROBE_RETRIES); } void
Re: ssh_config.5: refer consistently to HostName, not Hostname
On Tue, Jun 11 2019 14:40:03 +0100, Jason McIntyre wrote: > On Tue, Jun 11, 2019 at 03:11:03PM +0300, Lauri Tirkkonen wrote: > > Hi, trivial manpage diff. > > > > fixed, thanks. > jmc I noticed the followup commit that changes the canonical option name to 'Hostname'. I also like that capitalization better, but your diff seems to have been a bit incomplete; in particular the option should be called oHostname in code also, and a couple manpage mentions of HostName still remain. X11UseLocalhost is used consistently in comparison. diff --git a/usr.bin/ssh/clientloop.c b/usr.bin/ssh/clientloop.c index fdcca0e25ed..fd43ee4cbb4 100644 --- a/usr.bin/ssh/clientloop.c +++ b/usr.bin/ssh/clientloop.c @@ -121,7 +121,7 @@ extern int muxserver_sock; /* XXX use mux_client_cleanup() instead */ /* * Name of the host we are connecting to. This is the name given on the - * command line, or the HostName specified for the user-supplied name in a + * command line, or the Hostname specified for the user-supplied name in a * configuration file. */ extern char *host; diff --git a/usr.bin/ssh/readconf.c b/usr.bin/ssh/readconf.c index 27638663b78..5b9c97b7c8d 100644 --- a/usr.bin/ssh/readconf.c +++ b/usr.bin/ssh/readconf.c @@ -71,7 +71,7 @@ User foo Host fake.com - HostName another.host.name.real.org + Hostname another.host.name.real.org User blaah Port 34289 ForwardX11 no @@ -133,7 +133,7 @@ typedef enum { oGatewayPorts, oExitOnForwardFailure, oPasswordAuthentication, oRSAAuthentication, oChallengeResponseAuthentication, oXAuthLocation, - oIdentityFile, oHostName, oPort, oCipher, oRemoteForward, oLocalForward, + oIdentityFile, oHostname, oPort, oCipher, oRemoteForward, oLocalForward, oCertificateFile, oAddKeysToAgent, oIdentityAgent, oUser, oEscapeChar, oRhostsRSAAuthentication, oProxyCommand, oGlobalKnownHostsFile, oUserKnownHostsFile, oConnectionAttempts, @@ -225,7 +225,7 @@ static struct { { "certificatefile", oCertificateFile }, { "addkeystoagent", oAddKeysToAgent }, { "identityagent", oIdentityAgent }, - { "hostname", oHostName }, + { "hostname", oHostname }, { "hostkeyalias", oHostKeyAlias }, { "proxycommand", oProxyCommand }, { "port", oPort }, @@ -1102,7 +1102,7 @@ parse_char_array: max_entries = SSH_MAX_HOSTS_FILES; goto parse_char_array; - case oHostName: + case oHostname: charptr = &options->hostname; goto parse_string; @@ -2576,7 +2576,7 @@ dump_client_config(Options *o, const char *host) /* Most interesting options first: user, host, port */ dump_cfg_string(oUser, o->user); - dump_cfg_string(oHostName, host); + dump_cfg_string(oHostname, host); dump_cfg_int(oPort, o->port); /* Flag options */ diff --git a/usr.bin/ssh/scp.1 b/usr.bin/ssh/scp.1 index a2833dab041..29e97808ed1 100644 --- a/usr.bin/ssh/scp.1 +++ b/usr.bin/ssh/scp.1 @@ -164,7 +164,7 @@ For full details of the options listed below, and their possible values, see .It HostbasedKeyTypes .It HostKeyAlgorithms .It HostKeyAlias -.It HostName +.It Hostname .It IdentitiesOnly .It IdentityAgent .It IdentityFile diff --git a/usr.bin/ssh/sftp.1 b/usr.bin/ssh/sftp.1 index 259095885a7..a6757705941 100644 --- a/usr.bin/ssh/sftp.1 +++ b/usr.bin/ssh/sftp.1 @@ -241,7 +241,7 @@ For full details of the options listed below, and their possible values, see .It HostbasedKeyTypes .It HostKeyAlgorithms .It HostKeyAlias -.It HostName +.It Hostname .It IdentitiesOnly .It IdentityAgent .It IdentityFile diff --git a/usr.bin/ssh/ssh.1 b/usr.bin/ssh/ssh.1 index 9480eba8d3e..4c2c101d765 100644 --- a/usr.bin/ssh/ssh.1 +++ b/usr.bin/ssh/ssh.1 @@ -504,7 +504,7 @@ For full details of the options listed below, and their possible values, see .It HostbasedKeyTypes .It HostKeyAlgorithms .It HostKeyAlias -.It HostName +.It Hostname .It IdentitiesOnly .It IdentityAgent .It IdentityFile diff --git a/usr.bin/ssh/ssh.c b/usr.bin/ssh/ssh.c index defa3c1c337..aeefbe4f91c 100644 --- a/usr.bin/ssh/ssh.c +++ b/usr.bin/ssh/ssh.c @@ -146,7 +146,7 @@ char *config = NULL; /* * Name of the host we are connecting to. This is the name given on the - * command line, or the HostName specified for the user-supplied name in a + * command line, or the Hostname specified for the user-supplied name in a * configuration file. */ char *host; diff --git a/usr.bin/ssh/ssh_config.5 b/usr.bin/ssh/ssh_config.5 index 7bb6cde7ae1..719288b03a5 100644 --- a/usr.bin/ssh/ssh_config.5 +++ b/usr.bin/ssh/ssh_config.5 @@ -1223,8 +1223,8 @@ server running on some machine, or execute .Ic sshd -i somewhere. Host key management will be done using the -HostName of the host being connected (defaulting to the name typed by -the user). +.Cm Hostname +of the host being connected (defaulting to the name typed by the u
Re: [patch] use acme-client to sign certificated with ecdsa keys
On Wed, Jun 12, 2019 at 08:12:04AM +0200, Florian Obser wrote: > > I had a look yesterday and it looks mostly OK. > Something came up and I won't be around the next days. > > Someone can put it and and we can tweak it in tree or we wait a few > days. > okie dokie, will commit tonight when I get home unless someone beats me to it :-) > On Tue, Jun 11, 2019 at 01:37:24PM +0200, Renaud Allard wrote: > > > > > > On 6/11/19 10:17 AM, Renaud Allard wrote: > > > > > > Hello, > > > > > > Here is a patch with ecdsa and rsa in %token after the domain key name > > > > > > OK? comments? > > > > I just made a small modification in the formatting of acme.conf man page, > > putting keytype as an arg. And also a cleaner key.h > > > > OK? > > > Index: Makefile > > === > > RCS file: /cvs/src/usr.sbin/acme-client/Makefile,v > > retrieving revision 1.8 > > diff -u -p -r1.8 Makefile > > --- Makefile3 Jul 2017 22:21:47 - 1.8 > > +++ Makefile11 Jun 2019 11:35:24 - > > @@ -2,7 +2,7 @@ > > PROG= acme-client > > SRCS= acctproc.c base64.c certproc.c chngproc.c dbg.c > > dnsproc.c > > SRCS+= fileproc.c http.c jsmn.c json.c keyproc.c main.c > > netproc.c > > -SRCS+= parse.y revokeproc.c rsa.c util.c > > +SRCS+= parse.y revokeproc.c key.c util.c > > > > MAN= acme-client.1 acme-client.conf.5 > > > > Index: acctproc.c > > === > > RCS file: /cvs/src/usr.sbin/acme-client/acctproc.c,v > > retrieving revision 1.14 > > diff -u -p -r1.14 acctproc.c > > --- acctproc.c 8 Jun 2019 07:52:55 - 1.14 > > +++ acctproc.c 11 Jun 2019 11:35:24 - > > @@ -29,7 +29,7 @@ > > #include > > > > #include "extern.h" > > -#include "rsa.h" > > +#include "key.h" > > > > /* > > * Converts a BIGNUM to the form used in JWK. > > @@ -352,7 +352,9 @@ acctproc(int netsock, const char *acctke > > goto out; > > dodbg("%s: generated RSA account key", acctkey); > > } else { > > - if ((pkey = rsa_key_load(f, acctkey)) == NULL) > > + if ((pkey = key_load(f, acctkey)) == NULL) > > + goto out; > > + if (EVP_PKEY_type(pkey->type) != EVP_PKEY_RSA) > > goto out; > > doddbg("%s: loaded RSA account key", acctkey); > > } > > Index: acme-client.conf.5 > > === > > RCS file: /cvs/src/usr.sbin/acme-client/acme-client.conf.5,v > > retrieving revision 1.17 > > diff -u -p -r1.17 acme-client.conf.5 > > --- acme-client.conf.5 8 Jan 2019 06:46:29 - 1.17 > > +++ acme-client.conf.5 11 Jun 2019 11:35:24 - > > @@ -109,8 +109,10 @@ Specify a list of alternative names for > > The common name is included automatically if this option is present, > > but there is no automatic conversion/inclusion between "www." and > > plain domain name forms. > > -.It Ic domain key Ar file > > +.It Ic domain key Ar file Op Ar keytype > > The private key file for which the certificate will be obtained. > > +.Ar keytype > > +can be rsa or ecdsa. Defaults to rsa. > > .It Ic domain certificate Ar file > > The filename of the certificate that will be issued. > > This is optional if > > Index: extern.h > > === > > RCS file: /cvs/src/usr.sbin/acme-client/extern.h,v > > retrieving revision 1.12 > > diff -u -p -r1.12 extern.h > > --- extern.h8 Jun 2019 07:52:55 - 1.12 > > +++ extern.h11 Jun 2019 11:35:24 - > > @@ -276,6 +276,11 @@ char *json_fmt_signed(const char *, con > > int verbose; > > > > /* > > + * Should we switch to ecdsa? > > + */ > > +intecdsa; > > + > > +/* > > * What component is the process within (COMP__MAX for none)? > > */ > > enum comp proccomp; > > Index: key.c > > === > > RCS file: key.c > > diff -N key.c > > --- /dev/null 1 Jan 1970 00:00:00 - > > +++ key.c 11 Jun 2019 11:35:24 - > > @@ -0,0 +1,149 @@ > > +/* $Id: rsa.c,v 1.7 2018/07/28 15:25:23 tb Exp $ */ > > +/* > > + * Copyright (c) 2019 Renaud Allard > > + * Copyright (c) 2016 Kristaps Dzonsons > > + * > > + * Permission to use, copy, modify, and distribute this software for any > > + * purpose with or without fee is hereby granted, provided that the above > > + * copyright notice and this permission notice appear in all copies. > > + * > > + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHORS DISCLAIM ALL WARRANTIES > > + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF > > + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHORS BE LIABLE FOR > > + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES
Re: drm(4), multi-threaded task queues and barriers
> Date: Wed, 12 Jun 2019 17:04:10 +1000 > From: Jonathan Gray > > On Tue, Jun 11, 2019 at 09:10:46PM +0200, Mark Kettenis wrote: > > The drm(4) codebase really needs multi-threaded task queues since the > > code has taks that wait for the completion of other tasks that are > > submitted to the same task queue. Thank you Linux... > > > > Unfortunately the code also needs to wait for the completion of > > submitted tasks from other threads. This implemented using > > task_barrier(9). But unfortunately our current implementation only > > works for single-threaded task queues. > > > > The diff below fixes this by marking the barrier task with a flag and > > making sure that all threads of the task queue are syncchronized. > > This achived through a TASK_BARRIER flag that simply blocks the > > threads until the last unblocked thread sees the flag and executes the > > task. > > > > The diff also starts 4 threads for each workqueue that gets created by > > the drm(4) layer. The number 4 is a bit arbitrary but it is the > > number of threads that Linux creates per CPU for a so-called "unbound" > > workqueue which hopefully is enough to always make progress. > > Ignoring taskletq and systq use in burner_task/switchtask across > drivers and the local only backlight_schedule_update_status(), was it > intentional to exclude: > > workqueue.h: alloc_workqueue() > struct taskq *tq = taskq_create(name, 1, IPL_TTY, 0); > workqueue.h: alloc_ordered_workqueue() > struct taskq *tq = taskq_create(name, 1, IPL_TTY, 0); > workqueue.h: > #define system_power_efficient_wq ((struct workqueue_struct *)systq) Not entirely. But I don't want to spawn too many threads... If this diff seems to work, we might want to tweak the numbers a bit to see what works best. Whatever we do, the ordered workqueues need to stay single-threaded. > > Please test. If you experience a "hang" with this diff, please try to > > log in to the machine remotely over ssh and send me and jsg@ the > > output of "ps -AHwlk". > > > > Thanks, > > > > Mark > > > > > > Index: dev/pci/drm/drm_linux.c > > === > > RCS file: /cvs/src/sys/dev/pci/drm/drm_linux.c,v > > retrieving revision 1.38 > > diff -u -p -r1.38 drm_linux.c > > --- dev/pci/drm/drm_linux.c 9 Jun 2019 12:58:30 - 1.38 > > +++ dev/pci/drm/drm_linux.c 11 Jun 2019 18:54:38 - > > @@ -1399,15 +1399,15 @@ drm_linux_init(void) > > { > > if (system_wq == NULL) { > > system_wq = (struct workqueue_struct *) > > - taskq_create("drmwq", 1, IPL_HIGH, 0); > > + taskq_create("drmwq", 4, IPL_HIGH, 0); > > } > > if (system_unbound_wq == NULL) { > > system_unbound_wq = (struct workqueue_struct *) > > - taskq_create("drmubwq", 1, IPL_HIGH, 0); > > + taskq_create("drmubwq", 4, IPL_HIGH, 0); > > } > > if (system_long_wq == NULL) { > > system_long_wq = (struct workqueue_struct *) > > - taskq_create("drmlwq", 1, IPL_HIGH, 0); > > + taskq_create("drmlwq", 4, IPL_HIGH, 0); > > } > > > > if (taskletq == NULL) > > Index: dev/pci/drm/i915/intel_hotplug.c > > === > > RCS file: /cvs/src/sys/dev/pci/drm/i915/intel_hotplug.c,v > > retrieving revision 1.2 > > diff -u -p -r1.2 intel_hotplug.c > > --- dev/pci/drm/i915/intel_hotplug.c14 Apr 2019 10:14:52 - > > 1.2 > > +++ dev/pci/drm/i915/intel_hotplug.c11 Jun 2019 18:54:38 - > > @@ -619,7 +619,6 @@ void intel_hpd_init_work(struct drm_i915 > > INIT_WORK(&dev_priv->hotplug.hotplug_work, i915_hotplug_work_func); > > INIT_WORK(&dev_priv->hotplug.dig_port_work, i915_digport_work_func); > > INIT_WORK(&dev_priv->hotplug.poll_init_work, i915_hpd_poll_init_work); > > - dev_priv->hotplug.poll_init_work.tq = systq; > > INIT_DELAYED_WORK(&dev_priv->hotplug.reenable_work, > > intel_hpd_irq_storm_reenable_work); > > } > > Index: kern/kern_task.c > > === > > RCS file: /cvs/src/sys/kern/kern_task.c,v > > retrieving revision 1.25 > > diff -u -p -r1.25 kern_task.c > > --- kern/kern_task.c28 Apr 2019 04:20:40 - 1.25 > > +++ kern/kern_task.c11 Jun 2019 18:54:39 - > > @@ -43,6 +43,7 @@ struct taskq { > > TQ_S_DESTROYED > > }tq_state; > > unsigned int tq_running; > > + unsigned int tq_barrier; > > unsigned int tq_nthreads; > > unsigned int tq_flags; > > const char *tq_name; > > @@ -59,6 +60,7 @@ static const char taskq_sys_name[] = "sy > > struct taskq taskq_sys = { > > TQ_S_CREATED, > > 0, > > + 0, > > 1, > > 0, > > taskq_sys_name, > > @@ -77,6 +79,7 @@ static const char taskq_sys_mp_nam
Re: drm(4), multi-threaded task queues and barriers
On Tue, Jun 11, 2019 at 09:10:46PM +0200, Mark Kettenis wrote: > The drm(4) codebase really needs multi-threaded task queues since the > code has taks that wait for the completion of other tasks that are > submitted to the same task queue. Thank you Linux... > > Unfortunately the code also needs to wait for the completion of > submitted tasks from other threads. This implemented using > task_barrier(9). But unfortunately our current implementation only > works for single-threaded task queues. > > The diff below fixes this by marking the barrier task with a flag and > making sure that all threads of the task queue are syncchronized. > This achived through a TASK_BARRIER flag that simply blocks the > threads until the last unblocked thread sees the flag and executes the > task. > > The diff also starts 4 threads for each workqueue that gets created by > the drm(4) layer. The number 4 is a bit arbitrary but it is the > number of threads that Linux creates per CPU for a so-called "unbound" > workqueue which hopefully is enough to always make progress. Ignoring taskletq and systq use in burner_task/switchtask across drivers and the local only backlight_schedule_update_status(), was it intentional to exclude: workqueue.h: alloc_workqueue() struct taskq *tq = taskq_create(name, 1, IPL_TTY, 0); workqueue.h: alloc_ordered_workqueue() struct taskq *tq = taskq_create(name, 1, IPL_TTY, 0); workqueue.h: #define system_power_efficient_wq ((struct workqueue_struct *)systq) > > Please test. If you experience a "hang" with this diff, please try to > log in to the machine remotely over ssh and send me and jsg@ the > output of "ps -AHwlk". > > Thanks, > > Mark > > > Index: dev/pci/drm/drm_linux.c > === > RCS file: /cvs/src/sys/dev/pci/drm/drm_linux.c,v > retrieving revision 1.38 > diff -u -p -r1.38 drm_linux.c > --- dev/pci/drm/drm_linux.c 9 Jun 2019 12:58:30 - 1.38 > +++ dev/pci/drm/drm_linux.c 11 Jun 2019 18:54:38 - > @@ -1399,15 +1399,15 @@ drm_linux_init(void) > { > if (system_wq == NULL) { > system_wq = (struct workqueue_struct *) > - taskq_create("drmwq", 1, IPL_HIGH, 0); > + taskq_create("drmwq", 4, IPL_HIGH, 0); > } > if (system_unbound_wq == NULL) { > system_unbound_wq = (struct workqueue_struct *) > - taskq_create("drmubwq", 1, IPL_HIGH, 0); > + taskq_create("drmubwq", 4, IPL_HIGH, 0); > } > if (system_long_wq == NULL) { > system_long_wq = (struct workqueue_struct *) > - taskq_create("drmlwq", 1, IPL_HIGH, 0); > + taskq_create("drmlwq", 4, IPL_HIGH, 0); > } > > if (taskletq == NULL) > Index: dev/pci/drm/i915/intel_hotplug.c > === > RCS file: /cvs/src/sys/dev/pci/drm/i915/intel_hotplug.c,v > retrieving revision 1.2 > diff -u -p -r1.2 intel_hotplug.c > --- dev/pci/drm/i915/intel_hotplug.c 14 Apr 2019 10:14:52 - 1.2 > +++ dev/pci/drm/i915/intel_hotplug.c 11 Jun 2019 18:54:38 - > @@ -619,7 +619,6 @@ void intel_hpd_init_work(struct drm_i915 > INIT_WORK(&dev_priv->hotplug.hotplug_work, i915_hotplug_work_func); > INIT_WORK(&dev_priv->hotplug.dig_port_work, i915_digport_work_func); > INIT_WORK(&dev_priv->hotplug.poll_init_work, i915_hpd_poll_init_work); > - dev_priv->hotplug.poll_init_work.tq = systq; > INIT_DELAYED_WORK(&dev_priv->hotplug.reenable_work, > intel_hpd_irq_storm_reenable_work); > } > Index: kern/kern_task.c > === > RCS file: /cvs/src/sys/kern/kern_task.c,v > retrieving revision 1.25 > diff -u -p -r1.25 kern_task.c > --- kern/kern_task.c 28 Apr 2019 04:20:40 - 1.25 > +++ kern/kern_task.c 11 Jun 2019 18:54:39 - > @@ -43,6 +43,7 @@ struct taskq { > TQ_S_DESTROYED > }tq_state; > unsigned int tq_running; > + unsigned int tq_barrier; > unsigned int tq_nthreads; > unsigned int tq_flags; > const char *tq_name; > @@ -59,6 +60,7 @@ static const char taskq_sys_name[] = "sy > struct taskq taskq_sys = { > TQ_S_CREATED, > 0, > + 0, > 1, > 0, > taskq_sys_name, > @@ -77,6 +79,7 @@ static const char taskq_sys_mp_name[] = > struct taskq taskq_sys_mp = { > TQ_S_CREATED, > 0, > + 0, > 1, > TASKQ_MPSAFE, > taskq_sys_mp_name, > @@ -122,6 +125,7 @@ taskq_create(const char *name, unsigned > > tq->tq_state = TQ_S_CREATED; > tq->tq_running = 0; > + tq->tq_barrier = 0; > tq->tq_nthreads = nthreads; > tq->tq_name = name; > tq->tq_flags = flags; > @@ -161,6 +165,7 @@ taskq_destroy(struct taskq *