scsi-sg: pass flag to inhibit setting LUN

PATCH In review Jiří Pinkava 4 participants 7 messages Jan 2, 2014 – Jan 20, 2014

Subsystems (3)

SCSI Block layer SCSI SG driver

lore

Share & tools

lore

Jiří Pinkava

author

2014-01-02 13:33 UTC #0 lore raw

PATCH 1/1 scsi-sg: pass flag to inhibit setting LUN

Hi,

This patch implements support for inhibiting setting LUN number for SCSI custom command send via /dev/sgX with ioctl(.., SG_IO, ...) call.

This solves problems with some devices which claim support of SCSI v1/v2, but for some special purposes use custom commands which does not conform with standard message format. (Like mine Pentax digital single-lens reflex camera) It does not affect devices with SCSI v3 or latter.

For this purpose was earlier introduced SG_FLAG_LUN_INHIBIT / SG_FLAG_UNUSED_LUN_INHIBIT (glibc/kernel) flag, but the implementation was lost in some earlier code refactor.

The only possible issue I see is current implementation supports only SG driver but there is few more code paths where the similar ioctl can be invoked
(eg. anny SCSI device?). I'm not sure about, feel free to say anything about it.

---
 drivers/scsi/scsi.c       | 3 ++-
 drivers/scsi/sg.c         | 3 +++
 include/linux/blk_types.h | 2 ++
 3 files changed, 7 insertions(+), 1 deletion(-)

drivers/scsi/scsi.c +2 -1

index fe0bcb1..f6d5fc9 100644
@@ -693,7 +693,8 @@ int scsi_dispatch_cmd(struct scsi_cmnd *cmd)
      * If SCSI-2 or lower, store the LUN value in cmnd.
      */
     if (cmd->device->scsi_level <= SCSI_2 &&
-        cmd->device->scsi_level != SCSI_UNKNOWN) {
+        cmd->device->scsi_level != SCSI_UNKNOWN &&
+        !(cmd->request->cmd_flags & REQ_LUN_INHIBIT)) {
         cmd->cmnd[1] = (cmd->cmnd[1] & 0x1f) |
                    (cmd->device->lun << 5 & 0xe0);
     }

drivers/scsi/sg.c +0 -0

index df5e961..8e09015 100644
@@ -1663,6 +1663,9 @@ static int sg_start_req(Sg_request *srp, unsigned

char *cmd)

     rq->sense = srp->sense_b;
     rq->retries = SG_DEFAULT_RETRIES;

+ if (hp->flags & SG_FLAG_UNUSED_LUN_INHIBIT)
+ rq->cmd_flags |= REQ_LUN_INHIBIT;
+

     if ((dxfer_len <= 0) || (dxfer_dir == SG_DXFER_NONE))
         return 0;

include/linux/blk_types.h +1 -0

index 238ef0e..35d436b 100644
@@ -178,6 +178,7 @@ enum rq_flag_bits {
     __REQ_MIXED_MERGE,    /* merge of different types, fail separately */
     __REQ_KERNEL,         /* direct IO to kernel pages */
     __REQ_PM,        /* runtime pm request */
+    __REQ_LUN_INHIBIT,    /* pass through for

SG_FLAG_UNUSED_LUN_INHIBIT flag */
__REQ_END, /* last of chain of requests */
__REQ_NR_BITS, /* stops here */
};
@@ -230,6 +231,7 @@ enum rq_flag_bits {

 #define REQ_SECURE        (1ULL << __REQ_SECURE)
 #define REQ_KERNEL        (1ULL << __REQ_KERNEL)
 #define REQ_PM            (1ULL << __REQ_PM)

+#define REQ_LUN_INHIBIT (1ULL << __REQ_LUN_INHIBIT)
#define REQ_END (1ULL << __REQ_END)

#endif /* __LINUX_BLK_TYPES_H */

Signature · 1 line

1.8.5.2

Douglas Gilbert

maintainer

2014-01-03 20:10 UTC #1 lore raw

Quoted 15 lines · Jiří Pinkava

Hi,
This patch implements support for inhibiting setting LUN number for SCSI custom command send via /dev/sgX with ioctl(.., SG_IO, ...) call.
This solves problems with some devices which claim support of SCSI v1/v2, but for some special purposes use custom commands which does not conform with standard message format. (Like mine Pentax digital single-lens reflex camera) It does not affect devices with SCSI v3 or latter.
For this purpose was earlier introduced SG_FLAG_LUN_INHIBIT / SG_FLAG_UNUSED_LUN_INHIBIT (glibc/kernel) flag, but the implementation was lost in some earlier code refactor.

Quoted 5 lines

The only possible issue I see is current implementation supports only SG driver but there is few more code paths where the similar ioctl can be invoked
(eg. anny SCSI device?). I'm not sure about, feel free to say anything about it.

Hi,
SG_FLAG_LUN_INHIBIT was added to the sg driver around 15 years ago. The code to centralize the masking in scsi.c and remove the "LUN inhibit" capability arrived about 10 years ago. A handful of people have complained about it since. An you are the first to do something about it!

I have a few comments about the code (see below) but the logic seems okay to me. That said I would be very surprised if this is allowed back in the kernel. If you manage to do that then I'll be studying your technique (because many of my patches to the sg driver are not accepted).

So you can takes that as an "ack" with comments and good luck. I'll let others judge.

Quoted 11 lines

---
  drivers/scsi/scsi.c       | 3 ++-
  drivers/scsi/sg.c         | 3 +++
  include/linux/blk_types.h | 2 ++
  3 files changed, 7 insertions(+), 1 deletion(-)

drivers/scsi/scsi.c +0 -0

index fe0bcb1..f6d5fc9 100644

drivers/scsi/scsi.c int scsi_dispatch_cmd(struct scsi_cmnd *cmd) · line 700

Quoted 9 lines

drivers/scsi/scsi.c +2 -1

@@ -693,7 +693,8 @@ int scsi_dispatch_cmd(struct scsi_cmnd *cmd)
       * If SCSI-2 or lower, store the LUN value in cmnd.
       */
      if (cmd->device->scsi_level <= SCSI_2 &&
-        cmd->device->scsi_level != SCSI_UNKNOWN) {
+        cmd->device->scsi_level != SCSI_UNKNOWN &&
+        !(cmd->request->cmd_flags & REQ_LUN_INHIBIT)) {
          cmd->cmnd[1] = (cmd->cmnd[1] & 0x1f) |
                     (cmd->device->lun << 5 & 0xe0);
      }

The above being time sensitive code I was thinking a switch might make it clearer and give the compiler more chances to optimize:

      switch(cmd->device->scsi_level) {
      case SCSI_2:
      case SCSI_1_CCS:
      case SCSI_1:
              if (!(cmd->request->cmd_flags & REQ_LUN_INHIBIT)) {
                      cmd->cmnd[1] = (cmd->cmnd[1] & 0x1f) |
                                     (cmd->device->lun << 5 & 0xe0);
              }
              break;
      default:
              ;
      }

Hmm, can a switch's default be made "likely"?

More generally folks, with SAS SSDs available that can read at 1100 MB/sec we may want to spend time looking at the fast paths through the SCSI subsystem.

Quoted 30 lines

drivers/scsi/sg.c +3 -0

index df5e961..8e09015 100644
@@ -1663,6 +1663,9 @@ static int sg_start_req(Sg_request *srp, unsigned
 char *cmd)
      rq->sense = srp->sense_b;
      rq->retries = SG_DEFAULT_RETRIES;
 
+    if (hp->flags & SG_FLAG_UNUSED_LUN_INHIBIT)
+        rq->cmd_flags |=  REQ_LUN_INHIBIT;
+
      if ((dxfer_len <= 0) || (dxfer_dir == SG_DXFER_NONE))
          return 0;

include/linux/blk_types.h +1 -0

index 238ef0e..35d436b 100644
@@ -178,6 +178,7 @@ enum rq_flag_bits {
      __REQ_MIXED_MERGE,    /* merge of different types, fail separately */
      __REQ_KERNEL,         /* direct IO to kernel pages */
      __REQ_PM,        /* runtime pm request */
+    __REQ_LUN_INHIBIT,    /* pass through for

SG_FLAG_UNUSED_LUN_INHIBIT flag */
__REQ_END, /* last of chain of requests */
__REQ_NR_BITS, /* stops here */
};

include/linux/blk_types.h enum rq_flag_bits { · line 237

Quoted 7 lines

include/linux/blk_types.h +1 -0

@@ -230,6 +231,7 @@ enum rq_flag_bits {
  #define REQ_SECURE        (1ULL << __REQ_SECURE)
  #define REQ_KERNEL        (1ULL << __REQ_KERNEL)
  #define REQ_PM            (1ULL << __REQ_PM)
+#define REQ_LUN_INHIBIT    (1ULL << __REQ_LUN_INHIBIT)
  #define REQ_END            (1ULL << __REQ_END)
 
  #endif /* __LINUX_BLK_TYPES_H */

And I think the SG_FLAG_LUN_INHIBIT define should be re-instated to sg.h . For backward compatibility (for the last 10 years) please leave the SG_FLAG_UNUSED_LUN_INHIBIT define there. Both defines should have the same value.

Doug Gilbert

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Jiří Pinkava

author

2014-01-06 14:10 UTC #2 lore raw

Dne 3.1.2014 21:10, Douglas Gilbert napsal(a):

Quoted 76 lines · Jiří Pinkava

Quoted 16 lines
Hi,
This patch implements support for inhibiting setting LUN number for SCSI custom command send via /dev/sgX with ioctl(.., SG_IO, ...) call.
This solves problems with some devices which claim support of SCSI v1/v2, but for some special purposes use custom commands which does not conform with standard message format. (Like mine Pentax digital single-lens reflex camera) It does not affect devices with SCSI v3 or latter.
For this purpose was earlier introduced SG_FLAG_LUN_INHIBIT / SG_FLAG_UNUSED_LUN_INHIBIT (glibc/kernel) flag, but the implementation was lost in some earlier code refactor.
Quoted 5 lines
The only possible issue I see is current implementation supports only SG driver but there is few more code paths where the similar ioctl can be invoked
(eg. anny SCSI device?). I'm not sure about, feel free to say anything about it.
Hi,
SG_FLAG_LUN_INHIBIT was added to the sg driver around 15 years ago. The code to centralize the masking in scsi.c and remove the "LUN inhibit" capability arrived about 10 years ago. A handful of people have complained about it since. An you are the first to do something about it!
I have a few comments about the code (see below) but the logic seems okay to me. That said I would be very surprised if this is allowed back in the kernel. If you manage to do that then I'll be studying your technique (because many of my patches to the sg driver are not accepted).
So you can takes that as an "ack" with comments and good luck. I'll let others judge.
Quoted 11 lines
---
  drivers/scsi/scsi.c       | 3 ++-
  drivers/scsi/sg.c         | 3 +++
  include/linux/blk_types.h | 2 ++
  3 files changed, 7 insertions(+), 1 deletion(-)
drivers/scsi/scsi.c +0 -0
index fe0bcb1..f6d5fc9 100644
drivers/scsi/scsi.c int scsi_dispatch_cmd(struct scsi_cmnd *cmd) · line 700
Quoted 9 lines
drivers/scsi/scsi.c +2 -1
@@ -693,7 +693,8 @@ int scsi_dispatch_cmd(struct scsi_cmnd *cmd)
       * If SCSI-2 or lower, store the LUN value in cmnd.
       */
      if (cmd->device->scsi_level <= SCSI_2 &&
-        cmd->device->scsi_level != SCSI_UNKNOWN) {
+        cmd->device->scsi_level != SCSI_UNKNOWN &&
+        !(cmd->request->cmd_flags & REQ_LUN_INHIBIT)) {
          cmd->cmnd[1] = (cmd->cmnd[1] & 0x1f) |
                     (cmd->device->lun << 5 & 0xe0);
      }
The above being time sensitive code I was thinking a switch might make it clearer and give the compiler more chances to optimize:
      switch(cmd->device->scsi_level) {
      case SCSI_2:
      case SCSI_1_CCS:
      case SCSI_1:
              if (!(cmd->request->cmd_flags & REQ_LUN_INHIBIT)) {
                      cmd->cmnd[1] = (cmd->cmnd[1] & 0x1f) |
                                     (cmd->device->lun << 5 & 0xe0);
              }
              break;
      default:
              ;
      }

Quoted 1 line

Hmm, can a switch's default be made "likely"?

In switch can be used only __branch_check__ (E, C) or __builtin_expect(E, C), which
have too many '_' in name and I'm too afraid to use them.

switch (__builtin_expect(var, const)) {
...

if there is such a pressure for speed, unlikely(cmd->device->scsi_level <= SCSI_2)
might be used, but i does not have statistics about how much modern high-speed devices report itself as scsi_level <= SCSI_2 or scsi_level == SCSI_UNKNOWN.
(suppose the rate is pretty low).

The switch solution look more elegant.

Quoted 50 lines

More generally folks, with SAS SSDs available that can read at 1100 MB/sec we may want to spend time looking at the fast paths through the SCSI subsystem.
Quoted 31 lines
drivers/scsi/sg.c +3 -0
index df5e961..8e09015 100644
@@ -1663,6 +1663,9 @@ static int sg_start_req(Sg_request *srp, unsigned
 char *cmd)
      rq->sense = srp->sense_b;
      rq->retries = SG_DEFAULT_RETRIES;
 
+    if (hp->flags & SG_FLAG_UNUSED_LUN_INHIBIT)
+        rq->cmd_flags |=  REQ_LUN_INHIBIT;
+
      if ((dxfer_len <= 0) || (dxfer_dir == SG_DXFER_NONE))
          return 0;
include/linux/blk_types.h +0 -0
index 238ef0e..35d436b 100644
@@ -178,6 +178,7 @@ enum rq_flag_bits {
      __REQ_MIXED_MERGE,    /* merge of different types, fail
separately */
__REQ_KERNEL, /* direct IO to kernel pages */
__REQ_PM, /* runtime pm request */
+ __REQ_LUN_INHIBIT, /* pass through for
SG_FLAG_UNUSED_LUN_INHIBIT flag */
__REQ_END, /* last of chain of requests */
__REQ_NR_BITS, /* stops here */
};
include/linux/blk_types.h enum rq_flag_bits { · line 237
Quoted 7 lines
include/linux/blk_types.h +1 -0
@@ -230,6 +231,7 @@ enum rq_flag_bits {
  #define REQ_SECURE        (1ULL << __REQ_SECURE)
  #define REQ_KERNEL        (1ULL << __REQ_KERNEL)
  #define REQ_PM            (1ULL << __REQ_PM)
+#define REQ_LUN_INHIBIT    (1ULL << __REQ_LUN_INHIBIT)
  #define REQ_END            (1ULL << __REQ_END)
 
  #endif /* __LINUX_BLK_TYPES_H */
And I think the SG_FLAG_LUN_INHIBIT define should be re-instated to sg.h . For backward compatibility (for the last 10 years) please leave the SG_FLAG_UNUSED_LUN_INHIBIT define there. Both defines should have the same value.

Quoted 2 lines

Doug Gilbert

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Douglas Gilbert

maintainer

2014-01-19 15:56 UTC #3 lore raw

In reply to Douglas Gilbert

Quoted 117 lines · Douglas Gilbert

Quoted 15 lines · Jiří Pinkava
Hi,
This patch implements support for inhibiting setting LUN number for SCSI custom command send via /dev/sgX with ioctl(.., SG_IO, ...) call.
This solves problems with some devices which claim support of SCSI v1/v2, but for some special purposes use custom commands which does not conform with standard message format. (Like mine Pentax digital single-lens reflex camera) It does not affect devices with SCSI v3 or latter.
For this purpose was earlier introduced SG_FLAG_LUN_INHIBIT / SG_FLAG_UNUSED_LUN_INHIBIT (glibc/kernel) flag, but the implementation was lost in some earlier code refactor.
Quoted 5 lines
The only possible issue I see is current implementation supports only SG driver but there is few more code paths where the similar ioctl can be invoked
(eg. anny SCSI device?). I'm not sure about, feel free to say anything about it.
Hi,
SG_FLAG_LUN_INHIBIT was added to the sg driver around 15 years ago. The code to centralize the masking in scsi.c and remove the "LUN inhibit" capability arrived about 10 years ago. A handful of people have complained about it since. An you are the first to do something about it!
I have a few comments about the code (see below) but the logic seems okay to me. That said I would be very surprised if this is allowed back in the kernel. If you manage to do that then I'll be studying your technique (because many of my patches to the sg driver are not accepted).
So you can takes that as an "ack" with comments and good luck. I'll let others judge.
Quoted 11 lines
---
  drivers/scsi/scsi.c       | 3 ++-
  drivers/scsi/sg.c         | 3 +++
  include/linux/blk_types.h | 2 ++
  3 files changed, 7 insertions(+), 1 deletion(-)
drivers/scsi/scsi.c +0 -0
index fe0bcb1..f6d5fc9 100644
drivers/scsi/scsi.c int scsi_dispatch_cmd(struct scsi_cmnd *cmd) · line 700
Quoted 9 lines
drivers/scsi/scsi.c +2 -1
@@ -693,7 +693,8 @@ int scsi_dispatch_cmd(struct scsi_cmnd *cmd)
       * If SCSI-2 or lower, store the LUN value in cmnd.
       */
      if (cmd->device->scsi_level <= SCSI_2 &&
-        cmd->device->scsi_level != SCSI_UNKNOWN) {
+        cmd->device->scsi_level != SCSI_UNKNOWN &&
+        !(cmd->request->cmd_flags & REQ_LUN_INHIBIT)) {
          cmd->cmnd[1] = (cmd->cmnd[1] & 0x1f) |
                     (cmd->device->lun << 5 & 0xe0);
      }
The above being time sensitive code I was thinking a switch might make it clearer and give the compiler more chances to optimize:
       switch(cmd->device->scsi_level) {
       case SCSI_2:
       case SCSI_1_CCS:
       case SCSI_1:
               if (!(cmd->request->cmd_flags & REQ_LUN_INHIBIT)) {
                       cmd->cmnd[1] = (cmd->cmnd[1] & 0x1f) |
                                      (cmd->device->lun << 5 & 0xe0);
               }
               break;
       default:
               ;
       }
Hmm, can a switch's default be made "likely"?
More generally folks, with SAS SSDs available that can read at 1100 MB/sec we may want to spend time looking at the fast paths through the SCSI subsystem.
Quoted 35 lines
drivers/scsi/sg.c +3 -0
index df5e961..8e09015 100644
@@ -1663,6 +1663,9 @@ static int sg_start_req(Sg_request *srp, unsigned
 char *cmd)
      rq->sense = srp->sense_b;
      rq->retries = SG_DEFAULT_RETRIES;
 
+    if (hp->flags & SG_FLAG_UNUSED_LUN_INHIBIT)
+        rq->cmd_flags |=  REQ_LUN_INHIBIT;
+
      if ((dxfer_len <= 0) || (dxfer_dir == SG_DXFER_NONE))
          return 0;
include/linux/blk_types.h +1 -0
index 238ef0e..35d436b 100644
@@ -178,6 +178,7 @@ enum rq_flag_bits {
      __REQ_MIXED_MERGE,    /* merge of different types, fail separately */
      __REQ_KERNEL,         /* direct IO to kernel pages */
      __REQ_PM,        /* runtime pm request */
+    __REQ_LUN_INHIBIT,    /* pass through for
SG_FLAG_UNUSED_LUN_INHIBIT flag */
__REQ_END, /* last of chain of requests */
__REQ_NR_BITS, /* stops here */
};
include/linux/blk_types.h +1 -0
@@ -230,6 +231,7 @@ enum rq_flag_bits {
  #define REQ_SECURE        (1ULL << __REQ_SECURE)
  #define REQ_KERNEL        (1ULL << __REQ_KERNEL)
  #define REQ_PM            (1ULL << __REQ_PM)
+#define REQ_LUN_INHIBIT    (1ULL << __REQ_LUN_INHIBIT)
  #define REQ_END            (1ULL << __REQ_END)
 

Quoted 7 lines

include/linux/blk_types.h enum rq_flag_bits { · line 237
Quoted 2 lines
include/linux/blk_types.h +0 -0
  #endif /* __LINUX_BLK_TYPES_H */
And I think the SG_FLAG_LUN_INHIBIT define should be re-instated to sg.h . For backward compatibility (for the last 10 years) please leave the SG_FLAG_UNUSED_LUN_INHIBIT define there. Both defines should have the same value.

Not sure if this patch is going anywhere, but I do get queries periodically that boil down to a request for this capability to be re-instated.

In the last week someone sent me a usbmon dump showing how sg_raw worked (or at least obeyed the command line supplied cdb) from Windows but failed from Linux. Linux masking byte 1 of the cdb was the reason.

Doug Gilbert

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

James Bottomley

maintainer

2014-01-20 03:23 UTC #4 lore raw

Quoted 126 lines · Douglas Gilbert

Quoted 117 lines · Douglas Gilbert
Quoted 15 lines · Jiří Pinkava
Hi,
This patch implements support for inhibiting setting LUN number for SCSI custom command send via /dev/sgX with ioctl(.., SG_IO, ...) call.
This solves problems with some devices which claim support of SCSI v1/v2, but for some special purposes use custom commands which does not conform with standard message format. (Like mine Pentax digital single-lens reflex camera) It does not affect devices with SCSI v3 or latter.
For this purpose was earlier introduced SG_FLAG_LUN_INHIBIT / SG_FLAG_UNUSED_LUN_INHIBIT (glibc/kernel) flag, but the implementation was lost in some earlier code refactor.
Quoted 5 lines
The only possible issue I see is current implementation supports only SG driver but there is few more code paths where the similar ioctl can be invoked
(eg. anny SCSI device?). I'm not sure about, feel free to say anything about it.
Hi,
SG_FLAG_LUN_INHIBIT was added to the sg driver around 15 years ago. The code to centralize the masking in scsi.c and remove the "LUN inhibit" capability arrived about 10 years ago. A handful of people have complained about it since. An you are the first to do something about it!
I have a few comments about the code (see below) but the logic seems okay to me. That said I would be very surprised if this is allowed back in the kernel. If you manage to do that then I'll be studying your technique (because many of my patches to the sg driver are not accepted).
So you can takes that as an "ack" with comments and good luck. I'll let others judge.
Quoted 11 lines
---
  drivers/scsi/scsi.c       | 3 ++-
  drivers/scsi/sg.c         | 3 +++
  include/linux/blk_types.h | 2 ++
  3 files changed, 7 insertions(+), 1 deletion(-)
drivers/scsi/scsi.c +0 -0
index fe0bcb1..f6d5fc9 100644
drivers/scsi/scsi.c int scsi_dispatch_cmd(struct scsi_cmnd *cmd) · line 700
Quoted 9 lines
drivers/scsi/scsi.c +2 -1
@@ -693,7 +693,8 @@ int scsi_dispatch_cmd(struct scsi_cmnd *cmd)
       * If SCSI-2 or lower, store the LUN value in cmnd.
       */
      if (cmd->device->scsi_level <= SCSI_2 &&
-        cmd->device->scsi_level != SCSI_UNKNOWN) {
+        cmd->device->scsi_level != SCSI_UNKNOWN &&
+        !(cmd->request->cmd_flags & REQ_LUN_INHIBIT)) {
          cmd->cmnd[1] = (cmd->cmnd[1] & 0x1f) |
                     (cmd->device->lun << 5 & 0xe0);
      }
The above being time sensitive code I was thinking a switch might make it clearer and give the compiler more chances to optimize:
       switch(cmd->device->scsi_level) {
       case SCSI_2:
       case SCSI_1_CCS:
       case SCSI_1:
               if (!(cmd->request->cmd_flags & REQ_LUN_INHIBIT)) {
                       cmd->cmnd[1] = (cmd->cmnd[1] & 0x1f) |
                                      (cmd->device->lun << 5 & 0xe0);
               }
               break;
       default:
               ;
       }
Hmm, can a switch's default be made "likely"?
More generally folks, with SAS SSDs available that can read at 1100 MB/sec we may want to spend time looking at the fast paths through the SCSI subsystem.
Quoted 35 lines
drivers/scsi/sg.c +3 -0
index df5e961..8e09015 100644
@@ -1663,6 +1663,9 @@ static int sg_start_req(Sg_request *srp, unsigned
 char *cmd)
      rq->sense = srp->sense_b;
      rq->retries = SG_DEFAULT_RETRIES;
 
+    if (hp->flags & SG_FLAG_UNUSED_LUN_INHIBIT)
+        rq->cmd_flags |=  REQ_LUN_INHIBIT;
+
      if ((dxfer_len <= 0) || (dxfer_dir == SG_DXFER_NONE))
          return 0;
include/linux/blk_types.h +1 -0
index 238ef0e..35d436b 100644
@@ -178,6 +178,7 @@ enum rq_flag_bits {
      __REQ_MIXED_MERGE,    /* merge of different types, fail separately */
      __REQ_KERNEL,         /* direct IO to kernel pages */
      __REQ_PM,        /* runtime pm request */
+    __REQ_LUN_INHIBIT,    /* pass through for
SG_FLAG_UNUSED_LUN_INHIBIT flag */
__REQ_END, /* last of chain of requests */
__REQ_NR_BITS, /* stops here */
};
include/linux/blk_types.h +1 -0
@@ -230,6 +231,7 @@ enum rq_flag_bits {
  #define REQ_SECURE        (1ULL << __REQ_SECURE)
  #define REQ_KERNEL        (1ULL << __REQ_KERNEL)
  #define REQ_PM            (1ULL << __REQ_PM)
+#define REQ_LUN_INHIBIT    (1ULL << __REQ_LUN_INHIBIT)
  #define REQ_END            (1ULL << __REQ_END)
 
Quoted 7 lines
include/linux/blk_types.h enum rq_flag_bits { · line 237
Quoted 2 lines
include/linux/blk_types.h +0 -0
  #endif /* __LINUX_BLK_TYPES_H */
And I think the SG_FLAG_LUN_INHIBIT define should be re-instated to sg.h . For backward compatibility (for the last 10 years) please leave the SG_FLAG_UNUSED_LUN_INHIBIT define there. Both defines should have the same value.

Quoted 8 lines

Not sure if this patch is going anywhere, but I do get queries periodically that boil down to a request for this capability to be re-instated.
In the last week someone sent me a usbmon dump showing how sg_raw worked (or at least obeyed the command line supplied cdb) from Windows but failed from Linux. Linux masking byte 1 of the cdb was the reason.

OK, I'll look at this after the merge window, but please consider

1. If we're doing this, it has to be SG_IO via block and bsg as well, it can't be sg module only
2. It can't be per-command: a device can't rely on the CDB[1] bits for some commands and not for others, it must be all or nothing (otherwise how do the exception commands get addressed to non zero luns?)
3. Do we already have a flag we can use for this, like BLIST_NOLUN or BLIST_FORCELUN or BLIST_REPORTLUN2

James

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Martin K. Petersen

2014-01-03 20:22 UTC #5 lore raw

In reply to Jiří Pinkava

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Jiří" == Jiří Pinkava <[email protected]> writes:

Jiří> This patch implements support for inhibiting setting LUN number Jiří> for SCSI custom command send via /dev/sgX with ioctl(.., SG_IO, Jiří> ...) call.

Is it an absolute requirement that this is a per-command option or could we have a scsi_device quirk flag so we didn't have to plumb this through the entire I/O stack?

Signature · 5 lines

Martin K. Petersen	Oracle Linux Engineering
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Jiří Pinkava

author

2014-01-03 21:00 UTC #6 lore raw

Dne 3.1.2014 21:21, Martin K. Petersen napsal(a):

Quoted 9 lines

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Jiří" == Jiří Pinkava <[email protected]> writes:
Jiří> This patch implements support for inhibiting setting LUN number Jiří> for SCSI custom command send via /dev/sgX with ioctl(.., SG_IO, Jiří> ...) call.
Is it an absolute requirement that this is a per-command option or could we have a scsi_device quirk flag so we didn't have to plumb this through the entire I/O stack?

It should be enough to limit it to open file descriptor or sg subsystem.

This magic is expected to happen every time device is accessed trough sg subsystem, but not if it is accessed other way (eg. it is still some kind of common disk).

My understanding is that sg subsystem allows generic access to device, in extreme case I might want to send command to LUN which is not the active one.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html