[PATCH] target: cleanup target_core_transport.c for kernel-doc

[Randy Dunlap]

From: Randy Dunlap 

For exported functions that already have near-kernel-doc notation,
fix them to begin with "/**" and make a few corrections so that they
don't have any kernel-doc warnings.

Signed-off-by: Randy Dunlap 
Cc: "Nicholas A. Bellinger" 
Cc: linux-scsi@vger.kernel.org
Cc: target-de...@vger.kernel.org
Cc: Sagi Grimberg 
Cc: linux-r...@vger.kernel.org
Cc: "James E.J. Bottomley" 
Cc: "Martin K. Petersen" 
---
 drivers/target/target_core_transport.c |   17 ++---
 1 file changed, 10 insertions(+), 7 deletions(-)

--- linux-next-20171221.orig/drivers/target/target_core_transport.c
+++ linux-next-20171221/drivers/target/target_core_transport.c
@@ -1431,7 +1431,7 @@ transport_generic_map_mem_to_cmd(struct
return 0;
 }
 
-/*
+/**
  * target_submit_cmd_map_sgls - lookup unpacked lun and submit uninitialized
  *  se_cmd + use pre-allocated SGL memory.
  *
@@ -1441,7 +1441,7 @@ transport_generic_map_mem_to_cmd(struct
  * @sense: pointer to SCSI sense buffer
  * @unpacked_lun: unpacked LUN to reference for struct se_lun
  * @data_length: fabric expected data transfer length
- * @task_addr: SAM task attribute
+ * @task_attr: SAM task attribute
  * @data_dir: DMA data direction
  * @flags: flags for command submission from target_sc_flags_tables
  * @sgl: struct scatterlist memory for unidirectional mapping
@@ -1578,7 +1578,7 @@ int target_submit_cmd_map_sgls(struct se
 }
 EXPORT_SYMBOL(target_submit_cmd_map_sgls);
 
-/*
+/**
  * target_submit_cmd - lookup unpacked lun and submit uninitialized se_cmd
  *
  * @se_cmd: command descriptor to submit
@@ -1587,7 +1587,7 @@ EXPORT_SYMBOL(target_submit_cmd_map_sgls
  * @sense: pointer to SCSI sense buffer
  * @unpacked_lun: unpacked LUN to reference for struct se_lun
  * @data_length: fabric expected data transfer length
- * @task_addr: SAM task attribute
+ * @task_attr: SAM task attribute
  * @data_dir: DMA data direction
  * @flags: flags for command submission from target_sc_flags_tables
  *
@@ -2641,7 +2641,8 @@ int transport_generic_free_cmd(struct se
 }
 EXPORT_SYMBOL(transport_generic_free_cmd);
 
-/* target_get_sess_cmd - Add command to active ->sess_cmd_list
+/**
+ * target_get_sess_cmd - Add command to active ->sess_cmd_list
  * @se_cmd:command descriptor to add
  * @ack_kref:  Signal that fabric will perform an ack target_put_sess_cmd()
  */
@@ -2835,7 +2836,8 @@ void target_show_cmd(const char *pfx, st
 }
 EXPORT_SYMBOL(target_show_cmd);
 
-/* target_sess_cmd_list_set_waiting - Flag all commands in
+/**
+ * target_sess_cmd_list_set_waiting - Flag all commands in
  * sess_cmd_list to complete cmd_wait_comp.  Set
  * sess_tearing_down so no more commands are queued.
  * @se_sess:   session to flag
@@ -2870,7 +2872,8 @@ void target_sess_cmd_list_set_waiting(st
 }
 EXPORT_SYMBOL(target_sess_cmd_list_set_waiting);
 
-/* target_wait_for_sess_cmds - Wait for outstanding descriptors
+/**
+ * target_wait_for_sess_cmds - Wait for outstanding descriptors
  * @se_sess:session to wait for active I/O
  */
 void target_wait_for_sess_cmds(struct se_session *se_sess)

Re: [PATCH] scsi: remove extra white space at the end of the line

[Finn Thain]

Hi Bart,

On Thu, 21 Dec 2017, Bart Van Assche wrote:

> There are several reasons why most kernel maintainers ignore patches 
> like this one silently:
> 

I don't disagree with your conclusion but I think that these objections 
may have solutions, at least in principle. Your message prompted me write 
them down. I've long thought that our process has some room for 
improvement.

I think Linux needs an official automatic code reformatter. C.f. TeX and 
math typesetting. Not a bunch of regular expressions, but an actual C 
parser that can produce correct line breaks and indentation based on the 
depth of the parse tree and correctly deal with macro definitions and so 
on. The style guide is probably ambiguous and inconsistent so I'd make 
this reformatter the final arbiter on style matters.

Then it would be possible to automatically regenerate a patch (or rebase a 
commit) so it could still be applied to the tip even after the tip has had 
some clean-up. This becomes possible when all clean-up uses the same 
process, so it produces the same result.

> * Whitespace changes make it harder than necessary to backport patches 
>   to distro kernels. Before a patch that came later than the whitespace 
>   changes can be backported, the whitespace change has to be backported. 
>   Additionally, if a whitespace change touches many source files, the 
>   order in which to backport patches becomes really important.

When a post-clean-up commit needs to be backported to a branch made 
pre-clean-up, the prerequisite changes could be automatically generated 
for the old branch as needed.

Note that this is already possible where the clean-up is scripted e.g.
$ sed -i -e 's,[ \t]*$,,' drivers/scsi/scsi_lib.c
which is actually the subject of this thread.

> * Before a kernel developer posts a patch that fixes a bug she or he has
>   to verify the change history (git log -p) to figure out which patch 
>   introduced the bug. Patches that only change coding style pollute the 
>   change history.

A clean-up commit would be pollution but (assuming the code style rules 
don't change) as a one-off event that patch might be preferable to 
perpetuating readability issues indefinitely. IMHO open source code should 
aim to maximize readability and re-usability (in general).

> * Accepting a patch that only changes whitespace would open the
>   floodgates for other kernel coding style change patches. If a patch 
>   that only changes whitespace would get accepted it will become hard to 
>   keep other kernel coding style change patches out.

Given the right tooling, code style patches need never blight maintainers' 
inboxes again. Such patches could be ignored and/or interpreted as 
suggestions that the maintainer run a script to improve some part of their 
tree under development (given that the resulting merge conflicts would be 
resolved automatically).

In the event that someone needed to work on a messy file, they could start 
with an automatic clean-up but they would not submit the clean-up patch 
because the maintainer can just run the same automatic process. Reviewing 
that patch would be a tedious waste of time. The rest of the submission 
also becomes easier to review. E.g. a patch like this would end up 
shorter,

 static char foo[] = {
'b',
-   'a'
+   'a',
+   'r',
 };

because the missing comma would not appear in the diff, as it was fixed 
implicitly. Thus irrelevant style changes disappear from the submission 
and cease to distract reviewers from the effect of the changes, which is 
what matters.

The unreliable checkpatch style checks could then be replaced with a 
simple diff against the output of the reformatter. If a maintainer still 
receives patches with style issues but no other issues, they should be 
able to automatically correct the patch so it produces the correct code 
style and then commit the corrected version without everyone having to go 
through another email submission iteration (which polutes many inboxes). 
This need not break the rest of a patch series or patch queue, which could 
be automatically rebased/regenerated.

Of course, the fly in the ointment is that the reformatter tool would have 
to be infallible, or at least beyond reproach. This seems to imply either 
infinite bikeshedding or else a final decision from above and probably an 
implementation too (something like 'sparse').

AFAIK this idea is purely hypothetical so thanks for entertaining it.

Happy Christmas.

-- 

[PATCH v3 5/5] scsi_debug: add resp_write_scat function

[Douglas Gilbert]

Add resp_write_scat() function to support decoding WRITE SCATTERED
(16 and 32). Also weave resp_write_scat() into the cdb decoding
logic.

Signed-off-by: Douglas Gilbert 
---
 drivers/scsi/scsi_debug.c | 179 +-
 1 file changed, 176 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 3c56675c9b0c..32460c702c6a 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -93,6 +93,7 @@ static const char *sdebug_version_date = "20160430";
 #define MISCOMPARE_VERIFY_ASC 0x1d
 #define MICROCODE_CHANGED_ASCQ 0x1 /* with TARGET_CHANGED_ASC */
 #define MICROCODE_CHANGED_WO_RESET_ASCQ 0x16
+#define WRITE_ERROR_ASC 0xc
 
 /* Additional Sense Code Qualifier (ASCQ) */
 #define ACK_NAK_TO 0x3
@@ -327,7 +328,7 @@ enum sdeb_opcode_index {
SDEB_I_MAINT_IN = 14,
SDEB_I_MAINT_OUT = 15,
SDEB_I_VERIFY = 16, /* 10 only */
-   SDEB_I_VARIABLE_LEN = 17,   /* READ(32), WRITE(32) */
+   SDEB_I_VARIABLE_LEN = 17,   /* READ(32), WRITE(32), WR_SCAT(32) */
SDEB_I_RESERVE = 18,/* 6, 10 */
SDEB_I_RELEASE = 19,/* 6, 10 */
SDEB_I_ALLOW_REMOVAL = 20,  /* PREVENT ALLOW MEDIUM REMOVAL */
@@ -396,6 +397,7 @@ static int resp_log_sense(struct scsi_cmnd *, struct 
sdebug_dev_info *);
 static int resp_readcap(struct scsi_cmnd *, struct sdebug_dev_info *);
 static int resp_read_dt0(struct scsi_cmnd *, struct sdebug_dev_info *);
 static int resp_write_dt0(struct scsi_cmnd *, struct sdebug_dev_info *);
+static int resp_write_scat(struct scsi_cmnd *, struct sdebug_dev_info *);
 static int resp_start_stop(struct scsi_cmnd *, struct sdebug_dev_info *);
 static int resp_readcap16(struct scsi_cmnd *, struct sdebug_dev_info *);
 static int resp_get_lba_status(struct scsi_cmnd *, struct sdebug_dev_info *);
@@ -457,6 +459,9 @@ static const struct opcode_info_t vl_iarr[] = { /* 
VARIABLE LENGTH */
{0, 0x7f, 0xb, F_SA_HIGH | F_D_OUT | FF_MEDIA_IO, resp_write_dt0,
NULL, {32,  0xc7, 0, 0, 0, 0, 0x3f, 0x18, 0x0, 0xb, 0xfa,
   0, 0xff, 0xff, 0xff, 0xff} },/* WRITE(32) */
+   {0, 0x7f, 0x11, F_SA_HIGH | F_D_OUT | FF_MEDIA_IO, resp_write_scat,
+   NULL, {32,  0xc7, 0, 0, 0, 0, 0x3f, 0x18, 0x0, 0x11, 0xf8,
+  0, 0xff, 0xff, 0x0, 0x0} },  /* WRITE SCATTERED(32) */
 };
 
 static const struct opcode_info_t maint_in_iarr[] = {  /* MAINT IN */
@@ -528,8 +533,9 @@ static const struct opcode_info_t 
opcode_info_arr[SDEB_I_LAST_ELEMENT + 1] = {
resp_readcap16, sa_in_16_iarr, /* SA_IN(16), READ CAPACITY(16) */
{16,  0x10, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
 0xff, 0xff, 0xff, 0xff, 0x1, 0xc7} },
-   {0, 0, 0, F_INV_OP | FF_RESPOND, NULL, NULL, /* SA_OUT(16) */
-   {0,  0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0} },
+   {0, 0x9f, 0x12, F_SA_LOW | F_D_OUT | FF_MEDIA_IO, resp_write_scat,
+   NULL, {16,  0x12, 0xf9, 0x0, 0xff, 0xff, 0, 0, 0xff, 0xff, 0xff,
+   0xff, 0xff, 0xff, 0xff, 0xc7} },  /* SA_OUT(16), WRITE SCAT(16) */
{ARRAY_SIZE(maint_in_iarr), 0xa3, 0xa, F_SA_LOW | F_D_IN,
resp_report_tgtpgs, /* MAINT IN, REPORT TARGET PORT GROUPS */
maint_in_iarr, {12,  0xea, 0, 0, 0, 0, 0xff, 0xff, 0xff,
@@ -2988,6 +2994,173 @@ static int resp_write_dt0(struct scsi_cmnd *scp, struct 
sdebug_dev_info *devip)
return 0;
 }
 
+/*
+ * T10 has only specified WRITE SCATTERED(16) and WRITE SCATTERED(32).
+ * No READ GATHERED yet (requires bidi or long cdb holding gather list).
+ */
+static int resp_write_scat(struct scsi_cmnd *scp,
+  struct sdebug_dev_info *devip)
+{
+   u8 *cmd = scp->cmnd;
+   u8 *lrdp = NULL;
+   u8 *up;
+   u8 wrprotect;
+   u16 lbdof, num_lrd, k;
+   u32 num, num_by, bt_len, lbdof_blen, sg_off, cum_lb;
+   u32 lb_size = sdebug_sector_size;
+   u32 ei_lba;
+   u64 lba;
+   unsigned long iflags;
+   int ret, res;
+   bool is_16;
+   static const u32 lrd_size = 32; /* + parameter list header size */
+
+   if (cmd[0] == VARIABLE_LENGTH_CMD) {
+   is_16 = false;
+   wrprotect = (cmd[10] >> 5) & 0x7;
+   lbdof = get_unaligned_be16(cmd + 12);
+   num_lrd = get_unaligned_be16(cmd + 16);
+   bt_len = get_unaligned_be32(cmd + 28);
+   } else {/* that leaves WRITE SCATTERED(16) */
+   is_16 = true;
+   wrprotect = (cmd[2] >> 5) & 0x7;
+   lbdof = get_unaligned_be16(cmd + 4);
+   num_lrd = get_unaligned_be16(cmd + 8);
+   bt_len = get_unaligned_be32(cmd + 10);
+   if (unlikely(have_dif_prot)) {
+   if (sdebug_dif == T10_PI_TYPE2_PROTECTION &&
+   wrprotect) {
+ 

[PATCH v3 3/5] scsi_debug: do_device_access add sg offset argument

[Douglas Gilbert]

WRITE SCATTERED needs to take several "bites" out of the data-out buffer.
Expand the do_device_access() function to take a sg_skip argument.

Signed-off-by: Douglas Gilbert 
Reviewed-by: Bart Van Assche 
---
 drivers/scsi/scsi_debug.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 428165dbd486..4cd703f0aac0 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -2353,8 +2353,8 @@ static int check_device_access_params(struct scsi_cmnd 
*scp,
 }
 
 /* Returns number of bytes copied or -1 if error. */
-static int do_device_access(struct scsi_cmnd *scmd, u64 lba, u32 num,
-   bool do_write)
+static int do_device_access(struct scsi_cmnd *scmd, u32 sg_skip, u64 lba,
+   u32 num, bool do_write)
 {
int ret;
u64 block, rest = 0;
@@ -2380,14 +2380,15 @@ static int do_device_access(struct scsi_cmnd *scmd, u64 
lba, u32 num,
 
ret = sg_copy_buffer(sdb->table.sgl, sdb->table.nents,
   fake_storep + (block * sdebug_sector_size),
-  (num - rest) * sdebug_sector_size, 0, do_write);
+  (num - rest) * sdebug_sector_size, sg_skip, do_write);
if (ret != (num - rest) * sdebug_sector_size)
return ret;
 
if (rest) {
ret += sg_copy_buffer(sdb->table.sgl, sdb->table.nents,
fake_storep, rest * sdebug_sector_size,
-   (num - rest) * sdebug_sector_size, do_write);
+   sg_skip + ((num - rest) * sdebug_sector_size),
+   do_write);
}
 
return ret;
@@ -2648,7 +2649,7 @@ static int resp_read_dt0(struct scsi_cmnd *scp, struct 
sdebug_dev_info *devip)
}
}
 
-   ret = do_device_access(scp, lba, num, false);
+   ret = do_device_access(scp, 0, lba, num, false);
read_unlock_irqrestore(_rw, iflags);
if (unlikely(ret == -1))
return DID_ERROR << 16;
@@ -2936,7 +2937,7 @@ static int resp_write_dt0(struct scsi_cmnd *scp, struct 
sdebug_dev_info *devip)
}
}
 
-   ret = do_device_access(scp, lba, num, true);
+   ret = do_device_access(scp, 0, lba, num, true);
if (unlikely(scsi_debug_lbp()))
map_region(lba, num);
write_unlock_irqrestore(_rw, iflags);
@@ -3177,7 +3178,7 @@ static int resp_comp_write(struct scsi_cmnd *scp,
 * from data-in into arr. Safe (atomic) since write_lock held. */
fake_storep_hold = fake_storep;
fake_storep = arr;
-   ret = do_device_access(scp, 0, dnum, true);
+   ret = do_device_access(scp, 0, 0, dnum, true);
fake_storep = fake_storep_hold;
if (ret == -1) {
retval = DID_ERROR << 16;
-- 
2.14.1

[PATCH v3 0/5] scsi_debug: add write scattered support

[Douglas Gilbert]

While testing the WRITE SCATTERED command support in a new sg3_utils
utility (sg_write_x) it was helpful to have a target that supported
this command. This command might be attractive to other kernel
subsystems. Even if end devices don't support this command yet, it
would most likely be a performance win if SCSI LLDs supported it (by
breaking it down to a series of WRITE commands), as that would cut
down the overhead of the block layer, the ULD (e.g. sd) and the SCSI
midlevel.

Changes since v2:
  - remove over-zealous sanity check (lbdof >= bt_len) that rejected
valid invocations
  - change a kzalloc() failure to return SCSI_MLQUEUE_HOST_BUSY as
requested by reviewer

Changes since original version:
  - fix problem when fake_rw=1, identify media access commands
better with following rename
  - rename FF_DIRECT_IO to FF_MEDIA_IO
  - apply reviewer suggestions (Bart Van Assche)
  - expand in-code comments

Douglas Gilbert (5):
  scsi_debug: tab, kstrto changes
  scsi_debug: fix group_number mask
  scsi_debug: do_device_access add sg offset argument
  scsi_debug: ARRAY_SIZE and FF_MEDIA_IO
  scsi_debug: add resp_write_scat function

 drivers/scsi/scsi_debug.c | 541 +++---
 1 file changed, 366 insertions(+), 175 deletions(-)

-- 
2.14.1

[PATCH v3 1/5] scsi_debug: tab, kstrto changes

[Douglas Gilbert]

Some of my development tools tend to add spaces (my preference) rather
than tabs (kernel convention). Running unexpand to clean these spaces
up found more of them than checkpatch.pl did. Then checkpatch.pl
complained about other style violations in those newly tabbed lines.

Signed-off-by: Douglas Gilbert 
Reviewed-by: Bart Van Assche 
---
 drivers/scsi/scsi_debug.c | 174 +++---
 1 file changed, 87 insertions(+), 87 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index e4f037f0f38b..80a775b6e648 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -304,8 +304,8 @@ struct opcode_info_t {
u32 flags;  /* OR-ed set of SDEB_F_* */
int (*pfp)(struct scsi_cmnd *, struct sdebug_dev_info *);
const struct opcode_info_t *arrp;  /* num_attached elements or NULL */
-   u8 len_mask[16];/* len=len_mask[0], then mask for cdb[1]... */
-   /* ignore cdb bytes after position 15 */
+   u8 len_mask[16];/* len_mask[0]-->cdb_len, then mask for cdb */
+   /* 1 to min(cdb_len, 15); ignore cdb[15...] */
 };
 
 /* SCSI opcodes (first byte of cdb) of interest mapped onto these indexes */
@@ -340,7 +340,7 @@ enum sdeb_opcode_index {
SDEB_I_WRITE_SAME = 27, /* 10, 16 */
SDEB_I_SYNC_CACHE = 28, /* 10 only */
SDEB_I_COMP_WRITE = 29,
-   SDEB_I_LAST_ELEMENT = 30,   /* keep this last */
+   SDEB_I_LAST_ELEMENT = 30,   /* keep this last (previous + 1) */
 };
 
 
@@ -1900,7 +1900,7 @@ static unsigned char ctrl_m_pg[] = {0xa, 10, 2, 0, 0, 0, 
0, 0,
 static int resp_ctrl_m_pg(unsigned char * p, int pcontrol, int target)
 {  /* Control mode page for mode_sense */
unsigned char ch_ctrl_m_pg[] = {/* 0xa, 10, */ 0x6, 0, 0, 0, 0, 0,
-   0, 0, 0, 0};
+   0, 0, 0, 0};
unsigned char d_ctrl_m_pg[] = {0xa, 10, 2, 0, 0, 0, 0, 0,
 0, 0, 0x2, 0x4b};
 
@@ -2077,13 +2077,13 @@ static int resp_mode_sense(struct scsi_cmnd *scp,
len = resp_disconnect_pg(ap, pcontrol, target);
offset += len;
break;
-case 0x3:   /* Format device page, direct access */
+   case 0x3:   /* Format device page, direct access */
if (is_disk) {
len = resp_format_pg(ap, pcontrol, target);
offset += len;
} else
bad_pcode = true;
-break;
+   break;
case 0x8:   /* Caching page, direct access */
if (is_disk) {
len = resp_caching_pg(ap, pcontrol, target);
@@ -2099,7 +2099,7 @@ static int resp_mode_sense(struct scsi_cmnd *scp,
if ((subpcode > 0x2) && (subpcode < 0xff)) {
mk_sense_invalid_fld(scp, SDEB_IN_CDB, 3, -1);
return check_condition_result;
-   }
+   }
len = 0;
if ((0x0 == subpcode) || (0xff == subpcode))
len += resp_sas_sf_m_pg(ap + len, pcontrol, target);
@@ -2136,7 +2136,7 @@ static int resp_mode_sense(struct scsi_cmnd *scp,
} else {
mk_sense_invalid_fld(scp, SDEB_IN_CDB, 3, -1);
return check_condition_result;
-}
+   }
break;
default:
bad_pcode = true;
@@ -2172,8 +2172,8 @@ static int resp_mode_select(struct scsi_cmnd *scp,
mk_sense_invalid_fld(scp, SDEB_IN_CDB, mselect6 ? 4 : 7, -1);
return check_condition_result;
}
-res = fetch_to_dev_buffer(scp, arr, param_len);
-if (-1 == res)
+   res = fetch_to_dev_buffer(scp, arr, param_len);
+   if (-1 == res)
return DID_ERROR << 16;
else if (sdebug_verbose && (res < param_len))
sdev_printk(KERN_INFO, scp->device,
@@ -2239,8 +2239,8 @@ static int resp_temp_l_pg(unsigned char * arr)
 0x0, 0x1, 0x3, 0x2, 0x0, 65,
};
 
-memcpy(arr, temp_l_pg, sizeof(temp_l_pg));
-return sizeof(temp_l_pg);
+   memcpy(arr, temp_l_pg, sizeof(temp_l_pg));
+   return sizeof(temp_l_pg);
 }
 
 static int resp_ie_l_pg(unsigned char * arr)
@@ -2248,18 +2248,18 @@ static int resp_ie_l_pg(unsigned char * arr)
unsigned char ie_l_pg[] = {0x0, 0x0, 0x3, 0x3, 0x0, 0x0, 38,
};
 
-memcpy(arr, ie_l_pg, sizeof(ie_l_pg));
+   memcpy(arr, ie_l_pg, sizeof(ie_l_pg));
if (iec_m_pg[2] & 0x4) {/* TEST bit set */
arr[4] = THRESHOLD_EXCEEDED;
arr[5] = 0xff;
}
-

[PATCH v3 4/5] scsi_debug: ARRAY_SIZE and FF_MEDIA_IO

[Douglas Gilbert]

Reviewer suggested using the ARRAY_SIZE macro. That reduced one of the subtle
inter-dependencies in the parser's tables.

It is important that commands which simulate media access, indicate this in the
flags for that command. The flag to do that was FF_DIRECT_IO. On reflection
FF_MEDIA_IO seems a more accurate description.

Signed-off-by: Douglas Gilbert 
Reviewed-by: Bart Van Assche 
---
 drivers/scsi/scsi_debug.c | 163 +-
 1 file changed, 90 insertions(+), 73 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 4cd703f0aac0..3c56675c9b0c 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -232,7 +232,7 @@ static const char *sdebug_version_date = "20160430";
 #define F_M_ACCESS 0x800   /* media access */
 
 #define FF_RESPOND (F_RL_WLUN_OK | F_SKIP_UA | F_DELAY_OVERR)
-#define FF_DIRECT_IO (F_M_ACCESS | F_FAKE_RW)
+#define FF_MEDIA_IO (F_M_ACCESS | F_FAKE_RW)
 #define FF_SA (F_SA_HIGH | F_SA_LOW)
 
 #define SDEBUG_MAX_PARTS 4
@@ -322,12 +322,12 @@ enum sdeb_opcode_index {
SDEB_I_READ = 9,/* 6, 10, 12, 16 */
SDEB_I_WRITE = 10,  /* 6, 10, 12, 16 */
SDEB_I_START_STOP = 11,
-   SDEB_I_SERV_ACT_IN = 12,/* 12, 16 */
-   SDEB_I_SERV_ACT_OUT = 13,   /* 12, 16 */
+   SDEB_I_SERV_ACT_IN_16 = 12, /* add ...SERV_ACT_IN_12 if needed */
+   SDEB_I_SERV_ACT_OUT_16 = 13,/* add ...SERV_ACT_OUT_12 if needed */
SDEB_I_MAINT_IN = 14,
SDEB_I_MAINT_OUT = 15,
SDEB_I_VERIFY = 16, /* 10 only */
-   SDEB_I_VARIABLE_LEN = 17,
+   SDEB_I_VARIABLE_LEN = 17,   /* READ(32), WRITE(32) */
SDEB_I_RESERVE = 18,/* 6, 10 */
SDEB_I_RELEASE = 19,/* 6, 10 */
SDEB_I_ALLOW_REMOVAL = 20,  /* PREVENT ALLOW MEDIUM REMOVAL */
@@ -372,12 +372,12 @@ static const unsigned char opcode_ind_arr[256] = {
0, 0, 0, 0, 0, SDEB_I_ATA_PT, 0, 0,
SDEB_I_READ, SDEB_I_COMP_WRITE, SDEB_I_WRITE, 0, 0, 0, 0, 0,
0, 0, 0, SDEB_I_WRITE_SAME, 0, 0, 0, 0,
-   0, 0, 0, 0, 0, 0, SDEB_I_SERV_ACT_IN, SDEB_I_SERV_ACT_OUT,
+   0, 0, 0, 0, 0, 0, SDEB_I_SERV_ACT_IN_16, SDEB_I_SERV_ACT_OUT_16,
 /* 0xa0; 0xa0->0xbf: 12 byte cdbs */
SDEB_I_REPORT_LUNS, SDEB_I_ATA_PT, 0, SDEB_I_MAINT_IN,
 SDEB_I_MAINT_OUT, 0, 0, 0,
-   SDEB_I_READ, SDEB_I_SERV_ACT_OUT, SDEB_I_WRITE, SDEB_I_SERV_ACT_IN,
-0, 0, 0, 0,
+   SDEB_I_READ, 0 /* SDEB_I_SERV_ACT_OUT_12 */, SDEB_I_WRITE,
+0 /* SDEB_I_SERV_ACT_IN_12 */, 0, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0,
 /* 0xc0; 0xc0->0xff: vendor specific */
@@ -409,72 +409,78 @@ static int resp_xdwriteread_10(struct scsi_cmnd *, struct 
sdebug_dev_info *);
 static int resp_comp_write(struct scsi_cmnd *, struct sdebug_dev_info *);
 static int resp_write_buffer(struct scsi_cmnd *, struct sdebug_dev_info *);
 
-static const struct opcode_info_t msense_iarr[1] = {
+/*
+ * The following are overflow arrays for cdbs that "hit" the same index in
+ * the opcode_info_arr array. The most time sensitive (or commonly used) cdb
+ * should be placed in opcode_info_arr[], the others should be placed here.
+ */
+static const struct opcode_info_t msense_iarr[] = {
{0, 0x1a, 0, F_D_IN, NULL, NULL,
{6,  0xe8, 0xff, 0xff, 0xff, 0xc7, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0} },
 };
 
-static const struct opcode_info_t mselect_iarr[1] = {
+static const struct opcode_info_t mselect_iarr[] = {
{0, 0x15, 0, F_D_OUT, NULL, NULL,
{6,  0xf1, 0, 0, 0xff, 0xc7, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0} },
 };
 
-static const struct opcode_info_t read_iarr[3] = {
-   {0, 0x28, 0, F_D_IN | FF_DIRECT_IO, resp_read_dt0, NULL,/* READ(10) */
+static const struct opcode_info_t read_iarr[] = {
+   {0, 0x28, 0, F_D_IN | FF_MEDIA_IO, resp_read_dt0, NULL,/* READ(10) */
{10,  0xff, 0xff, 0xff, 0xff, 0xff, 0x3f, 0xff, 0xff, 0xc7, 0, 0,
 0, 0, 0, 0} },
-   {0, 0x8, 0, F_D_IN | FF_DIRECT_IO, resp_read_dt0, NULL, /* READ(6) */
+   {0, 0x8, 0, F_D_IN | FF_MEDIA_IO, resp_read_dt0, NULL, /* READ(6) */
{6,  0xff, 0xff, 0xff, 0xff, 0xc7, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0} },
-   {0, 0xa8, 0, F_D_IN | FF_DIRECT_IO, resp_read_dt0, NULL,/* READ(12) */
+   {0, 0xa8, 0, F_D_IN | FF_MEDIA_IO, resp_read_dt0, NULL,/* READ(12) */
{12,  0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xbf,
 0xc7, 0, 0, 0, 0} },
 };
 
-static const struct opcode_info_t write_iarr[3] = {
-   {0, 0x2a, 0, F_D_OUT | FF_DIRECT_IO, resp_write_dt0, NULL,   /* 10 */
-   {10,  0xfb, 0xff, 0xff, 0xff, 0xff, 0x3f, 0xff, 0xff, 0xc7, 0, 0,
-0, 0, 0, 0} },
-   {0, 0xa, 0, F_D_OUT | FF_DIRECT_IO, resp_write_dt0, NULL,/* 6 */
-   {6,  0xff, 0xff, 0xff, 0xff, 0xc7, 

[PATCH v3 2/5] scsi_debug: fix group_number mask

[Douglas Gilbert]

Various cdb masks incorrectly assumed the GROUP NUMBER field
was 5 bits long. It is actually 6 bits long. Correct.
Also fix mask failure (in same byte) to allow DLD0 in READ(16)
and WRITE(16).

Signed-off-by: Douglas Gilbert 
Reviewed-by: Bart Van Assche 
---
 drivers/scsi/scsi_debug.c | 28 ++--
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 80a775b6e648..428165dbd486 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -421,23 +421,23 @@ static const struct opcode_info_t mselect_iarr[1] = {
 
 static const struct opcode_info_t read_iarr[3] = {
{0, 0x28, 0, F_D_IN | FF_DIRECT_IO, resp_read_dt0, NULL,/* READ(10) */
-   {10,  0xff, 0xff, 0xff, 0xff, 0xff, 0x1f, 0xff, 0xff, 0xc7, 0, 0,
+   {10,  0xff, 0xff, 0xff, 0xff, 0xff, 0x3f, 0xff, 0xff, 0xc7, 0, 0,
 0, 0, 0, 0} },
{0, 0x8, 0, F_D_IN | FF_DIRECT_IO, resp_read_dt0, NULL, /* READ(6) */
{6,  0xff, 0xff, 0xff, 0xff, 0xc7, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0} },
{0, 0xa8, 0, F_D_IN | FF_DIRECT_IO, resp_read_dt0, NULL,/* READ(12) */
-   {12,  0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x9f,
+   {12,  0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xbf,
 0xc7, 0, 0, 0, 0} },
 };
 
 static const struct opcode_info_t write_iarr[3] = {
{0, 0x2a, 0, F_D_OUT | FF_DIRECT_IO, resp_write_dt0, NULL,   /* 10 */
-   {10,  0xfb, 0xff, 0xff, 0xff, 0xff, 0x1f, 0xff, 0xff, 0xc7, 0, 0,
+   {10,  0xfb, 0xff, 0xff, 0xff, 0xff, 0x3f, 0xff, 0xff, 0xc7, 0, 0,
 0, 0, 0, 0} },
{0, 0xa, 0, F_D_OUT | FF_DIRECT_IO, resp_write_dt0, NULL,/* 6 */
{6,  0xff, 0xff, 0xff, 0xff, 0xc7, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0} },
{0, 0xaa, 0, F_D_OUT | FF_DIRECT_IO, resp_write_dt0, NULL,   /* 12 */
-   {12,  0xfb, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x9f,
+   {12,  0xfb, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xbf,
 0xc7, 0, 0, 0, 0} },
 };
 
@@ -449,7 +449,7 @@ static const struct opcode_info_t sa_in_iarr[1] = {
 
 static const struct opcode_info_t vl_iarr[1] = {   /* VARIABLE LENGTH */
{0, 0x7f, 0xb, F_SA_HIGH | F_D_OUT | FF_DIRECT_IO, resp_write_dt0,
-   NULL, {32,  0xc7, 0, 0, 0, 0, 0x1f, 0x18, 0x0, 0xb, 0xfa,
+   NULL, {32,  0xc7, 0, 0, 0, 0, 0x3f, 0x18, 0x0, 0xb, 0xfa,
   0, 0xff, 0xff, 0xff, 0xff} },/* WRITE(32) */
 };
 
@@ -465,7 +465,7 @@ static const struct opcode_info_t maint_in_iarr[2] = {
 static const struct opcode_info_t write_same_iarr[1] = {
{0, 0x93, 0, F_D_OUT_MAYBE | FF_DIRECT_IO, resp_write_same_16, NULL,
{16,  0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-0xff, 0xff, 0xff, 0x1f, 0xc7} },
+0xff, 0xff, 0xff, 0x3f, 0xc7} },
 };
 
 static const struct opcode_info_t reserve_iarr[1] = {
@@ -508,11 +508,11 @@ static const struct opcode_info_t 
opcode_info_arr[SDEB_I_LAST_ELEMENT + 1] = {
 0, 0} },
{3, 0x88, 0, F_D_IN | FF_DIRECT_IO, resp_read_dt0, read_iarr,
{16,  0xfe, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-0xff, 0xff, 0xff, 0x9f, 0xc7} },   /* READ(16) */
+0xff, 0xff, 0xff, 0xff, 0xc7} },   /* READ(16) */
 /* 10 */
{3, 0x8a, 0, F_D_OUT | FF_DIRECT_IO, resp_write_dt0, write_iarr,
{16,  0xfa, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-0xff, 0xff, 0xff, 0x9f, 0xc7} },   /* WRITE(16) */
+0xff, 0xff, 0xff, 0xff, 0xc7} },   /* WRITE(16) */
{0, 0x1b, 0, 0, resp_start_stop, NULL,  /* START STOP UNIT */
{6,  0x1, 0, 0xf, 0xf7, 0xc7, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0} },
{1, 0x9e, 0x10, F_SA_LOW | F_D_IN, resp_readcap16, sa_in_iarr,
@@ -529,7 +529,7 @@ static const struct opcode_info_t 
opcode_info_arr[SDEB_I_LAST_ELEMENT + 1] = {
{10,  0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xc7,
 0, 0, 0, 0, 0, 0} },
{1, 0x7f, 0x9, F_SA_HIGH | F_D_IN | FF_DIRECT_IO, resp_read_dt0,
-   vl_iarr, {32,  0xc7, 0, 0, 0, 0, 0x1f, 0x18, 0x0, 0x9, 0xfe, 0,
+   vl_iarr, {32,  0xc7, 0, 0, 0, 0, 0x3f, 0x18, 0x0, 0x9, 0xfe, 0,
  0xff, 0xff, 0xff, 0xff} },/* VARIABLE LENGTH, READ(32) */
{1, 0x56, 0, F_D_OUT, NULL, reserve_iarr, /* RESERVE(10) */
{10,  0xff, 0xff, 0xff, 0, 0, 0, 0xff, 0xff, 0xc7, 0, 0, 0, 0, 0,
@@ -547,22 +547,22 @@ static const struct opcode_info_t 
opcode_info_arr[SDEB_I_LAST_ELEMENT + 1] = {
{0, 0x1d, F_D_OUT, 0, NULL, NULL,   /* SEND DIAGNOSTIC */
{6,  0xf7, 0, 0xff, 0xff, 0xc7, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0} },
{0, 0x42, 0, F_D_OUT | FF_DIRECT_IO, resp_unmap, NULL, /* UNMAP */
-   {10,  0x1, 0, 

Invest Loan

[JB]

Hello Sir,

In need of loan to start business, finance your project or expand your business 
 Write back if you are interested for further details.

Regards

Jack

Re: [PATCH 2/3] ses: skip error messages for invalid LUNs

[Hannes Reinecke]

On 12/22/2017 06:39 PM, James Bottomley wrote:
> On Thu, 2017-12-21 at 12:22 +0100, Hannes Reinecke wrote:
>> Some storage array set the 'Embedded enclosure' bit even though
>> no LUN is present, causing the first RECEIVE DIAGNOSTIC call to
>> be returned with sense code 'LOGICAL UNIT NOT SUPPORTED'.
>> This patch skips the annoying 'Failed to get diagnostic page 0x1'
>> messages for those cases.
> 
> What disagnostic pages does this thing support?  Can you do a receive
> diagnostic on page 0 to find out?  I suspect a lot of embedded
> enclosure services are simple and support page 7 only.  If it really
> refuses all diagnostic page requests (which would be a gross standards
> violation), then it should probably be blacklisted by inquiry string as
> unusable.
> 
If a LUN is connected it'll respond to the usual set (0, 1, 2, und 6).
Sending a RECEIVE DIAGNOSTIC to page 0 will probably yield the same
result (ie being returned with that sense code), but at least we won't
try to access the other pages.
But anyway, yes, the implementation is dodgy.
I'm in contact with the vendor and try to straighten things out.

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH 1/3] ses: make initial allocation size configurable

[Hannes Reinecke]

On 12/22/2017 06:14 PM, James Bottomley wrote:
> On Thu, 2017-12-21 at 12:22 +0100, Hannes Reinecke wrote:
>> Some storage arrays have an incorrect SES implementation which will
>> always return the allocation length of the CDB instead of the true
>> length of the requested page.
> 
> That's a pretty gross standards violation, is this common?  When the
> buffer is > than the data to return, does it get set then?  Because if
> not, we're going to be processing bogus data and no module parameter
> can fix this because the returned length depends on the number of
> elements in the enclosure making this parameter really unsafe unless
> you get it exactly right.
> 
It's actually the first time I've observed that; the vendor is already
alerted.
But yes, if the buffer is larger than the page the correct page size is
set, so we won't be suffering from the above issue.

>> With this patch one can modify the initial allocation size to
>> get the full output of those arrays.
> 
> This really doesn't look like the right way to do it.  Shouldn't we
> rather have a blacklist for these devices and simply do a page for each
> allocation for them (assuming they return the correct length when over
> subscribed).
> 
I really doubt it's worth it. It's just a single array line with a
particular firmware revision from what I've seen.
The one problem we're continually running into is that the blacklist
flags are essentially used up, so I'd be rather careful with adding some
not-that-common scenarios to it.
I would love to have the blacklist size increased, though; I do have a
few other issues which would rather benefit from being handled by a
blacklist flag ...

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [RFC] sg3_utils: sgp_dd: work around on pthread_cancel for android

[Bart Van Assche]

On Sat, 2017-12-23 at 10:19 +, Bean Huo (beanhuo) wrote:
> Doug wrote:
> > This seems to work in Linux but may not work in Android:
> > AC_SEARCH_LIBS([pthread_cancel], [pthread],
> > [AC_DEFINE(HAVE_PTHREAD_CANCEL, 1, [Found pthread_cancel])], [])
> > 
> > Bean, could you check?
> > 
> 
> Doug, I had tried that, it works, but this is for C codes macro definition in 
> the config.h.
> How can I figure out in the /src/Makefile.ac between Android OS and 
> non-Android?
> Can you give me some suggestions? Like this:
>  if OS_ANDROID
>  sgp_dd_LDADD = ../lib/libsgutils2.la @os_libs@
> else
> sgp_dd_LDADD = ../lib/libsgutils2.la @os_libs@ -lpthread
> endif

How about leaving out the following:
- Use AC_SEARCH_LIBS([pthread_cancel], [pthread]) and 
AC_CHECK_FUNC([pthread_cancel]) in
  configure.ac. 
- Use @LIBS@ in Makefile.am.

From 
https://www.gnu.org/software/autoconf/manual/autoconf-2.66/html_node/Libraries.html:
"If action-if-found is not specified, the default action prepends -llibrary to 
LIBS [...]".

Bart.

Re: [RFC] sg3_utils: sgp_dd: work around on pthread_cancel for android

[Bean Huo (beanhuo)]

Hi, Doug and Bart
Thanks so much for your prompt response.
See below my action:

>On 2017-12-22 01:24 PM, Bart Van Assche wrote:
>> On Fri, 2017-12-22 at 17:00 +, Bean Huo (beanhuo) wrote:
>>>   case "${host}" in
>>> +*-*-android*)
>>> +   AC_DEFINE_UNQUOTED(SG_ON_ANDROID, 1, [sg3_utils on
>android])
>>> +   AC_DEFINE_UNQUOTED(SG_LIB_LINUX, 1, [sg3_utils on linux])
>>> +AC_SUBST([os_cflags], [''])
>>> +AC_SUBST([os_libs], ['']) ;;
>>>   *-*-linux-gnu*)
>>> AC_DEFINE_UNQUOTED(SG_LIB_LINUX, 1, [sg3_utils on linux])
>>>   AC_SUBST([os_cflags], ['']) @@ -79,6 +84,7 @@
>>> AM_CONDITIONAL(OS_OSF, [echo $host_os | grep '^osf' > /dev/null])
>>>   AM_CONDITIONAL(OS_SOLARIS, [echo $host_os | grep '^solaris' >
>/dev/null])
>>>   AM_CONDITIONAL(OS_WIN32_MINGW, [echo $host_os | grep '^mingw' >
>/dev/null])
>>>   AM_CONDITIONAL(OS_WIN32_CYGWIN, [echo $host_os | grep '^cygwin' >
>>> /dev/null])
>>> +AM_CONDITIONAL(OS_ANDROID, [echo $host_os | grep 'android' >
>>> +/dev/null])
>>
>> Hello Bean,
>>
>> Please consider to use AC_CHECK_FUNC([pthread_cancel]) or similar to
>> check the availability of pthread_cancel(). Explicit distro checks are
>> much harder to maintain than checks for individual functions.
>
>That macro doesn't work in Linux. It doesn't work in the sense that Linux
>(Ubuntu 17.10) does have pthread_cancel but needs -lpthread in the link. So
>AC_CHECK_FUNC([pthread_cancel]) does not place '#define
>HAVE_PTHREAD_CANCEL 1' in config.h .
>
Correct, pthread_cancel needs -lpthread in the link, AC_CHECK_FUNC cannot
Properly figure out that.

>This seems to work in Linux but may not work in Android:
>AC_SEARCH_LIBS([pthread_cancel], [pthread],
>[AC_DEFINE(HAVE_PTHREAD_CANCEL, 1, [Found pthread_cancel])], [])
>
>Bean, could you check?
>
Doug, I had tried that, it works, but this is for C codes macro definition in 
the config.h.
How can I figure out in the /src/Makefile.ac between Android OS and non-Android?
Can you give me some suggestions? Like this:
 if OS_ANDROID
 sgp_dd_LDADD = ../lib/libsgutils2.la @os_libs@
else
sgp_dd_LDADD = ../lib/libsgutils2.la @os_libs@ -lpthread
endif
>Anyway, I like having an OS type for Android (which I have renamed from
>Bean's patch to SG_LIB_ANDROID to be consistent with my other OS naming).
>Overall the patch works in Linux.
>
Yes, we still need this OS type for Android, since there are several codes 
needed to be
Modified based on this OS type. For example, block device node folder in the 
Android
is /dev/block/sgx..., but for the other Linux based OS, it is /dev/sgx

Have a great holiday!
Bean Huo