On Tue, Apr 10, 2018 at 01:00:36PM -0400, Douglas Gilbert wrote:
> A patch titled: "[PATCH v2] scsi_debug: implement IMMED bit"
> introduced long delays to the Start stop unit (SSU) and
> Synchronize cache (SC) commands when the IMMED bit is clear.
> This patch makes those delays more realistic. It causes SSU
> to only delay when the start stop state is changed; SC only
> delays when there's been a write since the previous SC. It
> also reduced the SC delay from 1 second to 50 milliseconds.
>
> Signed-off-by: Douglas Gilbert
> ---
> This patch is in response to a report on the Linux scsi list
> indicating a significant slowdown in benchmarks using the
> original patch. The reporter seemed satisfield with an earlier
> version of this patch (in which the only difference was that
> the SC delay was 100 milliseconds).
>
> drivers/scsi/scsi_debug.c | 33 -
> 1 file changed, 24 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index 26ce022dd6f4..9ab19285a3d3 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
> @@ -234,11 +234,13 @@ static const char *sdebug_version_date = "20180128";
> #define F_INV_OP 0x200
> #define F_FAKE_RW0x400
> #define F_M_ACCESS 0x800 /* media access */
> -#define F_LONG_DELAY 0x1000
> +#define F_SSU_DELAY 0x1000
> +#define F_SYNC_DELAY 0x2000
>
> #define FF_RESPOND (F_RL_WLUN_OK | F_SKIP_UA | F_DELAY_OVERR)
> #define FF_MEDIA_IO (F_M_ACCESS | F_FAKE_RW)
> #define FF_SA (F_SA_HIGH | F_SA_LOW)
> +#define F_LONG_DELAY (F_SSU_DELAY | F_SYNC_DELAY)
>
> #define SDEBUG_MAX_PARTS 4
>
> @@ -510,7 +512,7 @@ static const struct opcode_info_t release_iarr[] = {
> };
>
> static const struct opcode_info_t sync_cache_iarr[] = {
> - {0, 0x91, 0, F_LONG_DELAY | F_M_ACCESS, resp_sync_cache, NULL,
> + {0, 0x91, 0, F_SYNC_DELAY | F_M_ACCESS, resp_sync_cache, NULL,
> {16, 0x6, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
>0xff, 0xff, 0xff, 0xff, 0x3f, 0xc7} }, /* SYNC_CACHE (16) */
> };
> @@ -553,7 +555,7 @@ static const struct opcode_info_t
> opcode_info_arr[SDEB_I_LAST_ELEMENT + 1] = {
> resp_write_dt0, write_iarr, /* WRITE(16) */
> {16, 0xfa, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
>0xff, 0xff, 0xff, 0xff, 0xff, 0xc7} },
> - {0, 0x1b, 0, F_LONG_DELAY, resp_start_stop, NULL,/* START STOP UNIT */
> + {0, 0x1b, 0, F_SSU_DELAY, resp_start_stop, NULL,/* START STOP UNIT */
> {6, 0x1, 0, 0xf, 0xf7, 0xc7, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0} },
> {ARRAY_SIZE(sa_in_16_iarr), 0x9e, 0x10, F_SA_LOW | F_D_IN,
> resp_readcap16, sa_in_16_iarr, /* SA_IN(16), READ CAPACITY(16) */
> @@ -606,7 +608,7 @@ static const struct opcode_info_t
> opcode_info_arr[SDEB_I_LAST_ELEMENT + 1] = {
> resp_write_same_10, write_same_iarr,/* WRITE SAME(10) */
> {10, 0xff, 0xff, 0xff, 0xff, 0xff, 0x3f, 0xff, 0xff, 0xc7, 0,
>0, 0, 0, 0, 0} },
> - {ARRAY_SIZE(sync_cache_iarr), 0x35, 0, F_LONG_DELAY | F_M_ACCESS,
> + {ARRAY_SIZE(sync_cache_iarr), 0x35, 0, F_SYNC_DELAY | F_M_ACCESS,
> resp_sync_cache, sync_cache_iarr,
> {10, 0x7, 0xff, 0xff, 0xff, 0xff, 0x3f, 0xff, 0xff, 0xc7, 0, 0,
>0, 0, 0, 0} }, /* SYNC_CACHE (10) */
> @@ -667,6 +669,7 @@ static bool sdebug_strict = DEF_STRICT;
> static bool sdebug_any_injecting_opt;
> static bool sdebug_verbose;
> static bool have_dif_prot;
> +static bool write_since_sync;
> static bool sdebug_statistics = DEF_STATISTICS;
>
> static unsigned int sdebug_store_sectors;
> @@ -1607,6 +1610,7 @@ static int resp_start_stop(struct scsi_cmnd *scp,
> {
> unsigned char *cmd = scp->cmnd;
> int power_cond, stop;
> + bool changing;
>
> power_cond = (cmd[4] & 0xf0) >> 4;
> if (power_cond) {
> @@ -1614,8 +1618,12 @@ static int resp_start_stop(struct scsi_cmnd *scp,
> return check_condition_result;
> }
> stop = !(cmd[4] & 1);
> + changing = atomic_read(&devip->stopped) == !stop;
> atomic_xchg(&devip->stopped, stop);
> - return (cmd[1] & 0x1) ? SDEG_RES_IMMED_MASK : 0; /* check IMMED bit */
> + if (!changing || cmd[1] & 0x1) /* state unchanged or IMMED set */
> + return SDEG_RES_IMMED_MASK;
> + else
> + return 0;
> }
>
> static sector_t get_sdebug_capacity(void)
> @@ -2473,6 +2481,7 @@ static int do_device_access(struct scsi_cmnd *scmd, u32
> sg_skip, u64 lba,
> if (do_write) {
> sdb = scsi_out(scmd);
> dir = DMA_TO_DEVICE;
> + write_since_sync = true;
> } else {
> sdb = scsi_in(scmd);
> dir = DMA_FROM_DEVICE;
> @@ -3583,6 +3592,7 @@ static int resp_get_lba_status(struct scsi_cmnd *scp,
> static
