On Tue, Jul 21, 2015 at 1:57 AM, Sagi Grimberg wrote:
> How were you able to get a chained SG list in the target code?
Local hack. So this bug can't be hit in current mainline code, but
patch improves the code and removes a hidden booby-trap, so I think it
makes sense to apply.
--
To unsubscribe
On Mon, Jul 27, 2015 at 1:09 AM, Hannes Reinecke wrote:
> Hmm? What happened to the original FIXME?
> Is it not required anymore?
Not sure I follow the question. The original FIXME was "/* FIXME:
Re-enable Global event handling.. */" and this patch indeed re-enables
global event handling. I don
On Mon, Oct 6, 2014 at 4:16 AM, Bart Van Assche wrote:
> On 10/02/14 19:30, Christoph Hellwig wrote:
>> Also if we want to merge scsi LLDDs that can take advantage of
>> multiqueue support it would probably be best if I take this via the SCSI
>> tree.
> Sending these patches to you is fine with m
At LSF this year, we had a discussion about error handling and in
particular the problem that SCSI midlayer error handling waits for the
entire SCSI host (HBA) to quiesce before it starts to abort commands
etc.
James made the suggestion that FC should handle things the way SAS
does, because SAS ha
From: Roland Dreier
Suppose an initiator sends a DATA IN command with an allocation length
shorter than the FC transfer length -- we get a target message like
TARGET_CORE[qla2xxx]: Expected Transfer Length: 256 does not match SCSI CDB
Length: 0 for SAM Opcode: 0x12
In that case, the
From: Roland Dreier
There is a nasty bug in the SCSI SG_IO ioctl that in some circumstances
leads to one process writing data into the address space of some other
random unrelated process if the ioctl is interrupted by a signal.
What happens is the following:
- A process issues an SG_IO ioctl
+ }
> bio_free_map_data(bmd);
> bio_put(bio);
> return ret;
Yes, looks reasonable -- I can't think of any reason why anyone would
ever want the bio code to copy to a random userspace address space.
Acked-by: Roland Dreier
--
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
From: Roland Dreier
There is a nasty bug in the SCSI SG_IO ioctl that in some circumstances
leads to one process writing data into the address space of some other
random unrelated process if the ioctl is interrupted by a signal.
What happens is the following:
- A process issues an SG_IO ioctl
On Wed, Aug 7, 2013 at 7:38 AM, David Milburn wrote:
> I was able to succesfully test this patch overnight, I had been experimenting
> with the
> sg driver setting the BIO_NULL_MAPPED flag in sg_rq_end_io_usercontext for a
> orphan process
> which prevented the corruption, but your solution seem
On Wed, Aug 7, 2013 at 9:31 AM, Douglas Gilbert wrote:
> So what kind of signal was leading to your "stomping on the memory"?
> Was it user generated or something like SIGIO, SIGPIPE or a RT signal?
It was sometimes SIGHUP (for reopening log files) and sometimes
SIGALARM (for various periodic thi
Jens / James, do you guys plan to send this to Linus for 3.11?
Triggering this bug is a bit esoteric but the impact is pretty nasty
(corrupting an unrelated process).
Thanks,
Roland
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vge
On Tue, Aug 20, 2013 at 1:08 PM, Nicholas A. Bellinger
wrote:
> Add a special case for COMPARE_AND_WRITE for the reverse data direction
> mapping used for pci_map_sg() + friends.
I don't understand this. In fact the whole patch series looks quite
confused. COMPARE AND WRITE is a normal Data-Out
On Wed, Aug 21, 2013 at 7:38 AM, Roland Dreier wrote:
> I don't understand this. In fact the whole patch series looks quite
> confused. COMPARE AND WRITE is a normal Data-Out command, with no
> requirement for special bidirectional handling or anything like that.
> The only
Thanks, applied at last. I'm assuming the vscsi changes are OK since
they look mechanical and we haven't heard anything from IBM people.
--
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://vge
> This series compiles, but is otherwise UNTESTED. I'll be working on that
> over the next few days, with an eye on getting as much of Bart's work
> into 3.8 as possible.
Hi Dave,
Great to have you back. Certainly I'd like to get this stuff into 3.8 too.
A couple of comments:
- I think the
Thanks, applied.
--
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
> I'm amenable to that, but we do need an agreed patch set, as Roland
> says. I also hate to apply the pressure, but I suspect -rc7 was the
> last -rc, so I'm expecting the merge window to open on 2/12.
I think the srp_transport bits are all simple and non-controversial.
So at least from my persp
On Mon, Nov 26, 2012 at 8:04 PM, David Dillow wrote:
> We can push it through James's tree if need be, but Bart's code is
> pretty self-contained, and going through the SCSI tree will introduce
> merge dependencies. It'd be much easier to push it all through the RDMA
> tree, especially if we want
From: Roland Dreier
Hi Nic,
A few fixes for TMR handling (fix crashes when a backend is really
slow, fix a reference leak if we get a TMR for a non-existent LUN) and
a couple of trivial cleanups in related code.
Roland Dreier (5):
target: Don't let abort handling free pending write com
From: Roland Dreier
If a backend IO takes a really long then an initiator might abort a
command, and then when it gives up on the abort, send a LUN reset too,
all before we process any of the original command or the abort. (The
abort will wait for the backend IO to complete too)
When the
From: Roland Dreier
When transport_lookup_tmr_lun() fails and we return a task management
response from target_complete_tmr_failure(), we need to call
transport_cmd_check_stop_to_fabric() to release the last ref to the
cmd after calling se_tfo->queue_tm_rsp(), or else we will never remove
From: Roland Dreier
The following sequence happens for write commands (or any other
commands with a data out phase):
- The transport calls target_submit_cmd(), which sets CMD_T_ACTIVE in
cmd->transport_state and sets cmd->t_state to TRANSPORT_NEW_CMD.
- Things
From: Roland Dreier
Signed-off-by: Roland Dreier
---
include/target/target_core_base.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/include/target/target_core_base.h
b/include/target/target_core_base.h
index 7cae236..02ed017 100644
--- a/include/target/target_core_base.h
+++ b/include
From: Roland Dreier
We do the same thing no matter which way the test goes, so just remove
the test and do what we're going to do.
The debug messages printed the wrong value of CMD_T_ACTIVE and don't
seem particularly useful, remove them too.
Signed-off-by: Roland Dreier
---
driv
On Wed, Dec 26, 2012 at 1:33 PM, Ben Hutchings wrote:
>> if (!(cmd->cmd_flags & ICF_OOO_CMDSN) && !cmd->immediate_cmd &&
>> - (cmd->cmd_sn >= conn->sess->exp_cmd_sn)) {
>> + iscsi_sna_gte(cmd->stat_sn, conn->sess->exp_cmd_sn)) {
>>
From: Roland Dreier
Commit 64c13330a389 ("iscsi-target: Fix bug in handling of ExpStatSN
ACK during u32 wrap-around") introduced a bug where we compare the
wrong SN against our ExpCmdSN.
Reported-by: Ben Hutchings
Signed-off-by: Roland Dreier
---
drivers/target/iscsi/iscsi_target_
On Tue, Jan 29, 2013 at 12:14 AM, Hannes Reinecke wrote:
> I would like to discuss at LSF the possible implementations
> and handling mechanism for this kind of failure scenarios.
I'd be interested in that discussion. With my Pure hat on, our array
can generate these thin provisioning threshold
On Mon, Jan 21, 2013 at 3:18 AM, Hannes Reinecke wrote:
> The current error handler still uses a 'target reset' (or, rather, bus
> reset) strategy, although the respective TMF has been obsoleted since
> SAM-3. SAM-5 defines an I_T nexus loss event instead, which so far has only
> been implemented
From: Roland Dreier
We're supposed to return LOGICAL BLOCK ADDRESS OUT OF RANGE, not
INVALID FIELD IN CDB.
Signed-off-by: Roland Dreier
---
drivers/target/target_core_sbc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/target/target_core_sbc.c b/drivers/t
From: Roland Dreier
SBC-3 (revision 35) says:
The PARAMETER LIST LENGTH field specifies the length in bytes of the
UNMAP parameter list that is available to be transferred from the
Data-Out Buffer. If the parameter list length is greater than zero
and less than 0008h (i.e
From: Roland Dreier
An empty parameter list (length == 0) is not an error, so succeed MODE
SELECT in this case. If the parameter list length is too small,
return the correct sense code of PARAMETER LIST LENGTH ERROR.
Signed-off-by: Roland Dreier
---
drivers/target/target_core_spc.c | 13
On Sat, Feb 9, 2013 at 1:23 AM, Chris Boot wrote:
>> + case TCM_PARAMETER_LIST_LENGTH_ERROR:
>> + /* CURRENT ERROR */
>> + buffer[0] = 0x70;
>> + buffer[SPC_ADD_SENSE_LEN_OFFSET] = 10;
>> + /* ILLEGAL REQUEST */
>> + buffe
From: Roland Dreier
If a SCSI device's old state is already SDEV_RUNNING and we're moving
to the same SDEV_RUNNING state, still wake the blockdev queue in
scsi_internal_device_unblock(). This fixes a case where we silently
hang SCSI commands forever during device discovery. One wa
On Thu, Mar 7, 2013 at 5:45 PM, Nicholas A. Bellinger
wrote:
> +EXPORT_SYMBOL(iscsit_get_transport);
It's not clear to me why this needs to be exported. Who would use it
outside the core iscsi target module?
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of
On Thu, Mar 7, 2013 at 10:02 PM, Nicholas A. Bellinger
wrote:
> Or and I discussed this point in the last status call, and given what
> the initiator did originally (eg: export iscsi_transport) he asked to
> keep it under drivers/infiniband/ulp/isert/ with the extra include bits.
>
> I'd have a sl
> With the old scsi_lib code, scsi_internal_device_unblock() will return
> an error at this point because the sdev state is already SDEV_RUNNING.
> This means we skip the call to blk_start_queue() and never actually
> start executing commands again.
>
> Fix this by still going ahead and finishing s
On Mon, Mar 11, 2013 at 11:08 AM, Mike Christie wrote:
>> If at the same time a SAS fabric event goes to the HBA, what can
>> happen is the following:
>>
>> - mpt2sas calls _scsih_block_io_all_device() ->
>> scsi_internal_device_block(sdev)
>>
>>(In response to some HBA firmware even
> I've been debugging various issues on the PowerPC 44x embedded
> architecture which happens to have non-coherent PCI DMA.
>
> One of the problem I'm hitting is that one really need to enforce
> kmalloc alignement to cache lines or bad things will happen (among
> others with USB), for some
> 2) Add the __dma_cacheline_aligned tag.
>
> But note that with #2 it could get quite ugly because the
> alignment and size both have a minimum that needs to be
> enforced, not just the alignment alone. So either:
>
> struct foo {
> unsigned int other_unrelated_stuff;
>
> s
> Actually, we already established on IRC that the lasi700 driver doesn't
> need this, principally because the parisc architecture doesn't do an
> invalidate for DMA_FROM_DEVICE but a flush and invalidate
> (architecturally, if you read our manuals, even pdc is entitled to write
> back dirty l
> > +#define __dma_aligned
> > __attribute__((aligned(ARCH_MIN_DMA_ALIGNMENT)))
> > +#define __dma_buffer __dma_buffer_line(__LINE__)
> > +#define __dma_buffer_line(line) __dma_aligned;\
> > + char __dma_pad_##line[0] __dma_aligned
> You i
> It's also incomplete as a fix because I don't see what guarantees the
> buffer size will always exceed cache line size
There's a macro trick that adds a pad member after the buffer too, so
that it gets rounded up to the cacheline size:
> +#define __dma_aligned
> __attribute__
> . . STGT read SCST read.STGT read
> SCST read.
> . . performance performance . performance
> performance .
> . . (0.5K, MB/s) (0.5K, MB/s) . (1 MB, MB/s)
> (1 MB, MB/s)
> Sorry, but I'm afraid you got this wrong. When the iSER transport is
> used instead of TCP, all data is sent via RDMA, including unsolicited
> data. If you have look at the iSER implementation in the Linux kernel
> (source files under drivers/infiniband/ulp/iser), you will see that
> all dat
From: Roland Dreier
Per SBC-3, since we report ANC_SUP==0 in VPD page B2h, we need to return
an error (ILLEGAL REQUEST/INVALID FIELD IN CDB) for all WRITE SAME
requests with ANCHOR==1.
Signed-off-by: Roland Dreier
---
drivers/target/target_core_sbc.c | 5 +
1 file changed, 5 insertions
From: Roland Dreier
The only place this struct member is touched is in one INIT_LIST_HEAD.
Signed-off-by: Roland Dreier
---
drivers/scsi/qla2xxx/qla_target.c | 2 --
drivers/scsi/qla2xxx/qla_target.h | 1 -
2 files changed, 3 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_target.c
b
From: Roland Dreier
Commit fbfe858fea2a ("target_core_spc: Include target device
descriptor in VPD page 83") added a new length variable, but (due to a
cut and paste mistake?) just checks scsi_name_len against 256 twice.
Fix this to check scsi_target_len for overflow too.
Signed-off-
From: Roland Dreier
The CMD_T_FAILED flag is set used in one place to record the result of a
trivial test, and it is only tested once, few lines later. We might as
well make the code simpler and easier to read by directly doing the test
of "success" where we want to use it.
Sig
On Sun, Jun 1, 2014 at 9:19 AM, Sagi Grimberg wrote:
> Although these patches involve 3 subsystems with different
> maintainers (scsi, iser, target) I would prefer seeing these
> patches included together.
Why? Because they break wire compatibility?
I hate to say it but even if they're merged a
> No, it does matter. Your suggestion doesn't work, because
> /sys/module/scsi_mod/parameters/ belongs to the module code. To create
> a new attribute there, you use the module_param() code -- and there's
> no way to have code called when your parameter is changed.
If I'm not misunderstandin
This looks like a really nice approach to me. Olaf?
- R.
-
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
From: Roland Dreier
Hi Nic,
Here's a series that's fundamentally about fixing a use-after-free in
qla_target code. It ends up being seven patches because I wanted to
make each step easy to review, and several of these are just cleanups
that stand on their own.
We have a few mo
From: Roland Dreier
The only place that sets qla_tgt_sess.tearing_down calls
target_splice_sess_cmd_list() immediately afterwards, without dropping
the lock it holds. That function sets se_session.sess_tearing_down,
so we can get rid of the qla_target-specific flag, and in the one
place that
From: Roland Dreier
There are no in-tree users of target_get_sess_cmd() outside of
target_core_transport.c. Any new code should use the higher-level
target_submit_cmd() interface. So let's un-export target_get_sess_cmd()
and make it static to the one file where it's actually used.
From: Roland Dreier
Cc: Chris Boot
Cc: Stefan Richter
Signed-off-by: Roland Dreier
---
drivers/target/sbp/sbp_target.c | 28
1 file changed, 12 insertions(+), 16 deletions(-)
diff --git a/drivers/target/sbp/sbp_target.c b/drivers/target/sbp/sbp_target.c
index
From: Roland Dreier
Target core code assumes that target_splice_sess_cmd_list() has set
sess_tearing_down and moved the list of pending commands to
sess_wait_list, no more commands will be added to the session; if any
are added, nothing keeps the se_session from being freed while the
command is
From: Roland Dreier
Now that target_submit_cmd() / target_get_sess_cmd() check
sess_tearing_down before adding commands to the list, we no longer
need the check in qlt_do_work(). In fact this check is racy anyway
(and that race is what inspired the change to add the check of
sess_tearing_down
From: Roland Dreier
We want it to be possible for target_submit_cmd() to return errors up
to its fabric module callers. For now just update the prototype to
return an int, and update all callers to handle non-zero return values
as an error.
Cc: Chad Dupuis
Cc: Arun Easi
Cc: Chris Boot
Cc
From: Roland Dreier
Since we set se_session.sess_tearing_down and stop new commands from
being added to se_session.sess_cmd_list before we wait for commands to
finish when freeing a session, there's no need for a separate
sess_wait_list -- if we let new commands be added to sess_cmd_list
On Mon, Jul 16, 2012 at 4:00 PM, Nicholas A. Bellinger
wrote:
> Mmmm. The original target_submit_cmd() code had been propagating up a
> return value, but then we decided (via Agrover's patch) that it made
> more sense for target_submit_cmd() to always handle exceptions via
> normal TFO callbacks,
On Mon, Jul 16, 2012 at 4:08 PM, Nicholas A. Bellinger
wrote:
> However, I'm still leaning towards a way to do this for tcm_qla2xxx that
> does not require all fabric callers to handle target_submit_cmd()
> exceptions directly..
>
> How about a special target_get_sess_cmd() failure callback to fre
> OK, I'll take a look at how you handle this...
So looking at commit bc187ea6c3b3 in the tree you just pushed out
("target: Check sess_tearing_down in target_get_sess_cmd()") it looks
like you just return from target_submit_cmd() if we fail to add the
command to sess_cmd_list -- doesn't this mean
On Mon, Jul 16, 2012 at 6:56 PM, Nicholas A. Bellinger
wrote:
>> Do you have a plan for how to handle this? Do we really want to plumb
>> through another callback to tell the fabric driver to free the command
>> in this case?
> I need to think more about this ahead of changing it back again for-
On Fri, Aug 17, 2012 at 6:02 PM, Nicholas A. Bellinger
wrote:
> No, or at least that is not what happens anymore with current target
> core + iscsi-target code..
>
> The se_cmd->data_length re-assignment here is what will be used by
> iscsi-target fabric code for all iSCSI descriptor related alloc
From: Roland Dreier
The qla2xxx firmware actually expects the task management response
code in a CTIO IOCB with SCSI status mode 1 to be in little-endian
byte order, ie the response code should be the first byte in the
sense_data[] array. The old code erroneously byte-swapped the
response code
On Wed, Sep 19, 2012 at 12:59 AM, James Bottomley
wrote:
> Is this also true on Big Endian Hardware? Because the fix you have
> assumes that the TIO IOCB with SCSI status mode 1 should be CPU
> endian ... that doesn't look right since this is passed directly over
> the PCI bus (and the PCI bus is
On Fri, Sep 21, 2012 at 1:02 AM, James Bottomley
wrote:
> The data in status1 appears to get used a word at a time ... what about
> the other three bytes you don't set; are they guaranteed to be zero? (in
> which case this works, it just looks wrong from the way the thing is
> used in the rest of
From: Roland Dreier
Hi everyone, a couple of qla2xxx target fixes that we've been shipping
for a while at Pure and that I've finally got around to cleaning up
and merging to the latest kernel tree...
Roland Dreier (2):
tcm_qla2xxx: Format VPD page 83h SCSI name string accord
From: Roland Dreier
My draft of SPC-4 says the following about the SCSI name string in
inquiry VPD page 83h:
The SCSI NAME STRING field starts with either:
a) the four UTF-8 characters 'eui.' concatenated with 16, 24, or
32 hexadecimal digits (i.e., the UTF-8 ch
From: Roland Dreier
It is possible for the target code to change the loop_id or s_id of a
target session in reaction to an FC fabric change. However, the
session structures are stored in tables that are indexed by these two
keys, and if we just change the session structure but leave the
On Tue, Aug 14, 2012 at 2:15 PM, Chad Dupuis wrote:
>
>
> On Tue, 14 Aug 2012, st...@purestorage.com wrote:
>
>> From: Steve Hodgson
>>
>> A few months ago our 2.6.39 based kernel started crashing almost 100%
>> of the time when running the selftests, after seeminly unrelated kernel
>> changes. I
From: Roland Dreier
Hi Nic,
Here's a series that makes aborts actually work on qla2xxx. Stopping
and releasing commands is quite convoluted so I'm not sure the first
patch is totally correct, but without it I can easily reproduce task
hangs or list corruption by having an initiat
From: Roland Dreier
- If we stop processing an already-aborted command in
target_execute_cmd(), then we need to complete t_transport_stop_comp
to wake up the the TMR handling thread, or else it will end up
waiting forever.
- If we've a already sent an "aborted" status f
From: Steve Hodgson
Search through the list of pending commands on the session list to find
the command the initiator is actually aborting, so that we can pass the
correct LUN to the core TMR handling code.
Signed-off-by: Steve Hodgson
Signed-off-by: Roland Dreier
---
drivers/scsi/qla2xxx
From: Roland Dreier
No need to have a goto where a return is clearer.
Signed-off-by: Roland Dreier
---
drivers/target/target_core_transport.c | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/target/target_core_transport.c
b/drivers/target
From: Roland Dreier
Signed-off-by: Roland Dreier
---
drivers/target/target_core_transport.c | 24 ++--
1 file changed, 10 insertions(+), 14 deletions(-)
diff --git a/drivers/target/target_core_transport.c
b/drivers/target/target_core_transport.c
index 0f29d70..f225f90
On Sun, Mar 5, 2017 at 8:35 AM, Sebastian Herbszt wrote:
> Just like Hannes I do favour integration. I guess it could be
> comparable to qla2xxx + tcm_qla2xxx, lpfc + lpfc_scst and
> lpfc + tcm_lpfc. That approach might even help Bart with his
> target driver unification if he didn't give up on th
efine DRV_NAME "ib_srp"
+#define PFXDRV_NAME ": "
+#define DRV_VERSION"0.01"
+#define DRV_RELDATE"January 11, 2005"
+
+MODULE_AUTHOR("Roland Dreier");
+MODULE_DESCRIPTION("InfiniBand SCSI RDMA Protocol driv
78 matches
Mail list logo
