public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
From: "Martin K. Petersen" <martin.petersen@oracle.com>
To: weiping zhang <zhangweiping@didichuxing.com>
Cc: martin.petersen@oracle.com, jejb@linux.vnet.ibm.com,
	linux-scsi@vger.kernel.org
Subject: Re: [PATCH v3] scsi: sd: add new match array for cache_type
Date: Tue, 30 Jan 2018 22:22:41 -0500	[thread overview]
Message-ID: <yq1lggeempq.fsf@oracle.com> (raw)
In-Reply-To: <20180123144439.GA15247@localhost.didichuxing.com> (weiping zhang's message of "Tue, 23 Jan 2018 22:44:44 +0800")


Weiping,

> add user friendly command strings sd_wr_cache to enable/disable
> write&read cache. user can enable both write and read cache by one of
> the following commands:

I remain unconvinced that introducing redundant option strings is a
benefit.

These shorthand forms may seem obvious to you, but not to everyone
else. Adding to the murkiness is that fact that write cache must be
*enabled* and read cache *disabled* in the protocol. The fact that the
??rN and RCD columns below contain inverse values just emphasizes that
point that "0" and "1" are not a particularly good interface either.

It is also confusing that the existing longhand forms will be printed
when the user subsequently queries the state instead of what they used
to toggle it.


This, however, is pretty much what I was looking for:

> Encoding                          | WCE RCD | Write_cache Read_cache
> --------------------------------------------------------------------
> write through              / w0r1 | 0   0   | off         on
> none                       / w0r0 | 0   1   | off         off
> write back                 / w1r1 | 1   0   | on          on
> write back, no read (daft) / w1r0 | 1   1   | on          off

Please drop the w?r? options and submit a patch with this table and a
note about the "temporary" prefix. Put it in
Documentation/scsi/sd-parameters.txt or something to that effect.

Bonus points for also documenting the remaining sd sysfs attributes and
their options.

Thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

  reply	other threads:[~2018-01-31  3:22 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-23 14:44 [PATCH v3] scsi: sd: add new match array for cache_type weiping zhang
2018-01-31  3:22 ` Martin K. Petersen [this message]
2018-02-01 14:15   ` Weiping Zhang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=yq1lggeempq.fsf@oracle.com \
    --to=martin.petersen@oracle.com \
    --cc=jejb@linux.vnet.ibm.com \
    --cc=linux-scsi@vger.kernel.org \
    --cc=zhangweiping@didichuxing.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox