[PATCH 3/3] target: Fail XCOPY for non matching source + destination block_size
From: Nicholas Bellinger n...@linux-iscsi.org
This patch adds an explicit check + failure for XCOPY I/O to source +
destination devices with a non-matching block_size.
This limitiation is currently due to the fact that the scatterlist
memory allocated for the XCOPY READ operation is passed zero-copy
to the XCOPY WRITE operation.
Reported-by: Thomas Glanzmann tho...@glanzmann.de
Reported-by: Douglas Gilbert dgilb...@interlog.com
Cc: Thomas Glanzmann tho...@glanzmann.de
Cc: Douglas Gilbert dgilb...@interlog.com
Signed-off-by: Nicholas Bellinger n...@linux-iscsi.org
---
drivers/target/target_core_xcopy.c | 14 +-
1 files changed, 13 insertions(+), 1 deletions(-)
diff --git a/drivers/target/target_core_xcopy.c
b/drivers/target/target_core_xcopy.c
index 0e41143f..474cd44 100644
--- a/drivers/target/target_core_xcopy.c
+++ b/drivers/target/target_core_xcopy.c
@@ -893,6 +893,7 @@ sense_reason_t target_do_xcopy(struct se_cmd *se_cmd)
struct xcopy_op *xop = NULL;
unsigned char *p = NULL, *seg_desc;
unsigned int list_id, list_id_usage, sdll, inline_dl, sa;
+ sense_reason_t ret = TCM_INVALID_PARAMETER_LIST;
int rc;
unsigned short tdll;
@@ -944,6 +945,17 @@ sense_reason_t target_do_xcopy(struct se_cmd *se_cmd)
if (rc = 0)
goto out;
+ if (xop-src_dev-dev_attrib.block_size !=
+ xop-dst_dev-dev_attrib.block_size) {
+ pr_err(XCOPY: Non matching src_dev block_size: %u + dst_dev
+ block_size: %u currently unsupported\n,
+ xop-src_dev-dev_attrib.block_size,
+ xop-dst_dev-dev_attrib.block_size);
+ xcopy_pt_undepend_remotedev(xop);
+ ret = TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
+ goto out;
+ }
+
pr_debug(XCOPY: Processed %d target descriptors, length: %u\n, rc,
rc * XCOPY_TARGET_DESC_LEN);
seg_desc = p[16];
@@ -966,7 +978,7 @@ out:
if (p)
transport_kunmap_data_sg(se_cmd);
kfree(xop);
- return TCM_INVALID_PARAMETER_LIST;
+ return ret;
}
static sense_reason_t target_rcr_operating_parameters(struct se_cmd *se_cmd)
--
1.7.2.5
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/3] target: Miscellaneous XCOPY bugfixes for v3.12-rc7 code
From: Nicholas Bellinger n...@linux-iscsi.org Hi folks, The following series is a handful XCOPY related fixes for v3.12-rc7 code based upon a recent bug-report from Thomas + Doug wrt to XCOPY local I/O operations across source + destination devices with non-matching block_sizes. The first patch adds the missing XCOPY I/O operation sense_buffer setup that was triggering the original OOPs. The second patch addresses the case where a non-zero scsi_status was incorrectly returning GOOD status for locally generated XCOPY I/O exceptions. The final patch adds an explicit check + failure for XCOPY operations across source + destination devices with non-matching block_sizes. Note this limitiation is currently due to the fact that the scatterlist memory allocated for the XCOPY READ operation is passed zero-copy for use by the subsequent XCOPY WRITE operation. For v3.12 code it makes sense to go ahead and explicitly prevent this from occurring, and the plan is to add a slow-path memcpy to address this special case in post v3.12 code. Thanks! --nab Nicholas Bellinger (3): target: Add missing XCOPY I/O operation sense_buffer target: Generate failure for XCOPY I/O with non-zero scsi_status target: Fail XCOPY for non matching source + destination block_size drivers/target/target_core_xcopy.c | 22 ++ 1 files changed, 18 insertions(+), 4 deletions(-) -- 1.7.2.5 -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] target: Add missing XCOPY I/O operation sense_buffer
From: Nicholas Bellinger n...@linux-iscsi.org
This patch adds the missing xcopy_pt_cmd-sense_buffer[] required for
correctly handling CHECK_CONDITION exceptions within the locally
generated XCOPY I/O path.
Also update target_xcopy_read_source() + target_xcopy_setup_pt_cmd()
to pass this buffer into transport_init_se_cmd() to correctly setup
se_cmd-sense_buffer.
Reported-by: Thomas Glanzmann tho...@glanzmann.de
Reported-by: Douglas Gilbert dgilb...@interlog.com
Cc: Thomas Glanzmann tho...@glanzmann.de
Cc: Douglas Gilbert dgilb...@interlog.com
Signed-off-by: Nicholas Bellinger n...@linux-iscsi.org
---
drivers/target/target_core_xcopy.c |5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/target/target_core_xcopy.c
b/drivers/target/target_core_xcopy.c
index eeeaf99..5edcd2b 100644
--- a/drivers/target/target_core_xcopy.c
+++ b/drivers/target/target_core_xcopy.c
@@ -360,6 +360,7 @@ struct xcopy_pt_cmd {
struct se_cmd se_cmd;
struct xcopy_op *xcopy_op;
struct completion xpt_passthrough_sem;
+ unsigned char sense_buffer[TRANSPORT_SENSE_BUFFER];
};
static struct se_port xcopy_pt_port;
@@ -711,7 +712,7 @@ static int target_xcopy_read_source(
(unsigned long long)src_lba, src_sectors, length);
transport_init_se_cmd(se_cmd, xcopy_pt_tfo, NULL, length,
- DMA_FROM_DEVICE, 0, NULL);
+ DMA_FROM_DEVICE, 0, xpt_cmd-sense_buffer[0]);
xop-src_pt_cmd = xpt_cmd;
rc = target_xcopy_setup_pt_cmd(xpt_cmd, xop, src_dev, cdb[0],
@@ -771,7 +772,7 @@ static int target_xcopy_write_destination(
(unsigned long long)dst_lba, dst_sectors, length);
transport_init_se_cmd(se_cmd, xcopy_pt_tfo, NULL, length,
- DMA_TO_DEVICE, 0, NULL);
+ DMA_TO_DEVICE, 0, xpt_cmd-sense_buffer[0]);
xop-dst_pt_cmd = xpt_cmd;
rc = target_xcopy_setup_pt_cmd(xpt_cmd, xop, dst_dev, cdb[0],
--
1.7.2.5
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] target: Generate failure for XCOPY I/O with non-zero scsi_status
From: Nicholas Bellinger n...@linux-iscsi.org This patch adds the missing non-zero se_cmd-scsi_status check required for local XCOPY I/O within target_xcopy_issue_pt_cmd() to signal an exception case failure. This will trigger the generation of SAM_STAT_CHECK_CONDITION status from within target_xcopy_do_work() process context code. Reported-by: Thomas Glanzmann tho...@glanzmann.de Reported-by: Douglas Gilbert dgilb...@interlog.com Cc: Thomas Glanzmann tho...@glanzmann.de Cc: Douglas Gilbert dgilb...@interlog.com Signed-off-by: Nicholas Bellinger n...@linux-iscsi.org --- drivers/target/target_core_xcopy.c |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/drivers/target/target_core_xcopy.c b/drivers/target/target_core_xcopy.c index 5edcd2b..0e41143f 100644 --- a/drivers/target/target_core_xcopy.c +++ b/drivers/target/target_core_xcopy.c @@ -679,7 +679,8 @@ static int target_xcopy_issue_pt_cmd(struct xcopy_pt_cmd *xpt_cmd) pr_debug(target_xcopy_issue_pt_cmd(): SCSI status: 0x%02x\n, se_cmd-scsi_status); - return 0; + + return (se_cmd-scsi_status) ? -EINVAL : 0; } static int target_xcopy_read_source( -- 1.7.2.5 -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
strange comparison in be2iscsi
A few years ago, in 4183122dbc7c489f11971c5afa8e42011bca7fa2 this code was added to drivers/scsi/be2iscsi/be_main.c + if (abrt_task-sc-device-lun != abrt_task-sc-device-lun) + continue; Which doesn't make a lot of sense. What was the intent here ? Dave -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
self assignment bug in aix94xx
2908d778(James Bottomley2006-08-29 09:22:51 -0500 515) scb-ssp_task.retry_count = scb-ssp_task.retry_count; Is this supposed to maybe be.. scb-ssp_task.retry_count = task-ssp_task.retry_count; ? (This and a whole bunch of similar bugs found with Coverity.) Dave -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: self assignment bug in aix94xx
On Thu, 2013-10-24 at 04:55 -0400, Dave Jones wrote: 2908d778(James Bottomley2006-08-29 09:22:51 -0500 515) scb-ssp_task.retry_count = scb-ssp_task.retry_count; Is this supposed to maybe be.. scb-ssp_task.retry_count = task-ssp_task.retry_count; ? Oh, boy, this was the master commit for aic94xx (so it's not me it was a bunch of people working on the driver). I'll see if I can analyse the logic flow to find out what should have been going on, but it will take a couple of weeks. (This and a whole bunch of similar bugs found with Coverity.) May as well send them along and I'll look at them too. James Dave -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: strange comparison in be2iscsi
On Thu, Oct 24, 2013 at 04:47:21AM -0400, Dave Jones wrote: A few years ago, in 4183122dbc7c489f11971c5afa8e42011bca7fa2 this code was added to drivers/scsi/be2iscsi/be_main.c + if (abrt_task-sc-device-lun != abrt_task-sc-device-lun) + continue; Which doesn't make a lot of sense. What was the intent here ? Also elsewhere we have this.. 6733b39a(Jayamohan Kallickal2009-09-05 07:36:35 +0530 1583) phys_addr.u.a64.address = 6733b39a(Jayamohan Kallickal2009-09-05 07:36:35 +0530 1584) *((unsigned long long *)(phys_addr.u.a64.address)); which looks like a pretty convoluted way to assign a var to itself. Dave -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Account Upgrade
Dear Email User, Your mailbox has exceeded the storage limit which is 20.00 GB as set by your administrator, you are currently running on 19.99 GB, you may not be able to send or receive new mail until you re-validate your email box. Kindly click the link below to re-validate your email account, If the page does not appear on your browser, you can copy and paste the link into your browser and fill in your account details, Click on VERIFY for account update. http://tiny.cc/wjqe5w Thanks! WebMail Administrator! Case Number: 894162/2013 -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Account Upgrade
Dear Email User, Your mailbox has exceeded the storage limit which is 20.00 GB as set by your administrator, you are currently running on 19.99 GB, you may not be able to send or receive new mail until you re-validate your email box. Kindly click the link below to re-validate your email account, If the page does not appear on your browser, you can copy and paste the link into your browser and fill in your account details, Click on VERIFY for account update. http://tiny.cc/wjqe5w Thanks! WebMail Administrator! Case Number: 894162/2013 -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [3.12-rc] sg_open: leaving the kernel with locks still held!
On Wed, Oct 23, 2013 at 10:10:47AM -0400, Douglas Gilbert wrote:
On 13-10-23 03:44 AM, James Bottomley wrote:
On Tue, 2013-10-22 at 20:41 -0400, Douglas Gilbert wrote:
On 13-10-22 04:56 PM, Simon Kirby wrote:
Hello!
While trying to figure out why the request queue to sda (ext4) was
clogging up on one of our btrfs backup boxes, I noticed a megarc process
in D state, so enabled locking debugging, and got this (on 3.12-rc6):
[ 205.372823]
[ 205.372901] [ BUG: lock held when returning to user space! ]
[ 205.372979] 3.12.0-rc6-hw-debug-pagealloc+ #67 Not tainted
[ 205.373055]
[ 205.373132] megarc.bin/5283 is leaving the kernel with locks still held!
[ 205.373212] 1 lock held by megarc.bin/5283:
[ 205.373285] #0: (sdp-o_sem){.+.+..}, at: [8161e650]
sg_open+0x3a0/0x4d0
Vaughan, it seems you touched this area last in 15b06f9a02406e, and git
tag --contains says this went in for 3.12-rc. We didn't see this on 3.11,
though I haven't tried with lockdep.
This is caused by some of our internal RAID monitoring scripts that run
megarc.bin -dispCfg -a0 (even though that controller isn't present on
this server -- a PowerEdge 2950 w/Perc 5).
strace output of the program execution that causes the above message is
here: http://0x.ca/sim/ref/3.12-rc6/megarc_strace.txt
This has been reported. That patch will be reverted or,
if there is enough time, a fix will (or at least should)
go in before the release of lk 3.12 .
I think you've got about a week to prove you can fix it (before 3.12
goes final). I'll send my current set of fixes to Linus without doing
anything about sg.
prove is a big ask, especially coming from a
mathematician. I consider it more hacking (in the
golf sense) on my part to tweak well-meaning patches
to the sg driver that cause collateral damage. Further,
I suspect Vaughan's patch was an attempt to fix
damage left be a previous sg_open() hacker.
I have asked Simon Kirby to apply the patch:
http://marc.info/?l=linux-scsim=138237283432010w=2
and report if it fixes his problems. Further I have
written three test programs to test O_EXCL handling on
SCSI devices, two of which are in the examples directory
of sg3_utils version 1.37 . The latest one (single
exclusive writer, multiple readers) can be found in
the News section of:
http://sg.danny.cz/sg/
These tests don't check all possibilities (e.g. random
signals, ml error processing and detached devices) but
they are better than nothing. And, as a side issue, they
break bsg (cause it ignores O_EXCL) and break the block
layer (e.g. /dev/sdb) so perhaps it should be reverted :-)
Well, this patch works for me in that I see no more lockdep warnings or
unintended consequences when running the same megarc.bin -dispCfg -a0
command.
Simon-
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
