Re: standards revisions

2012-10-07 Thread James Bottomley
On Sat, 2012-10-06 at 23:11 -0700, Nicholas A. Bellinger wrote:
 On Sat, 2012-10-06 at 21:49 -0400, Christoph Hellwig wrote:
  Currenly all non-pscsi bakcneds report their standards version as
  SPC 2 via -get_device_rev.
 
 No, the proper on-the-wire bits to signal SPC-3 compliance are already
 being returned by virtual backend drivers within standard INQUIRY
 payload data.  
 
 Notice the comment:
 
 root@tifa:/usr/src/target-pending.git# grep SCSI_SPC_2 drivers/target/*.c
 drivers/target/target_core_file.c:return SCSI_SPC_2; /* Returns SPC-3 in 
 Initiator Data */
 drivers/target/target_core_iblock.c:  return SCSI_SPC_2; /* Returns SPC-3 in 
 Initiator Data */
 drivers/target/target_core_rd.c:  return SCSI_SPC_2; /* Returns SPC-3 in 
 Initiator Data */

That's causing confusion, I think!

   In addition to putting it into the
  inquirty data in spc_emulate_inquiry_std we also use it in two
  places to check on the version of the persistent reservation and
  alua support.  What is the benefit of supporting the old-style SCSI 2
  reservations and ALUA support when they can't be used at all with
  the virtual backends, and can only be used in the corner case emulated
  ALUA/PR support for pscsi?
  
 
 It's the include/scsi/scsi.h SCSI_3 + SCSI_SPC_* version definition
 names that are incorrect:
 
 #define SCSI_UNKNOWN0
 #define SCSI_1  1
 #define SCSI_1_CCS  2
 #define SCSI_2  3
 #define SCSI_3  4/* SPC */
 #define SCSI_SPC_2  5
 #define SCSI_SPC_3  6
 
 from spc4r30 section 6.4.2 Standard INQUIRY data, Table 142 -- Version:
 
00h The device server does not claim conformance to any standard.
 01h to 02h Obsolete
03h The device server complies to ANSI INCITS 301-1997 (a withdrawn 
 standard).
04h The device server complies to ANSI INCITS 351-2001 (SPC-2).
05h The device server complies to ANSI INCITS 408-2005 (SPC-3).
06h The device server complies to this standard.
 
 How about the following patch to fix the long-standing incorrect name
 usage of these three scsi.h defines..?

Because it's not incorrect.  Your confusion is that the SCSI_ constants
should match the inquiry data ... they shouldn't.  Look where we set
scsi_level in scsi_scan.c:708-713.  We have to fit an extra identifier
for SCSI_1_CCS so the scsi_level doesn't actually match the inquiry
data; it's incremented by 1 for SCSI_3 and above.  SCSI_3 does exist, by
the way, it was defined in the first version of SPC and there are some
devices which conform to it.

James


--
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: [PATCH, resend] Fix race between starved list processing and device removal

2012-10-07 Thread James Bottomley
On Mon, 2012-09-24 at 15:14 +0200, Bart Van Assche wrote:
 Avoid that the sdev reference count can drop to zero before
 the queue is run by scsi_run_queue(). Also avoid that the sdev
 reference count can drop to zero in the same function by invoking
 __blk_run_queue().
[...]   if (scsi_device_set_state(sdev, SDEV_CANCEL) != 0)
 @@ -977,6 +978,10 @@ void __scsi_remove_device(struct scsi_device *sdev)
   blk_cleanup_queue(sdev-request_queue);
   cancel_work_sync(sdev-requeue_work);
  
 + spin_lock_irqsave(shost-host_lock, flags);
 + list_del(sdev-starved_entry);
 + spin_unlock_irqrestore(shost-host_lock, flags);
 +

This hunk doesn't make much sense.  It seems to be orthogonal to the
problem listed in the changelog and this action is done on last put
anyway.

James


--
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: Request for improved commit tracking between fcoe and scsi trees

2012-10-07 Thread James Bottomley
On Wed, 2012-10-03 at 15:23 -0400, Neil Horman wrote:
 James, Robert-
   I've been doing lots of backports of FCoE code to the RHEL tree these
 last few months, and I've noticed something fairly irritating, and I was
 wondering if you two could help me out with it (in fact you two are the only 
 two
 which can).  I noticed that commits which are accepted into the FCoE tree that
 get passed upstream through the scsi tree have their commit hashes altered.  I
 can't find any examples currently, due to the fact that you, Robert, have
 recently re-cloned your git tree at open-fcoe.org, so all this nastiness has
 been covered up currently, but if things don't change, this issue will quickly
 resurface.
 
 Regardless, This makes it _really_ difficult to track a given patchs' 
 traversal 
 between trees upstream, and makes my life as a distro subsystem maintainer 
 fairly
 painful.  Normally I would just live with it, but I can't see any reason why 
 it
 should be this way, given that git can easily prevent this with a pull.  
 James,
 Robert, could you two please work out a way to provide commit hash consistency
 between your trees?  It would make mine (and I'm sure many other people's)
 lives, much easier.

I'm reluctant to commit to any tracking process that relies on stable
commit ids simply because they're illusory in git:  if we find a bug in
a commit (or even worse a bisection failure) in a tree, we fix it there,
which causes the commit id to change.

The way I do this type of tracking is via the Subject: line ... why
can't you use that?

James


--
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: Request for improved commit tracking between fcoe and scsi trees

2012-10-07 Thread James Bottomley
On Thu, 2012-10-04 at 20:25 +, Love, Robert W wrote:
 On 10/3/2012 12:23 PM, Neil Horman wrote:
  James, Robert-
  I've been doing lots of backports of FCoE code to the RHEL tree these
  last few months, and I've noticed something fairly irritating, and I was
  wondering if you two could help me out with it (in fact you two are the 
  only two
  which can).  I noticed that commits which are accepted into the FCoE tree 
  that
  get passed upstream through the scsi tree have their commit hashes altered. 
   I
  can't find any examples currently, due to the fact that you, Robert, have
  recently re-cloned your git tree at open-fcoe.org, so all this nastiness has
  been covered up currently, but if things don't change, this issue will 
  quickly
  resurface.
 
  Regardless, This makes it _really_ difficult to track a given patchs' 
  traversal
  between trees upstream, and makes my life as a distro subsystem maintainer 
  fairly
  painful.  Normally I would just live with it, but I can't see any reason 
  why it
  should be this way, given that git can easily prevent this with a pull.  
  James,
  Robert, could you two please work out a way to provide commit hash 
  consistency
  between your trees?  It would make mine (and I'm sure many other people's)
  lives, much easier.
 
 I had included pull URLs in the covermails of my updates, but I haven't 
 lately. I will make sure to do that from now on.

Actually, I'm happy to do a pull based process with signed tags going
forwards.  However:

  Bart had a complaint 
 about a misspelling in a commit message of a patch in my last update. I 
 just resent that three patch series with the corrected commit message. I 
 included a signed-tag to pull from in the covermail.

That change changed the commit id and gives a graphic illustration of
why any tracking process based on git commit ids is wrong.

James


--
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: [RESEND][PATCH 0/20][SCSI] mpt3sas: Driver submission v01.100.00.00

2012-10-07 Thread James Bottomley
On Sat, 2012-09-29 at 19:48 +0530, sreekanth.re...@lsi.com wrote:
 This is new scsi lld device driver from LSI supporting the SAS 3.0
 standard.
 
 Here is list of new 12gb host controllers:
 
   LSI SAS3004
   LSI SAS3008
   LSI SAS3108
 
 Signed-off-by: Sreekanth Reddy sreekanth.re...@lsi.com
 Reviewed-by: Nagalakshmi Nandigama nagalakshmi.nandig...@lsi.com

This doesn't seem to compile:

drivers/scsi/mpt3sas/mpt3sas_scsih.c: In function
‘_scsih_sas_broadcast_primitive_event’:
drivers/scsi/mpt3sas/mpt3sas_scsih.c:5368:2: error: ‘event_data’
undeclared (first use in this function)
drivers/scsi/mpt3sas/mpt3sas_scsih.c:5368:2: note: each undeclared
identifier is reported only once for each function it appears in
drivers/scsi/mpt3sas/mpt3sas_ctl.c: In function
‘mpt3sas_ctl_reset_handler’:
drivers/scsi/mpt3sas/mpt3sas_ctl.c:475:3: error: expected ‘)’ before
‘MPT3SAS_FMT9’
make[3]: *** [drivers/scsi/mpt3sas/mpt3sas_scsih.o] Error 1
make[3]: *** Waiting for unfinished jobs
make[3]: *** [drivers/scsi/mpt3sas/mpt3sas_ctl.o] Error 1
make[2]: *** [drivers/scsi/mpt3sas] Error 2
make[2]: *** Waiting for unfinished jobs
make[1]: *** [drivers/scsi] Error 2
make: *** [drivers] Error 2

Did you miss something in the split?

James


--
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


[Bug 47701] When too many disks fall out at the same time, RCU hangs

2012-10-07 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=47701


Brad Campbell lists2...@fnarfbargle.com changed:

   What|Removed |Added

 CC||lists2...@fnarfbargle.com




--- Comment #3 from Brad Campbell lists2...@fnarfbargle.com  2012-10-07 
14:20:46 ---
Apparently fixed as of 3.6.0-07201-ged5062d (current git as of 8 hours ago).

-- 
Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.
--
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: standards revisions

2012-10-07 Thread Christoph Hellwig
On Sat, Oct 06, 2012 at 11:11:50PM -0700, Nicholas A. Bellinger wrote:
 On Sat, 2012-10-06 at 21:49 -0400, Christoph Hellwig wrote:
  Currenly all non-pscsi bakcneds report their standards version as
  SPC 2 via -get_device_rev.
 
 No, the proper on-the-wire bits to signal SPC-3 compliance are already
 being returned by virtual backend drivers within standard INQUIRY
 payload data.  

I missed that, but it doesn't matter for the point I was making, which
is the code to special case the SCSI_2 case, which can't happen for
any virtual backend.  In addition it also can't happen for pscsi as
we don't wire up any command emulation but REPORT LUNS for it any more,
effectively making it dead code.

--
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: standards revisions

2012-10-07 Thread Nicholas A. Bellinger
On Sun, 2012-10-07 at 10:51 -0400, Christoph Hellwig wrote: 
 On Sat, Oct 06, 2012 at 11:11:50PM -0700, Nicholas A. Bellinger wrote:
  On Sat, 2012-10-06 at 21:49 -0400, Christoph Hellwig wrote:
   Currenly all non-pscsi bakcneds report their standards version as
   SPC 2 via -get_device_rev.
  
  No, the proper on-the-wire bits to signal SPC-3 compliance are already
  being returned by virtual backend drivers within standard INQUIRY
  payload data.  
 
 I missed that, but it doesn't matter for the point I was making, which
 is the code to special case the SCSI_2 case, which can't happen for
 any virtual backend.

Regardless of if an virtual backend reports a SPC-3 version (0x05) in
INQUIRY response, an initiator is still allowed to fall back to using
legacy SCSI-2 reservations instead of SPC-3 persistent reservations.  I
can think of at least one mainstream SCSI initiator that does this.

Also, I think your mistaken about ALUA and SCSI-2 compatibility.  ALUA
is an SPC-3 and above specific feature.

 In addition it also can't happen for pscsi as
 we don't wire up any command emulation but REPORT LUNS for it any more,
 effectively making it dead code.
 

pSCSI has always NOP'ed the reservation + ALUA function pointers within
core_setup_reservations() and core_setup_alua().

--nab

--
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: standards revisions

2012-10-07 Thread Nicholas A. Bellinger
On Sun, 2012-10-07 at 09:16 +0100, James Bottomley wrote:
 On Sat, 2012-10-06 at 23:11 -0700, Nicholas A. Bellinger wrote:
  On Sat, 2012-10-06 at 21:49 -0400, Christoph Hellwig wrote:
   Currenly all non-pscsi bakcneds report their standards version as
   SPC 2 via -get_device_rev.
  
  No, the proper on-the-wire bits to signal SPC-3 compliance are already
  being returned by virtual backend drivers within standard INQUIRY
  payload data.  
  
  Notice the comment:
  
  root@tifa:/usr/src/target-pending.git# grep SCSI_SPC_2 drivers/target/*.c
  drivers/target/target_core_file.c:  return SCSI_SPC_2; /* Returns SPC-3 in 
  Initiator Data */
  drivers/target/target_core_iblock.c:return SCSI_SPC_2; /* Returns 
  SPC-3 in Initiator Data */
  drivers/target/target_core_rd.c:return SCSI_SPC_2; /* Returns SPC-3 in 
  Initiator Data */
 
 That's causing confusion, I think!
 
In addition to putting it into the
   inquirty data in spc_emulate_inquiry_std we also use it in two
   places to check on the version of the persistent reservation and
   alua support.  What is the benefit of supporting the old-style SCSI 2
   reservations and ALUA support when they can't be used at all with
   the virtual backends, and can only be used in the corner case emulated
   ALUA/PR support for pscsi?
   
  
  It's the include/scsi/scsi.h SCSI_3 + SCSI_SPC_* version definition
  names that are incorrect:
  
  #define SCSI_UNKNOWN0
  #define SCSI_1  1
  #define SCSI_1_CCS  2
  #define SCSI_2  3
  #define SCSI_3  4/* SPC */
  #define SCSI_SPC_2  5
  #define SCSI_SPC_3  6
  
  from spc4r30 section 6.4.2 Standard INQUIRY data, Table 142 -- Version:
  
 00h The device server does not claim conformance to any standard.
  01h to 02h Obsolete
 03h The device server complies to ANSI INCITS 301-1997 (a withdrawn 
  standard).
 04h The device server complies to ANSI INCITS 351-2001 (SPC-2).
 05h The device server complies to ANSI INCITS 408-2005 (SPC-3).
 06h The device server complies to this standard.
  
  How about the following patch to fix the long-standing incorrect name
  usage of these three scsi.h defines..?
 
 Because it's not incorrect.  Your confusion is that the SCSI_ constants
 should match the inquiry data ... they shouldn't.

No, my current confusion is stemming from why it's OK for SCSI_SPC_[2,3]
constant names+values to not match what is actually defined in SPC-4
drafts for what target INQUIRY emulation bits end up going 'over the
wire'.

 Look where we set
 scsi_level in scsi_scan.c:708-713.  We have to fit an extra identifier
 for SCSI_1_CCS so the scsi_level doesn't actually match the inquiry
 data; it's incremented by 1 for SCSI_3 and above.

The extra increment by 1 is required by some Linux/SCSI client
implementation dependent requirements, yes..?

 SCSI_3 does exist, by
 the way, it was defined in the first version of SPC and there are some
 devices which conform to it.
 

Regardless, SCSI_SPC_[2,3] - SCSI_SPC[3,4] constants for target-core +
scsi-core should still be re-named to avoid the extra confusion this
introduces for folks reading code.

--nab

--
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: [PATCH, resend] Fix race between starved list processing and device removal

2012-10-07 Thread Bart Van Assche

On 10/07/12 12:47, James Bottomley wrote:

On Mon, 2012-09-24 at 15:14 +0200, Bart Van Assche wrote:

Avoid that the sdev reference count can drop to zero before
the queue is run by scsi_run_queue(). Also avoid that the sdev
reference count can drop to zero in the same function by invoking
__blk_run_queue().

[...]   if (scsi_device_set_state(sdev, SDEV_CANCEL) != 0)

@@ -977,6 +978,10 @@ void __scsi_remove_device(struct scsi_device *sdev)
blk_cleanup_queue(sdev-request_queue);
cancel_work_sync(sdev-requeue_work);

+   spin_lock_irqsave(shost-host_lock, flags);
+   list_del(sdev-starved_entry);
+   spin_unlock_irqrestore(shost-host_lock, flags);
+


This hunk doesn't make much sense.  It seems to be orthogonal to the
problem listed in the changelog and this action is done on last put
anyway.


Removing an sdev from the starved list in __scsi_remove_device() has the 
advantage that it is guaranteed that the get_device() call added in 
scsi_run_queue() will succeed. A possible alternative is to leave the 
starved list removal code in scsi_device_dev_release_usercontext() and 
to invoke __blk_run_queue() in scsi_run_queue() only if the get_device() 
call in that function succeeded. Does this mean that you prefer the 
second option - something like the (untested) code below ?


if (get_device(sdev-sdev_gendev)) {
spin_unlock(shost-host_lock);

spin_lock(sdev-request_queue-queue_lock);
__blk_run_queue(sdev-request_queue);
spin_unlock(sdev-request_queue-queue_lock);

put_device(sdev-sdev_gendev);
spin_lock(shost-host_lock);
}

Bart.

--
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: [PATCH 0/3] Insert ATA transport objects in SCSI syfs topology.

2012-10-07 Thread Dan Williams
On Thu, Oct 4, 2012 at 9:56 AM, Gwendal Grignou gwen...@google.com wrote:
 What's the benefit of this?
 + To unify ata transport sysfs topology with other scsi transport.

My concern is the thrash and breakage to switch the ordering around
given the (minor) growing pains injecting an ata_port into the device
path caused.  Although, it seems like Aaron has caught where this
reversal broke things at the cost of some additional special-casing (4
files changed, 25 insertions(+), 13 deletions(-)).  Patch 1 also
creates a problem for bisections as the code that assumes
dev/port/host will break.

I don't know... I'm all for consistency, but if the only justification
is to make the transports look the same we'll still have a glaring
transport difference between ata and sas.  In the sas case one
hba/host spanning all possible sas domains vs the ata case of a
guaranteed ata_port per ata domain relationship with at least one
host per port.  The port does live higher in the topology in the ata
case.

 + To easily map a ata_port with its associated scsi_host structure.

Not sure this is getting any easier.  There would now be three options
based on kernel version: look for the ata_port as a host attribute,
look at the host's parent, or look for the host's child.

--
Dan
--
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