Re: [PATCH v2 12/12] IB/srp: Add multichannel support

2014-10-21 Thread Christoph Hellwig 
On Mon, Oct 20, 2014 at 01:57:21PM +0200, Bart Van Assche wrote:
 That pr_err() statement was convenient while debugging the multiqueue code
 in the SRP initiator driver but can be left out. Would you agree with
 leaving the above three lines of debug code out instead of adding an
 additional argument to scsi_host_find_tag() ?

Feel free to remove the check.

--
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 11/12] IB/srp: Eliminate free_reqs list

2014-10-21 Thread Christoph Hellwig 
On Mon, Oct 20, 2014 at 01:47:53PM +0200, Bart Van Assche wrote:
 The only reason why patches 10/12 and 11/12 are separate patches
 is to reduce the size of individual patches and hence to make it
 easier to review these patches. If everyone agrees I'm fine with
 folding patch 11/12 into patch 10/12.

I would prefer to merge the two patches.

--
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 04/12] scsi_tcq.h: Add support for multiple hardware queues

2014-10-21 Thread Christoph Hellwig 
On Mon, Oct 20, 2014 at 02:01:25PM +0200, Bart Van Assche wrote:
 On 10/19/14 18:12, Sagi Grimberg wrote:
 On 10/7/2014 4:04 PM, Bart Van Assche wrote:
 -req = blk_queue_find_tag(sdev-request_queue, tag);
 +req = blk_queue_find_tag(sdev-request_queue, tag);
 
 Why is this line different?
 
 This is because the indentation has been modified from 8xspacetab into
 tabtab. I can leave out that change if you want.

Please keep it.

--
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 07/12] IB/srp: Avoid that I/O hangs due to a cable pull during LUN scanning

2014-10-21 Thread Christoph Hellwig 
On Mon, Oct 20, 2014 at 02:15:07PM +0200, Bart Van Assche wrote:
 How about renaming this function into shost_sdev_count() and moving its
 declaration to scsi/scsi_device.h and its implementation to
 drivers/scsi/scsi_lib.c ?

I'd prefer to defer this until we have an actual need for it elsewhere.

--
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: EH action after scsi_remove_host, was: Re: [PATCH v2 12/12] IB/srp: Add multichannel support

2014-10-21 Thread Christoph Hellwig 
On Mon, Oct 20, 2014 at 03:53:38PM +0200, Bart Van Assche wrote:
 The above assignment statement has been reported to fix a kernel oops that
 could be triggered by cable pulling. Regarding fixing the root cause: some
 time ago I had posted a patch series that makes scsi_remove_host() wait
 until all error handler callback functions have finished and also that
 prevents that any new error handler function calls are initiated after
 scsi_remove_host() has finished
 (http://thread.gmane.org/gmane.linux.scsi/82572/focus=87985). Should I
 repost that patch series ?

Please keep the workaround in srp for now, and then resend the series,
including a new patch to remove the workaround in srp.

--
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 04/12] scsi_tcq.h: Add support for multiple hardware queues

2014-10-21 Thread Sagi Grimberg 

On 10/21/2014 11:49 AM, Christoph Hellwig wrote:

On Mon, Oct 20, 2014 at 02:01:25PM +0200, Bart Van Assche wrote:

On 10/19/14 18:12, Sagi Grimberg wrote:

On 10/7/2014 4:04 PM, Bart Van Assche wrote:

-req = blk_queue_find_tag(sdev-request_queue, tag);
+req = blk_queue_find_tag(sdev-request_queue, tag);


Why is this line different?


This is because the indentation has been modified from 8xspacetab into
tabtab. I can leave out that change if you want.


Please keep it.



I don't have a big objection on this, but the problem with leaving this
stuff is that it tends to screw up git blame...
--
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 12/12] IB/srp: Add multichannel support

2014-10-21 Thread Sagi Grimberg 

On 10/20/2014 3:56 PM, Bart Van Assche wrote:

On 10/19/14 19:36, Sagi Grimberg wrote:

On 10/7/2014 4:07 PM, Bart Van Assche wrote:

  * comp_vector, a number in the range 0..n-1 specifying the
-  MSI-X completion vector. Some HCA's allocate multiple (n)
-  MSI-X vectors per HCA port. If the IRQ affinity masks of
-  these interrupts have been configured such that each MSI-X
-  interrupt is handled by a different CPU then the comp_vector
-  parameter can be used to spread the SRP completion workload
-  over multiple CPU's.
+  MSI-X completion vector of the first RDMA channel. Some
+  HCA's allocate multiple (n) MSI-X vectors per HCA port. If
+  the IRQ affinity masks of these interrupts have been
+  configured such that each MSI-X interrupt is handled by a
+  different CPU then the comp_vector parameter can be used to
+  spread the SRP completion workload over multiple CPU's.


This is fairly not trivial for the user...

Aren't we requesting a bit too much awareness here?
Can't we just make it work? The user hands out ch_count - why can't
you do some least-used logic here?

Maybe we can even go with per-cpu QPs and discard comp_vector argument?
this would probably bring the best performance, wouldn't it?
(fallback to least-used logic in case HW support less vectors)


Hello Sagi,

The only reason the comp_vector parameter is still supported is because
of backwards compatibility. What I expect is that users will set the
ch_count parameter but not the comp_vector parameter.


Agreed...



Using one QP per CPU thread does not necessarily result in the best
performance. In the tests I ran performance was about 4% better when
using one QP for each pair of CPU threads (with hyperthreading enabled).


I usually don't like using defaults based on empirical experiments on
specific workloads. IMO, going either full blown MQ (per-cpu), or
go SQ for default.

But that is just my opinion...
you call it.




+static unsigned ch_count;
+module_param(ch_count, uint, 0444);
+MODULE_PARM_DESC(ch_count,
+ Number of RDMA channels to use for communication with an
SRP target. Using more than one channel improves performance if the
HCA supports multiple completion vectors. The default value is the
minimum of four times the number of online CPU sockets and the number
of completion vectors supported by the HCA.);


Why? how did you get to this magic equation?


On the systems I have access to measurements have shown that this choice
for the ch_count parameter results in a significant performance
improvement without consuming too many system resources. The performance
difference when using more than four channels was small. This means that
the exact value of this parameter is not that important. What matters to
me is that users can benefit from improved performance even if the
ch_count kernel module parameter has been left to its default value.


I do like the idea of giving users high performance out-of-the-box. But
as I wrote below, I less like the idea of basing your choice on
experiments.

Sagi.



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 v2 12/12] IB/srp: Add multichannel support

2014-10-21 Thread Sagi Grimberg 

On 10/7/2014 4:07 PM, Bart Van Assche wrote:

Improve performance by using multiple RDMA/RC channels per SCSI
host for communication with an SRP target. About the
implementation:
- Introduce a loop over all channels in the code that uses
   target-ch.
- Set the SRP_MULTICHAN_MULTI flag during login for the creation
   of the second and subsequent channels.
- RDMA completion vectors are chosen such that RDMA completion
   interrupts are handled by the CPU socket that submitted the I/O
   request. As one can see in this patch it has been assumed if a
   system contains n CPU sockets and m RDMA completion vectors
   have been assigned to an RDMA HCA that IRQ affinity has been
   configured such that completion vectors [i*m/n..(i+1)*m/n) are
   bound to CPU socket i with 0 = i  n.
- Modify srp_free_ch_ib() and srp_free_req_data() such that it
   becomes safe to invoke these functions after the corresponding
   allocation function failed.
- Add a ch_count sysfs attribute per target port.

Signed-off-by: Bart Van Assche bvanass...@acm.org
Cc: Sagi Grimberg sa...@mellanox.com
Cc: Sebastian Parschauer sebastian.rie...@profitbricks.com


SNIP


spin_lock_irqsave(ch-lock, flags);
ch-req_lim += be32_to_cpu(rsp-req_lim_delta);
@@ -1906,7 +1970,7 @@ static int srp_queuecommand(struct Scsi_Host *shost, 
struct scsi_cmnd *scmnd)
goto err;


Bart,

Any chance you can share some perf output on this code?
I'm interested of knowing the contention on target-lock that is
still taken on the IO path across channels.

Can we think on how to avoid it?

Also would like to understand the where did the bottleneck transition.

Sagi.
--
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: [GIT PULL] target updates for v3.18-rc2

2014-10-21 Thread Sagi Grimberg 

On 10/21/2014 2:39 AM, Nicholas A. Bellinger wrote:

Hi Linus,

Here are the target updates for v3.18-rc2 code.  These where originally
destined for -rc1, but due to the combination of travel last week for
KVM Forum and my mistake of taking the three week merge window
literally, the pull request slipped..  Apologies for that.

A heads-up that you'll hit one minor conflict with scsi.git, that was
caught by sfr in linux-next here:

http://marc.info/?l=linux-nextm=141223868207635w=2

Please go ahead and pull from:

git://git.kernel.org/pub/scm/linux/kernel/git/nab/target-pending.git 
for-next

Things where reasonably quiet this round.  The highlights include:

   - New userspace backend driver (target_core_user.ko) by Shaohua Li
 and Andy Grover
   - A number of cleanups in target, iscsi-taret and qla_target code
 from Joern Engel
   - Fix an OOPs related to queue full handling with CHECK_CONDITION
 status from Quinn Tran
   - Fix to disable TX completion interrupt coalescing in iser-target,
 that was causing problems on some hardware
   - Fix for PR APTPL metadata handling with demo-mode ACLs

I'm most excited about the new backend driver that uses UIO + shared
memory ring to dispatch I/O and control commands into user-space.  This
was probably the most requested feature by users over the last couple of
years, and opens up a new area of development + porting of existing
user-space storage applications to LIO.  Thanks to Shaohua + Andy for
making this happen.

Also another honorable mention, a new Xen PV SCSI driver was merged via
the xen/tip.git tree recently, which puts us now at 10 target drivers in
upstream!  Thanks to David Vrabel + Juergen Gross for their work to get
this code merged.

Thank you,



Nic,

Why are these fixes are not included?

Target/iser: Fix initiator_depth and responder_resources
Target/iser: Avoid calling rdma_disconnect twice
Target/iser: Don't put isert_conn inside disconnected handler
Target/iser: Get isert_conn reference once got to connected_handler

They are sitting around for some time and  were supposed to make it to
3.17, I don't want these to miss 3.18 too...

Sagi.
--
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: [PATCHv7 00/27] scsi logging update (the boring part)

2014-10-21 Thread Christoph Hellwig 
On Mon, Oct 20, 2014 at 08:52:03AM +0200, Hannes Reinecke wrote:
 here is now the seventh iteration of my scsi logging update.
 With this I've included another patch for ratelimiting
 I/O error messages, which resolves the outstanding issue
 noted by Robert Elliot.

What tree is this against?  This doesn't seem to touch the ufs driver,
which in 3.18-rc has grown calls to some of the functions you remove.

--
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 1/2] ipr: Convert to generic DMA API

2014-10-21 Thread Christoph Hellwig 
Wendy, Brian,

can you give me an ACK for this 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

Re: [PATCHv7 00/27] scsi logging update (the boring part)

2014-10-21 Thread Hannes Reinecke 
On 10/21/2014 12:55 PM, Christoph Hellwig wrote:
 On Mon, Oct 20, 2014 at 08:52:03AM +0200, Hannes Reinecke wrote:
 here is now the seventh iteration of my scsi logging update.
 With this I've included another patch for ratelimiting
 I/O error messages, which resolves the outstanding issue
 noted by Robert Elliot.
 
 What tree is this against?  This doesn't seem to touch the ufs driver,
 which in 3.18-rc has grown calls to some of the functions you remove.
 
Hmm. I _thought_ to have it done against core-for-3.18; but I'll
double check ...

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: [dm-devel] [PATCH 1/1] multipath-tools: Change path checker for IBM IPR devices

2014-10-21 Thread Christoph Hellwig 
On Mon, Oct 06, 2014 at 05:50:32PM -0400, wenxi...@linux.vnet.ibm.com wrote:
 Sorry it took some time since we need to re-config the systems for this test.
 
 With Christoph's new patch only, still saw the failure.
 With Christoph's new patch + Brian's patch, works fine, didn't see the
 failure.

Can one of you send me a tested series with both patches?

Thanks!
--
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: [PATCHv7 00/27] scsi logging update (the boring part)

2014-10-21 Thread Christoph Hellwig 
On Tue, Oct 21, 2014 at 01:02:48PM +0200, Hannes Reinecke wrote:
  What tree is this against?  This doesn't seem to touch the ufs driver,
  which in 3.18-rc has grown calls to some of the functions you remove.
  
 Hmm. I _thought_ to have it done against core-for-3.18; but I'll
 double check ...

The core-for-3.18 tree indeed doesn't have this changes, but as 3.18-rc1
is out that doesn't really matter.  Please prepare it against Linus'
latest tree.  I will probably push out a core-for-3.19 today, but it
won't have a lot of patches at this point.
--
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] Fix comment typo in Scsi_Host struct definition

2014-10-21 Thread Christoph Hellwig 
On Thu, Oct 09, 2014 at 12:40:07PM +0100, Mark Knibbs wrote:
 Hi,
 
 The comment for the max_channel, max_id and max_lun parameters
 refers to the first two, but actually means the last two.
 
 Signed-off-by: Mark Knibbs ma...@clara.co.uk

I just noticed that I have already applied an equal patch from
Sebastian Herbszt, so I'll skip your version.
--
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 1/1] ufs: scsi: fix sparse errors in ufshcd_system_suspend

2014-10-21 Thread Christoph Hellwig 
On Sat, Oct 11, 2014 at 04:06:29AM -0700, Christoph Hellwig wrote:
 On Sun, Oct 05, 2014 at 03:44:16PM -0700, Subhash Jadavani wrote:
  Looks good, Reviewed-by: Subhash Jadavani subha...@codeaurora.org
 
 I'm getting a bit lost with all the UFS fixes, can you prepared a series
 with all the fixups required for 3.18 for me please?

Are you going to prepare a set of fixups for 3.18, or should I try to
cram them together?  I supect you'd do a much better job than I could.
--
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

[GIT PULL] osd: Boaz Harrosh - change of email

2014-10-21 Thread Boaz Harrosh 
Hi Sir Linus

A small administrative stuff, was on vacation and the old email bounced on me.
I was hoping to still make the 3 weeks merge window, but it was 2 weeks at the
end.
Your call if to make this wait for next window. Needless to say that it is
ZERO risk, just change of email.

Based on commit [bfe01a5b] Linux 3.17

3 patches available in the git repository at:

  git://git.open-osd.org/linux-open-osd.git for-linus

for you to fetch changes up to 1fa3a002b2546c42c343c77c144871285896ced5:

  Boaz Harrosh - fix email in Documentation (2014-10-19 20:36:36 +0300)


Boaz Harrosh (3):
  MAINTAINERS: Change Boaz Harrosh's email
  Boaz Harrosh - Fix broken email address
  Boaz Harrosh - fix email in Documentation

 Documentation/scsi/osd.txt  | 3 +--
 MAINTAINERS | 2 +-
 drivers/scsi/osd/Kbuild | 2 +-
 drivers/scsi/osd/Kconfig| 2 +-
 drivers/scsi/osd/osd_debug.h| 2 +-
 drivers/scsi/osd/osd_initiator.c| 4 ++--
 drivers/scsi/osd/osd_uld.c  | 4 ++--
 fs/exofs/Kbuild | 2 +-
 fs/exofs/common.h   | 2 +-
 fs/exofs/dir.c  | 2 +-
 fs/exofs/exofs.h| 2 +-
 fs/exofs/file.c | 2 +-
 fs/exofs/inode.c| 2 +-
 fs/exofs/namei.c| 2 +-
 fs/exofs/ore.c  | 4 ++--
 fs/exofs/ore_raid.c | 2 +-
 fs/exofs/ore_raid.h | 2 +-
 fs/exofs/super.c| 2 +-
 fs/exofs/symlink.c  | 2 +-
 fs/exofs/sys.c  | 2 +-
 fs/nfs/objlayout/objio_osd.c| 2 +-
 fs/nfs/objlayout/objlayout.c| 2 +-
 fs/nfs/objlayout/objlayout.h| 2 +-
 fs/nfs/objlayout/pnfs_osd_xdr_cli.c | 2 +-
 include/linux/pnfs_osd_xdr.h| 2 +-
 include/scsi/osd_initiator.h| 2 +-
 include/scsi/osd_ore.h  | 2 +-
 include/scsi/osd_protocol.h | 4 ++--
 include/scsi/osd_sec.h  | 2 +-
 include/scsi/osd_sense.h| 2 +-
 include/scsi/osd_types.h| 2 +-
 31 files changed, 35 insertions(+), 36 deletions(-)

Thanks, have a good Kernel Cycle
Boaz
--
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 27/27] scsi: ratelimit I/O error messages

2014-10-21 Thread Elliott, Robert (Server Storage) 


 -Original Message-
 From: Hannes Reinecke [mailto:h...@suse.de]
 Sent: Monday, 20 October, 2014 1:53 AM
 To: James Bottomley
 Cc: Christoph Hellwig; linux-scsi@vger.kernel.org; Elliott, Robert (Server
 Storage); Hannes Reinecke
 Subject: [PATCH 27/27] scsi: ratelimit I/O error messages
 
 There can be quite a lot of I/O error messages, even on smaller
 machines. So we need to ratelimit them to not overwhelm logging.
 
 Signed-off-by: Hannes Reinecke h...@suse.de
 ---
  drivers/scsi/scsi_lib.c | 30 ++
  1 file changed, 18 insertions(+), 12 deletions(-)
 
 diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
 index 2df485f..9d248d2 100644
 --- a/drivers/scsi/scsi_lib.c
 +++ b/drivers/scsi/scsi_lib.c
 @@ -22,6 +22,7 @@
  #include linux/hardirq.h
  #include linux/scatterlist.h
  #include linux/blk-mq.h
 +#include linux/ratelimit.h
 
  #include scsi/scsi.h
  #include scsi/scsi_cmnd.h
 @@ -1037,18 +1038,23 @@ void scsi_io_completion(struct scsi_cmnd *cmd,
 unsigned int good_bytes)
   switch (action) {
   case ACTION_FAIL:
   /* Give up and fail the remainder of the request */
 - if (unlikely(scsi_logging_level))
 - level = SCSI_LOG_LEVEL(SCSI_LOG_MLQUEUE_SHIFT,
 -SCSI_LOG_MLQUEUE_BITS);
 - /*
 -  * if logging is enabled the failure will be printed
 -  * in scsi_log_completion(), so avoid duplicate messages
 -  */
 - if (!level  !(req-cmd_flags  REQ_QUIET)) {
 - scsi_print_result(cmd, NULL, FAILED);
 - if (driver_byte(result)  DRIVER_SENSE)
 - scsi_print_sense(cmd);
 - scsi_print_command(cmd);
 + if (unlikely(scsi_logging_level)) {

I don't think wrapping all that inside:
if (unlikely(scsi_logging_level))
is correct.  That means unrelated logging levels (e.g., LLQUEUE)
have an impact on whether this print happens.

 + static DEFINE_RATELIMIT_STATE(_rs,
 + DEFAULT_RATELIMIT_INTERVAL,
 + DEFAULT_RATELIMIT_BURST);
 + level = SCSI_LOG_LEVEL(SCSI_LOG_MLCOMPLETE_SHIFT,
 +SCSI_LOG_MLCOMPLETE_BITS);
 + /*
 +  * if logging is enabled the failure will be printed
 +  * in scsi_log_completion(), so avoid duplicate messages
 +  */
 + if (__ratelimit(_rs)  !level 
 + !(req-cmd_flags  REQ_QUIET)) {

__ratelimit should only be invoked if the other two are 
leading towards printing, since it is fairly involved
(including a spinlock):
if (!level  !(req-cmd_flags  REQ_QUIET)  
(__ratelimit(_rs)) {

 + scsi_print_result(cmd, NULL, FAILED);
 + if (driver_byte(result)  DRIVER_SENSE)
 + scsi_print_sense(cmd);
 + scsi_print_command(cmd);
 + }
   }
   if (!scsi_end_request(req, error, blk_rq_err_bytes(req), 0))
   return;

Here are some results from removing devices during sequential reads.

With the scsi logging level clear, just 10 block layer prints show up
(that outer if blocks everything):
[48973.034156] blk_update_request: critical target error, dev sdr, sector 
7447280
[48973.035298] blk_update_request: critical target error, dev sdr, sector 
7591344
[48973.035302] blk_update_request: critical target error, dev sdr, sector 
7591352
[48973.035351] blk_update_request: critical target error, dev sdr, sector 
7591360
[48973.035355] blk_update_request: critical target error, dev sdr, sector 
7591368
[48973.035391] blk_update_request: critical target error, dev sdr, sector 
7591376
[48973.035394] blk_update_request: critical target error, dev sdr, sector 
7591384
[48973.035436] blk_update_request: critical target error, dev sdr, sector 
7591392
[48973.035439] blk_update_request: critical target error, dev sdr, sector 
7591400
[48973.035471] blk_update_request: critical target error, dev sdr, sector 
7591408

With scsi_logging_level --set --error=5, there are: 
* 10 FAILED Result prints
  * the first has no CDB
  * at the end, there is a much later CDB print not preceded by a FAILED print
* 10 blk_update_request: critical target error prints
  * their sector number matches the preceding CDB print
* 1 blk_update_request: 539 callbacks suppressed print
* fio printed 33 io_u errors
  * the first at offset 27118616, matching the first print
  * one of the later ones is at offset 28297168, matching the last print
* no callbacks suppressed messages from scsi_io_completion


[49190.331376] sd 2:0:0:1: [sds] FAILED Result: hostbyte=DID_OK

Re: [GIT PULL] target updates for v3.18-rc2

2014-10-21 Thread Nicholas A. Bellinger 
On Tue, 2014-10-21 at 12:24 +0300, Sagi Grimberg wrote:
 On 10/21/2014 2:39 AM, Nicholas A. Bellinger wrote:
  Hi Linus,
 
  Here are the target updates for v3.18-rc2 code.  These where originally
  destined for -rc1, but due to the combination of travel last week for
  KVM Forum and my mistake of taking the three week merge window
  literally, the pull request slipped..  Apologies for that.
 
  A heads-up that you'll hit one minor conflict with scsi.git, that was
  caught by sfr in linux-next here:
 
  http://marc.info/?l=linux-nextm=141223868207635w=2
 
  Please go ahead and pull from:
 
  git://git.kernel.org/pub/scm/linux/kernel/git/nab/target-pending.git 
  for-next
 
  Things where reasonably quiet this round.  The highlights include:
 
 - New userspace backend driver (target_core_user.ko) by Shaohua Li
   and Andy Grover
 - A number of cleanups in target, iscsi-taret and qla_target code
   from Joern Engel
 - Fix an OOPs related to queue full handling with CHECK_CONDITION
   status from Quinn Tran
 - Fix to disable TX completion interrupt coalescing in iser-target,
   that was causing problems on some hardware
 - Fix for PR APTPL metadata handling with demo-mode ACLs
 
  I'm most excited about the new backend driver that uses UIO + shared
  memory ring to dispatch I/O and control commands into user-space.  This
  was probably the most requested feature by users over the last couple of
  years, and opens up a new area of development + porting of existing
  user-space storage applications to LIO.  Thanks to Shaohua + Andy for
  making this happen.
 
  Also another honorable mention, a new Xen PV SCSI driver was merged via
  the xen/tip.git tree recently, which puts us now at 10 target drivers in
  upstream!  Thanks to David Vrabel + Juergen Gross for their work to get
  this code merged.
 
  Thank you,
 
 
 Nic,
 
 Why are these fixes are not included?
 
 Target/iser: Fix initiator_depth and responder_resources
 Target/iser: Avoid calling rdma_disconnect twice
 Target/iser: Don't put isert_conn inside disconnected handler
 Target/iser: Get isert_conn reference once got to connected_handler
 
 They are sitting around for some time and  were supposed to make it to
 3.17, I don't want these to miss 3.18 too...
 

Those went in for v3.17 with the appropriate CC's for stable.

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/infiniband/ulp/isert?id=c2f88b17a1d97ca4ecd96cc22333a7a4f1407d39
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/infiniband/ulp/isert?id=0fc4ea701fcf5bc51ace4e288af5be741465f776
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/infiniband/ulp/isert?id=38a8316b5d80ddee071d493bae567185c07de359
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/infiniband/ulp/isert?id=1a92e17e39b9a5d476e0a7aa52280e76321834ee

--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 27/27] scsi: ratelimit I/O error messages

2014-10-21 Thread Elliott, Robert (Server Storage) 


 -Original Message-
 From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi-
 ow...@vger.kernel.org] On Behalf Of Elliott, Robert (Server Storage)
...
 With scsi_logging_level --set --error=5, there are:
 * 10 FAILED Result prints
   * the first has no CDB
   * at the end, there is a much later CDB print not preceded by a
 FAILED print
 * 10 blk_update_request: critical target error prints
   * their sector number matches the preceding CDB print
 * 1 blk_update_request: 539 callbacks suppressed print
 * fio printed 33 io_u errors
   * the first at offset 27118616, matching the first print
   * one of the later ones is at offset 28297168, matching the last
 print
 * no callbacks suppressed messages from scsi_io_completion
...
 [49190.337402] sd 2:0:0:1: [sds] FAILED Result: hostbyte=DID_OK
 driverbyte=DRIVER_SENSE
 [49190.337404] sd 2:0:0:1: [sds] Sense Key : Hardware Error [current]
 [49190.337406] sd 2:0:0:1: [sds] Add. Sense: Logical unit failure
 [49190.337408] sd 2:0:0:1: [sds] CDB:
 [49190.337412] Read(10): 28 00 01 9d cc 58 00 00 08 00
 [49190.337413] blk_update_request: critical target error, dev sds,
 sector 27118680 0x19dcc58
 [49190.337417] blk_update_request: critical target error, dev sds,
 sector 27118688 0x19dcc60
 [49190.554043] sd 2:0:0:1: [sds] CDB:
 [49190.555178] Read(10): 28 00 01 af c7 d0 00 00 08 00 28297168
...

After triggering another error after lunch, a ratelimit message
for scsi_io_completion did finally appear:
[57601.577000] scsi_io_completion: 536 callbacks suppressed
[57601.578865] sd 2:0:0:0: [sdr] FAILED Result: hostbyte=DID_OK 
driverbyte=DRIVER_SENSE
[57601.581535] sd 2:0:0:0: [sdr] Sense Key : Hardware Error [current]
[57601.584406] sd 2:0:0:0: [sdr] Add. Sense: Logical unit failure
[57601.586419] sd 2:0:0:0: [sdr] CDB:
[57601.587662] Read(10): 28 00 2e 92 90 00 00 00 08 00

---
Rob ElliottHP Server Storage





--
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] scsi: Add Hyper-V logical block provisioning quirks

2014-10-21 Thread KY Srinivasan 


 -Original Message-
 From: Sitsofe Wheeler [mailto:sits...@gmail.com]
 Sent: Monday, October 20, 2014 9:46 PM
 To: KY Srinivasan
 Cc: Jeff Leung; James Bottomley; Christoph Hellwig; Haiyang Zhang; Christoph
 Hellwig; Hannes Reinecke; linux-scsi@vger.kernel.org; linux-
 ker...@vger.kernel.org; de...@linuxdriverproject.org
 Subject: Re: [PATCH 0/3] scsi: Add Hyper-V logical block provisioning quirks
 
 On Sun, Oct 12, 2014 at 01:21:01AM +, KY Srinivasan wrote:
 
   -Original Message-
   From: Jeff Leung [mailto:jle...@v10networks.ca]
   Sent: Saturday, October 11, 2014 1:22 PM
  
On the current release of Windows (windows 10), we are advertising
SPC3 compliance.
We are ok with declaring compliance to SPC3 in our drivers.
   If you are going to declare SPC3 compliance in the drivers, are you
   going to put in checks to ensure that SPC-3 compliance doesn't get
   accidentally enabled for hypervisors below Win10?
  
   I do know for a fact that Ubuntu's kernels already force SPC3
   compliance to enable specific features such as TRIM on earlier
   versions of Hyper-V (Namely Hyper-V 2012 and 2012 R2).
  You are right; Ubuntu has been carrying a patch  that was doing just
  this and this has been working without any issues on many earlier
  versions of Windows. (2012 and 2012 R2).  On windows 10 we don't need
  any changes in the Linux driver as the host itself is advertising SPC3
  compliance. Based on the testing we have done with Ubuntu, we are
  comfortable picking up that patch.
 
 OK this seems to be the patch currently carried by Ubuntu:
 
 From ff2c5fa3fa9adf0b919b9425e71a8ba044c31a7d Mon Sep 17 00:00:00 2001
 From: Andy Whitcroft a...@canonical.com
 Date: Fri, 13 Sep 2013 17:49:16 +0100
 Subject: [PATCH] scsi: hyper-v storsvc switch up to SPC-3
 
 Suggested-By: James Bottomley
 james.bottom...@hansenpartnership.com
 Signed-off-by: Andy Whitcroft a...@canonical.com
 ---
  drivers/scsi/storvsc_drv.c |8 
  1 file changed, 8 insertions(+)
 
 diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c index
 9969fa1..3903c8a 100644
 --- a/drivers/scsi/storvsc_drv.c
 +++ b/drivers/scsi/storvsc_drv.c
 @@ -1441,6 +1441,14 @@ static int storvsc_device_configure(struct
 scsi_device *sdevice)
 
   sdevice-no_write_same = 1;
 
 + /*
 +  * hyper-v lies about its capabilities indicating it is only SPC-2
 +  * compliant, but actually implements the core SPC-3 features.
 +  * If we pretend to be SPC-3, we send RC16 which activates trim and
 +  * will query the appropriate VPD pages to enable trim.
 +  */
 + sdevice-scsi_level = SCSI_SPC_3;
 +
   return 0;
  }
 
 --
 1.7.9.5
 
 (Downloaded from
 http://kernel.ubuntu.com/git?p=jsalisbury/stable/trusty/ubuntu-
 trusty.git;a=patch;h=ff2c5fa3fa9adf0b919b9425e71a8ba044c31a7d
 ).
 
 I think it's unwise to override the scsi_level at this particular point 
 because
 you are going to do it for ALL Hyper-V disks (perhaps all Hyper-V SCSI
 devices?)...

I agree with you. I was only referring virtual hard disks (VHDs) being 
presented to the guest and
not pass-through devices. Furthermore, we would fix up the scsi conformance 
level based on the
version of the host we are running on.

Regards,

K. Y
 
 Here's the SCSI inquiry information reported by a USB 2 hard disk being
 passed passed-through by one of my 2012 R2 hosts:
 
 # sg_inq /dev/sdc
 standard INQUIRY:
   PQual=0  Device_type=0  RMB=0  version=0x02  [SCSI-2]
   [AERC=0]  [TrmTsk=0]  NormACA=0  HiSUP=0  Resp_data_format=1
   SCCS=0  ACC=0  TPGS=0  3PC=0  Protect=0  [BQue=0]
   EncServ=0  MultiP=0  [MChngr=0]  [ACKREQQ=0]  Addr16=0
   [RelAdr=0]  WBus16=0  Sync=0  Linked=0  [TranDis=0]  CmdQue=0
 length=36 (0x24)   Peripheral device type: disk
  Vendor identification: MDT MD50
  Product identification: 00AAKS-00TMA0
  Product revision level:
 
 Is it OK to replace a scsi_level of SCSI-2 with SCSI_SPC_3? Additionally is it
 also OK to force SCSI_SPC_3 on Hyper-V 2008?
 
NryزXvؖ){nlj{zX}zj:v zZzf~zwڢ)jyA
   
i
  N?r??yb?X??ǧv?^?)޺{.n?+{zX????ܨ}???Ơz?j:+v???
 zZ+??+zf???h???~i???z??w??)ߢf??^jǫy?m??@A?a??
 ?
 
 0??h???i
 
 ^^^ Where do these characters come from? I've occasionally seen them on
 emails from other Microsoft folks posting to LKML too...
 
 --
 Sitsofe | http://sucs.org/~sits/

RE: [PATCH] scsi: storvsc: Force SPC-3 for Win8 Hosts or Later

2014-10-21 Thread KY Srinivasan 


 -Original Message-
 From: Jeff Leung [mailto:jle...@v10networks.ca]
 Sent: Monday, October 20, 2014 10:39 PM
 To: linux-scsi@vger.kernel.org; linux-ker...@vger.kernel.org;
 de...@linuxdriverproject.org
 Cc: KY Srinivasan; james.bottom...@hansenpartnership.com; Jeff Leung
 Subject: [PATCH] scsi: storvsc: Force SPC-3 for Win8 Hosts or Later
 
 This patch forces SPC-3 for Hyper-V disks on the VMBus. As Windows 10's
 virtual SAS bus is SPC-3 compliant and there are no negative side effects on
 forcing SPC-3 compliance for Win8 hosts, this patch enables SPC-3 compliance
 by forcing it on for hosts with versions later than Win8.
 
 Forcing SPC-3 compliance for hosts earlier than Win10 also enables TRIM
 support.
 
 Suggested by: James Bottomley
 james.bottom...@hansenpartnership.com
 Signed-off-by: Jeff Leung jle...@v10networks.ca
 ---
  drivers/scsi/storvsc_drv.c |9 +
  1 files changed, 9 insertions(+), 0 deletions(-)
 
 diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c index
 ed0f899..afcc68e 100644
 --- a/drivers/scsi/storvsc_drv.c
 +++ b/drivers/scsi/storvsc_drv.c
 @@ -1449,6 +1449,15 @@ static int storvsc_device_configure(struct
 scsi_device *sdevice)
 
   sdevice-no_write_same = 1;
 
 + /*
 +  * If the host is Win2k12 or later, we pretend to be SPC-3 compliant
 +  * and send RC16 which activates TRIM. We will only enable this on a
 +  * host with levels greater than VERSION_WIN8
 +  */
 + if (vmbus_proto_version = VERSION_WIN8) {

I would want this hack not to be in Windows 10. We can have this hack if the 
host is either
Ws2012 or ws2012 r2 (VERSION_WIN8 or VERSION_WIN8_1). Also this hack should 
apply only 
to VHD's being presented and not pass through devices.

K. Y
 + sdevice-scsi_level = SCSI_SPC_3;
 + }
 +
   return 0;
  }
 
 --
 1.7.2.5

--
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 1/1] ufs: scsi: fix sparse errors in ufshcd_system_suspend

2014-10-21 Thread Subhash Jadavani 
 Are you going to prepare a set of fixups for 3.18, or should I try to cram
them together?  I supect you'd do a much better job than I could.

Agreed. There are 3 patches already posted for sparse warning fixes but then
there were 5 new reports of sparse warning. So we (Dolev Raviv and myself)
would try to fix them soon and post all 8 patches as single patch series so
that it would be easy trace. In addition, there will be one more fix for the
issue reported by Akinobu Mita, will include it in the same in patch series.
We should be able to send you these fixes by early next week if not earlier.

-Original Message-
From: linux-scsi-ow...@vger.kernel.org
[mailto:linux-scsi-ow...@vger.kernel.org] On Behalf Of Christoph Hellwig
Sent: Tuesday, October 21, 2014 7:06 AM
To: Subhash Jadavani
Cc: 'Dolev Raviv'; james.bottom...@hansenpartnership.com;
linux-scsi@vger.kernel.org; linux-arm-...@vger.kernel.org;
santos...@gmail.com
Subject: Re: [PATCH 1/1] ufs: scsi: fix sparse errors in
ufshcd_system_suspend

On Sat, Oct 11, 2014 at 04:06:29AM -0700, Christoph Hellwig wrote:
 On Sun, Oct 05, 2014 at 03:44:16PM -0700, Subhash Jadavani wrote:
  Looks good, Reviewed-by: Subhash Jadavani subha...@codeaurora.org
 
 I'm getting a bit lost with all the UFS fixes, can you prepared a 
 series with all the fixups required for 3.18 for me please?

Are you going to prepare a set of fixups for 3.18, or should I try to cram
them together?  I supect you'd do a much better job than I could.
--
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

--
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] vhost-scsi: Take configfs group dependency during VHOST_SCSI_SET_ENDPOINT

2014-10-21 Thread Nicholas A. Bellinger 
Hey Paolo,

On Thu, 2014-10-09 at 12:49 +0200, Paolo Bonzini wrote:
 Il 09/10/2014 10:49, Paolo Bonzini ha scritto:
  
  It does not happen if you close QEMU with SIGTERM, ctrl-c, or with the
  quit command, because no attempt is done to bring down the VM data
  structures (or free memory, or close file descriptors) in case of a
  fatal exit.  The kernel should do that for us.
 
 ... and in the case of vhost-scsi, doesn't it do that when
 vhost_scsi_release calls vhost_scsi_clear_endpoint?
 

Thanks for the extra clarifications here.

The SIGTERM, ctrl-c and quit command cases happen as you describe, and
invoke vhost_scsi_release() - vhost_scsi_clear_endpoint() to drop the
endpoint reference.

AFAICT, it's the SIGKILL case that is problematic both with and without
this patch.  With the patch, the configfs dependency on the vhost-scsi
endpoint group is left in place, thus preventing the group (and
underlying target_core_mod) from being removed until a
VHOST_SCSI_CLEAR_ENDPOINT with the same wwpn is called to drop the
original reference.

Without the patch, the group can still be removed at any time, but any
subsequent VHOST_SCSI_SET_ENDPOINT attempts with the original wwpn will
fail after SIGKILL, because the original reference is still in place.

So I held off on pushing this patch to -rc1 for the moment, but even
with the above limitation preventing group shutdown after SIGKILL, I
think it's still better to obtain the configfs dependency to prevent the
removal of endpoints while there are active references.

That said, I'm still unsure how to address the SIGKILL case, and what's
the most sane way to drop dead references after it happens, and how
vhost-scsi should be differentiating between dead and active references.

Any ideas..?

--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 scsi] libcxgbi : support ipv6 address host_param

2014-10-21 Thread Mike Christie 
On 10/16/2014 05:59 PM, Anish Bhatt wrote:
 libcxgbi was always returning an ipv4 address for ISCSI_HOST_PARAM_IPADDRESS,
 return appropriate address based on address family
 
 Signed-off-by: Anish Bhatt an...@chelsio.com
 Signed-off-by: Karen Xie k...@chelsio.com
 ---
  drivers/scsi/cxgbi/libcxgbi.c | 42 +-
  drivers/scsi/cxgbi/libcxgbi.h |  5 -
  2 files changed, 37 insertions(+), 10 deletions(-)
 
 diff --git a/drivers/scsi/cxgbi/libcxgbi.c b/drivers/scsi/cxgbi/libcxgbi.c
 index 6a2001d..403330a 100644
 --- a/drivers/scsi/cxgbi/libcxgbi.c
 +++ b/drivers/scsi/cxgbi/libcxgbi.c
 @@ -397,6 +397,35 @@ EXPORT_SYMBOL_GPL(cxgbi_hbas_add);
   *   If the source port is outside our allocation range, the caller is
   *   responsible for keeping track of their port usage.
   */
 +
 +static struct cxgbi_sock *find_sock_on_port(struct cxgbi_device *cdev,
 + unsigned char port_id)
 +{
 + struct cxgbi_ports_map *pmap = cdev-pmap;
 + unsigned int i;
 + unsigned int used;
 +
 + if (!pmap-max_connect || !pmap-used)
 + return NULL;
 +
 + spin_lock_bh(pmap-lock);
 + used = pmap-used;
 + for (i = 0; used  i  pmap-max_connect; i++) {
 + struct cxgbi_sock *csk = pmap-port_csk[i];
 +
 + if (csk) {
 + if (csk-port_id == port_id) {
 + spin_unlock_bh(pmap-lock);
 + return csk;
 + }
 + used--;
 + }
 + }
 + spin_unlock_bh(pmap-lock);
 +
 + return NULL;
 +}
 +
  static int sock_get_port(struct cxgbi_sock *csk)
  {
   struct cxgbi_device *cdev = csk-cdev;
 @@ -747,6 +776,7 @@ static struct cxgbi_sock *cxgbi_check_route6(struct 
 sockaddr *dst_addr)
   csk-daddr6.sin6_addr = daddr6-sin6_addr;
   csk-daddr6.sin6_port = daddr6-sin6_port;
   csk-daddr6.sin6_family = daddr6-sin6_family;
 + csk-saddr6.sin6_family = daddr6-sin6_family;
   csk-saddr6.sin6_addr = pref_saddr;
  
   neigh_release(n);
 @@ -2645,12 +2675,14 @@ int cxgbi_get_host_param(struct Scsi_Host *shost, 
 enum iscsi_host_param param,
   break;
   case ISCSI_HOST_PARAM_IPADDRESS:
   {
 - __be32 addr;
 -
 - addr = cxgbi_get_iscsi_ipv4(chba);
 - len = sprintf(buf, %pI4, addr);
 + struct cxgbi_sock *csk = find_sock_on_port(chba-cdev,
 +chba-port_id);
 + if (csk) {
 + len = sprintf(buf, %pIS,
 +   (struct sockaddr *)csk-saddr);
 + }
   log_debug(1  CXGBI_DBG_ISCSI,
 - hba %s, ipv4 %pI4.\n, chba-ndev-name, addr);
 +   hba %s, addr %s.\n, chba-ndev-name, buf);
   break;
   }
   default:
 diff --git a/drivers/scsi/cxgbi/libcxgbi.h b/drivers/scsi/cxgbi/libcxgbi.h
 index 1d98fad..2c7cb1c 100644
 --- a/drivers/scsi/cxgbi/libcxgbi.h
 +++ b/drivers/scsi/cxgbi/libcxgbi.h
 @@ -700,11 +700,6 @@ static inline void cxgbi_set_iscsi_ipv4(struct cxgbi_hba 
 *chba, __be32 ipaddr)
   chba-ndev-name);
  }
  
 -static inline __be32 cxgbi_get_iscsi_ipv4(struct cxgbi_hba *chba)
 -{
 - return chba-ipv4addr;
 -}
 -
  struct cxgbi_device *cxgbi_device_register(unsigned int, unsigned int);
  void cxgbi_device_unregister(struct cxgbi_device *);
  void cxgbi_device_unregister_all(unsigned int flag);
 


Looks ok.

Reviewed-by: Mike Christie micha...@cs.wisc.edu
--
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] block: remove artifical max_hw_sectors cap

2014-10-21 Thread Jens Axboe 
On 10/17/2014 07:12 AM, Christoph Hellwig wrote:
 On Wed, Oct 01, 2014 at 02:12:58PM -0700, Christoph Hellwig wrote:
 On Wed, Oct 01, 2014 at 06:59:34PM +, Elliott, Robert (Server Storage) 
 wrote:
 One supporting example: A low limit interferes with creation of
 full stripe writes for RAID controllers.

 Yes, both Linux mdraid and parity raids are often crippled by the
 default.  The mdadm default of 512kb for each chunk combined with
 the default limit very much guarantees a horrible out of the box
 experience.
 
 Jens, can you apply this patch once the 3.19 queue opens?

Yup, lets get it applied.

-- 
Jens Axboe

--
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: virtio_blk - kernel BUG at drivers/virtio/virtio_ring.c:160!

2014-10-21 Thread Jens Axboe 
On 10/17/2014 07:27 AM, Christoph Hellwig wrote:
 On Thu, Oct 16, 2014 at 11:17:39AM -0400, Brian Foster wrote:
 Hi all,

 Hopefully this is the right list for this report...

 I hit the following kernel bug reliably by running xfstests test
 generic/234 against XFS using 10GB LVM test/scratch volumes on top of a
 ~100GB virtio_blk block device. The virt block device is file-backed on
 the host.
 
 Jens, I thought the segment merging bug was fixed a while ago.  Did we
 manage to not include parts of it for 3.17?

Mings patch went in after 3.17, iirc. Ming?

 [ cut here ]
 kernel BUG at drivers/virtio/virtio_ring.c:160!
 invalid opcode:  [#1] SMP
 Modules linked in: xfs libcrc32c cfg80211 rfkill snd_hda_codec_generic ppdev 
 snd_hda_intel snd_hda_controller snd_hda_codec snd_hwdep snd_seq 
 snd_seq_device snd_pcm snd_timer snd serio_raw virtio_balloon soundcore 
 virtio_console parport_pc parport i2c_piix4 sunrpc virtio_blk virtio_net qxl 
 drm_kms_helper ttm ata_generic virtio_pci virtio_ring drm virtio pata_acpi
 CPU: 0 PID: 1442 Comm: xfsaild/dm-3 Not tainted 3.17.0+ #97
 Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
 task: 8800c6483460 ti: 880118034000 task.ti: 880118034000
 RIP: 0010:[a00729e5]  [a00729e5] 
 virtqueue_add_sgs+0x415/0x430 [virtio_ring]
 RSP: 0018:880118037678  EFLAGS: 00010002
 RAX: 88011808b000 RBX: 8801180377e8 RCX: 0003
 RDX: ea0003667102 RSI: 8801180377d0 RDI: 880118037730
 RBP: 8801180376e8 R08: 8800d78caf70 R09: 0020
 R10: 8800c6483460 R11: 8800c6484050 R12: 8801180377e8
 R13: 0002 R14: 0081 R15: 0020
 FS:  () GS:88011ae0() knlGS:
 CS:  0010 DS:  ES:  CR0: 8005003b
 CR2: 7fffa85ff0c0 CR3: 01e14000 CR4: 06f0
 Stack:
  880118037698 0304 8800d78caf70 8801180377d0
  0003 88011808b000 880118037730 8802
  8801180376e8 8800d78caf70 880118037730 0002
 Call Trace:
  [a006a42f] __virtblk_add_req+0xdf/0x1c0 [virtio_blk]
  [a006a5f2] ? virtio_queue_rq+0xe2/0x280 [virtio_blk]
  [a006a616] virtio_queue_rq+0x106/0x280 [virtio_blk]
  [813d71f1] __blk_mq_run_hw_queue+0x1d1/0x350
  [813d7bb0] blk_mq_run_hw_queue+0x70/0xa0
  [813d8a6d] blk_mq_insert_requests+0xfd/0x2d0
  [813d9a2b] blk_mq_flush_plug_list+0x13b/0x160
  [813cd5c1] blk_flush_plug_list+0xc1/0x240
  [813d8f2a] blk_sq_make_request+0x2ea/0x5d0
  [81656395] ? dm_get_live_table+0x5/0xb0
  [813c7760] generic_make_request+0xe0/0x130
  [813c7828] submit_bio+0x78/0x160
  [a02ffcc6] _xfs_buf_ioapply+0x2e6/0x420 [xfs]
  [a0300328] ? __xfs_buf_delwri_submit+0x1d8/0x5b0 [xfs]
  [a02fff22] xfs_buf_submit+0xd2/0x300 [xfs]
  [a0300328] __xfs_buf_delwri_submit+0x1d8/0x5b0 [xfs]
  [a03021af] ? xfs_buf_delwri_submit_nowait+0x2f/0x50 [xfs]
  [a03021af] xfs_buf_delwri_submit_nowait+0x2f/0x50 [xfs]
  [a0340635] xfsaild+0x275/0xe30 [xfs]
  [a03403c0] ? xfs_trans_ail_cursor_first+0xb0/0xb0 [xfs]
  [810cda79] kthread+0xf9/0x110
  [810cd980] ? kthread_create_on_node+0x250/0x250
  [8182717c] ret_from_fork+0x7c/0xb0
  [810cd980] ? kthread_create_on_node+0x250/0x250
 Code: ff 0f 1f 44 00 00 eb 84 48 8b 4d b8 8b 55 b0 48 c7 c6 19 42 07 a0 48 
 c7 c7 78 50 07 a0 31 c0 31 db e8 10 5d 3b e1 e9 4d fd ff ff 0f 0b bb fb ff 
 ff ff e9 41 fd ff ff 66 66 66 66 66 66 2e 0f 1f
 RIP  [a00729e5] virtqueue_add_sgs+0x415/0x430 [virtio_ring]
  RSP 880118037678
 ---[ end trace 823f74f9a11abe26 ]---

 This occurs on the latest tot kernel (commit 0429fbc0bdc2) but appears
 to originate sometime during the 3.16 development cycle. A bisect lands
 on the following commit:

 05f1dd53 block: add queue flag for disabling SG merging

 To corroborate that, the appended diff appears to work around the
 problem for me (included as a data point, not a fix, as I'm not familiar
 with the block layer). Let me know if I can provide any more info,
 thanks!

 Brian

 ---8---

 diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
 index 0a58140..5861bd72 100644
 --- a/drivers/block/virtio_blk.c
 +++ b/drivers/block/virtio_blk.c
 @@ -634,7 +634,7 @@ static int virtblk_probe(struct virtio_device *vdev)
  vblk-tag_set.ops = virtio_mq_ops;
  vblk-tag_set.queue_depth = virtblk_queue_depth;
  vblk-tag_set.numa_node = NUMA_NO_NODE;
 -vblk-tag_set.flags = BLK_MQ_F_SHOULD_MERGE;
 +vblk-tag_set.flags = BLK_MQ_F_SHOULD_MERGE|BLK_MQ_F_SG_MERGE;
  vblk-tag_set.cmd_size =
  sizeof(struct virtblk_req) +
  sizeof(struct scatterlist) * sg_elems;
 --
 To unsubscribe from this list: send the line unsubscribe linux-scsi in
 the body

Re: [GIT PULL] osd: Boaz Harrosh - change of email

2014-10-21 Thread James Bottomley 
On Tue, 2014-10-21 at 17:05 +0300, Boaz Harrosh wrote:
 Hi Sir Linus
 
 A small administrative stuff, was on vacation and the old email bounced on me.
 I was hoping to still make the 3 weeks merge window, but it was 2 weeks at the
 end.
 Your call if to make this wait for next window. Needless to say that it is
 ZERO risk, just change of email.
 
 Based on commit [bfe01a5b] Linux 3.17
 
 3 patches available in the git repository at:
 
   git://git.open-osd.org/linux-open-osd.git for-linus
 
 for you to fetch changes up to 1fa3a002b2546c42c343c77c144871285896ced5:
 
   Boaz Harrosh - fix email in Documentation (2014-10-19 20:36:36 +0300)
 
 
 Boaz Harrosh (3):
   MAINTAINERS: Change Boaz Harrosh's email
   Boaz Harrosh - Fix broken email address
   Boaz Harrosh - fix email in Documentation
 
  Documentation/scsi/osd.txt  | 3 +--
  MAINTAINERS | 2 +-
  drivers/scsi/osd/Kbuild | 2 +-
  drivers/scsi/osd/Kconfig| 2 +-
  drivers/scsi/osd/osd_debug.h| 2 +-
  drivers/scsi/osd/osd_initiator.c| 4 ++--
  drivers/scsi/osd/osd_uld.c  | 4 ++--
  fs/exofs/Kbuild | 2 +-
  fs/exofs/common.h   | 2 +-
  fs/exofs/dir.c  | 2 +-
  fs/exofs/exofs.h| 2 +-
  fs/exofs/file.c | 2 +-
  fs/exofs/inode.c| 2 +-
  fs/exofs/namei.c| 2 +-
  fs/exofs/ore.c  | 4 ++--
  fs/exofs/ore_raid.c | 2 +-
  fs/exofs/ore_raid.h | 2 +-
  fs/exofs/super.c| 2 +-
  fs/exofs/symlink.c  | 2 +-
  fs/exofs/sys.c  | 2 +-
  fs/nfs/objlayout/objio_osd.c| 2 +-
  fs/nfs/objlayout/objlayout.c| 2 +-
  fs/nfs/objlayout/objlayout.h| 2 +-
  fs/nfs/objlayout/pnfs_osd_xdr_cli.c | 2 +-
  include/linux/pnfs_osd_xdr.h| 2 +-
  include/scsi/osd_initiator.h| 2 +-
  include/scsi/osd_ore.h  | 2 +-
  include/scsi/osd_protocol.h | 4 ++--
  include/scsi/osd_sec.h  | 2 +-
  include/scsi/osd_sense.h| 2 +-
  include/scsi/osd_types.h| 2 +-
  31 files changed, 35 insertions(+), 36 deletions(-)

This is a bit of an unnecessary massive churn.  No one expects the
author named in the file to stay up to date, especially because the
@domain.com usually credits the company who paid for the code, so it's
left in as a kind of mark of respect for them.  I'm not saying it
applies in your case, just that it creates the common expectation of
in-file authors needing to be traced through the MAINTAINERS file.

Could you not just update the MAINTAINERS file only, like everyone else?

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] vhost-scsi: Take configfs group dependency during VHOST_SCSI_SET_ENDPOINT

2014-10-21 Thread Michael S. Tsirkin 
On Tue, Oct 21, 2014 at 12:05:31PM -0700, Nicholas A. Bellinger wrote:
 Hey Paolo,
 
 On Thu, 2014-10-09 at 12:49 +0200, Paolo Bonzini wrote:
  Il 09/10/2014 10:49, Paolo Bonzini ha scritto:
   
   It does not happen if you close QEMU with SIGTERM, ctrl-c, or with the
   quit command, because no attempt is done to bring down the VM data
   structures (or free memory, or close file descriptors) in case of a
   fatal exit.  The kernel should do that for us.
  
  ... and in the case of vhost-scsi, doesn't it do that when
  vhost_scsi_release calls vhost_scsi_clear_endpoint?
  
 
 Thanks for the extra clarifications here.
 
 The SIGTERM, ctrl-c and quit command cases happen as you describe, and
 invoke vhost_scsi_release() - vhost_scsi_clear_endpoint() to drop the
 endpoint reference.
 
 AFAICT, it's the SIGKILL case that is problematic both with and without
 this patch.  With the patch, the configfs dependency on the vhost-scsi
 endpoint group is left in place, thus preventing the group (and
 underlying target_core_mod) from being removed until a
 VHOST_SCSI_CLEAR_ENDPOINT with the same wwpn is called to drop the
 original reference.
 
 Without the patch, the group can still be removed at any time, but any
 subsequent VHOST_SCSI_SET_ENDPOINT attempts with the original wwpn will
 fail after SIGKILL, because the original reference is still in place.
 
 So I held off on pushing this patch to -rc1 for the moment, but even
 with the above limitation preventing group shutdown after SIGKILL, I
 think it's still better to obtain the configfs dependency to prevent the
 removal of endpoints while there are active references.
 
 That said, I'm still unsure how to address the SIGKILL case, and what's
 the most sane way to drop dead references after it happens, and how
 vhost-scsi should be differentiating between dead and active references.
 
 Any ideas..?
 
 --nab

Need to use some other file (not sysfs), cleanup on release.

-- 
MST
--
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] ips: Remove unnecessary METHOD_TRACE

2014-10-21 Thread Joe Perches 
METHOD_TRACE is a poorman's function tracer.
Use the actual function tracer instead (ftrace)

Signed-off-by: Joe Perches j...@perches.com
---
 drivers/scsi/ips.c | 177 -
 1 file changed, 177 deletions(-)

diff --git a/drivers/scsi/ips.c b/drivers/scsi/ips.c
index e5afc38..a483244 100644
--- a/drivers/scsi/ips.c
+++ b/drivers/scsi/ips.c
@@ -153,8 +153,6 @@
  *NOTE: only works when IPS_DEBUG compile directive is 
used.
  *   1  - Normal debug messages
  *   2  - Verbose debug messages
- *   11 - Method trace (non interrupt)
- *   12 - Method trace (includes interrupt)
  *
  * noi2o- Don't use I2O Queues (ServeRAID 4 only)
  * nommap   - Don't use memory mapped I/O
@@ -216,11 +214,9 @@ module_param(ips, charp, 0);
  scb-scsi_cmd-sc_data_direction)
 
 #ifdef IPS_DEBUG
-#define METHOD_TRACE(s, i)if (ips_debug = (i+10)) printk(KERN_NOTICE s 
\n);
 #define DEBUG(i, s)   if (ips_debug = i) printk(KERN_NOTICE s \n);
 #define DEBUG_VAR(i, s, v...) if (ips_debug = i) printk(KERN_NOTICE s \n, 
v);
 #else
-#define METHOD_TRACE(s, i)
 #define DEBUG(i, s)
 #define DEBUG_VAR(i, s, v...)
 #endif
@@ -563,8 +559,6 @@ ips_detect(struct scsi_host_template * SHT)
 {
int i;
 
-   METHOD_TRACE(ips_detect, 1);
-
 #ifdef MODULE
if (ips)
ips_setup(ips);
@@ -654,8 +648,6 @@ ips_release(struct Scsi_Host *sh)
ips_ha_t *ha;
int i;
 
-   METHOD_TRACE(ips_release, 1);
-
scsi_remove_host(sh);
 
for (i = 0; i  IPS_MAX_ADAPTERS  ips_sh[i] != sh; i++) ;
@@ -788,8 +780,6 @@ int ips_eh_abort(struct scsi_cmnd *SC)
int ret;
struct Scsi_Host *host;
 
-   METHOD_TRACE(ips_eh_abort, 1);
-
if (!SC)
return (FAILED);
 
@@ -846,8 +836,6 @@ static int __ips_eh_reset(struct scsi_cmnd *SC)
ips_scb_t *scb;
ips_copp_wait_item_t *item;
 
-   METHOD_TRACE(ips_eh_reset, 1);
-
 #ifdef NO_IPS_RESET
return (FAILED);
 #else
@@ -1052,8 +1040,6 @@ static int ips_queue_lck(struct scsi_cmnd *SC, void 
(*done) (struct scsi_cmnd *)
ips_ha_t *ha;
ips_passthru_t *pt;
 
-   METHOD_TRACE(ips_queue, 1);
-
ha = (ips_ha_t *) SC-device-host-hostdata;
 
if (!ha)
@@ -1157,8 +1143,6 @@ static int ips_biosparam(struct scsi_device *sdev, struct 
block_device *bdev,
int sectors;
int cylinders;
 
-   METHOD_TRACE(ips_biosparam, 1);
-
if (!ha)
/* ?!?! host adater info invalid */
return (0);
@@ -1234,8 +1218,6 @@ do_ipsintr(int irq, void *dev_id)
struct Scsi_Host *host;
int irqstatus;
 
-   METHOD_TRACE(do_ipsintr, 2);
-
ha = (ips_ha_t *) dev_id;
if (!ha)
return IRQ_NONE;
@@ -1281,8 +1263,6 @@ ips_intr_copperhead(ips_ha_t * ha)
IPS_STATUS cstatus;
int intrstatus;
 
-   METHOD_TRACE(ips_intr, 2);
-
if (!ha)
return 0;
 
@@ -1345,8 +1325,6 @@ ips_intr_morpheus(ips_ha_t * ha)
IPS_STATUS cstatus;
int intrstatus;
 
-   METHOD_TRACE(ips_intr_morpheus, 2);
-
if (!ha)
return 0;
 
@@ -1412,8 +1390,6 @@ ips_info(struct Scsi_Host *SH)
char *bp;
ips_ha_t *ha;
 
-   METHOD_TRACE(ips_info, 1);
-
ha = IPS_HA(SH);
 
if (!ha)
@@ -1495,8 +1471,6 @@ static int ips_is_passthru(struct scsi_cmnd *SC)
 {
unsigned long flags;
 
-   METHOD_TRACE(ips_is_passthru, 1);
-
if (!SC)
return (0);
 
@@ -1572,8 +1546,6 @@ ips_make_passthru(ips_ha_t *ha, struct scsi_cmnd *SC, 
ips_scb_t *scb, int intr)
int i, ret;
 struct scatterlist *sg = scsi_sglist(SC);
 
-   METHOD_TRACE(ips_make_passthru, 1);
-
 scsi_for_each_sg(SC, sg, scsi_sg_count(SC), i)
length += sg-length;
 
@@ -1911,8 +1883,6 @@ ips_usrcmd(ips_ha_t * ha, ips_passthru_t * pt, ips_scb_t 
* scb)
IPS_SG_LIST sg_list;
uint32_t cmd_busaddr;
 
-   METHOD_TRACE(ips_usrcmd, 1);
-
if ((!scb) || (!pt) || (!ha))
return (0);
 
@@ -1998,8 +1968,6 @@ ips_cleanup_passthru(ips_ha_t * ha, ips_scb_t * scb)
 {
ips_passthru_t *pt;
 
-   METHOD_TRACE(ips_cleanup_passthru, 1);
-
if ((!scb) || (!scb-scsi_cmd) || (!scsi_sglist(scb-scsi_cmd))) {
DEBUG_VAR(1, (%s%d) couldn't cleanup after passthru,
  ips_name, ha-host_num);
@@ -2036,8 +2004,6 @@ ips_cleanup_passthru(ips_ha_t * ha, ips_scb_t * scb)
 static int
 ips_host_info(ips_ha_t *ha, struct seq_file *m)
 {
-   METHOD_TRACE(ips_host_info, 1);
-
seq_printf(m, \nIBM ServeRAID General Information:\n\n);
 
if ((le32_to_cpu(ha-nvram-signature) == IPS_NVRAM_P5_SIG) 
@@ -2155,8 +2121,6 @@ ips_host_info(ips_ha_t *ha, struct seq_file

Re: virtio_blk - kernel BUG at drivers/virtio/virtio_ring.c:160!

2014-10-21 Thread Ming Lei 
On Wed, Oct 22, 2014 at 4:05 AM, Jens Axboe ax...@kernel.dk wrote:
 On 10/17/2014 07:27 AM, Christoph Hellwig wrote:
 On Thu, Oct 16, 2014 at 11:17:39AM -0400, Brian Foster wrote:
 Hi all,

 Hopefully this is the right list for this report...

 I hit the following kernel bug reliably by running xfstests test
 generic/234 against XFS using 10GB LVM test/scratch volumes on top of a
 ~100GB virtio_blk block device. The virt block device is file-backed on
 the host.

 Jens, I thought the segment merging bug was fixed a while ago.  Did we
 manage to not include parts of it for 3.17?

 Mings patch went in after 3.17, iirc. Ming?

Sorry, that patch is wrong[1], attachment patch should fix the issue.

[1] http://marc.info/?l=linux-kernelm=141290430004361w=2


Thanks,
-- 
Ming Lei
From dde19549352da3fea516d89bbfa25d08b784e229 Mon Sep 17 00:00:00 2001
From: Ming Lei tom.leim...@gmail.com
Date: Wed, 22 Oct 2014 08:30:30 +0800
Subject: [PATCH] blk-merge: recaculate segment if it isn't less than max
 segments

The problem is introduced by commit 764f612c6c3c231b(blk-merge:
don't compute bi_phys_segments from bi_vcnt for cloned bio),
and merge is needed if number of current segment isn't less than
max segments.

Strictly speaking, bio-bi_vcnt shouldn't be used here since
it may not be accurate in cases of both cloned bio or bio cloned
from, but bio_segments() is a bit expensive, and bi_vcnt is still
the biggest number, so the approach should work.

Signed-off-by: Ming Lei tom.leim...@gmail.com
---
 block/blk-merge.c |5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/block/blk-merge.c b/block/blk-merge.c
index ba99351..b3ac40a 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -99,16 +99,17 @@ void blk_recount_segments(struct request_queue *q, struct bio *bio)
 {
 	bool no_sg_merge = !!test_bit(QUEUE_FLAG_NO_SG_MERGE,
 			q-queue_flags);
+	bool merge_not_need = bio-bi_vcnt  queue_max_segments(q);
 
 	if (no_sg_merge  !bio_flagged(bio, BIO_CLONED) 
-			bio-bi_vcnt  queue_max_segments(q))
+			merge_not_need)
 		bio-bi_phys_segments = bio-bi_vcnt;
 	else {
 		struct bio *nxt = bio-bi_next;
 
 		bio-bi_next = NULL;
 		bio-bi_phys_segments = __blk_recalc_rq_segments(q, bio,
-no_sg_merge);
+no_sg_merge  merge_not_need);
 		bio-bi_next = nxt;
 	}
 
-- 
1.7.9.5

Re: [GIT PULL] osd: Boaz Harrosh - change of email

2014-10-21 Thread J. Bruce Fields 
On Tue, Oct 21, 2014 at 11:04:12PM +0200, James Bottomley wrote:
 On Tue, 2014-10-21 at 17:05 +0300, Boaz Harrosh wrote:
  Hi Sir Linus
  
  A small administrative stuff, was on vacation and the old email bounced on 
  me.
  I was hoping to still make the 3 weeks merge window, but it was 2 weeks at 
  the
  end.
  Your call if to make this wait for next window. Needless to say that it is
  ZERO risk, just change of email.
  
  Based on commit [bfe01a5b] Linux 3.17
  
  3 patches available in the git repository at:
  
git://git.open-osd.org/linux-open-osd.git for-linus
  
  for you to fetch changes up to 1fa3a002b2546c42c343c77c144871285896ced5:
  
Boaz Harrosh - fix email in Documentation (2014-10-19 20:36:36 +0300)
  
  
  Boaz Harrosh (3):
MAINTAINERS: Change Boaz Harrosh's email
Boaz Harrosh - Fix broken email address
Boaz Harrosh - fix email in Documentation
  
   Documentation/scsi/osd.txt  | 3 +--
   MAINTAINERS | 2 +-
   drivers/scsi/osd/Kbuild | 2 +-
   drivers/scsi/osd/Kconfig| 2 +-
   drivers/scsi/osd/osd_debug.h| 2 +-
   drivers/scsi/osd/osd_initiator.c| 4 ++--
   drivers/scsi/osd/osd_uld.c  | 4 ++--
   fs/exofs/Kbuild | 2 +-
   fs/exofs/common.h   | 2 +-
   fs/exofs/dir.c  | 2 +-
   fs/exofs/exofs.h| 2 +-
   fs/exofs/file.c | 2 +-
   fs/exofs/inode.c| 2 +-
   fs/exofs/namei.c| 2 +-
   fs/exofs/ore.c  | 4 ++--
   fs/exofs/ore_raid.c | 2 +-
   fs/exofs/ore_raid.h | 2 +-
   fs/exofs/super.c| 2 +-
   fs/exofs/symlink.c  | 2 +-
   fs/exofs/sys.c  | 2 +-
   fs/nfs/objlayout/objio_osd.c| 2 +-
   fs/nfs/objlayout/objlayout.c| 2 +-
   fs/nfs/objlayout/objlayout.h| 2 +-
   fs/nfs/objlayout/pnfs_osd_xdr_cli.c | 2 +-
   include/linux/pnfs_osd_xdr.h| 2 +-
   include/scsi/osd_initiator.h| 2 +-
   include/scsi/osd_ore.h  | 2 +-
   include/scsi/osd_protocol.h | 4 ++--
   include/scsi/osd_sec.h  | 2 +-
   include/scsi/osd_sense.h| 2 +-
   include/scsi/osd_types.h| 2 +-
   31 files changed, 35 insertions(+), 36 deletions(-)
 
 This is a bit of an unnecessary massive churn.  No one expects the
 author named in the file to stay up to date, especially because the
 @domain.com usually credits the company who paid for the code, so it's
 left in as a kind of mark of respect for them.  I'm not saying it
 applies in your case, just that it creates the common expectation of
 in-file authors needing to be traced through the MAINTAINERS file.
 
 Could you not just update the MAINTAINERS file only, like everyone else?

I guess there's also .mailmap, though it doesn't look like that's being
used much.

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