Re: mpt2sas and mpt3sas merge (again)

2014-07-14 Thread Martin K. Petersen
> "James" == James Bottomley  writes:

James> I support the concept, since I think everyone told LSI at the
James> time that splitting the drivers would become a maintenance
James> nightmare.

And it is. I'd like to see these drivers merged as well.

James> One of the big reasons we don't have a lot of leverage with them
James> is that they always seem to slide updates around upstream via the
James> distros (often, it has to be admitted the DKM route), so if Red
James> Hat, SUSE, Oracle and Canonical can agree not to accept LSI
James> updates until the driver is done this way, we'd have a lot more
James> leverage.

Unless it's a trivial bug fix Oracle won't take anything that's not
accepted upstream. We are working with Avago/LSI to streamline their
driver process to be a better fit for both upstream development and
distro kernels.

-- 
Martin K. Petersen  Oracle Linux Engineering
--
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: mpt2sas stuck installing

2014-07-14 Thread Joe Lawrence
On Sat, 12 Jul 2014 03:13:07 +
"Elliott, Robert (Server Storage)"  wrote:

> 
> 
> > -Original Message-
> > From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi-
> > ow...@vger.kernel.org] On Behalf Of Joe Lawrence
> ...
> > In your crash stack trace, the scsi error handler has issued a host
> > reset, but then crashed in mpt2sas_base_get_iocstate.  Reading through
> > _scsih_shutdown, I don't believe that the mpt2sas .shutdown path takes
> > any precaution before heading down mpt2sas_base_detach to free adapter
> > resources.  The ordinary .remove path looks similar, though it does
> > call sas_remove_host before freeing resources, *then* scsi_remove_host
> > and scsi_host_put.
> > 
> > I wonder if this ordering needs to be reversed (and added to
> > _scsih_shutdown) to properly de-register from the SCSI midlayer prior
> > to removing the controller instance.
> > 
> > Regards,
> > 
> > -- Joe
> 
> Nagalakshmi was working on an mpt3sas patch for the scsi-mq tree 
> to do just that.  I don't recall if the patch has made it into the 
> scsi-mq tree yet. Apparently it's also needed for non-mq and mpt2sas.
> 
> It is making this change:
> > sas_remove_host(shost);
> > +   scsi_remove_host(shost);
> > mpt3sas_base_detach(ioc);
> > list_del(&ioc->list);
> > -   scsi_remove_host(shost);
> > scsi_host_put(shost);
> 
> We are making a similar change in hpsa.  Doing so led to a general 
> protection fault, which unveiled that we also needed to change 
> cancel_delayed_work() calls to cancel_delayed_work_sync() to 
> ensure there are no worker functions still active after the 
> scsi_host structure is unregistered.

It looks like Sreekanth posted those patches today -- the new ordering
seems correct, but is there any reason why _scsih_shutdown skips
{sas,scsi}_remove_host calls?

Regards,

-- Joe
--
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: scsi-mq V2

2014-07-14 Thread Benjamin LaHaise
Hi Robert,

On Sun, Jul 13, 2014 at 05:15:15PM +, Elliott, Robert (Server Storage) 
wrote:
> > > I will see if that solves the problem with the scsi-mq-3 tree, or
> > > at least some of the bisect trees leading up to it.
> > 
> > scsi-mq-3 is still going after 45 minutes.  I'll leave it running
> > overnight.
> > 
> 
> That has been going strong for 18 hours, so I think that's the patch
> we need.

Thanks for taking the time to narrow this down.  I've applied the fix to 
my aio-fixes tree at git://git.kvack.org/~bcrl/aio-fixes.git and fowarded 
it on to Linus as well.

-ben
-- 
"Thought is the essence of where you are now."
--
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: mpt2sas and mpt3sas merge (again)

2014-07-14 Thread Tomas Henzl
On 07/14/2014 05:10 PM, Hannes Reinecke wrote:
> On 07/14/2014 04:57 PM, James Bottomley wrote:
>> On Mon, 2014-07-14 at 16:39 +0200, Hannes Reinecke wrote:
>>> On 07/14/2014 04:17 PM, James Bottomley wrote:
> [ .. ]
 This isn't really a democracy; it's about who maintains the drivers and
 right now it's LSI (or whatever their new name is).

 One of the big reasons we don't have a lot of leverage with them is that
 they always seem to slide updates around upstream via the distros
 (often,  it has to be admitted the DKM route), so if Red Hat, SUSE,
 Oracle and Canonical can agree not to accept LSI updates until the
 driver is done this way, we'd have a lot more leverage.

We have a strong 'upstream first' policy in our releases for new drivers
and for new patches, so in theory upstream could use the force, I still  prefer
a softer way that is discussing it with Avago(LSI) and getting their ack. 


>>> Hmm. We (as SUSE) have been striving to have a 'upstream first'
>>> policy. IE for any new release the drivers have to be upstream
>>> before we consider including it in our release.
>>> This is most certainly true for the upcoming SLE-12 release, and
>>> also has been enforced for the current SLES11 SP3 release.
>>>
>>> This is official company policy, and has been communicated to all
>>> our partners.
>>> We do accept driver updates (ie patches which are not upstream ATM),
>>> but only on the understanding that the vendor will have to push the
>>> patches upstream eventually.
>>> If they don't the patches will be kicked out of the next release.
>>> (Which is what happened to the mptsas v4 release; it never made it
>>> upstream and so got dropped from SLE-12).
>>>
>>> However, this cuts both ways; we cannot go and tell our partners to
>>> change the driver if upstream hasn't done it first.
>> I'm not saying we need to go into why this happened.  Just that I'd like
>> community agreement amongst the distros before trying to force the
>> issue.  I accept that the distros respond to their TAMs as well as the
>> community, but if there's going to be TAM push back, I'd at least like
>> to hear about it so I can have a word with the relevant people.
>>
>>> So the push has to come from us (as the linux kernel developers);
>>> after all, we should make the decision what goes in and what
>>> doesn't. If a driver is in a bad state (and it's actually us which
>>> defines the 'bad state') we should be discussing on how we would
>>> like to improve things.
>>> If the maintainer proves unwilling to implement our suggestions we
>>> can always go ahead and implement a separate driver.
>> Then we need a maintainer of that driver ... remember this is a fat
>> firmware driver with a proprietary interface.  It's hard to maintain and
>> update without docs ... unless you happen to have an NDA copy?
>>
> Hmm. _if_ the driver is similar to the original one (which was the 
> idea) it should be reasonably trivial to port the latest changes 
> from the original driver to the merged one.

I think you expect more or less that after that new driver is created it
will be maintained again by Avago(LSI), so let us hear what they think first.


>From my point of view the not optimal thing already has happened, both driver 
>exist
and both are included in our major releases.
 
The change is probably better for the long-term maintainability of the driver,
but from the distro perspective any switch to a new driver adds some new work,
because we don't want to remove any particular old driver which has been 
included to
a major release. 
That said, I agree that we (Red Hat) will manage the change, since it is the 
better
approach for the long-term maintenance of the driver in the upstream". 

Cheers,
Tomas

>
>>> Look what happened to hpsa; this was the pretty much the showcase on
>>> how it should be done:
>>> Tomo went ahead and re-implemented the cciss driver, and eventually
>>> HP adopted it as their main driver.
>>> I agree that was pretty much the optimal case, though:-)
>> The best is to get LSI to agree, yes ... hence the need for unanimity.
>>
> Agreed. Let's see what LSI has to say here.
>
> Cheers,
>
> Hannes

--
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: mpt2sas and mpt3sas merge (again)

2014-07-14 Thread Hannes Reinecke

On 07/14/2014 04:57 PM, James Bottomley wrote:

On Mon, 2014-07-14 at 16:39 +0200, Hannes Reinecke wrote:

On 07/14/2014 04:17 PM, James Bottomley wrote:

[ .. ]


This isn't really a democracy; it's about who maintains the drivers and
right now it's LSI (or whatever their new name is).

One of the big reasons we don't have a lot of leverage with them is that
they always seem to slide updates around upstream via the distros
(often,  it has to be admitted the DKM route), so if Red Hat, SUSE,
Oracle and Canonical can agree not to accept LSI updates until the
driver is done this way, we'd have a lot more leverage.


Hmm. We (as SUSE) have been striving to have a 'upstream first'
policy. IE for any new release the drivers have to be upstream
before we consider including it in our release.
This is most certainly true for the upcoming SLE-12 release, and
also has been enforced for the current SLES11 SP3 release.

This is official company policy, and has been communicated to all
our partners.
We do accept driver updates (ie patches which are not upstream ATM),
but only on the understanding that the vendor will have to push the
patches upstream eventually.
If they don't the patches will be kicked out of the next release.
(Which is what happened to the mptsas v4 release; it never made it
upstream and so got dropped from SLE-12).

However, this cuts both ways; we cannot go and tell our partners to
change the driver if upstream hasn't done it first.


I'm not saying we need to go into why this happened.  Just that I'd like
community agreement amongst the distros before trying to force the
issue.  I accept that the distros respond to their TAMs as well as the
community, but if there's going to be TAM push back, I'd at least like
to hear about it so I can have a word with the relevant people.


So the push has to come from us (as the linux kernel developers);
after all, we should make the decision what goes in and what
doesn't. If a driver is in a bad state (and it's actually us which
defines the 'bad state') we should be discussing on how we would
like to improve things.
If the maintainer proves unwilling to implement our suggestions we
can always go ahead and implement a separate driver.


Then we need a maintainer of that driver ... remember this is a fat
firmware driver with a proprietary interface.  It's hard to maintain and
update without docs ... unless you happen to have an NDA copy?

Hmm. _if_ the driver is similar to the original one (which was the 
idea) it should be reasonably trivial to port the latest changes 
from the original driver to the merged one.



Look what happened to hpsa; this was the pretty much the showcase on
how it should be done:
Tomo went ahead and re-implemented the cciss driver, and eventually
HP adopted it as their main driver.
I agree that was pretty much the optimal case, though:-)


The best is to get LSI to agree, yes ... hence the need for unanimity.


Agreed. Let's see what LSI has to say here.

Cheers,

Hannes
--
Dr. Hannes Reinecke   zSeries & Storage
h...@suse.de  +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
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: [RFC 0/2] target: userspace pass-through backend

2014-07-14 Thread Stefan Hajnoczi
On Tue, Jul 1, 2014 at 9:11 PM, Andy Grover  wrote:
> Shaohua Li wrote an initial implementation of this, late last year[1].
> Starting from that, I started working on some alternate implementation
> choices, and ended up with something rather different.
>
> Please take a look and let me know what you think. Patch 1 is a
> design and overview doc, and patch 2 is the actual code, along with
> implementation rationale.
>
> Thanks -- Andy
>
> [1] http://thread.gmane.org/gmane.linux.scsi.target.devel/5044
>
> Andy Grover (2):
>   target: Add documentation on the target userspace pass-through driver
>   target: Add a user-passthrough backstore
>
>  Documentation/target/tcmu-design.txt   |  210 +++
>  drivers/target/Kconfig |5 +
>  drivers/target/Makefile|1 +
>  drivers/target/target_core_transport.c |4 +
>  drivers/target/target_core_user.c  | 1078 
> 
>  drivers/target/target_core_user.h  |  126 
>  6 files changed, 1424 insertions(+)
>  create mode 100644 Documentation/target/tcmu-design.txt
>  create mode 100644 drivers/target/target_core_user.c
>  create mode 100644 drivers/target/target_core_user.h

Hi Andy,
Just wanted to let you know that a userspace backstore would
potentially be useful for QEMU.  QEMU supports a number of disk image
formats (VMDK, VHDX, qcow2, and more).  Make these available as SCSI
LUNs on the host or to remote SCSI initiators is cool.

We currently have a tool called qemu-nbd that exports disk images
using the Network Block Device protocol.  Your userspace backstore
provides other options like iSCSI target or loopback access on the
host.

I took a quick look at the patch and imagine it's not hard to hook up
to QEMU.  Looks promising!

Stefan
--
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: mpt2sas and mpt3sas merge (again)

2014-07-14 Thread James Bottomley
On Mon, 2014-07-14 at 16:39 +0200, Hannes Reinecke wrote:
> On 07/14/2014 04:17 PM, James Bottomley wrote:
> > On Mon, 2014-07-14 at 11:22 +0200, Hannes Reinecke wrote:
> >> On 07/14/2014 10:35 AM, Christoph Hellwig wrote:
> >>> Back when the mpt3sas driver was first posted I suggested that it should
> >>> be merged into mpt2sas, but my proposal didn't get much traction.
> >>>
> >>> Illumos has now produced a shared driver and shown that the difference
> >>> are basically limited to a different S/G list format [1], and a quick
> >>> experiment on the Linux drivers confirms this mostly - the additional
> >>> differences are various smaller workarounds for specific hardware
> >>> revisions.
> >>>
> >>> I think we'd all be served much better with a merged driver.
> >>>
> >>> [1] http://thread.gmane.org/gmane.os.illumos.devel/17341
> >>
> >> Please. Pretty please.
> >
> > I support the concept, since I think everyone told LSI at the time that
> > splitting the drivers would become a maintenance nightmare.
> >
> >> I've started a merge myself, but then never found time to finalize it.
> >>
> >> Carrying three basically identical drivers just creates a
> >> maintenance overhead for everyone involved.
> >>
> >> The original idea of splitting the driver and just maintaining the
> >> latest has never really worked out; all fixes to the latest driver
> >> turned out to be applicable to the others, too.
> >> So it just increased the workload for the maintainer for no real
> >> gain. I would _strongly_ vote for it.
> >
> > This isn't really a democracy; it's about who maintains the drivers and
> > right now it's LSI (or whatever their new name is).
> >
> > One of the big reasons we don't have a lot of leverage with them is that
> > they always seem to slide updates around upstream via the distros
> > (often,  it has to be admitted the DKM route), so if Red Hat, SUSE,
> > Oracle and Canonical can agree not to accept LSI updates until the
> > driver is done this way, we'd have a lot more leverage.
> >
> Hmm. We (as SUSE) have been striving to have a 'upstream first' 
> policy. IE for any new release the drivers have to be upstream 
> before we consider including it in our release.
> This is most certainly true for the upcoming SLE-12 release, and 
> also has been enforced for the current SLES11 SP3 release.
> 
> This is official company policy, and has been communicated to all 
> our partners.
> We do accept driver updates (ie patches which are not upstream ATM), 
> but only on the understanding that the vendor will have to push the 
> patches upstream eventually.
> If they don't the patches will be kicked out of the next release.
> (Which is what happened to the mptsas v4 release; it never made it 
> upstream and so got dropped from SLE-12).
> 
> However, this cuts both ways; we cannot go and tell our partners to 
> change the driver if upstream hasn't done it first.

I'm not saying we need to go into why this happened.  Just that I'd like
community agreement amongst the distros before trying to force the
issue.  I accept that the distros respond to their TAMs as well as the
community, but if there's going to be TAM push back, I'd at least like
to hear about it so I can have a word with the relevant people.

> So the push has to come from us (as the linux kernel developers); 
> after all, we should make the decision what goes in and what 
> doesn't. If a driver is in a bad state (and it's actually us which 
> defines the 'bad state') we should be discussing on how we would 
> like to improve things.
> If the maintainer proves unwilling to implement our suggestions we 
> can always go ahead and implement a separate driver.

Then we need a maintainer of that driver ... remember this is a fat
firmware driver with a proprietary interface.  It's hard to maintain and
update without docs ... unless you happen to have an NDA copy?

> Look what happened to hpsa; this was the pretty much the showcase on 
> how it should be done:
> Tomo went ahead and re-implemented the cciss driver, and eventually 
> HP adopted it as their main driver.
> I agree that was pretty much the optimal case, though:-)

The best is to get LSI to agree, yes ... hence the need for unanimity.

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: mpt2sas and mpt3sas merge (again)

2014-07-14 Thread Hannes Reinecke

On 07/14/2014 04:17 PM, James Bottomley wrote:

On Mon, 2014-07-14 at 11:22 +0200, Hannes Reinecke wrote:

On 07/14/2014 10:35 AM, Christoph Hellwig wrote:

Back when the mpt3sas driver was first posted I suggested that it should
be merged into mpt2sas, but my proposal didn't get much traction.

Illumos has now produced a shared driver and shown that the difference
are basically limited to a different S/G list format [1], and a quick
experiment on the Linux drivers confirms this mostly - the additional
differences are various smaller workarounds for specific hardware
revisions.

I think we'd all be served much better with a merged driver.

[1] http://thread.gmane.org/gmane.os.illumos.devel/17341


Please. Pretty please.


I support the concept, since I think everyone told LSI at the time that
splitting the drivers would become a maintenance nightmare.


I've started a merge myself, but then never found time to finalize it.

Carrying three basically identical drivers just creates a
maintenance overhead for everyone involved.

The original idea of splitting the driver and just maintaining the
latest has never really worked out; all fixes to the latest driver
turned out to be applicable to the others, too.
So it just increased the workload for the maintainer for no real
gain. I would _strongly_ vote for it.


This isn't really a democracy; it's about who maintains the drivers and
right now it's LSI (or whatever their new name is).

One of the big reasons we don't have a lot of leverage with them is that
they always seem to slide updates around upstream via the distros
(often,  it has to be admitted the DKM route), so if Red Hat, SUSE,
Oracle and Canonical can agree not to accept LSI updates until the
driver is done this way, we'd have a lot more leverage.

Hmm. We (as SUSE) have been striving to have a 'upstream first' 
policy. IE for any new release the drivers have to be upstream 
before we consider including it in our release.
This is most certainly true for the upcoming SLE-12 release, and 
also has been enforced for the current SLES11 SP3 release.


This is official company policy, and has been communicated to all 
our partners.
We do accept driver updates (ie patches which are not upstream ATM), 
but only on the understanding that the vendor will have to push the 
patches upstream eventually.

If they don't the patches will be kicked out of the next release.
(Which is what happened to the mptsas v4 release; it never made it 
upstream and so got dropped from SLE-12).


However, this cuts both ways; we cannot go and tell our partners to 
change the driver if upstream hasn't done it first.


So the push has to come from us (as the linux kernel developers); 
after all, we should make the decision what goes in and what 
doesn't. If a driver is in a bad state (and it's actually us which 
defines the 'bad state') we should be discussing on how we would 
like to improve things.
If the maintainer proves unwilling to implement our suggestions we 
can always go ahead and implement a separate driver.


Look what happened to hpsa; this was the pretty much the showcase on 
how it should be done:
Tomo went ahead and re-implemented the cciss driver, and eventually 
HP adopted it as their main driver.

I agree that was pretty much the optimal case, though:-)

Cheers,

Hannes
--
Dr. Hannes Reinecke   zSeries & Storage
h...@suse.de  +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
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: mpt2sas and mpt3sas merge (again)

2014-07-14 Thread James Bottomley
On Mon, 2014-07-14 at 11:22 +0200, Hannes Reinecke wrote:
> On 07/14/2014 10:35 AM, Christoph Hellwig wrote:
> > Back when the mpt3sas driver was first posted I suggested that it should
> > be merged into mpt2sas, but my proposal didn't get much traction.
> >
> > Illumos has now produced a shared driver and shown that the difference
> > are basically limited to a different S/G list format [1], and a quick
> > experiment on the Linux drivers confirms this mostly - the additional
> > differences are various smaller workarounds for specific hardware
> > revisions.
> >
> > I think we'd all be served much better with a merged driver.
> >
> > [1] http://thread.gmane.org/gmane.os.illumos.devel/17341
> 
> Please. Pretty please.

I support the concept, since I think everyone told LSI at the time that
splitting the drivers would become a maintenance nightmare.

> I've started a merge myself, but then never found time to finalize it.
> 
> Carrying three basically identical drivers just creates a 
> maintenance overhead for everyone involved.
> 
> The original idea of splitting the driver and just maintaining the 
> latest has never really worked out; all fixes to the latest driver 
> turned out to be applicable to the others, too.
> So it just increased the workload for the maintainer for no real 
> gain. I would _strongly_ vote for it.

This isn't really a democracy; it's about who maintains the drivers and
right now it's LSI (or whatever their new name is).

One of the big reasons we don't have a lot of leverage with them is that
they always seem to slide updates around upstream via the distros
(often,  it has to be admitted the DKM route), so if Red Hat, SUSE,
Oracle and Canonical can agree not to accept LSI updates until the
driver is done this way, we'd have a lot more leverage.

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 v2] scsi_debug: support scsi-mq, queues and locks

2014-07-14 Thread Douglas Gilbert

On 14-07-13 06:55 PM, Elliott, Robert (Server Storage) wrote:




-Original Message-
From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi-
ow...@vger.kernel.org] On Behalf Of Douglas Gilbert
Sent: Monday, 07 July, 2014 8:31 AM
To: SCSI development list
Subject: [PATCH v2] scsi_debug: support scsi-mq, queues and locks

Resend, looks like the list does not like html attachments.


This v2 patch is against Christoph's core-for-3.17 branch which
includes scsi-mq V2. Here is a link to a partially updated
version of the scsi_debug html page.
http://sg.danny.cz/scsi/sdebug26.html


Reviewed-by: Robert Elliott 

A few minor concerns:
1. In scsi_debug_abort, does num_aborts needs to be atomic - can
the SCSI midlayer have concurrent .eh_abort_handler calls
in progress?

+static int scsi_debug_abort(struct scsi_cmnd *SCpnt)
+   ++num_aborts;

(I don't think this patch changes that from before...)

2. Same question for:
num_dev_resets
num_target_resets
num_bus_resets
num_host_resets

(I don't think this patch changes that from before...)


These are only informational (i.e. only consumed by
'cat /proc/scsi/scsi_debug/'). This could be addressed
if more meat is added to the various ".eh*" entry points.
I'd look for help from Hannes (cc-ed) in this area.


3. schedule_resp includes this comment about the new TASK SET
FULL injection code:
+   /* if (tsf) simulate device reporting SCSI status of TASK SET FULL.
+* Might override existing CHECK CONDITION. */

If a TASK SET FULL is injected over a CHECK CONDITION/
UNIT ATTENTION created by check_readiness():
+   k = find_first_bit(devip->uas_bm, SDEBUG_NUM_UAS);
...
+   clear_bit(k, devip->uas_bm);

then it looks like that unit attention is lost forever.


Yes. The driver could make the distinction between SCSI
errors found early in device server processing (e.g. UAs
and Illegal Requests) from errors found later such as
Medium Error. But that adds complexity. The simplest
approach would be to skip TSF injection if any error is
being reported. In the rare case where the driver wants
to delay the response and has no space (i.e. an attempt
to exceed CAN_QUEUE/scsi_debug_max_queue) the error could
take precedence by skipping the delay and doing an
in-thread scsi_done() call.


4. In scsi_debug_show_info:
+   "num_tgts=%d, shared (ram) size=%d MB, opts=0x%x, "

and the modparam string describing that variable:
MODULE_PARM_DESC(dev_size_mb, "size in MB of ram shared by devs(def=8)");

the units are really MiB, not MB.

(I don't think this patch changes that from before...)


Yes.


5. For the UNMAP command, this modparam:
MODULE_PARM_DESC(lbprz, "unmapped blocks return 0 on read (def=1)");
always causes unmap_region to zero out the blocks:
 if (scsi_debug_lbprz) {
 memset(fake_storep +
lba * scsi_debug_sector_size, 0,
scsi_debug_sector_size *
scsi_debug_unmap_granularity);
 }

That doesn't recognize that unmap requests via UNMAP commands are just
hints/suggestions, not mandatory.  The same is true in ATA for the
DATA SET MANAGEMENT/TRIM command.

Zeroing out is fine for when resp_write_same is the caller of
unmap_region and either NDOB=1 or the Data-Out Buffer contains all
zeros - if WRITE SAME with UNMAP=1 doesn't cause an unmap, it
still writes all zeros to the blocks.

When resp_unmap is the caller, though, there is no guarantee that
the data will change.

Maybe another modparam should be included to cause the driver
to purposely ignore unmap requests?  That might help more people
realize the danger in these commands.  (e.g., I think mdraid
assumes UNMAP will result in zeros for RAID-5/6 volumes,
which means parity will be calculated wrong if the drive
doesn't really unmap).

(I don't think this patch changes that from before...)


I consider the PI+LBP parts of this driver to be maintained
by Martin Petersen (cc-ed) and I'm hoping he will look at
this area (and its lock safety) when the dust settles.


I noticed that scsi_debug is reporting SPC-3 compliance and
that probably should be upped to SPC-4.


In summary, I would like to leave this oversized "v2" patch
as is. Then address some of the issues raised here as
a series of small, follow-up patches including some input
from those cc-ed in this reply.


BTW The driver documentation at:
  http://sg.danny.cz/sg/sdebug26.html
has been updated reflecting this v2 patch. There was a temporary
version at: http://sg.danny.cz/scsi/sdebug26.html
which will be removed (or made a symlink to the former).

Doug Gilbert

--
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: mpt2sas and mpt3sas merge (again)

2014-07-14 Thread Hannes Reinecke

On 07/14/2014 10:35 AM, Christoph Hellwig wrote:

Back when the mpt3sas driver was first posted I suggested that it should
be merged into mpt2sas, but my proposal didn't get much traction.

Illumos has now produced a shared driver and shown that the difference
are basically limited to a different S/G list format [1], and a quick
experiment on the Linux drivers confirms this mostly - the additional
differences are various smaller workarounds for specific hardware
revisions.

I think we'd all be served much better with a merged driver.

[1] http://thread.gmane.org/gmane.os.illumos.devel/17341


Please. Pretty please.

I've started a merge myself, but then never found time to finalize it.

Carrying three basically identical drivers just creates a 
maintenance overhead for everyone involved.


The original idea of splitting the driver and just maintaining the 
latest has never really worked out; all fixes to the latest driver 
turned out to be applicable to the others, too.
So it just increased the workload for the maintainer for no real 
gain. I would _strongly_ vote for it.


Cheers,

Hannes
--
Dr. Hannes Reinecke   zSeries & Storage
h...@suse.de  +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
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: scsi-mq V2

2014-07-14 Thread Sagi Grimberg

On 7/8/2014 5:48 PM, Christoph Hellwig wrote:


I've pushed out a new scsi-mq.3 branch, which has been rebased on the
latest core-for-3.17 tree + the "RFC: clean up command setup" series
from June 29th.  Robert Elliot found a problem with not fully zeroed
out UNMAP CDBs, which is fixed by the saner discard handling in that
series.

There is a new patch to factor the code from the above series for
blk-mq use, which I've attached below.  Besides that the only changes
are minor merge fixups in the main blk-mq usage patch.


Hey Christoph & Co,

I'd like to share some benchmarks I took on this patch set using iSER 
initiator (+2 pre-submitted performance improvements) vs LIO iSER target.
I ran workloads I think are interesting use-cases (single LUN with 1,2,4 
IO threads up to a fully occupied system doing IO to multiple LUNs).
Overall (except 2 strange anomalies) seems that scsi-mq patches 
(use_blk_mq=N) roughly sustains traditional scsi performance.
On the other hand scsi-mq code path (use_blk_mq=Y) on its own clearly 
shows better performance (tables below).


At first I too hit the aio issues discussed in this thread and converted 
to scsi-mq.3-no-rebase for testing (thanks Doug & Rob for raising it).
I must say that for some reason I get very low numbers for writes vs. 
reads (writes perf stuck at ~20K IOPs per thread), this happens
on 3.16-rc2 even before scsi-mq patches. Did anyone step on this as well 
or is it just a weird problem I'm having in my setup?
Anyway this is why my benchmarks shows only randread IO pattern (getting 
familiar numbers). I need to figure out whats wrong

with IO writes - I'll start bisecting on this.

I also reviewed the patch set and at this point, I don't have any 
comments. So you can add to the series:
Reviewed-by: Sagi Grimberg '' (or Tested-by - 
whatever you choose).


I want to state that I tested a traditional iSER initiator - no scsi-mq 
adoption at all.
I started looking into adopting scsi-mq to iSCSI/iSER recently and I 
must that say the scsi-mq adoption is not so
trivial due to iSCSI session-wide CmdSN/StatSN ordering constraints 
(can't just use more RDMA channels per connection...)
I'll be on vacation for the next couple of weeks, so I'll start a 
separate thread to get the community input on this matter.



Results: table entries are KIOPS(CPU%)
3.16-rc2 (scsi-mq patches reverted)
   Threads/LUN   1   24
#LUNs
 1 231(6.5%)   355(18.5%)   337(31.1%)
 2 446(13.6%)  673(37.2%)   654(49.8%)
 4 594(25%)960(49.41%) 1165(99.3%)
 81018(50.3%) 1563(99.6%)  1696(99.9%)
 16   1660(86.5%) 1731(99.6%)  1710(100%)


3.16-rc2 (scsi-mq included, use_blk_mq=N)
   Threads/LUN   1   24
#LUNs
 1 231(6.5%)   351(18.5%)   337(31.4%)
 2 446(13.6%)  660(37.3%)   647(50%)
 4 591(25%)967(49.7%)  1136(98.1%)
 81014(52.1%) 1296(100%)   1470(100%)
 16   1741(100%)  1761(100%)   1853(100%)


3.16-rc2 (scsi-mq included, use_blk_mq=Y)
   Threads/LUN   1   24
#LUNs
 1 265(6.4%)   465(13.4%)   572(27.9%)
 2 507(13.4%)  902(27.8%)  1034(45.9%)
 4 697(25%)   1197(49.5%)  1477(98.6%)
 81257(53.6%) 1856(98.7%)  1906(100%)
 16   1991(100%)  2021(100%)   2020(100%)

Notes:
- IOPs measurements are the average of a 60 seconds runs.
- The CPU measurement is the total usage across all CPUs, In order
  to understand per-CPU utilization value should be normalized to 16
  cores.
- scsi_mq (use_blk_mq=N) has roughly the same performance as
  traditional scsi IO path but I see an anomaly in test cases
  {8 LUNs, 2/4 threads per LUN}. This may result in NUMA
  misalignment for threads/interrupts – requires further
  investigation.
- iSER initiator has no Multi-Queue awareness.

Testing environment:
- Initiator and target systems of 16 (8x2) cores (Hyperthreading
  disabled).
- CPU model: Intel(R) Xeon(R) @ 2.60GHz
- Block Layer settings:
- scheduler=noop
- rq_affinity=1
- add_random=0
- nomerges=1
- Single FDR link between the target and initiator.
- Device model: Mellanox ConnectIB (the numbers are also familiar
  with Mellanox ConnectX-3).
- MSIX interrupt vectors were spread across system cores.
- irqbalancer was disabled.
- scsi_host settings:
- cmd_per_lun=32 (default)
- can_queue=113 (default)
- In the multi-LUN test cases, each LUN exposed via different
  scsi_host (iSCSI session).

Software:
- fio version: 2.0.13
- LIO iSER target (target-pending for-next)
- Null backing devices (NULLIO)
- Upstream based iSER initiator + internal pre-submitted
  performance enhancements.

fio configuration:
rw=randread
bs=1k
iodepth=128
loops=1
ioengine=libaio
direct=1
invalidate=1
fsync_on_close=1
randrepeat=1
norandommap

Cheers,
Sag

Re: [PATCH V3 1/7] Drivers: scsi: storvsc: Change the limits to reflect the values on the host

2014-07-14 Thread Hannes Reinecke

On 07/14/2014 11:00 AM, Christoph Hellwig wrote:

On Mon, Jul 14, 2014 at 10:57:53AM +0200, Hannes Reinecke wrote:

Okay, that's fine by me.


Should I take this as a reviewed-by tag?


Yes, please do.

Cheers,

Hannes
--
Dr. Hannes Reinecke   zSeries & Storage
h...@suse.de  +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
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 V3 1/7] Drivers: scsi: storvsc: Change the limits to reflect the values on the host

2014-07-14 Thread Christoph Hellwig
On Mon, Jul 14, 2014 at 10:57:53AM +0200, Hannes Reinecke wrote:
> Okay, that's fine by me.

Should I take this as a reviewed-by tag?

--
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 V3 1/7] Drivers: scsi: storvsc: Change the limits to reflect the values on the host

2014-07-14 Thread Hannes Reinecke

On 07/14/2014 10:30 AM, Christoph Hellwig wrote:

On Mon, Jul 14, 2014 at 08:15:17AM +0200, Hannes Reinecke wrote:

Limiting max_lun to 255 will make the driver to _not_ respond to LUNs higher
than that; ie Well-known LUN won't work here.
Also the SCSI stack will be using REPORT LUNS anyway since you're
advertising SPC-2 compliance. So your driver runs into issues if Hyper-V
would ever return more than 256 LUNs with the REPORT LUN command or if any
of the LUNs has an addressing scheme other than
'0x00'.
I would suggest to raise this to the technical limit (ie the largest LUN
which the _protocol_ supports) and let REPORT LUNS deal with the actual
LUNs.


I suspect hypverv doesn't support anything more.  For now I'd be
inclined to just put it in ASAP and if your suggestion works out fix it
up later, although I'll wait a bit for more review feedback.


Okay, that's fine by me.

Cheers,

Hannes
--
Dr. Hannes Reinecke   zSeries & Storage
h...@suse.de  +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
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


mpt2sas and mpt3sas merge (again)

2014-07-14 Thread Christoph Hellwig
Back when the mpt3sas driver was first posted I suggested that it should
be merged into mpt2sas, but my proposal didn't get much traction.

Illumos has now produced a shared driver and shown that the difference
are basically limited to a different S/G list format [1], and a quick
experiment on the Linux drivers confirms this mostly - the additional
differences are various smaller workarounds for specific hardware
revisions.

I think we'd all be served much better with a merged driver.

[1] http://thread.gmane.org/gmane.os.illumos.devel/17341
--
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


scsi: SCSI_FC_TGT_ATTRS

2014-07-14 Thread Paul Bolle
0) Commit 73685a458f2e ("tgt: removal") landed in next-20140714. It
states that "the CONFIG_SCSI_TGT, CONFIG_SCSI_SRP_TGT_ATTRS and
CONFIG_SCSI_FC_TGT_ATTRS kbuild variable[s] [...] are no longer needed".
It removed the Kconfig entry for SCSI_SRP_TGT_ATTRS, but it left the
entries for SCSI_TGT and SCSI_FC_TGT_ATTRS untouched.

1) Both SCSI_TGT and SCSI_FC_TGT_ATTRS appear to be unused after that
commit. Is a patch to remove their Kconfig entries queued somewhere?


Paul Bolle

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

2014-07-14 Thread Christoph Hellwig
On Mon, Jul 14, 2014 at 10:29:04AM +0200, Paul Bolle wrote:
> 0) Commit 73685a458f2e ("tgt: removal") landed in next-20140714. It
> states that "the CONFIG_SCSI_TGT, CONFIG_SCSI_SRP_TGT_ATTRS and
> CONFIG_SCSI_FC_TGT_ATTRS kbuild variable[s] [...] are no longer needed".
> It removed the Kconfig entry for SCSI_SRP_TGT_ATTRS, but it left the
> entries for SCSI_TGT and SCSI_FC_TGT_ATTRS untouched.
> 
> 1) Both SCSI_TGT and SCSI_FC_TGT_ATTRS appear to be unused after that
> commit. Is a patch to remove their Kconfig entries queued somewhere?

We missed those two options, feel free to send a 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


Re: [PATCH V3 1/7] Drivers: scsi: storvsc: Change the limits to reflect the values on the host

2014-07-14 Thread Christoph Hellwig
On Mon, Jul 14, 2014 at 08:15:17AM +0200, Hannes Reinecke wrote:
> Limiting max_lun to 255 will make the driver to _not_ respond to LUNs higher
> than that; ie Well-known LUN won't work here.
> Also the SCSI stack will be using REPORT LUNS anyway since you're
> advertising SPC-2 compliance. So your driver runs into issues if Hyper-V
> would ever return more than 256 LUNs with the REPORT LUN command or if any
> of the LUNs has an addressing scheme other than
> '0x00'.
> I would suggest to raise this to the technical limit (ie the largest LUN
> which the _protocol_ supports) and let REPORT LUNS deal with the actual
> LUNs.

I suspect hypverv doesn't support anything more.  For now I'd be
inclined to just put it in ASAP and if your suggestion works out fix it
up later, although I'll wait a bit for more review feedback.

--
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] eata: remove driver_lock

2014-07-14 Thread Christoph Hellwig
port_detect is only called from the module_init routine and thus implicitly
serialized, so remove the driver lock which was held over potentially
sleeping function calls.

Signed-off-by: Christoph Hellwig 
Reported-by: Arthur Marsh 
Tested-by: Arthur Marsh 
---
 drivers/scsi/eata.c | 9 -
 1 file changed, 9 deletions(-)

diff --git a/drivers/scsi/eata.c b/drivers/scsi/eata.c
index 03372cf..980898e 100644
--- a/drivers/scsi/eata.c
+++ b/drivers/scsi/eata.c
@@ -837,7 +837,6 @@ struct hostdata {
 static struct Scsi_Host *sh[MAX_BOARDS];
 static const char *driver_name = "EATA";
 static char sha[MAX_BOARDS];
-static DEFINE_SPINLOCK(driver_lock);
 
 /* Initialize num_boards so that ihdlr can work while detect is in progress */
 static unsigned int num_boards = MAX_BOARDS;
@@ -1097,8 +1096,6 @@ static int port_detect(unsigned long port_base, unsigned 
int j,
goto fail;
}
 
-   spin_lock_irq(&driver_lock);
-
if (do_dma(port_base, 0, READ_CONFIG_PIO)) {
 #if defined(DEBUG_DETECT)
printk("%s: detect, do_dma failed at 0x%03lx.\n", name,
@@ -1265,10 +1262,7 @@ static int port_detect(unsigned long port_base, unsigned 
int j,
}
 #endif
 
-   spin_unlock_irq(&driver_lock);
sh[j] = shost = scsi_register(tpnt, sizeof(struct hostdata));
-   spin_lock_irq(&driver_lock);
-
if (shost == NULL) {
printk("%s: unable to register host, detaching.\n", name);
goto freedma;
@@ -1345,8 +1339,6 @@ static int port_detect(unsigned long port_base, unsigned 
int j,
else
sprintf(dma_name, "DMA %u", dma_channel);
 
-   spin_unlock_irq(&driver_lock);
-
for (i = 0; i < shost->can_queue; i++)
ha->cp[i].cp_dma_addr = pci_map_single(ha->pdev,
  &ha->cp[i],
@@ -1439,7 +1431,6 @@ static int port_detect(unsigned long port_base, unsigned 
int j,
   freeirq:
free_irq(irq, &sha[j]);
   freelock:
-   spin_unlock_irq(&driver_lock);
release_region(port_base, REGION_SIZE);
   fail:
return 0;
-- 
1.9.1

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