Re: Add ioctl for disk cache flush
> > From: David Gwynne > > Date: Mon, 31 Oct 2016 10:38:49 +1000 > > > > > On 31 Oct 2016, at 09:18, Stefan Fritsch wrote: > > > > > > On Sun, 30 Oct 2016, Stefan Fritsch wrote: > > >> I agree with all your comments (and should have reviewed the initial > > >> patch > > >> better). Here is an updated diff. > > > > > > The FWRITE check was missing in wdioctl(). Next try: > > > > the diff looks fine to me. > > > > whats the use of the ioctl though? why do we need extra sync cache things? > > Right. This smells like papering over real bugs. Here's the thoughts I have exchanged with sf. This issue should not be per filesystem, not even per partition. It seems this should keep track of when any partition converts from RW -> R access (either via direct open, or via mount), and if so schedule the required sync operation (to occur sometime soon). That way if many such RW->R transitions occur, they won't serialize and create a performance hit. Furthermore I don't believe userland should have access to this. We've made it 30 years without needing this, why do we need it now.
Re: Add ioctl for disk cache flush
> From: David Gwynne > Date: Mon, 31 Oct 2016 10:38:49 +1000 > > > On 31 Oct 2016, at 09:18, Stefan Fritsch wrote: > > > > On Sun, 30 Oct 2016, Stefan Fritsch wrote: > >> I agree with all your comments (and should have reviewed the initial patch > >> better). Here is an updated diff. > > > > The FWRITE check was missing in wdioctl(). Next try: > > the diff looks fine to me. > > whats the use of the ioctl though? why do we need extra sync cache things? Right. This smells like papering over real bugs.
Re: Add ioctl for disk cache flush
> On 31 Oct 2016, at 09:18, Stefan Fritsch wrote: > > On Sun, 30 Oct 2016, Stefan Fritsch wrote: >> I agree with all your comments (and should have reviewed the initial patch >> better). Here is an updated diff. > > The FWRITE check was missing in wdioctl(). Next try: the diff looks fine to me. whats the use of the ioctl though? why do we need extra sync cache things? dlg > > diff --git sys/dev/ata/wd.c sys/dev/ata/wd.c > index 5e2461f..ca5ac90 100644 > --- sys/dev/ata/wd.c > +++ sys/dev/ata/wd.c > @@ -26,7 +26,7 @@ > */ > > /*- > - * Copyright (c) 1998 The NetBSD Foundation, Inc. > + * Copyright (c) 1998, 2003, 2004 The NetBSD Foundation, Inc. > * All rights reserved. > * > * This code is derived from software contributed to The NetBSD Foundation > @@ -138,7 +138,7 @@ void wdstart(void *); > void __wdstart(struct wd_softc*, struct buf *); > void wdrestart(void *); > int wd_get_params(struct wd_softc *, u_int8_t, struct ataparams *); > -void wd_flushcache(struct wd_softc *, int); > +int wd_flushcache(struct wd_softc *, int); > void wd_standby(struct wd_softc *, int); > > /* XXX: these should go elsewhere */ > @@ -848,6 +848,14 @@ wdioctl(dev_t dev, u_long xfer, caddr_t addr, int flag, > struct proc *p) > } > #endif > > + case DIOCCACHESYNC: > + if ((flag & FWRITE) == 0) { > + error = EBADF; > + goto exit; > + } > + error = wd_flushcache(wd, AT_WAIT); > + goto exit; > + > default: > error = wdc_ioctl(wd->drvp, xfer, addr, flag, p); > goto exit; > @@ -1067,13 +1075,13 @@ wd_get_params(struct wd_softc *wd, u_int8_t flags, > struct ataparams *params) > } > } > > -void > +int > wd_flushcache(struct wd_softc *wd, int flags) > { > struct wdc_command wdc_c; > > if (wd->drvp->ata_vers < 4) /* WDCC_FLUSHCACHE is here since ATA-4 */ > - return; > + return EIO; > bzero(&wdc_c, sizeof(struct wdc_command)); > wdc_c.r_command = (wd->sc_flags & WDF_LBA48 ? WDCC_FLUSHCACHE_EXT : > WDCC_FLUSHCACHE); > @@ -1088,20 +1096,28 @@ wd_flushcache(struct wd_softc *wd, int flags) > if (wdc_exec_command(wd->drvp, &wdc_c) != WDC_COMPLETE) { > printf("%s: flush cache command didn't complete\n", > wd->sc_dev.dv_xname); > + return EIO; > } > + if (wdc_c.flags & ERR_NODEV) > + return ENODEV; > if (wdc_c.flags & AT_TIMEOU) { > printf("%s: flush cache command timeout\n", > wd->sc_dev.dv_xname); > + return EIO; > + } > + if (wdc_c.flags & AT_ERROR) { > + if (wdc_c.r_error == WDCE_ABRT) /* command not supported */ > + return ENODEV; > + printf("%s: flush cache command: error 0x%x\n", > + wd->sc_dev.dv_xname, wdc_c.r_error); > + return EIO; > } > if (wdc_c.flags & AT_DF) { > printf("%s: flush cache command: drive fault\n", > wd->sc_dev.dv_xname); > + return EIO; > } > - /* > - * Ignore error register, it shouldn't report anything else > - * than COMMAND ABORTED, which means the device doesn't support > - * flush cache > - */ > + return 0; > } > > void > diff --git sys/scsi/sd.c sys/scsi/sd.c > index 30fb36b..e5a0139 100644 > --- sys/scsi/sd.c > +++ sys/scsi/sd.c > @@ -2,7 +2,7 @@ > /*$NetBSD: sd.c,v 1.111 1997/04/02 02:29:41 mycroft Exp $ */ > > /*- > - * Copyright (c) 1998 The NetBSD Foundation, Inc. > + * Copyright (c) 1998, 2003, 2004 The NetBSD Foundation, Inc. > * All rights reserved. > * > * This code is derived from software contributed to The NetBSD Foundation > @@ -96,7 +96,7 @@ int sd_vpd_block_limits(struct sd_softc *, int); > int sd_vpd_thin(struct sd_softc *, int); > int sd_thin_params(struct sd_softc *, int); > int sd_get_parms(struct sd_softc *, struct disk_parms *, int); > -void sd_flush(struct sd_softc *, int); > +int sd_flush(struct sd_softc *, int); > > void viscpy(u_char *, u_char *, int); > > @@ -999,6 +999,15 @@ sdioctl(dev_t dev, u_long cmd, caddr_t addr, int flag, > struct proc *p) > error = sd_ioctl_cache(sc, cmd, (struct dk_cache *)addr); > goto exit; > > + case DIOCCACHESYNC: > + if (!ISSET(flag, FWRITE)) { > + error = EBADF; > + goto exit; > + } > + if ((sc->flags & SDF_DIRTY) != 0 || *(int *)addr != 0) > + error = sd_flush(sc, 0); > + goto exit; > + > default: > if (part != RAW_PART) { > error = ENOTTY; > @@ -1876,19 +1885,20 @@ die: > return (SDGP_RESULT_OFFLINE); > } > > -void > +int > sd_flush(struct sd_softc *sc, int flags) > { > struct scsi_link *link; > struct scsi_xfer *xs; >
Re: Add ioctl for disk cache flush
On Sun, 30 Oct 2016, Stefan Fritsch wrote: > I agree with all your comments (and should have reviewed the initial patch > better). Here is an updated diff. The FWRITE check was missing in wdioctl(). Next try: diff --git sys/dev/ata/wd.c sys/dev/ata/wd.c index 5e2461f..ca5ac90 100644 --- sys/dev/ata/wd.c +++ sys/dev/ata/wd.c @@ -26,7 +26,7 @@ */ /*- - * Copyright (c) 1998 The NetBSD Foundation, Inc. + * Copyright (c) 1998, 2003, 2004 The NetBSD Foundation, Inc. * All rights reserved. * * This code is derived from software contributed to The NetBSD Foundation @@ -138,7 +138,7 @@ void wdstart(void *); void __wdstart(struct wd_softc*, struct buf *); void wdrestart(void *); int wd_get_params(struct wd_softc *, u_int8_t, struct ataparams *); -void wd_flushcache(struct wd_softc *, int); +int wd_flushcache(struct wd_softc *, int); void wd_standby(struct wd_softc *, int); /* XXX: these should go elsewhere */ @@ -848,6 +848,14 @@ wdioctl(dev_t dev, u_long xfer, caddr_t addr, int flag, struct proc *p) } #endif + case DIOCCACHESYNC: + if ((flag & FWRITE) == 0) { + error = EBADF; + goto exit; + } + error = wd_flushcache(wd, AT_WAIT); + goto exit; + default: error = wdc_ioctl(wd->drvp, xfer, addr, flag, p); goto exit; @@ -1067,13 +1075,13 @@ wd_get_params(struct wd_softc *wd, u_int8_t flags, struct ataparams *params) } } -void +int wd_flushcache(struct wd_softc *wd, int flags) { struct wdc_command wdc_c; if (wd->drvp->ata_vers < 4) /* WDCC_FLUSHCACHE is here since ATA-4 */ - return; + return EIO; bzero(&wdc_c, sizeof(struct wdc_command)); wdc_c.r_command = (wd->sc_flags & WDF_LBA48 ? WDCC_FLUSHCACHE_EXT : WDCC_FLUSHCACHE); @@ -1088,20 +1096,28 @@ wd_flushcache(struct wd_softc *wd, int flags) if (wdc_exec_command(wd->drvp, &wdc_c) != WDC_COMPLETE) { printf("%s: flush cache command didn't complete\n", wd->sc_dev.dv_xname); + return EIO; } + if (wdc_c.flags & ERR_NODEV) + return ENODEV; if (wdc_c.flags & AT_TIMEOU) { printf("%s: flush cache command timeout\n", wd->sc_dev.dv_xname); + return EIO; + } + if (wdc_c.flags & AT_ERROR) { + if (wdc_c.r_error == WDCE_ABRT) /* command not supported */ + return ENODEV; + printf("%s: flush cache command: error 0x%x\n", + wd->sc_dev.dv_xname, wdc_c.r_error); + return EIO; } if (wdc_c.flags & AT_DF) { printf("%s: flush cache command: drive fault\n", wd->sc_dev.dv_xname); + return EIO; } - /* -* Ignore error register, it shouldn't report anything else -* than COMMAND ABORTED, which means the device doesn't support -* flush cache -*/ + return 0; } void diff --git sys/scsi/sd.c sys/scsi/sd.c index 30fb36b..e5a0139 100644 --- sys/scsi/sd.c +++ sys/scsi/sd.c @@ -2,7 +2,7 @@ /* $NetBSD: sd.c,v 1.111 1997/04/02 02:29:41 mycroft Exp $ */ /*- - * Copyright (c) 1998 The NetBSD Foundation, Inc. + * Copyright (c) 1998, 2003, 2004 The NetBSD Foundation, Inc. * All rights reserved. * * This code is derived from software contributed to The NetBSD Foundation @@ -96,7 +96,7 @@ int sd_vpd_block_limits(struct sd_softc *, int); intsd_vpd_thin(struct sd_softc *, int); intsd_thin_params(struct sd_softc *, int); intsd_get_parms(struct sd_softc *, struct disk_parms *, int); -void sd_flush(struct sd_softc *, int); +intsd_flush(struct sd_softc *, int); void viscpy(u_char *, u_char *, int); @@ -999,6 +999,15 @@ sdioctl(dev_t dev, u_long cmd, caddr_t addr, int flag, struct proc *p) error = sd_ioctl_cache(sc, cmd, (struct dk_cache *)addr); goto exit; + case DIOCCACHESYNC: + if (!ISSET(flag, FWRITE)) { + error = EBADF; + goto exit; + } + if ((sc->flags & SDF_DIRTY) != 0 || *(int *)addr != 0) + error = sd_flush(sc, 0); + goto exit; + default: if (part != RAW_PART) { error = ENOTTY; @@ -1876,19 +1885,20 @@ die: return (SDGP_RESULT_OFFLINE); } -void +int sd_flush(struct sd_softc *sc, int flags) { struct scsi_link *link; struct scsi_xfer *xs; struct scsi_synchronize_cache *cmd; + int error; if (sc->flags & SDF_DYING) - return; + return (ENXIO); link = sc->sc_link; if (link->quirks & SDEV_NOSYNCCACHE) - return; + r
Re: acpivout(4): brightness adjustment quirk
Not sure if it applies to your Dell machine (and probably too obvious), but on my Lenovo lap-top the brightness hotkeys only worked after adjusting a BIOS setting to enable them. On Sun, Oct 30, 2016 at 12:48 PM, Anton Lindqvist wrote: > I'm trying to fix the brightness hotkeys on my Dell Latitude 3160. > Adjusting the brightness using wsconsctl(1) works, as opposed of the > keys. Turns out it's not possible to move the next or previous BCL level > relative to the current one on this particular machine. Instead four > steps is the minimum required in order for the brightness to actually > change. Attached is a patch which finds the number of steps needed in > order to get the brightness to change. Any feedback would be > appreciated. > > Index: acpivout.c > === > RCS file: /cvs/src/sys/dev/acpi/acpivout.c,v > retrieving revision 1.12 > diff -u -p -r1.12 acpivout.c > --- acpivout.c 29 Mar 2016 17:52:04 - 1.12 > +++ acpivout.c 30 Oct 2016 14:05:52 - > @@ -57,6 +57,7 @@ struct acpivout_softc { > > int *sc_bcl; > size_t sc_bcl_len; > + size_t sc_bcl_step; > }; > > void acpivout_brightness_cycle(struct acpivout_softc *); > @@ -178,8 +179,11 @@ acpivout_brightness_up(struct acpivout_s > if (cur_level == sc->sc_bcl[sc->sc_bcl_len - 1]) > return; > > - for (i = 0; i < sc->sc_bcl_len && cur_level != sc->sc_bcl[i]; i++); > - acpivout_set_brightness(sc, sc->sc_bcl[i + 1]); > + for (i = 0; i < sc->sc_bcl_len && cur_level != sc->sc_bcl[i]; i++) > + continue; > + KASSERT(i < sc->sc_bcl_len - 1); > + i += i + sc->sc_bcl_step < sc->sc_bcl_len ? sc->sc_bcl_step : 1; > + acpivout_set_brightness(sc, sc->sc_bcl[i]); > } > > void > @@ -197,8 +201,11 @@ acpivout_brightness_down(struct acpivout > if (cur_level == sc->sc_bcl[0]) > return; > > - for (i = 0; i < sc->sc_bcl_len && cur_level != sc->sc_bcl[i]; i++); > - acpivout_set_brightness(sc, sc->sc_bcl[i - 1]); > + for (i = 0; i < sc->sc_bcl_len && cur_level != sc->sc_bcl[i]; i++) > + continue; > + KASSERT(i > 0); > + i -= i > sc->sc_bcl_step ? sc->sc_bcl_step : 1; > + acpivout_set_brightness(sc, sc->sc_bcl[i]); > } > > void > @@ -262,8 +269,8 @@ acpivout_set_brightness(struct acpivout_ > void > acpivout_get_bcl(struct acpivout_softc *sc) > { > - int i, j, value; > - struct aml_value res; > + struct aml_valueres; > + int cur_level, i, inc, j, value; > > DPRINTF(("Getting _BCL!")); > aml_evalname(sc->sc_acpi, sc->sc_devnode, "_BCL", 0, NULL, &res); > @@ -295,6 +302,30 @@ acpivout_get_bcl(struct acpivout_softc * > sc->sc_bcl[j] = sc->sc_bcl[j - 1]; > sc->sc_bcl[j] = value; > } > + > + /* > +* Some implementations doesn't change the brightness when moving > to the > +* next or previous BCL value relative to the current one. > Calculate the > +* step needed by incrementally adjusting the brightness. > +*/ > + cur_level = acpivout_get_brightness(sc); > + for (i = 0; i < sc->sc_bcl_len && cur_level != sc->sc_bcl[i]; i++) > + continue; > + inc = i < sc->sc_bcl_len/2 ? 1 : -1; > + sc->sc_bcl_step = 1; > + for (;;) { > + i += inc; > + if (i < 0 || i >= sc->sc_bcl_len) > + break; > + > + acpivout_set_brightness(sc, sc->sc_bcl[i]); > + if (acpivout_get_brightness(sc) != cur_level) > + break; > + > + sc->sc_bcl_step++; > + } > + /* Restore brightness */ > + acpivout_set_brightness(sc, cur_level); > > err: > aml_freevalue(&res); > >
Re: step missing from "2016/10/14" followig -current?
Peter Hessler writes: > On 2016 Oct 30 (Sun) at 12:28:36 +0100 (+0100), Ingo Schwarze wrote: > :> Everybody would have had, had it been included in /etc/skel/.cvsrc :^) > :> Worth adding? > : > :I (mildly) oppose that. Providing customization files by default > :is an oxymoron that provokes exactly the problems seen here. People > :become so used to it that communication breaks down - some people > :assume the default customization to be the default, others assume > :the defaults to be the defaults. These default customization files > :are also undocumented, so they cause default behaviour of utilities > :to differ from what is described in the manpages. At least we should > :not pile more poo on top of the turd. > : > :We want sane defaults, and we want simple defaults. Where traditional > :defaults are suboptimal but can't easily be changed, i still think > :that having one set of defaults is saner and simpler than having > :two sets of defaults: One traditional, documented plus one arguably > :more convenient, but undocumented set... > : > :Whatever, i expect others will disagree, so i'm unlikely to argue > :this further, and i can certainly live with whatever happens in > :/etc/skel/, i never use it anyway. > : > :Yours, > : Ingo > > On one hand, I 100% agree with Ingo. Our instructions MUST NOT assume > configuration files / environment have things we depend on. > > On the other hand, if we do provide environment files, we MUST ensure > they are sane. adding -d to cvs update is pretty much required when > using OpenBSD's cvs tree. Agreed. > > :> Index: etc/skel/dot.cvsrc > :> === > :> RCS file: /cvs/src/etc/skel/dot.cvsrc,v > :> retrieving revision 1.2 > :> diff -u -p -r1.2 dot.cvsrc > :> --- etc/skel/dot.cvsrc 31 Mar 2015 18:34:40 - 1.2 > :> +++ etc/skel/dot.cvsrc 29 Oct 2016 17:44:30 - > :> @@ -1,6 +1,6 @@ > :> # $OpenBSD: dot.cvsrc,v 1.2 2015/03/31 18:34:40 naddy Exp $ > :> # > :> diff -uNp > :> -update -P > :> +update -Pd > :> checkout -P > :> rdiff -u > : > > OK Also ok jca@ -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Re: update net/py-pcapy
Peter Hessler writes: > On 2016 Oct 30 (Sun) at 15:17:34 +0100 (+0100), Jeremie Courreges-Anglas > wrote: > :Peter Hessler writes: > : > :> I would like to use pcapy in a python3 module, so we need to update it. > :> > :> However, when I run the test program, I get the following error: > :> > :> python2.7:/usr/local/lib/python2.7/site-packages/pcapy.so: undefined > :> symbol '_Z10bpf_filterPK8bpf_insnPKhjj' > :> Traceback (most recent call last): > :> File "/usr/local/tests/pcapytests.py", line 8, in > :> import pcapy > :> ImportError: Cannot load specified object > :> > :> Based on what I can tell, pcapy.so is linked against libpcap, which > :> should provide those symbols. > :> > :> $ ldd /usr/local/lib/python2.7/site-packages/pcapy.so > :> /usr/local/lib/python2.7/site-packages/pcapy.so: > :> StartEnd Type Open Ref GrpRef Name > :> 10ffb9019000 10ffb921f000 dlib 20 0 > /usr/local/lib/python2.7/site-packages/pcapy.so > :> 10ff88f9c000 10ff891c1000 rlib 01 0 > /usr/lib/libpcap.so.8.1 > :> 10ff5d7d6000 10ff5daed000 rlib 01 0 > /usr/lib/libstdc++.so.57.0 > :> 10ff9925f000 10ff99636000 rlib 01 0 > /usr/local/lib/libpython2.7.so.0.0 > :> 10fee0ee9000 10fee111 rlib 03 0 > /usr/lib/libm.so.10.0 > :> 10ff63fa3000 10ff641b2000 rlib 01 0 > /usr/lib/libpthread.so.23.0 > :> 10fef2919000 10fef2b26000 rlib 01 0 > /usr/lib/libutil.so.12.1 > :> > :> Ideas? > : > :C++ name mangling is affecting the functions declared in net/bpf.h. > :Probably a __BEGIN_DECLS / __END_DECLS pair missing. > : > :-- > :jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE > : > > Yup, that was it. > > How risky is that? Should this be tested in a bulk? The potential for breakage seems low to me. A failure case I can think of: some c++ port now correctly detects the function, activates some code, but this code fails to build for whatever reason. > (moved to tech@) OK? ok jca@ I wonder whether _bpf_filter should be exposed at all. > > Index: sys/net/bpf.h > === > RCS file: /cvs/openbsd/src/sys/net/bpf.h,v > retrieving revision 1.58 > diff -u -p -u -p -r1.58 bpf.h > --- sys/net/bpf.h 12 Sep 2016 16:24:37 - 1.58 > +++ sys/net/bpf.h 30 Oct 2016 16:21:32 - > @@ -276,11 +276,13 @@ struct bpf_ops { > #define BPF_STMT(code, k) { (u_int16_t)(code), 0, 0, k } > #define BPF_JUMP(code, k, jt, jf) { (u_int16_t)(code), jt, jf, k } > > +__BEGIN_DECLS > u_int bpf_filter(const struct bpf_insn *, const u_char *, u_int, > u_int) > __bounded((__buffer__, 2, 4)); > > u_int _bpf_filter(const struct bpf_insn *, const struct bpf_ops *, >const void *, u_int); > +__END_DECLS > > #ifdef _KERNEL > struct ifnet; -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Re: update net/py-pcapy
On 2016 Oct 30 (Sun) at 15:17:34 +0100 (+0100), Jeremie Courreges-Anglas wrote: :Peter Hessler writes: : :> I would like to use pcapy in a python3 module, so we need to update it. :> :> However, when I run the test program, I get the following error: :> :> python2.7:/usr/local/lib/python2.7/site-packages/pcapy.so: undefined :> symbol '_Z10bpf_filterPK8bpf_insnPKhjj' :> Traceback (most recent call last): :> File "/usr/local/tests/pcapytests.py", line 8, in :> import pcapy :> ImportError: Cannot load specified object :> :> Based on what I can tell, pcapy.so is linked against libpcap, which :> should provide those symbols. :> :> $ ldd /usr/local/lib/python2.7/site-packages/pcapy.so :> /usr/local/lib/python2.7/site-packages/pcapy.so: :> StartEnd Type Open Ref GrpRef Name :> 10ffb9019000 10ffb921f000 dlib 20 0 /usr/local/lib/python2.7/site-packages/pcapy.so :> 10ff88f9c000 10ff891c1000 rlib 01 0 /usr/lib/libpcap.so.8.1 :> 10ff5d7d6000 10ff5daed000 rlib 01 0 /usr/lib/libstdc++.so.57.0 :> 10ff9925f000 10ff99636000 rlib 01 0 /usr/local/lib/libpython2.7.so.0.0 :> 10fee0ee9000 10fee111 rlib 03 0 /usr/lib/libm.so.10.0 :> 10ff63fa3000 10ff641b2000 rlib 01 0 /usr/lib/libpthread.so.23.0 :> 10fef2919000 10fef2b26000 rlib 01 0 /usr/lib/libutil.so.12.1 :> :> Ideas? : :C++ name mangling is affecting the functions declared in net/bpf.h. :Probably a __BEGIN_DECLS / __END_DECLS pair missing. : :-- :jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE : Yup, that was it. How risky is that? Should this be tested in a bulk? (moved to tech@) OK? Index: sys/net/bpf.h === RCS file: /cvs/openbsd/src/sys/net/bpf.h,v retrieving revision 1.58 diff -u -p -u -p -r1.58 bpf.h --- sys/net/bpf.h 12 Sep 2016 16:24:37 - 1.58 +++ sys/net/bpf.h 30 Oct 2016 16:21:32 - @@ -276,11 +276,13 @@ struct bpf_ops { #define BPF_STMT(code, k) { (u_int16_t)(code), 0, 0, k } #define BPF_JUMP(code, k, jt, jf) { (u_int16_t)(code), jt, jf, k } +__BEGIN_DECLS u_int bpf_filter(const struct bpf_insn *, const u_char *, u_int, u_int) __bounded((__buffer__, 2, 4)); u_int _bpf_filter(const struct bpf_insn *, const struct bpf_ops *, const void *, u_int); +__END_DECLS #ifdef _KERNEL struct ifnet; -- Every program has two purposes -- one for which it was written and another for which it wasn't.
acpivout(4): brightness adjustment quirk
I'm trying to fix the brightness hotkeys on my Dell Latitude 3160. Adjusting the brightness using wsconsctl(1) works, as opposed of the keys. Turns out it's not possible to move the next or previous BCL level relative to the current one on this particular machine. Instead four steps is the minimum required in order for the brightness to actually change. Attached is a patch which finds the number of steps needed in order to get the brightness to change. Any feedback would be appreciated. Index: acpivout.c === RCS file: /cvs/src/sys/dev/acpi/acpivout.c,v retrieving revision 1.12 diff -u -p -r1.12 acpivout.c --- acpivout.c 29 Mar 2016 17:52:04 - 1.12 +++ acpivout.c 30 Oct 2016 14:05:52 - @@ -57,6 +57,7 @@ struct acpivout_softc { int *sc_bcl; size_t sc_bcl_len; + size_t sc_bcl_step; }; void acpivout_brightness_cycle(struct acpivout_softc *); @@ -178,8 +179,11 @@ acpivout_brightness_up(struct acpivout_s if (cur_level == sc->sc_bcl[sc->sc_bcl_len - 1]) return; - for (i = 0; i < sc->sc_bcl_len && cur_level != sc->sc_bcl[i]; i++); - acpivout_set_brightness(sc, sc->sc_bcl[i + 1]); + for (i = 0; i < sc->sc_bcl_len && cur_level != sc->sc_bcl[i]; i++) + continue; + KASSERT(i < sc->sc_bcl_len - 1); + i += i + sc->sc_bcl_step < sc->sc_bcl_len ? sc->sc_bcl_step : 1; + acpivout_set_brightness(sc, sc->sc_bcl[i]); } void @@ -197,8 +201,11 @@ acpivout_brightness_down(struct acpivout if (cur_level == sc->sc_bcl[0]) return; - for (i = 0; i < sc->sc_bcl_len && cur_level != sc->sc_bcl[i]; i++); - acpivout_set_brightness(sc, sc->sc_bcl[i - 1]); + for (i = 0; i < sc->sc_bcl_len && cur_level != sc->sc_bcl[i]; i++) + continue; + KASSERT(i > 0); + i -= i > sc->sc_bcl_step ? sc->sc_bcl_step : 1; + acpivout_set_brightness(sc, sc->sc_bcl[i]); } void @@ -262,8 +269,8 @@ acpivout_set_brightness(struct acpivout_ void acpivout_get_bcl(struct acpivout_softc *sc) { - int i, j, value; - struct aml_value res; + struct aml_valueres; + int cur_level, i, inc, j, value; DPRINTF(("Getting _BCL!")); aml_evalname(sc->sc_acpi, sc->sc_devnode, "_BCL", 0, NULL, &res); @@ -295,6 +302,30 @@ acpivout_get_bcl(struct acpivout_softc * sc->sc_bcl[j] = sc->sc_bcl[j - 1]; sc->sc_bcl[j] = value; } + + /* +* Some implementations doesn't change the brightness when moving to the +* next or previous BCL value relative to the current one. Calculate the +* step needed by incrementally adjusting the brightness. +*/ + cur_level = acpivout_get_brightness(sc); + for (i = 0; i < sc->sc_bcl_len && cur_level != sc->sc_bcl[i]; i++) + continue; + inc = i < sc->sc_bcl_len/2 ? 1 : -1; + sc->sc_bcl_step = 1; + for (;;) { + i += inc; + if (i < 0 || i >= sc->sc_bcl_len) + break; + + acpivout_set_brightness(sc, sc->sc_bcl[i]); + if (acpivout_get_brightness(sc) != cur_level) + break; + + sc->sc_bcl_step++; + } + /* Restore brightness */ + acpivout_set_brightness(sc, cur_level); err: aml_freevalue(&res);
Re: step missing from "2016/10/14" followig -current?
On 2016 Oct 30 (Sun) at 12:28:36 +0100 (+0100), Ingo Schwarze wrote: :> Everybody would have had, had it been included in /etc/skel/.cvsrc :^) :> Worth adding? : :I (mildly) oppose that. Providing customization files by default :is an oxymoron that provokes exactly the problems seen here. People :become so used to it that communication breaks down - some people :assume the default customization to be the default, others assume :the defaults to be the defaults. These default customization files :are also undocumented, so they cause default behaviour of utilities :to differ from what is described in the manpages. At least we should :not pile more poo on top of the turd. : :We want sane defaults, and we want simple defaults. Where traditional :defaults are suboptimal but can't easily be changed, i still think :that having one set of defaults is saner and simpler than having :two sets of defaults: One traditional, documented plus one arguably :more convenient, but undocumented set... : :Whatever, i expect others will disagree, so i'm unlikely to argue :this further, and i can certainly live with whatever happens in :/etc/skel/, i never use it anyway. : :Yours, : Ingo On one hand, I 100% agree with Ingo. Our instructions MUST NOT assume configuration files / environment have things we depend on. On the other hand, if we do provide environment files, we MUST ensure they are sane. adding -d to cvs update is pretty much required when using OpenBSD's cvs tree. :> Index: etc/skel/dot.cvsrc :> === :> RCS file: /cvs/src/etc/skel/dot.cvsrc,v :> retrieving revision 1.2 :> diff -u -p -r1.2 dot.cvsrc :> --- etc/skel/dot.cvsrc 31 Mar 2015 18:34:40 - 1.2 :> +++ etc/skel/dot.cvsrc 29 Oct 2016 17:44:30 - :> @@ -1,6 +1,6 @@ :> # $OpenBSD: dot.cvsrc,v 1.2 2015/03/31 18:34:40 naddy Exp $ :> # :> diff -uNp :> -update -P :> +update -Pd :> checkout -P :> rdiff -u : OK -- Man 1: Ask me what the most important thing about telling a good joke is. Man 2: OK, what is the most impo -- Man 1: TIMING!
Re: step missing from "2016/10/14" followig -current?
Hi, Raf Czlonka wrote on Sat, Oct 29, 2016 at 06:46:56PM +0100: > On Sat, Oct 29, 2016 at 08:39:47AM BST, Theo Buehler wrote: >> I suppose everybody has 'update -Pd' in their ~/.cvsrc schwarze@isnote $ cat ~/.cvsrc ~/.Xdefaults ~/.login ~/.mailrc ~/.profile cat: /home/schwarze/.cvsrc: No such file or directory cat: /home/schwarze/.Xdefaults: No such file or directory cat: /home/schwarze/.login: No such file or directory cat: /home/schwarze/.mailrc: No such file or directory cat: /home/schwarze/.profile: No such file or directory I'm probably repeating myself, but there are people who avoid as much customization as they can, because customizing stuff makes it harder to maintain systems and to work on unfamiliar systems. In my personal opinion, saved keystrokes are not worth the above downsides, finger memory does the trick just fine. (I know that many disagree, of course - no problem with that. I don't say that support for customization files should be deleted.) >> or automatically knew to give the -d flag to create directories... >> 'cvs up' should probably be expanded to the full 'cvs -q up -Pd' >> on that page. I'll fix it later when I'm on a good machine if >> nobody beats me to it. Thanks for fixing it. > Everybody would have had, had it been included in /etc/skel/.cvsrc :^) > Worth adding? I (mildly) oppose that. Providing customization files by default is an oxymoron that provokes exactly the problems seen here. People become so used to it that communication breaks down - some people assume the default customization to be the default, others assume the defaults to be the defaults. These default customization files are also undocumented, so they cause default behaviour of utilities to differ from what is described in the manpages. At least we should not pile more poo on top of the turd. We want sane defaults, and we want simple defaults. Where traditional defaults are suboptimal but can't easily be changed, i still think that having one set of defaults is saner and simpler than having two sets of defaults: One traditional, documented plus one arguably more convenient, but undocumented set... Whatever, i expect others will disagree, so i'm unlikely to argue this further, and i can certainly live with whatever happens in /etc/skel/, i never use it anyway. Yours, Ingo > Index: etc/skel/dot.cvsrc > === > RCS file: /cvs/src/etc/skel/dot.cvsrc,v > retrieving revision 1.2 > diff -u -p -r1.2 dot.cvsrc > --- etc/skel/dot.cvsrc31 Mar 2015 18:34:40 - 1.2 > +++ etc/skel/dot.cvsrc29 Oct 2016 17:44:30 - > @@ -1,6 +1,6 @@ > # $OpenBSD: dot.cvsrc,v 1.2 2015/03/31 18:34:40 naddy Exp $ > # > diff -uNp > -update -P > +update -Pd > checkout -P > rdiff -u
Re: Add ioctl for disk cache flush
On Mon, 17 Oct 2016, Alexander Bluhm wrote: > > Part 1: Add an ioctl: > > > > > > add DIOCCACHESYNC > > > > Add an ioctl to tell storage devices to flush their internal caches. > > Ported from netbsd by pedro@ > > > > OK? > > Some remarks inline Sorry for the long delay, I was very busy in the last weeks. I agree with all your comments (and should have reviewed the initial patch better). Here is an updated diff. Cheers, Stefan diff --git sys/dev/ata/wd.c sys/dev/ata/wd.c index 5e2461f..a644cbf 100644 --- sys/dev/ata/wd.c +++ sys/dev/ata/wd.c @@ -26,7 +26,7 @@ */ /*- - * Copyright (c) 1998 The NetBSD Foundation, Inc. + * Copyright (c) 1998, 2003, 2004 The NetBSD Foundation, Inc. * All rights reserved. * * This code is derived from software contributed to The NetBSD Foundation @@ -138,7 +138,7 @@ void wdstart(void *); void __wdstart(struct wd_softc*, struct buf *); void wdrestart(void *); int wd_get_params(struct wd_softc *, u_int8_t, struct ataparams *); -void wd_flushcache(struct wd_softc *, int); +int wd_flushcache(struct wd_softc *, int); void wd_standby(struct wd_softc *, int); /* XXX: these should go elsewhere */ @@ -848,6 +848,10 @@ wdioctl(dev_t dev, u_long xfer, caddr_t addr, int flag, struct proc *p) } #endif + case DIOCCACHESYNC: + error = wd_flushcache(wd, AT_WAIT); + goto exit; + default: error = wdc_ioctl(wd->drvp, xfer, addr, flag, p); goto exit; @@ -1067,13 +1071,13 @@ wd_get_params(struct wd_softc *wd, u_int8_t flags, struct ataparams *params) } } -void +int wd_flushcache(struct wd_softc *wd, int flags) { struct wdc_command wdc_c; if (wd->drvp->ata_vers < 4) /* WDCC_FLUSHCACHE is here since ATA-4 */ - return; + return EIO; bzero(&wdc_c, sizeof(struct wdc_command)); wdc_c.r_command = (wd->sc_flags & WDF_LBA48 ? WDCC_FLUSHCACHE_EXT : WDCC_FLUSHCACHE); @@ -1088,20 +1092,28 @@ wd_flushcache(struct wd_softc *wd, int flags) if (wdc_exec_command(wd->drvp, &wdc_c) != WDC_COMPLETE) { printf("%s: flush cache command didn't complete\n", wd->sc_dev.dv_xname); + return EIO; } + if (wdc_c.flags & ERR_NODEV) + return ENODEV; if (wdc_c.flags & AT_TIMEOU) { printf("%s: flush cache command timeout\n", wd->sc_dev.dv_xname); + return EIO; + } + if (wdc_c.flags & AT_ERROR) { + if (wdc_c.r_error == WDCE_ABRT) /* command not supported */ + return ENODEV; + printf("%s: flush cache command: error 0x%x\n", + wd->sc_dev.dv_xname, wdc_c.r_error); + return EIO; } if (wdc_c.flags & AT_DF) { printf("%s: flush cache command: drive fault\n", wd->sc_dev.dv_xname); + return EIO; } - /* -* Ignore error register, it shouldn't report anything else -* than COMMAND ABORTED, which means the device doesn't support -* flush cache -*/ + return 0; } void diff --git sys/scsi/sd.c sys/scsi/sd.c index 30fb36b..e5a0139 100644 --- sys/scsi/sd.c +++ sys/scsi/sd.c @@ -2,7 +2,7 @@ /* $NetBSD: sd.c,v 1.111 1997/04/02 02:29:41 mycroft Exp $ */ /*- - * Copyright (c) 1998 The NetBSD Foundation, Inc. + * Copyright (c) 1998, 2003, 2004 The NetBSD Foundation, Inc. * All rights reserved. * * This code is derived from software contributed to The NetBSD Foundation @@ -96,7 +96,7 @@ int sd_vpd_block_limits(struct sd_softc *, int); intsd_vpd_thin(struct sd_softc *, int); intsd_thin_params(struct sd_softc *, int); intsd_get_parms(struct sd_softc *, struct disk_parms *, int); -void sd_flush(struct sd_softc *, int); +intsd_flush(struct sd_softc *, int); void viscpy(u_char *, u_char *, int); @@ -999,6 +999,15 @@ sdioctl(dev_t dev, u_long cmd, caddr_t addr, int flag, struct proc *p) error = sd_ioctl_cache(sc, cmd, (struct dk_cache *)addr); goto exit; + case DIOCCACHESYNC: + if (!ISSET(flag, FWRITE)) { + error = EBADF; + goto exit; + } + if ((sc->flags & SDF_DIRTY) != 0 || *(int *)addr != 0) + error = sd_flush(sc, 0); + goto exit; + default: if (part != RAW_PART) { error = ENOTTY; @@ -1876,19 +1885,20 @@ die: return (SDGP_RESULT_OFFLINE); } -void +int sd_flush(struct sd_softc *sc, int flags) { struct scsi_link *link; struct scsi_xfer *xs; struct scsi_synchronize_cache *cmd; + int error; if (sc->flags & SDF_DYING) - return; + return (ENXIO); link = sc->