> On 31 Oct 2016, at 09:18, Stefan Fritsch <[email protected]> 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;
> 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;
> + return (0);
>
> /*
> * Issue a SYNCHRONIZE CACHE. Address 0, length 0 means "all remaining
> @@ -1899,7 +1909,7 @@ sd_flush(struct sd_softc *sc, int flags)
> xs = scsi_xs_get(link, flags);
> if (xs == NULL) {
> SC_DEBUG(link, SDEV_DB1, ("cache sync failed to get xs\n"));
> - return;
> + return (EIO);
> }
>
> cmd = (struct scsi_synchronize_cache *)xs->cmd;
> @@ -1909,10 +1919,14 @@ sd_flush(struct sd_softc *sc, int flags)
> xs->timeout = 100000;
> xs->flags |= SCSI_IGNORE_ILLEGAL_REQUEST;
>
> - if (scsi_xs_sync(xs) == 0)
> - sc->flags &= ~SDF_DIRTY;
> - else
> - SC_DEBUG(link, SDEV_DB1, ("cache sync failed\n"));
> + error = scsi_xs_sync(xs);
>
> scsi_xs_put(xs);
> +
> + if (error)
> + SC_DEBUG(link, SDEV_DB1, ("cache sync failed\n"));
> + else
> + sc->flags &= ~SDF_DIRTY;
> +
> + return (error);
> }
> diff --git sys/sys/dkio.h sys/sys/dkio.h
> index c774e19..0a373af 100644
> --- sys/sys/dkio.h
> +++ sys/sys/dkio.h
> @@ -83,4 +83,6 @@ struct dk_diskmap {
>
> #define DIOCMAP _IOWR('d', 119, struct dk_diskmap)
>
> +#define DIOCCACHESYNC _IOW('d', 120, int) /* sync cache (force?)
> */
> +
> #endif /* _SYS_DKIO_H_ */
>
