On Fri, Apr 24, 2015 at 10:32:54AM +0200, Hannes Reinecke wrote:
> Ancient, and pretty much obsolete by now.
And still doesn't address the fixme. You probably should remove the
cmd_per_lun setting entirely.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a
> config SCSI_ADVANSYS
> tristate "AdvanSys SCSI support"
> - depends on SCSI && VIRT_TO_BUS && !ARM
> + depends on SCSI && !ARM
Care to remove the bogus call to dma_cache_sync and allow compiling on
arm as well?
--
To unsubscribe from this list: send the line "unsubscribe linux-scs
On Sun, Apr 26, 2015 at 04:57:01PM +0200, Ondrej Zary wrote:
> This patch breaks my setup: "modprobe advansys" hangs.
>
> It works when all other patches are applied except this one.
Oh damn, looks like the code does have issues in this case.
So let's just drop this change for now.
Jusr curious
T10 PI is just another optional feature, LLDDs should work without
the infrastructure.
Signed-off-by: Christoph Hellwig
---
drivers/scsi/Kconfig | 1 -
drivers/scsi/virtio_scsi.c | 11 ++-
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/Kconfig b
On Wed, Apr 29, 2015 at 10:17:59AM +0900, Akinobu Mita wrote:
> This change broke ufs driver.
I'd claim the ufs driver, or rather more specifily the ufs spec
had already been broken. That whole concept of keeping references
to scsi devices to send commands to for host state changes is just
fundam
On Thu, Apr 30, 2015 at 10:56:42AM +0200, Bart Van Assche wrote:
> Introduce the helper function srp_wait_for_queuecommand().
> Move the definition of scsi_request_fn_active(). This patch
> does not change any functionality. A second call to
> scsi_wait_for_queuecommand() will be introduced in the
Signed-off-by: Christoph Hellwig
---
drivers/md/dm-mpath.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
index 6395347..01e5f8e 100644
--- a/drivers/md/dm-mpath.c
+++ b/drivers/md/dm-mpath.c
@@ -732,6 +732,9 @@ static int parse_hw_handler
.
Signed-off-by: Christoph Hellwig
---
drivers/md/dm-mpath.c | 13 -
drivers/scsi/device_handler/scsi_dh.c | 101 +-
include/scsi/scsi_device.h| 1 -
include/scsi/scsi_dh.h| 5 --
4 files changed, 27 insertions
This way we can reused the same code any attachment method, not just those
requested from dm-mpath.
Signed-off-by: Christoph Hellwig
---
drivers/md/dm-mpath.c | 7 ---
drivers/scsi/device_handler/scsi_dh.c | 35 ++-
include/scsi/scsi_dh.h
The I/O submission and completion pathes call into the device handler
without any synchronization agains detachment. So disallow detaching
device handlers at runtime.
Signed-off-by: Christoph Hellwig
Reviewed-by: Martin K. Petersen
Reviewed-by: Hannes Reinecke
---
drivers/scsi/scsi_dh.c | 5
On Wed, May 06, 2015 at 02:46:18PM +0300, Boaz Harrosh wrote:
> > - memset(rq->__cmd, 0, sizeof(rq->__cmd));
> > +
> > + rq->block_pc = kzalloc(sizeof(*rq->block_pc) + cmd_len, gfp);
>
> I wish you would not embed a dynamic allocation here for any
> driver regardless. This extra allocation doe
On Tue, May 12, 2015 at 10:09:23PM -0700, Nicholas A. Bellinger wrote:
> > Why do we still need TRANSPORT_LUNFLAGS_INITIATOR_ACCESS? Isn't deve
> > being not-NULL enough?
> >
>
> Yep, I think this can go as well. Dropping now.
Also might be worth to add a helper that has the ->se_lun_acl check
On Mon, May 18, 2015 at 11:05:47PM -0700, Nicholas A. Bellinger wrote:
> > [ 12.830576] kernel BUG at ../drivers/target/target_core_device.c:337!
> >
>
> How did you hit this..?
tcm_node --block iblock_0/array /dev/sda
line=$(tcm_loop --createnexus=0)
wwn=$(echo $line | awk '{print $15}')
tcm
On Tue, May 12, 2015 at 09:25:33AM +, Nicholas A. Bellinger wrote:
> From: Nicholas Bellinger
>
> This patch converts the fixed size se_portal_group->tpg_lun_list[]
> to use modern RCU with hlist_head in order to support an arbitary
> number of se_lun ports per target endpoint.
>
> It includ
On Tue, May 19, 2015 at 08:22:31AM +0200, Christoph Hellwig wrote:
> On Mon, May 18, 2015 at 11:05:47PM -0700, Nicholas A. Bellinger wrote:
> > > [ 12.830576] kernel BUG at ../drivers/target/target_core_device.c:337!
> > >
> >
> > How did you hit this..?
>
Can you put up a git branch with these? Without that or a known
good baseline it's hard to test, or even to do a deep review.
--
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.or
> - spin_lock_irqsave(&se_sess->se_node_acl->device_list_lock, flags);
> - se_cmd->se_deve = se_sess->se_node_acl->device_list[unpacked_lun];
> - if (se_cmd->se_deve->lun_flags & TRANSPORT_LUNFLAGS_INITIATOR_ACCESS) {
> - struct se_dev_entry *deve = se_cmd->se_deve;
> -
> +
On Fri, May 22, 2015 at 06:11:04AM +, Nicholas A. Bellinger wrote:
> + clear_bit(1, &orig->pr_reg);
Can you call it ->flags and give the bit a meaningful name?
> diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c
> index c0b593a..d29b39c 100644
> --- a/drivers/
Oh, you've actually go it - still would make sense to move it the beginning of
the series.
--
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
> This patch converts the fixed size se_portal_group->tpg_lun_list[]
> to use modern RCU with hlist_head in order to support an arbitary
> number of se_lun ports per target endpoint.
>
> It includes dropping core_tpg_alloc_lun() from core_dev_add_lun(),
> and calling it directly from target_fabric
On Fri, May 22, 2015 at 01:55:30AM -0700, Nicholas A. Bellinger wrote:
> > This update will now be racy, ditto for the read/write_bytes update
> > later.
>
> This should become an atomic_long_t increment, yes..?
Yes.
> Yes, this helper is from your patch above.
>
> Considering there is a single
On Fri, May 22, 2015 at 02:05:57AM -0700, Nicholas A. Bellinger wrote:
> On Fri, 2015-05-22 at 10:26 +0200, Christoph Hellwig wrote:
> > On Fri, May 22, 2015 at 06:11:04AM +, Nicholas A. Bellinger wrote:
> > > + clear_bit(1, &orig->pr_reg);
> >
> > Can y
>
> -/*
> - * this function can be called with struct se_device->dev_reservation_lock
> - * when register_move = 1
> - */
> static void __core_scsi3_add_registration(
> struct se_device *dev,
> struct se_node_acl *nacl,
> @@ -1023,6 +1021,7 @@ static void __core_scsi3_add_registratio
> @@ -683,7 +679,7 @@ void core_tpg_remove_lun(
> dev->export_count--;
> spin_unlock(&dev->se_port_lock);
>
> - lun->lun_se_dev = NULL;
> + rcu_assign_pointer(lun->lun_se_dev, NULL);
> }
What guarantees that the se_device stays around for
This makes lockdep very unhappy, rightly so. If you execute
one end_io function inside another you basŃ–cally nest every possible
lock taken in the I/O completion path. Also adding more work
to the hardirq path generally isn't a smart idea. Can you explain
what issues you were seeing and how much
On Thu, Jun 04, 2015 at 12:06:09AM -0700, Nicholas A. Bellinger wrote:
> So I've been using tcm_loop + RAMDISK backends for prototyping, but this
> patch is intended for vhost-scsi so it can avoid the unnecessary
> queue_work() context switch within target_complete_cmd() for all backend
> driver ty
Hi Sathya,
please start your effort per your plans. I don't think we'll need
separate binaries as we should be fine with a MODULE_ALIAS and
registering two misc devices. But compared to the overall effort
that's a fairly minor descision which we can revisit while going ahead.
Keeping the user i
> diff --git a/drivers/target/target_core_tpg.c
> b/drivers/target/target_core_tpg.c
> index 3fbb0d4..aa08d6b 100644
> --- a/drivers/target/target_core_tpg.c
> +++ b/drivers/target/target_core_tpg.c
> @@ -37,6 +37,7 @@
>
> #include
> #include
> +#include
> #include
>
> #include "target
On Tue, Jun 16, 2015 at 06:46:14PM -0400, Martin K. Petersen wrote:
>
> Brian,
>
> I only have minor nits wrt. your patch since you did what I asked.
> However, now that I'm less jet lagged and blurry eyed I wonder if
> the tweak below wouldn't suffice?
This would be my preferred version as well
What's the benefit of the SAS transport class writeout? I honestly
always saw tcm_loop as a simple loopback driver, with the different
transport IDs in the PR code as a gimmick. Note that vhost and
xen-blkback copies that style and I did plan to consolidate it
in common code.
--
To unsubscribe fr
On Thu, Jun 18, 2015 at 11:43:41AM +0200, Hannes Reinecke wrote:
> Strictly speaking SPC doesn't require READ CAPACITY and friends
> to be supported while in the port is in standby.
But it does allow it. We should always aim to implement the best
possible behavior instead of aiming for the worst.
On Thu, Jun 18, 2015 at 11:43:42AM +0200, Hannes Reinecke wrote:
> When LUNs are mapped with a demo-mode initiator we're missing
> the 'write_protect' attribute to set a LUN read-only.
I don't think it's a good idea as-is. The target core clearly
differenciates between demo mode and production mo
On Fri, Jun 19, 2015 at 08:40:23AM +0200, Hannes Reinecke wrote:
> Having a list here implies that 'se_lun' can have _several_
> se_dev_entry structure attached to it, which I found rather curious.
>
> Can you give me an example where this might be the case?
> Or can we replace the list with a sim
> @@ -79,7 +79,8 @@ static int _scsih_scan_finished(struct Scsi_Host *shost,
> unsigned long time);
>
> /* global parameters */
> LIST_HEAD(mpt2sas_ioc_list);
> -
> +/* global ioc lock for list operations */
> +spinlock_t gioc_lock;
> /* local parameters */
> static u8 scsi_io_cb_idx = -1;
>
> --- a/drivers/target/target_core_alua.c
> +++ b/drivers/target/target_core_alua.c
> @@ -1880,12 +1880,19 @@ static void core_alua_put_tg_pt_gp_from_name(
> static void __target_attach_tg_pt_gp(struct se_lun *lun,
> struct t10_alua_tg_pt_gp *tg_pt_gp)
> {
> + struct se_dev_entr
On Thu, Jun 11, 2015 at 10:01:28AM +0200, Hannes Reinecke wrote:
> SAM mandates that an BUS DEVICE RESET FUNCTION OCCURRED
> UA needs to be send after a LUN RESET tmr has completed.
>
> Signed-off-by: Hannes Reinecke
> ---
> drivers/target/target_core_transport.c | 11 +++
> 1 file chang
> + hlist_for_each_entry_rcu(tmp, &nacl->lun_entry_hlist, link) {
> + if (tmp == new)
> + continue;
> + core_scsi3_ua_allocate(tmp, 0x3F,
> + ASCQ_3FH_REPORTED_LUNS_DATA_HAS_CHANGED);
> +
On Fri, Jun 19, 2015 at 03:09:34PM +0200, Hannes Reinecke wrote:
> This harks back to my previous mail:
> Under which circumstances will there be more than one se_dev_entry
> structures in lun_deve_list?
> Isn't there a 1:1 relationship?
No. See my answer to your previous mail.
--
To unsubscribe
Feel free to take over the series as your ALUA changes rely on it.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
On Fri, Jun 19, 2015 at 03:21:27PM +0200, Hannes Reinecke wrote:
> > Each initiator has it's own dev entry.
> >
> But isn't 'se_lun' per initiator, too?
No.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
Looks good,
Reviewed-by: Christoph Hellwig
--
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
Please use a descriptive one line Subject line and put the rest of
your changelog into the message body.
--
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
On Mon, Jun 08, 2015 at 08:50:51PM -0700, Calvin Owens wrote:
> These objects can be referenced concurrently throughout the driver, we
> need a way to make sure threads can't delete them out from under each
> other.
>
> Signed-off-by: Calvin Owens
Thsi doesn't make sense without users of the ref
>
> +struct _sas_device *
> +mpt2sas_scsih_sas_device_get_by_sas_address_nolock(struct MPT2SAS_ADAPTER
> *ioc,
> +u64 sas_address)
Any chance to use a shorter name for this function? E.g.
__mpt2sas_get_sdev_by_addr ?
> +{
> + struct _sas_device *sas_device;
> +
> + BUG_ON(!spin_is_
On Mon, Jun 08, 2015 at 08:50:54PM -0700, Calvin Owens wrote:
> The fw_event_work struct is concurrently referenced at shutdown, so
> add a refcount to protect it.
Same comment here - a refcount that isn't used isn't useful, please fold
into the next patch.
--
To unsubscribe from this list: send
On Mon, Jun 08, 2015 at 08:50:55PM -0700, Calvin Owens wrote:
> This refactors the fw_event code to use the new refcount.
I spent some time looking over this code because it's so convoluted.
In general I think code should either embeed one work_struct (and it
really doesn't seem to need a delayed
On Mon, Jun 08, 2015 at 08:50:56PM -0700, Calvin Owens wrote:
> Since the fw_event deletes itself from the list, cleanup_queue() can
> walk onto garbage pointers or walk off into freed memory.
>
> This refactors the code in _scsih_fw_event_cleanup_queue() to not
> iterate over the fw_event_list wi
On Mon, Jun 08, 2015 at 08:50:53PM -0700, Calvin Owens wrote:
> We cannot iterate over the list without holding a lock for the entire
> duration, or we risk corrupting random memory if items are added or
> deleted as we iterate.
>
> This refactors code such that it always holds the lock when itera
rot;
>^
>
> Signed-off-by: Stephen Rothwell
Thanks, this looks good:
Reviewed-by: Christoph Hellwig
--
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
On Mon, Jun 29, 2015 at 06:05:25PM +0300, Sagi Grimberg wrote:
> Instead of open coding the sense buffer construction, use
> scsi scsi_build_sense_buffer() and scsi_set_sense_information()
> helpers.
>
> This patch also fixes wrong setting of descriptor format sense data
> for t10-pi integrity err
Why is this a binary attribute? Needs an interface specific in
Documentation/ABI/ to start with, and a Ccto stable seems very
aggressive.
--
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://v
On Thu, Jun 25, 2015 at 04:24:22PM +0530, Nagarajkumar Narayanan wrote:
> This patch contains the fix for kernel oops on issuing setpci reset
> along with sysfs, cli access
>
> spinlock initialization modified to DEFINE_SPINLOCK as per the
> comments for previous version of the patch
Why is the p
On Wed, Jul 01, 2015 at 11:04:08AM +0200, Vitaly Kuznetsov wrote:
> SPC-2/3/4 specs state that "The standard INQUIRY data (see table ...)
> shall contain at least 36 bytes". Hyper-V host doesn't always honor this
> requirement, e.g. when there is no physical device present at a particular
> LUN hos
ks good,
Reviewed-by: Christoph Hellwig
Sagi: if you send on Barts patches you probably should add your signoff
as well.
--
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
> +static void translate_sense_reason(struct se_cmd *cmd, int r)
> +{
Can you please pass the sense_reason_t argument to this function
and do all the __force int casting inside this function?
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majo
On Mon, Jul 06, 2015 at 11:02:27AM +0300, Sagi Grimberg wrote:
> Instead of open coding the sense buffer construction, use
> scsi scsi_build_sense_buffer() and scsi_set_sense_information()
> helpers which moved to scsi_common.
>
> This patch also fixes wrong setting of descriptor format sense data
On Wed, Jul 08, 2015 at 11:17:47AM +0530, Nagarajkumar Narayanan wrote:
> Note: pci_access_mutex is used only if nytro warpdrive cards
> (ioc->is_warpdrive based on device id) are used
> as we could not test this case with other SAS2 HBA cards
> We can remove this check if this behaviour confirme
On Tue, Jul 07, 2015 at 06:59:37AM -0700, Bart Van Assche wrote:
> Please note that my test was run with CONFIG_SLUB_DEBUG=y which causes a red
> zone to be allocated before and after each block of allocated memory. Could
> that explain the kmalloc-96 objects ?
96 is almost guaranteed to be the se
> + if (r == (__force int)TCM_CHECK_CONDITION_UNIT_ATTENTION) {
You probably want to compare reason here to avoid the cast.
Otherwise looks good,
Reviewed-by: Christoph Hellwig
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a mess
Looks good,
Reviewed-by: Christoph Hellwig
--
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
udes should be fixed up the same way, but that's a
different story).
Otherwise looks good:
Reviewed-by: Christoph Hellwig
--
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
On Mon, Jul 06, 2015 at 04:15:08PM +0300, Sagi Grimberg wrote:
> PI errors should be reported in a descriptor format sense data.
> Fix that by adding a desc_format flag to struct sense_info and pass
> it to scsi_build_sense_buffer() to do the right thing.
Do we really need the additional flag? We
On Wed, Jul 08, 2015 at 01:36:04PM +0300, Sagi Grimberg wrote:
> We don't have any other information today, but sector is not the only
> information that is requires a descriptor format, so maybe it will be a
> bit awkward to condition the descriptor format on the sector info?
The only reason why
On Wed, Jul 08, 2015 at 12:59:18PM +0200, Hannes Reinecke wrote:
> Actually it's controlled by the D_SENSE bit in the Control mode page
> (that's bit[2] of byte 2 in the control mode page).
> Which is currently set to '0', ie we will be returning fixed sense
> information.
> _If_ we were to report
On Wed, Jul 08, 2015 at 01:09:46PM +0200, Hannes Reinecke wrote:
> Add an 'access_state' attribute to display the LUN access state.
Should we just reuse the ALUA values here as they part of the spec
and map the legacy implemetation values to it?
I'd also really love to store the access_state vari
On Wed, Jul 08, 2015 at 11:07:05AM +0200, Hannes Reinecke wrote:
> As scsi_dh.c is now always compiled in we should be moving
> the 'dh_state' attribute to the generic code.
>
> Signed-off-by: Hannes Reinecke
Looks fine, but a few coding style nitpicks:
> + if (!sdev->handler) {
> +
On Wed, Jul 08, 2015 at 06:00:03PM +0300, Sagi Grimberg wrote:
> In fixed size sense format the information field is a four byte
> field.
This looks correct to me,
Reviewed-by: Christoph Hellwig
But: this will truncate > 32bit sector numbers. Maybe we need to
enable descriptor for
;
> Additionally, we cannot iterate over the sas_device_list without
> holding the lock, or we risk corrupting random memory if items are
> added or deleted as we iterate. This patch refactors _scsih_probe_sas()
> to use the sas_device_list in a safe way.
>
> Cc: Christoph Hellw
Looks good,
Reviewed-by: Christoph Hellwig
--
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
Looks good:
Reviewed-by: Christoph Hellwig
I wonder if we should start to gradually phase out these tuables in the
host template. It's not really more complicated to set them directly
in the host anyway.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" i
On Sun, Jul 12, 2015 at 01:37:03PM +0300, Sagi Grimberg wrote:
> Fixed size sense data information field is only 32 bits which
> means the sector (64 bits) information will be truncated.
>
> Move to descriptor format sense data to correctly report full
> sector information.
I think this needs to
On Tue, Jul 14, 2015 at 02:59:51PM +0300, Sagi Grimberg wrote:
> diff --git a/include/target/target_core_backend.h
> b/include/target/target_core_backend.h
> index 1e5c8f9..6ce370f 100644
> --- a/include/target/target_core_backend.h
> +++ b/include/target/target_core_backend.h
> @@ -3,6 +3,9 @@
>
;
> Signed-off-by: Sagi Grimberg
> Reviewed-by: Hannes Reinecke
> Reviewed-by: Martin K. Petersen
Looks good,
Reviewed-by: Christoph Hellwig
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kerne
i target and libata.
>
> Reported-by: Hannes Reinecke
> Signed-off-by: Sagi Grimberg
> Reviewed-by: Martin K. Petersen
> Cc: Tejun Heo
Looks good,
Reviewed-by: Christoph Hellwig
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a
ense data to
> correctly report sector information.
>
> Reported-by: Christoph Hellwig
> Signed-off-by: Sagi Grimberg
Looks good,
Reviewed-by: Christoph Hellwig
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kerne
On Wed, Jul 15, 2015 at 10:55:39AM +0300, Sagi Grimberg wrote:
> In case encountered a PI error, use scsi_set_sense_information
> instead of open coding information descriptor format.
>
> Signed-off-by: Sagi Grimberg
Looks good,
Reviewed-by: Christoph Hellwig
--
To unsubscribe fr
An array can't issue a reservation, the initiator needs to register
it. Right now the only way to do it is through SG_IO passthrough,
which is a best luck effort it I/O isn't also using SG_IO and can't
be properly supported because of that.
However I will submit an in-kernel reservation API soon
On Thu, Jul 16, 2015 at 05:07:03AM +, Christophe Varoqui wrote:
> For reference the opensvc crm does use type 5 pr, and aims for all paths
> registered. It still does not make use of the multipathd pr janitoring
> features, and uses sg_persist directly for pr status and actions.
The type doesn
Looks good,
but what's up with your From lines:
On Mon, Jul 20, 2015 at 04:29:49PM -0700, Spencer Baugh wrote:
> From: Spencer Baugh
>
> From: Chris Zankel
plus another address for you in the actual email From line. Who
did actually write this patch?
--
To unsubscribe from this list: send th
Looks good too, but same From: issue as the last patch.
--
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
Thanks Tony,
this looks good to me. In the long run I'll see how we can hide
these implementation details in the lib/scatterlist.c code instead
of burdening it on the users.
Reviewed-by: Christoph Hellwig
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" i
NVALID_CDB_FIELD;
The mix of || and | here is odd, why not just:
if (!(cdb[1] & 1) || cdb[2] || cdb[3])
> + if (!(cdb[4] & 1) || ((cdb[4] & 2) | (cdb[4] & 4)))
Same here.
But except for this nitpicking the patch looks good:
Reviewed-by: Christoph Hellwig
--
To unsu
Looks good,
Reviewed-by: Christoph Hellwig
--
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
On Wed, Jul 22, 2015 at 03:08:17PM -0700, Spencer Baugh wrote:
> done:
> + /*
> + * If no real LUNs are accessible, report an allocation length
> + * of 1 LUN to account for virtual LUN 0.
> + */
> + if (lun_count == 0)
> + lun_count = 1;
> +
Shouldn't the code
Looks good,
Reviewed-by: Christoph Hellwig
(altough normal Linux style would be to add parenthesis around the
logial and)
--
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.
On Tue, Jul 21, 2015 at 03:07:55PM -0700, Spencer Baugh wrote:
> From: Dilip Kumar Uppugandla
>
> Invoking get_cmd_state for qla2xxx always returns 0. Instead change it
> to return the actual fabric state from qla_tgt_cmd. This will help with
> debugging.
I think the ->get_cmd_state callback sho
On Thu, Jul 23, 2015 at 03:19:32PM -0700, Spencer Baugh wrote:
> From: Alexei Potashnik
>
> If command didn't match a LUN and we're sending check condition, the
> target_cmd_complete ftrace point will crash because it assumes that
> cmd->t_task_cdb has been set.
>
> The fix will temporarily set
On Wed, Jul 08, 2015 at 01:09:44PM +0200, Hannes Reinecke wrote:
> +++ b/drivers/scsi/device_handler/scsi_dh_alua.c
> @@ -264,8 +264,11 @@ static int alua_check_vpd(struct scsi_device *sdev,
> struct alua_dh_data *h)
> int device_id_size, device_id_type = 0;
> struct alua_port_group *t
On Fri, Jul 24, 2015 at 04:40:42PM +0200, Hannes Reinecke wrote:
> Yeah, possibly. After all, the variable isn't expected to change
> under rcu_read_lock().
Actually it can and will change, that's the point. But if you use
a local variable you keep a single version of it, which won't be
freed unt
As per the comment on patch 3 I'd rather expose the ALUA state in the core
SCSI code. But having this alua_state attribute in core SCSI code sounds
fine to me.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majord
This seems to be a bit of a catchall. Can you split the logging changes
from actual error code logic changes and describe the latter in more detail?
--
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
mp;rq->cmd[6]);
This is unrelated to the rest of the patch, can you split it out,
maybe together with other get/put_unaligned conversions?
Otherwise looks good:
Reviewed-by: Christoph Hellwig
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a mes
> - memset(h->buff, 0, stpg_len);
> - h->buff[4] = TPGS_STATE_OPTIMIZED & 0x0f;
> - h->buff[6] = (h->group_id >> 8) & 0xff;
> - h->buff[7] = h->group_id & 0xff;
> + memset(stpg_data, 0, stpg_len);
> + stpg_data[4] = TPGS_STATE_OPTIMIZED & 0x0f;
> + put_unaligned_be16(gro
Seems like this should use scsi_execute_req_flags instead so that it doesn't
have to deal with the raw sense buffer.
--
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/majordom
On Wed, Jul 08, 2015 at 11:06:09AM +0200, Hannes Reinecke wrote:
> + pg = kzalloc(sizeof(struct alua_port_group), GFP_KERNEL);
> + if (!pg) {
> + sdev_printk(KERN_WARNING, sdev,
> + "%s: kzalloc port group failed\n",
> + ALUA_DH_NA
Looks good,
Reviewed-by: Christoph Hellwig
--
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
0x0a)
> + err = SCSI_DH_RETRY;
> + if (sense_hdr.sense_key == UNIT_ATTENTION)
else if or just but cases in the same condition for clarity?
Otherwise looks fine:
Reviewed-by: Christoph Hellwig
--
To unsubscribe from this list: send the line "unsubscribe l
On Wed, Jul 08, 2015 at 11:06:12AM +0200, Hannes Reinecke wrote:
> Parse VPD descriptor to figure out the device identification.
> As devices might implement several descriptors the order
> of preference is:
> - NAA IEE Registered Extended
> - EUI-64 based 16-byte
> - EUI-64 based 12-byte
> - NAA I
> + charwork_q_name[264];
create_workqueue and friends now accept printf-like format
string, so there is no need for this temporary buffer.
> + int error;
> + struct completion init_complete;
Please rename error to init_error and only ass
Which debug printk do you care about? I'd much prefer having a trace
point inside the driver which could even pretty print it instead of the
hack where a driver defined binary value is printed by the core.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a me
1 - 100 of 4514 matches
Mail list logo
