Re: standards revisions

2012-10-07 Thread Christoph Hellwig
On Sun, Oct 07, 2012 at 10:31:37AM -0700, Nicholas A. Bellinger wrote:
> 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.

Yes, but we have a different code path for the mixed SCSI-2/SPC-3
reservations from the pure SCSI-2 mode, based on the function pointers
set up in core_setup_reservations().  As far as I can see we could
only reach the SCSI-2 path there for a pscsi device that has the
emulate_reservations flag set, and even then we would never actually hit
the code the function pointers point to as we don't actually support
emulated reservations for pscsi.

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

What I mean is that int core_setup_alua again has a special code path
for < SCSI-3 levels, which has the same reachability issues as for the
reservations above.

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

Unless the emulate_reservations or emulate_alua flags are set, in which
case we will set the other functions pointers.  That being said I can't
see why the function pointers are even needed, as the non-noop versions
are careful enough to do the right thing for pscsi as we'll never have
registrations set.  I'll send a series of patches to clean all this up.

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


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


[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  changed:

   What|Removed |Added

 CC||lists2...@fnarfbargle.com




--- Comment #3 from Brad Campbell   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: [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 
> Reviewed-by: Nagalakshmi Nandigama 

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


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