[PATCH-FOR-38] target: Fix READ_CAPACITY_16 regression

[PATCH-FOR-38] target: Fix READ_CAPACITY_16 regression

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

Hi James,

This patch fixes a regression for READ_CAPACITY to trigger SAI READ_CAPACITY_16 for
>= 0xffffffff virtual backends.  This occured during v4.0.0-rc6 when the backend
read_capacity* handlers where moved into generic target_core_cdb.c code, and the
'unsigned long long blocks_long' piece was dropped from target_emulate_readcapacity().

This fix has been tested with TCM_Loop on .38-rc7 with lio-4.1 and is working as expected
with large block virtual backends:

[67826.897061] TARGET_CORE[loopback]->TPG[1]_LUN[0] - Adding READ-WRITE access for LUN in Demo Mode
[67826.897061] scsi 7:0:1:0: Direct-Access     LIO-ORG  FILEIO           4.0  PQ: 0 ANSI: 5
[67826.900933] sd 7:0:1:0: [sdd] 2621440000001 512-byte logical blocks: (1.34 PB/1.19 PiB)
[67826.901510] sd 7:0:1:0: [sdd] Write Protect is off
[67826.901684] sd 7:0:1:0: [sdd] Mode Sense: 2f 00 00 00

Please include into scsi-rc-fixes going to Linus for-38 mainline target code.

Reported-by: Ben Jarvis <bjarvismn@gmail.com>
Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: Signed-off-by: Nicholas A. Bellinger <nab@linux-iscsi.org>
---
 drivers/target/target_core_cdb.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/drivers/target/target_core_cdb.c b/drivers/target/target_core_cdb.c
index 366080b..7f19c8b 100644
--- a/drivers/target/target_core_cdb.c
+++ b/drivers/target/target_core_cdb.c
@@ -667,7 +667,13 @@ target_emulate_readcapacity(struct se_cmd *cmd)
 {
 	struct se_device *dev = SE_DEV(cmd);
 	unsigned char *buf = cmd->t_task->t_task_buf;
-	u32 blocks = dev->transport->get_blocks(dev);
+	unsigned long long blocks_long = dev->transport->get_blocks(dev);
+	u32 blocks;
+
+	if (blocks_long >= 0x00000000ffffffff)
+		blocks = 0xffffffff;
+	else
+		blocks = (u32)blocks_long;
 
 	buf[0] = (blocks >> 24) & 0xff;
 	buf[1] = (blocks >> 16) & 0xff;
-- 
1.5.6.5

Re: [PATCH-FOR-38] target: Fix READ_CAPACITY_16 regression

[James Bottomley]

On Wed, 2011-03-02 at 15:52 -0800, Nicholas A. Bellinger wrote:
> From: Nicholas Bellinger <nab@linux-iscsi.org>
> 
> Hi James,
> 
> This patch fixes a regression for READ_CAPACITY to trigger SAI READ_CAPACITY_16 for
> >= 0xffffffff virtual backends.  This occured during v4.0.0-rc6 when the backend
> read_capacity* handlers where moved into generic target_core_cdb.c code, and the
> 'unsigned long long blocks_long' piece was dropped from target_emulate_readcapacity().
> 
> This fix has been tested with TCM_Loop on .38-rc7 with lio-4.1 and is working as expected
> with large block virtual backends:
> 
> [67826.897061] TARGET_CORE[loopback]->TPG[1]_LUN[0] - Adding READ-WRITE access for LUN in Demo Mode
> [67826.897061] scsi 7:0:1:0: Direct-Access     LIO-ORG  FILEIO           4.0  PQ: 0 ANSI: 5
> [67826.900933] sd 7:0:1:0: [sdd] 2621440000001 512-byte logical blocks: (1.34 PB/1.19 PiB)
> [67826.901510] sd 7:0:1:0: [sdd] Write Protect is off
> [67826.901684] sd 7:0:1:0: [sdd] Mode Sense: 2f 00 00 00
> 
> Please include into scsi-rc-fixes going to Linus for-38 mainline target code.

So the piece you didn't say is that this only affects volumes >2TB
volume which misreport the capacity (which is actually what the bug
description should have been).

That's hardly an oopsworthy fix at this stage ... I'll put it into misc
with a cc stable.

James

Re: [PATCH-FOR-38] target: Fix READ_CAPACITY_16 regression

[Nicholas A. Bellinger]

On Fri, 2011-03-11 at 17:17 -0600, James Bottomley wrote:
> On Wed, 2011-03-02 at 15:52 -0800, Nicholas A. Bellinger wrote:
> > From: Nicholas Bellinger <nab@linux-iscsi.org>
> > 
> > Hi James,
> > 
> > This patch fixes a regression for READ_CAPACITY to trigger SAI READ_CAPACITY_16 for
> > >= 0xffffffff virtual backends.  This occured during v4.0.0-rc6 when the backend
> > read_capacity* handlers where moved into generic target_core_cdb.c code, and the
> > 'unsigned long long blocks_long' piece was dropped from target_emulate_readcapacity().
> > 
> > This fix has been tested with TCM_Loop on .38-rc7 with lio-4.1 and is working as expected
> > with large block virtual backends:
> > 
> > [67826.897061] TARGET_CORE[loopback]->TPG[1]_LUN[0] - Adding READ-WRITE access for LUN in Demo Mode
> > [67826.897061] scsi 7:0:1:0: Direct-Access     LIO-ORG  FILEIO           4.0  PQ: 0 ANSI: 5
> > [67826.900933] sd 7:0:1:0: [sdd] 2621440000001 512-byte logical blocks: (1.34 PB/1.19 PiB)
> > [67826.901510] sd 7:0:1:0: [sdd] Write Protect is off
> > [67826.901684] sd 7:0:1:0: [sdd] Mode Sense: 2f 00 00 00
> > 
> > Please include into scsi-rc-fixes going to Linus for-38 mainline target code.
> 
> So the piece you didn't say is that this only affects volumes >2TB
> volume which misreport the capacity (which is actually what the bug
> description should have been).
> 
> That's hardly an oopsworthy fix at this stage ... I'll put it into misc
> with a cc stable.
> 

This has already been picked up by AKPM last week, and (I assume) will
be queued to Linus for .38-FINAL via -mm.

Considering that is a critical fix that is not trigger by some obsecure
corner case, I am not sure why this would be deferred for for a
v2.6.38.x release when we already have a proper fix in place..?

--nab

Re: [PATCH-FOR-38] target: Fix READ_CAPACITY_16 regression

[Nicholas A. Bellinger]

On Fri, 2011-03-11 at 18:40 -0800, Andrew Morton wrote:
> On Fri, 11 Mar 2011 17:46:11 -0800 "Nicholas A. Bellinger" <nab@linux-iscsi.org> wrote:
> 
> > On Fri, 2011-03-11 at 17:17 -0600, James Bottomley wrote:
> > > On Wed, 2011-03-02 at 15:52 -0800, Nicholas A. Bellinger wrote:
> > > > From: Nicholas Bellinger <nab@linux-iscsi.org>
> > > > 
> > > > Hi James,
> > > > 
> > > > This patch fixes a regression for READ_CAPACITY to trigger SAI READ_CAPACITY_16 for
> > > > >= 0xffffffff virtual backends.  This occured during v4.0.0-rc6 when the backend
> > > > read_capacity* handlers where moved into generic target_core_cdb.c code, and the
> > > > 'unsigned long long blocks_long' piece was dropped from target_emulate_readcapacity().
> > > > 
> > > > This fix has been tested with TCM_Loop on .38-rc7 with lio-4.1 and is working as expected
> > > > with large block virtual backends:
> > > > 
> > > > [67826.897061] TARGET_CORE[loopback]->TPG[1]_LUN[0] - Adding READ-WRITE access for LUN in Demo Mode
> > > > [67826.897061] scsi 7:0:1:0: Direct-Access     LIO-ORG  FILEIO           4.0  PQ: 0 ANSI: 5
> > > > [67826.900933] sd 7:0:1:0: [sdd] 2621440000001 512-byte logical blocks: (1.34 PB/1.19 PiB)
> > > > [67826.901510] sd 7:0:1:0: [sdd] Write Protect is off
> > > > [67826.901684] sd 7:0:1:0: [sdd] Mode Sense: 2f 00 00 00
> > > > 
> > > > Please include into scsi-rc-fixes going to Linus for-38 mainline target code.
> > > 
> > > So the piece you didn't say is that this only affects volumes >2TB
> > > volume which misreport the capacity (which is actually what the bug
> > > description should have been).
> > > 
> > > That's hardly an oopsworthy fix at this stage ... I'll put it into misc
> > > with a cc stable.
> > > 
> > 
> > This has already been picked up by AKPM last week, and (I assume) will
> > be queued to Linus for .38-FINAL via -mm.
> 
> Not really.  I often grab important-looking fixes which cross my
> desk, in case they end up getting mis-scheduled or mislaid.
> 
> > Considering that is a critical fix that is not trigger by some obsecure
> > corner case, I am not sure why this would be deferred for for a
> > v2.6.38.x release when we already have a proper fix in place..?
> 
> 2.6.38.1 is OK.  No sane people will run 2.6.38 anyway ;)

Ok, thanks for the clarification here Andrew.  A for .38.1 item it shall
be.

Best Regards,

--nab

Re: [PATCH-FOR-38] target: Fix READ_CAPACITY_16 regression

[Andrew Morton]

On Fri, 11 Mar 2011 17:46:11 -0800 "Nicholas A. Bellinger" <nab@linux-iscsi.org> wrote:

> On Fri, 2011-03-11 at 17:17 -0600, James Bottomley wrote:
> > On Wed, 2011-03-02 at 15:52 -0800, Nicholas A. Bellinger wrote:
> > > From: Nicholas Bellinger <nab@linux-iscsi.org>
> > > 
> > > Hi James,
> > > 
> > > This patch fixes a regression for READ_CAPACITY to trigger SAI READ_CAPACITY_16 for
> > > >= 0xffffffff virtual backends.  This occured during v4.0.0-rc6 when the backend
> > > read_capacity* handlers where moved into generic target_core_cdb.c code, and the
> > > 'unsigned long long blocks_long' piece was dropped from target_emulate_readcapacity().
> > > 
> > > This fix has been tested with TCM_Loop on .38-rc7 with lio-4.1 and is working as expected
> > > with large block virtual backends:
> > > 
> > > [67826.897061] TARGET_CORE[loopback]->TPG[1]_LUN[0] - Adding READ-WRITE access for LUN in Demo Mode
> > > [67826.897061] scsi 7:0:1:0: Direct-Access     LIO-ORG  FILEIO           4.0  PQ: 0 ANSI: 5
> > > [67826.900933] sd 7:0:1:0: [sdd] 2621440000001 512-byte logical blocks: (1.34 PB/1.19 PiB)
> > > [67826.901510] sd 7:0:1:0: [sdd] Write Protect is off
> > > [67826.901684] sd 7:0:1:0: [sdd] Mode Sense: 2f 00 00 00
> > > 
> > > Please include into scsi-rc-fixes going to Linus for-38 mainline target code.
> > 
> > So the piece you didn't say is that this only affects volumes >2TB
> > volume which misreport the capacity (which is actually what the bug
> > description should have been).
> > 
> > That's hardly an oopsworthy fix at this stage ... I'll put it into misc
> > with a cc stable.
> > 
> 
> This has already been picked up by AKPM last week, and (I assume) will
> be queued to Linus for .38-FINAL via -mm.

Not really.  I often grab important-looking fixes which cross my
desk, in case they end up getting mis-scheduled or mislaid.

> Considering that is a critical fix that is not trigger by some obsecure
> corner case, I am not sure why this would be deferred for for a
> v2.6.38.x release when we already have a proper fix in place..?

2.6.38.1 is OK.  No sane people will run 2.6.38 anyway ;)