I really think this heuristic belongs in the kernel. I think there is a
desire to make the policy a knob (the old, "I prefer slow and safe over
fast and dangerous; well use a ups! they don't!" debate).
So instead of bioctl I think we need a sysctl, for example hw.diskcache,
that by default is enabled which is the drive manufacturers suggested
setting. Then if so desired one can turn it off.
Or do people think this would be too large a hammer and would like to
have a more granular control?
On Wed, Mar 02, 2011 at 05:54:23AM -0500, Okan Demirmen wrote:
> On Sun 2011.02.20 at 10:30 -0500, Okan Demirmen wrote:
> > On Sun 2011.02.20 at 13:28 +0100, Mark Kettenis wrote:
> > > > Date: Sun, 20 Feb 2011 07:03:25 -0500
> > > > From: Kenneth R Westerback <kwesterb...@rogers.com>
> > > >
> > > > On Sun, Feb 20, 2011 at 12:39:06PM +0100, Mark Kettenis wrote:
> > > > > > Date: Sun, 20 Feb 2011 19:54:21 +1000
> > > > > > From: David Gwynne <l...@animata.net>
> > > > > >
> > > > > > > how to manipulate write cache policy?
> > > > > >
> > > > > > the lsi firmwares dont implement handling of the mod page changes
> > > > > > unfortunately. you could call the ioctl this implements yourself
> > > > > > though from userland.
> > > > >
> > > > > David, while I think that implementing the cache manipulation ioctls
> > > > > for mpii(4) is a good idea, there is a problem here. We don't have a
> > > > > tool in base that actually issues those ioctls. And unless I'm
> > > > > misreading the diff, this still leaves the cache disabled on the
> > > > > stupid Dell.
> > > >
> > > > DIOCSCACHE is called in sdattach() to enable write cache for all
> > > > disks that DIOCGCACHE reports as having write cache disabled. Or are
> > > > you concerned that we have no way to manipulate it from userland
> > > > if/when the default needs to be modified?
> > >
> > > Ah, that's the bit I was missing. A userland tool to display and
> > > manipulate the cache settings would still be good though.
> > > Functionality should probably be added to bioctl(8). A bit
> > > unfortunate that both the -c and -C options are already taken.
> >
> > Ah, I had a diff for bioctl (enable/disable WCE/RCD) based on dlg's
> > sample, but I think marco wanted more of a policy of when to do WCE/RCD
> > rather than a switch - I'll send it along when I get home later this
> > week.
>
> I'm not certain this is wanted, but I said I would forward along this
> very simplisitc patch, so here it is. If something like this is wanted,
> it can be re-worked to take multiple args to -e and such, but again,
> only if this is deemed necessary in a userland tool outside of scsi(8).
>
> Index: bioctl.8
> ===================================================================
> RCS file: /cvs/src/sbin/bioctl/bioctl.8,v
> retrieving revision 1.84
> diff -u -p -r1.84 bioctl.8
> --- bioctl.8 22 Dec 2010 16:25:32 -0000 1.84
> +++ bioctl.8 2 Mar 2011 10:44:23 -0000
> @@ -35,6 +35,7 @@
> .Op Fl hiqv
> .Op Fl a Ar alarm-function
> .Op Fl b Ar channel:target[.lun]
> +.Op Fl e Ar flag
> .Op Fl H Ar channel:target[.lun]
> .Op Fl R Ar device \*(Ba channel:target[.lun]
> .Op Fl u Ar channel:target[.lun]
> @@ -128,6 +129,24 @@ digits to four or less.
> .It Fl i
> Enumerate the selected RAID devices.
> This is the default if no other option is given.
> +.It Fl e Ar flag
> +Pass
> +.Ar flag
> +to
> +.Nm .
> +May be one of:
> +.Bl -tag -width disable -compact
> +.It Ar q
> +Query the read/write cache status.
> +.It Ar R
> +Enable the read cache.
> +.It Ar r
> +Disable the read cache.
> +.It Ar W
> +Enable the write cache.
> +.It Ar w
> +Disable the write cache.
> +.El
> .It Fl q
> Show vendor, product, revision, and serial number for the given disk.
> .It Fl R Ar device \*(Ba channel:target[.lun]
> Index: bioctl.c
> ===================================================================
> RCS file: /cvs/src/sbin/bioctl/bioctl.c,v
> retrieving revision 1.98
> diff -u -p -r1.98 bioctl.c
> --- bioctl.c 1 Dec 2010 19:40:18 -0000 1.98
> +++ bioctl.c 2 Mar 2011 10:44:23 -0000
> @@ -77,6 +77,7 @@ void bio_changepass(char *);
> u_int32_t bio_createflags(char *);
> char *bio_vis(char *);
> void bio_diskinq(char *);
> +void bio_cache(char *, char *);
>
> int devh = -1;
> int human;
> @@ -97,17 +98,17 @@ main(int argc, char *argv[])
> char *devicename = NULL;
> char *realname = NULL, *al_arg = NULL;
> char *bl_arg = NULL, *dev_list = NULL;
> - char *key_disk = NULL;
> + char *key_disk = NULL, *ca_arg = NULL;
> const char *errstr;
> int ch, rv, blink = 0, changepass = 0, diskinq = 0;
> - int ss_func = 0;
> + int ss_func = 0, diskcache = 0;
> u_int16_t cr_level = 0;
> int biodev = 0;
>
> if (argc < 2)
> usage();
>
> - while ((ch = getopt(argc, argv, "a:b:C:c:dH:hik:l:Pp:qr:R:svu:")) !=
> + while ((ch = getopt(argc, argv, "a:b:C:c:de:H:hik:l:Pp:qr:R:svu:")) !=
> -1) {
> switch (ch) {
> case 'a': /* alarm */
> @@ -133,6 +134,10 @@ main(int argc, char *argv[])
> /* delete volume */
> func |= BIOC_DELETERAID;
> break;
> + case 'e': /* cache */
> + diskcache = 1;
> + ca_arg = optarg;
> + break;
> case 'u': /* unblink */
> func |= BIOC_BLINK;
> blink = BIOC_SBUNBLINK;
> @@ -219,6 +224,8 @@ main(int argc, char *argv[])
>
> if (diskinq) {
> bio_diskinq(devicename);
> + } else if (diskcache) {
> + bio_cache(devicename, ca_arg);
> } else if (changepass && !biodev) {
> bio_changepass(devicename);
> } else if (func & BIOC_INQ) {
> @@ -252,7 +259,8 @@ usage(void)
> fprintf(stderr,
> "usage: %s [-hiqv] [-a alarm-function] "
> "[-b channel:target[.lun]]\n"
> - "\t[-H channel:target[.lun]] "
> + "\t[-e flag] "
> + "[-H channel:target[.lun]] "
> "[-R device | channel:target[.lun]\n"
> "\t[-u channel:target[.lun]] "
> "device\n"
> @@ -1104,4 +1112,43 @@ derive_key_pkcs(int rounds, u_int8_t *ke
> memset(passphrase, 0, sizeof(passphrase));
>
> return;
> +}
> +
> +void
> +bio_cache(char *sd_dev, char *arg)
> +{
> + int set = 1;
> + struct dk_cache dkc;
> +
> + if (ioctl(devh, DIOCGCACHE, &dkc) == -1)
> + err(1, "DIOCGCACHE");
> +
> + switch (arg[0]) {
> + case 'q': /* query cache */
> + set = 0;
> + break;
> + case 'r': /* disable read cache */
> + dkc.rdcache = 0;
> + break;
> + case 'R': /* enable read cache */
> + dkc.rdcache = 1;
> + break;
> + case 'w': /* disable write cache */
> + dkc.wrcache = 0;
> + break;
> + case 'W': /* enable write cache */
> + dkc.wrcache = 1;
> + break;
> + default:
> + errx(1, "invalid cache option: %s", arg);
> + }
> +
> + if (set) {
> + if (ioctl(devh, DIOCSCACHE, &dkc) == -1)
> + err(1, "ioctl DIOCSCACHE");
> + }
> +
> + printf("%s: write cache: %s, read cache: %s\n", sd_dev,
> + dkc.wrcache ? "enabled" : "disabled",
> + dkc.rdcache ? "enabled" : "disabled");
> }
