Re: [PATCH-v2 0/6] vhost/scsi: Add T10 PI SGL passthrough support

2014-06-10 Thread Paolo Bonzini
Il 10/06/2014 09:07, Nicholas A. Bellinger ha scritto: OK, finally went over this, looks good to me: Acked-by: Michael S. Tsirkin m...@redhat.com However, we really should stop making more changes before fixing ANY_LAYOUT in this driver. virtio 1.0 should be out soon and that makes

Re: [PATCH v1 3/3] TARGET/sbc,loopback: Adjust command data length in case pi exists on the wire

2014-06-10 Thread Paolo Bonzini
Il 10/06/2014 10:04, Nicholas A. Bellinger ha scritto: That said, there is one other small qla2xxx change to enable per-session PI that is currently missing in Quinn's patch in scsi/for-next code. Sooo, I'll go ahead and include Sagi's patches with the vhost-scsi change below if there are no

Re: [PATCH-v2 0/6] vhost/scsi: Add T10 PI SGL passthrough support

2014-06-09 Thread Paolo Bonzini
Il 08/06/2014 18:05, Michael S. Tsirkin ha scritto: OK, finally went over this, looks good to me: Acked-by: Michael S. Tsirkin m...@redhat.com However, we really should stop making more changes before fixing ANY_LAYOUT in this driver. virtio 1.0 should be out soon and that makes ANY_LAYOUT a

Re: [PATCH] scsi: ibmvscsi: protect abort handler from done-scmd in flight

2014-06-05 Thread Paolo Bonzini
Il 05/06/2014 08:16, Liu Ping Fan ha scritto: Take the following scene in guest: seqA: scsi_done() - gapX (before taking REQ_ATOM_COMPLETE) seqB: scmd_eh_abort_handler()- ...- ibmvscsi_eh_abort_handler()- ...-scsi_put_command(scmd) If seqA is scheduled at gapX, and seqB reclaims scmd.

[PATCH v2 0/6] virtio-scsi patches for 3.16 + midlayer fix

2014-06-04 Thread Paolo Bonzini
): virtio_scsi: remove ACCESS_ONCE() and smp_read_barrier_depends() virtio-scsi: replace target spinlock with seqcount Paolo Bonzini (2): virtio-scsi: avoid cancelling uninitialized work items virtio-scsi: fix various bad behavior on aborted requests Ulrich Obergfell (1): scsi_error: fix

[PATCH v2 4/6] scsi_error: fix invalid setting of host byte

2014-06-04 Thread Paolo Bonzini
comments. - Paolo] Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- v1-v2: fix all occurrences [Bart] drivers/scsi/scsi_error.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c index f17aa7aa7879

[PATCH v2 6/6] virtio-scsi: Implement change_queue_depth for virtscsi targets

2014-06-04 Thread Paolo Bonzini
up to the device maximum when the BUSY condition has resolved. Signed-off-by: Venkatesh Srinivas venkate...@google.com Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- drivers/scsi/virtio_scsi.c | 33 + 1 file changed, 33 insertions(+) diff --git a/drivers/scsi

[PATCH v2 5/6] virtio-scsi: fix various bad behavior on aborted requests

2014-06-04 Thread Paolo Bonzini
or oopses. Cc: sta...@vger.kernel.org Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- drivers/scsi/virtio_scsi.c | 22 ++ 1 file changed, 22 insertions(+) diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c index d66c4ee2c774..fda9fb35 100644

[PATCH v2 3/6] virtio-scsi: avoid cancelling uninitialized work items

2014-06-04 Thread Paolo Bonzini
Calling the workqueue interface on uninitialized work items isn't a good idea even if they're zeroed. It's not failing catastrophically only through happy accidents. Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- drivers/scsi/virtio_scsi.c | 4 +++- 1 file changed, 3 insertions(+), 1

[PATCH v2 1/6] virtio_scsi: remove ACCESS_ONCE() and smp_read_barrier_depends()

2014-06-04 Thread Paolo Bonzini
req_vq is read from scsi-req_vqs[vq-index - VIRTIO_SCSI_VQ_BASE] instead of tgt-req_vq, so remove the unnecessary barrier. Also remove related comment about the barrier. Cc: Paolo Bonzini pbonz...@redhat.com Signed-off-by: Ming Lei tom.leim...@gmail.com Signed-off-by: Paolo Bonzini pbonz

[PATCH v2 2/6] virtio-scsi: replace target spinlock with seqcount

2014-06-04 Thread Paolo Bonzini
@canonical.com [Add initialization in virtscsi_target_alloc. - Paolo] Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- drivers/scsi/virtio_scsi.c | 42 +- 1 file changed, 29 insertions(+), 13 deletions(-) diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi

Re: [PATCH v2 0/6] virtio-scsi patches for 3.16 + midlayer fix

2014-06-04 Thread Paolo Bonzini
Il 04/06/2014 13:21, Bart Van Assche ha scritto: Thanks for the quick respin. However, since you are mentioning that in v2 all occurrences of scmd-result |= DID_TIME_OUT 16 have been addressed, this made me wonder whether you had noticed the following code in scsi_decide_disposition() ?

[PATCH v3 6/6] virtio-scsi: Implement change_queue_depth for virtscsi targets

2014-06-04 Thread Paolo Bonzini
up to the device maximum when the BUSY condition has resolved. Signed-off-by: Venkatesh Srinivas venkate...@google.com Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- drivers/scsi/virtio_scsi.c | 33 + 1 file changed, 33 insertions(+) diff --git a/drivers/scsi

[PATCH v3 5/6] virtio-scsi: fix various bad behavior on aborted requests

2014-06-04 Thread Paolo Bonzini
or oopses. Cc: sta...@vger.kernel.org Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- drivers/scsi/virtio_scsi.c | 22 ++ 1 file changed, 22 insertions(+) diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c index d66c4ee2c774..fda9fb35 100644

[PATCH v3 2/6] virtio-scsi: replace target spinlock with seqcount

2014-06-04 Thread Paolo Bonzini
@canonical.com [Add initialization in virtscsi_target_alloc. - Paolo] Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- drivers/scsi/virtio_scsi.c | 42 +- 1 file changed, 29 insertions(+), 13 deletions(-) diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi

[PATCH v3 4/6] scsi_error: fix invalid setting of host byte

2014-06-04 Thread Paolo Bonzini
comments. - Paolo] Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- v1-v2: fix all occurrences [Bart] except one v2-v3: really fix all occurrences [Bart] drivers/scsi/scsi_error.c | 6 +++--- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/scsi

[PATCH v3 3/6] virtio-scsi: avoid cancelling uninitialized work items

2014-06-04 Thread Paolo Bonzini
Calling the workqueue interface on uninitialized work items isn't a good idea even if they're zeroed. It's not failing catastrophically only through happy accidents. Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- drivers/scsi/virtio_scsi.c | 4 +++- 1 file changed, 3 insertions(+), 1

[PATCH v3 1/6] virtio_scsi: remove ACCESS_ONCE() and smp_read_barrier_depends()

2014-06-04 Thread Paolo Bonzini
req_vq is read from scsi-req_vqs[vq-index - VIRTIO_SCSI_VQ_BASE] instead of tgt-req_vq, so remove the unnecessary barrier. Also remove related comment about the barrier. Cc: Paolo Bonzini pbonz...@redhat.com Signed-off-by: Ming Lei tom.leim...@gmail.com Signed-off-by: Paolo Bonzini pbonz

[PATCH v3 0/6] virtio-scsi patches for 3.16 + midlayer fix

2014-06-04 Thread Paolo Bonzini
fix all occurrences in patch 4 Ming Lei (2): virtio_scsi: remove ACCESS_ONCE() and smp_read_barrier_depends() virtio-scsi: replace target spinlock with seqcount Paolo Bonzini (2): virtio-scsi: avoid cancelling uninitialized work items virtio-scsi: fix various bad behavior on aborted

Re: [PATCH v3 5/6] virtio-scsi: fix various bad behavior on aborted requests

2014-06-04 Thread Paolo Bonzini
Il 04/06/2014 19:29, Venkatesh Srinivas ha scritto: Do you really want to poll the request VQs for completions if the TMF was rejected? I wasn't sure, but bugs in this path are hard enough that I preferred the safer patch. TMF ABORT may return FUNCTION REJECTED if the command to abort

Re: [PATCH 5/6] [SCSI] Look up and store NAA if VPD page 0x83 is present

2014-06-03 Thread Paolo Bonzini
Il 03/06/2014 03:00, Martin K. Petersen ha scritto: Paolo == Paolo Bonzini pbonz...@redhat.com writes: + sdev_printk(KERN_ERR, sdev, + %s: VPD page 0x83 NAA descriptor not found\n, __func__); + + return; Paolo I suspect this error will be relatively common. You're right. But we would like

[PATCH 5/6] virtio-scsi: fix various bad behavior on aborted requests

2014-06-03 Thread Paolo Bonzini
or oopses. Cc: sta...@vger.kernel.org Cc: Ulrich Obergfell uober...@redhat.com Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- drivers/scsi/virtio_scsi.c | 22 ++ 1 file changed, 22 insertions(+) diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c index

[PATCH 6/6] virtio-scsi: Implement change_queue_depth for virtscsi targets

2014-06-03 Thread Paolo Bonzini
up to the device maximum when the BUSY condition has resolved. Signed-off-by: Venkatesh Srinivas venkate...@google.com Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- drivers/scsi/virtio_scsi.c | 33 + 1 file changed, 33 insertions(+) diff --git a/drivers/scsi

[PATCH 4/6] scsi_error: fix invalid setting of host byte

2014-06-03 Thread Paolo Bonzini
will corrupt the result field and initiate an unwanted command retry. Fix this by using set_host_byte instead, following the model of commit 2082ebc45af9c9c648383b8cde0dc1948eadbf31. Cc: sta...@vger.kernel.org Signed-off-by: Ulrich Obergfell uober...@redhat.com Signed-off-by: Paolo Bonzini pbonz

[PATCH 1/6] virtio_scsi: remove ACCESS_ONCE() and smp_read_barrier_depends()

2014-06-03 Thread Paolo Bonzini
req_vq is read from scsi-req_vqs[vq-index - VIRTIO_SCSI_VQ_BASE] instead of tgt-req_vq, so remove the unnecessary barrier. Also remove related comment about the barrier. Cc: Paolo Bonzini pbonz...@redhat.com Signed-off-by: Ming Lei tom.leim...@gmail.com Signed-off-by: Paolo Bonzini pbonz

[PATCH resend 0/6] virtio-scsi patches for 3.16 + a midlayer one-liner

2014-06-03 Thread Paolo Bonzini
Ming Lei (2): virtio_scsi: remove ACCESS_ONCE() and smp_read_barrier_depends() virtio-scsi: replace target spinlock with seqcount Paolo Bonzini (2): virtio-scsi: avoid cancelling uninitialized work items virtio-scsi: fix various bad behavior on aborted requests Ulrich Obergfell (1

Re: [PATCH 5/6] [SCSI] Look up and store NAA if VPD page 0x83 is present

2014-06-02 Thread Paolo Bonzini
Il 29/05/2014 05:52, Martin K. Petersen ha scritto: + sdev_printk(KERN_ERR, sdev, + %s: VPD page 0x83 NAA descriptor not found\n, __func__); + + return; I suspect this error will be relatively common. libata for example has if (ata_id_has_wwn(args-id)) {

Re: [RFC 0/9] fix for the race issue between scsi timer and in-flight scmd

2014-05-30 Thread Paolo Bonzini
Il 30/05/2014 10:15, Liu Ping Fan ha scritto: When running io stress test on large latency scsi-disk, e.g guest with virtscsi on a nfs image. It can trigger the BUG_ON(test_bit(REQ_ATOM_COMPLETE, req-atomic_flags)); in blk_start_request(). Since there is a race between latency finishing scmd

Re: [PATCH] virtio-scsi: replace target spinlock with seqcount

2014-05-28 Thread Paolo Bonzini
Il 28/05/2014 03:48, Ming Lei ha scritto: On Wed, May 28, 2014 at 12:57 AM, Paolo Bonzini pbonz...@redhat.com wrote: Il 27/05/2014 18:50, Venkatesh Srinivas ha scritto: Hi, I think this patch has a small race involving just two commands: 1. The first command to a target

Re: [target:for-next 20/20] drivers/scsi/virtio_scsi.c:531:48: error: dereferencing pointer to incomplete type

2014-05-28 Thread Paolo Bonzini
Il 28/05/2014 00:21, Nicholas A. Bellinger ha scritto: On Mon, 2014-05-26 at 13:30 -0400, Martin K. Petersen wrote: Nic == Nicholas A Bellinger n...@linux-iscsi.org writes: What about #ifdef'ing VIRTIO_SCSI_F_T10_PI support out if !CONFIG_BLK_DEV_INTEGRITY? Nic I figured it was slightly

Re: [PATCH 1/3] Remove two cancel_delayed_work() calls from the error handler

2014-05-27 Thread Paolo Bonzini
Il 27/05/2014 10:56, James Bottomley ha scritto: The whole reason BUG_ON doesn't leave a log trace is to try to prevent corruption propagating to the data storage devices. What you propose would be inviting that corruption in the name of getting a log entry. Even this is not entirely correct.

Re: [PATCH 2/3] block: Introduce blk_rq_completed()

2014-05-27 Thread Paolo Bonzini
Il 27/05/2014 12:21, James Bottomley ha scritto: I could also see us one day extending the TMF capability to abort any running command, which would make even an assertion of block timed out or completed invalid. Actually the assertion would remain valid, and that's exactly what Bart wants to

Re: [PATCH 2/3] block: Introduce blk_rq_completed()

2014-05-27 Thread Paolo Bonzini
Il 27/05/2014 12:59, James Bottomley ha scritto: On Tue, 2014-05-27 at 12:47 +0200, Paolo Bonzini wrote: Il 27/05/2014 12:21, James Bottomley ha scritto: I could also see us one day extending the TMF capability to abort any running command, which would make even an assertion of block timed out

Re: [PATCH 2/3] block: Introduce blk_rq_completed()

2014-05-27 Thread Paolo Bonzini
Il 27/05/2014 13:26, James Bottomley ha scritto: You could use a different mechanism than a softirq to tell the abort were successful, for example by overriding scsi_done. But with respect to the block layer, the mechanics of avoiding the race and double-free would probably be the same. I

Re: [PATCH] virtio-scsi: replace target spinlock with seqcount

2014-05-27 Thread Paolo Bonzini
Il 27/05/2014 18:50, Venkatesh Srinivas ha scritto: Hi, I think this patch has a small race involving just two commands: 1. The first command to a target is in virtscsi_pick_vq(), after atomic_inc_return(tgt-tgt_lock)) but before write_seqcount_begin() 2. A second command to the same target

Re: [PATCH 1/3] Remove two cancel_delayed_work() calls from the error handler

2014-05-26 Thread Paolo Bonzini
(). Signed-off-by: Bart Van Assche bvanass...@acm.org Cc: Hannes Reinecke h...@suse.de Cc: Paolo Bonzini pbonz...@redhat.com Cc: Christoph Hellwig h...@infradead.org Cc: Jens Axboe ax...@fb.com Cc: Joe Lawrence joe.lawre...@stratus.com --- drivers/scsi/scsi.c | 2 +- drivers/scsi

Re: [target:for-next 20/20] drivers/scsi/virtio_scsi.c:531:48: error: dereferencing pointer to incomplete type

2014-05-23 Thread Paolo Bonzini
Il 22/05/2014 22:46, Nicholas A. Bellinger ha scritto: Hi Fengguang, On Thu, 2014-05-22 at 11:13 +0800, kbuild test robot wrote: tree: git://git.kernel.org/pub/scm/linux/kernel/git/nab/target-pending.git for-next head: 4baaa7d589e24bfe87dfd6c7a1229108be404a28 commit:

Re: dangling pointers and/or reentrancy in scmd_eh_abort_handler?

2014-05-23 Thread Paolo Bonzini
Il 23/05/2014 03:28, Elliott, Robert (Server Storage) ha scritto: Depending on the transport, there may be a race condition between the command status and the ABORT TASK response; the ABORT TASK response might get back first. I think that means scsi_eh_abort_handler's call to

Re: [PATCH RFC] Remove the cancel_delayed_work() call from scsi_put_command()

2014-05-23 Thread Paolo Bonzini
Il 23/05/2014 08:09, Hannes Reinecke ha scritto: And when freeing a command we absolutely need to make sure that the workqueue is empty. So calling cancel_delayed_work() was the obvious thing to do. You would need cancel_delayed_work_sync, but if it really happened that the work item is

Re: [PATCH RFC] Remove the cancel_delayed_work() call from scsi_put_command()

2014-05-23 Thread Paolo Bonzini
Il 23/05/2014 12:37, Bart Van Assche ha scritto: On 05/23/14 11:24, Paolo Bonzini wrote: Il 23/05/2014 08:09, Hannes Reinecke ha scritto: And when freeing a command we absolutely need to make sure that the workqueue is empty. So calling cancel_delayed_work() was the obvious thing to do. You

Re: [PATCH-v2 6/6] virtio-scsi: Enable DIF/DIX modes in SCSI host LLD

2014-05-22 Thread Paolo Bonzini
in the pull request, Acked-by: Paolo Bonzini pbonz...@redhat.com for getting this in via the target tree. Paolo v4 changes: - Convert virtio_scsi_init_hdr_pi to use pi_bytesout + pi_bytesin (mst + paolo + nab) - Use blk_integrity-tuple_size to calculate pi bytes (nab) v3 changes

Re: [PATCH-v2 0/6] vhost/scsi: Add T10 PI SGL passthrough support

2014-05-22 Thread Paolo Bonzini
Il 22/05/2014 04:26, Nicholas A. Bellinger ha scritto: From: Nicholas Bellinger n...@linux-iscsi.org Hi MST, MKP, Paolo Co, Here is the v2 patch series for adding T1O protection information (PI) SGL passthrough support between virtio-scsi LLD + vhost-scsi fabric endpoints. Following MST's

Re: [PATCH RFC] Remove the cancel_delayed_work() call from scsi_put_command()

2014-05-22 Thread Paolo Bonzini
Il 21/05/2014 15:30, Bart Van Assche ha scritto: +static bool scmd_being_handled_in_other_context(struct scsi_cmnd *scmd) +{ + struct Scsi_Host *shost = scmd-device-host; + struct scsi_cmnd *c; + unsigned long flags; + bool ret = false; + + if

Re: dangling pointers and/or reentrancy in scmd_eh_abort_handler?

2014-05-21 Thread Paolo Bonzini
Il 21/05/2014 16:16, Mark Wu ha scritto: Is it possible that when scsi_done is invoked, the scsi command or the request has been freed and then reallocated for a new I/O request? Because of this the bit flag REQ_ATOM_COMPLETE becomes unreliable. Thanks! It is up to the driver to ensure that

Re: [PATCH 6/6] virtio-scsi: Enable DIF/DIX modes in SCSI host LLD

2014-05-20 Thread Paolo Bonzini
Il 19/05/2014 21:07, Nicholas A. Bellinger ha scritto: Hi MST, So I've finally got some cycles to get back to this code, and wanted to verify the outstanding items you had previously raised: - Convert vhost-scsi to be independent of IOV layout using memcpy_fromiovecend. (Does this effect

Re: [PATCH] virtio_scsi: don't call virtqueue_add_sgs(... GFP_NOIO) holding spinlock.

2014-05-20 Thread Paolo Bonzini
, - sizeof cmd-req.tmf, sizeof cmd-resp.tmf, - GFP_NOIO) 0) + sizeof cmd-req.tmf, sizeof cmd-resp.tmf) 0) goto out; wait_for_completion(comp); Acked-by: Paolo Bonzini pbonz...@redhat.com

Re: dangling pointers and/or reentrancy in scmd_eh_abort_handler?

2014-05-20 Thread Paolo Bonzini
Il 20/05/2014 10:10, Bart Van Assche ha scritto: REQ_ATOM_COMPLETE is already set before scsi_eh_scmd_add() is called since that function is only invoked after the block layer has marked a request as complete. The only callers of scsi_eh_scmd_add() are scsi_softirq_done(), scsi_times_out() and

dangling pointers and/or reentrancy in scmd_eh_abort_handler?

2014-05-19 Thread Paolo Bonzini
Hi all, I'm trying to understand asynchronous abort in the current upstream code, and the code seems to have some dubious locking. Here are some examples of the issue: 1) dangling pointers: scsi_put_command calls cancel_delayed_work(), but that doesn't mean that the scmd_eh_abort_handler

Re: dangling pointers and/or reentrancy in scmd_eh_abort_handler?

2014-05-19 Thread Paolo Bonzini
Il 19/05/2014 17:08, Bart Van Assche ha scritto: On 05/19/14 16:08, Paolo Bonzini wrote: 2) reentrancy: the softirq handler and scmd_eh_abort_handler can run concurrently, and call scsi_finish_command without any lock protecting the calls. You can then get memory corruption. I'm not sure

Re: [PATCH v1] virtio_scsi: remove ACCESS_ONCE() and smp_read_barrier_depends()

2014-05-08 Thread Paolo Bonzini
because req_vq is read from scsi-req_vqs[vq-index - VIRTIO_SCSI_VQ_BASE] instead of tgt-req_vq, so remove the unnecessary barrier. Also remove related comment about the barrier. Cc: Paolo Bonzini pbonz...@redhat.com Signed-off-by: Ming Lei tom.leim...@gmail.com --- v1: - fix comment

Re: virtio-scsi: two questions related with picking up queue

2014-05-08 Thread Paolo Bonzini
Il 08/05/2014 12:44, Ming Lei ha scritto: On Wed, 07 May 2014 18:43:45 +0200 Paolo Bonzini pbonz...@redhat.com wrote: Per-CPU spinlocks have bad scalability problems, especially if you're overcommitting. Writing req_vq is not at all rare. OK, thought about it further, and I believe

Re: virtio-scsi: two questions related with picking up queue

2014-05-08 Thread Paolo Bonzini
Il 08/05/2014 14:55, Ming Lei ha scritto: On Thu, May 8, 2014 at 8:17 PM, Paolo Bonzini pbonz...@redhat.com wrote: Il 08/05/2014 12:44, Ming Lei ha scritto: On Wed, 07 May 2014 18:43:45 +0200 Paolo Bonzini pbonz...@redhat.com wrote: Per-CPU spinlocks have bad scalability problems

Re: virtio-scsi: two questions related with picking up queue

2014-05-07 Thread Paolo Bonzini
Il 07/05/2014 03:07, Ming Lei ha scritto: Hi Paolo, On Tue, May 6, 2014 at 9:17 PM, Paolo Bonzini pbonz...@redhat.com wrote: Il 06/05/2014 11:26, Ming Lei ha scritto: Hi Paolo and All, One question is about ACCESS_ONCE() in virtscsi_pick_vq(), looks it needn't since both reading and writing

Re: [PATCH 2/2] virtio_scsi: remove smp_read_barrier_depends

2014-05-07 Thread Paolo Bonzini
: Paolo Bonzini pbonz...@redhat.com Signed-off-by: Ming Lei tom.leim...@gmail.com --- drivers/scsi/virtio_scsi.c | 52 +++- 1 file changed, 3 insertions(+), 49 deletions(-) diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c index aead20f

Re: [PATCH] virtio_scsi: remove ACCESS_ONCE() and smp_read_barrier_depends()

2014-05-07 Thread Paolo Bonzini
Il 07/05/2014 16:34, Ming Lei ha scritto: * - * An interesting effect of this policy is that only writes to req_vq need to - * take the tgt_lock. Read can be done outside the lock because: - * - * - writes of req_vq only occur when atomic_inc_return(tgt-reqs) returns 1. - * In that

Re: [PATCH] virtio_scsi: remove ACCESS_ONCE() and smp_read_barrier_depends()

2014-05-07 Thread Paolo Bonzini
Il 07/05/2014 18:03, Ming Lei ha scritto: * * - likewise, decrements of reqs only occur when reqs != 0. If the decremented * value is zero, the first CPU that enters virtscsi_queuecommand_multi will * modify req_vq and the others will spin on tgt_lock. The fact should be obvious too,

Re: virtio-scsi: two questions related with picking up queue

2014-05-07 Thread Paolo Bonzini
Il 07/05/2014 18:24, Ming Lei ha scritto: On Tue, 06 May 2014 15:17:15 +0200 Paolo Bonzini pbonz...@redhat.com wrote: Il 06/05/2014 11:26, Ming Lei ha scritto: Hi Paolo and All, One question is about ACCESS_ONCE() in virtscsi_pick_vq(), looks it needn't since both reading and writing tgt

Re: virtio-scsi: two questions related with picking up queue

2014-05-06 Thread Paolo Bonzini
Il 06/05/2014 11:26, Ming Lei ha scritto: Hi Paolo and All, One question is about ACCESS_ONCE() in virtscsi_pick_vq(), looks it needn't since both reading and writing tgt-req_vq holds tgt-tgt_lock. You're right. It should be possible to avoid the lock in virtscsi_pick_vq like this:

Re: [GIT PULL] SCSI fixes for 3.15-rc3

2014-04-28 Thread Paolo Bonzini
Il 25/04/2014 16:59, James Bottomley ha scritto: This is a set of seven fixes, three (hpsa) and free'd command references correcting bugs in the last round of updates and the remaining four correcting problems within the SCSI error handler that was causing a deadlock within USB. The patch is

Re: [PATCH 1/6] virtio-scsi.h: Add virtio_scsi_cmd_req_pi + VIRTIO_SCSI_F_T10_PI bits

2014-04-12 Thread Paolo Bonzini
Il 09/04/2014 09:22, Michael S. Tsirkin ha scritto: The interface uses bytes instead of iovecs as the unit, and vhost-scsi can add the (temporary...) requirement that do_pi_nbytes and di_pi_nbytes comprise an integer number of iovecs. Paolo That would be a reasonable intermediate step, but

Re: [PATCH] virtio-scsi: Skip setting affinity on uninitialized vq

2014-04-11 Thread Paolo Bonzini
. Not a problem though; the queues are few and this is not a hot path anyway. Acked-by: Paolo Bonzini pbonz...@redhat.com Paolo -- 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

Re: [PATCH] virtio-scsi: Skip setting affinity on uninitialized vq

2014-04-11 Thread Paolo Bonzini
Il 11/04/2014 03:23, Fam Zheng ha scritto: virtscsi_init calls virtscsi_remove_vqs on err, even before initializing the vqs. The latter calls virtscsi_set_affinity, so let's check the pointer there before setting affinity on it. This fixes a panic when setting device's num_queues=2 on RHEL 6.5:

Re: [PATCH 16/39] virtio_scsi: use cmd_size

2014-03-25 Thread Paolo Bonzini
Il 25/03/2014 16:31, Christoph Hellwig ha scritto: Paolo, do you have a virtio_scsi tree to picks this up in? We now have all the infrastructure for it in James' tree. I don't, I usually just give my Acked-by and James picks it up. Paolo -- To unsubscribe from this list: send the line

Re: [RFCv2 5/7] vhost/scsi: Enable T10 PI IOV - SGL memory mapping

2014-03-17 Thread Paolo Bonzini
Il 17/03/2014 06:32, Nicholas A. Bellinger ha scritto: + if (vq-iov[0].iov_len == sizeof(v_req_pi)) { + req = (unsigned char *)v_req_pi; + target = v_req_pi.lun[1]; + req_size = sizeof(v_req_pi); +

Re: [PATCH] virtio-scsi: Change sense buffer size to 252

2014-03-06 Thread Paolo Bonzini
Il 06/03/2014 09:47, Fam Zheng ha scritto: According to SPC-4, section 4.5.2.1, 252 is the limit of sense data. So increase the value. Signed-off-by: Fam Zheng f...@redhat.com --- include/linux/virtio_scsi.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git

Re: [PATCH] virtio-scsi: Change sense buffer size to 252

2014-03-06 Thread Paolo Bonzini
Il 06/03/2014 12:22, Hannes Reinecke ha scritto: On 03/06/2014 11:09 AM, Paolo Bonzini wrote: Il 06/03/2014 09:47, Fam Zheng ha scritto: According to SPC-4, section 4.5.2.1, 252 is the limit of sense data. So increase the value. Signed-off-by: Fam Zheng f...@redhat.com --- include/linux

Re: [PATCH] vhost/scsi: Check LUN structure byte 0 is set to 1, per spec

2014-02-24 Thread Paolo Bonzini
Reviewed-by: Paolo Bonzini pbonz...@redhat.com -- 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 04/17] scsi: avoid useless free_list lock roundtrips

2014-02-07 Thread Paolo Bonzini
); - if (likely(cmd != NULL)) + if (cmd) scsi_pool_free_command(shost-cmd_pool, cmd); put_device(dev); Reviewed-by: Paolo Bonzini pbonz...@redhat.com -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord

Re: [PATCH 06/17] scsi: add support for per-host cmd pools

2014-02-07 Thread Paolo Bonzini
Il 05/02/2014 13:39, Christoph Hellwig ha scritto: + pool = scsi_find_host_cmd_pool(shost); Should you have a WARN_ON somewhere if shost-hostt-cmd_size shost-unchecked_isa_dma? Apart from this, Reviewed-by: Paolo Bonzini pbonz...@redhat.com Paolo + if (!pool

Re: [PATCH 07/17] virtio_scsi: use cmd_size

2014-02-07 Thread Paolo Bonzini
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 Acked-by: Paolo Bonzini pbonz...@redhat.com -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body

Re: [PATCH 2/2] target_core_spc: bounds check for spc_emulate_inquiry()

2013-12-20 Thread Paolo Bonzini
Il 20/12/2013 07:47, Hannes Reinecke ha scritto: How about changing the local buffer to heap memory instead, and bumping SE_INQUIRY_BUF to 1024 bytes..? Ok. But then we should have a quick check at the start if (cmd-data_length SE_INQUIRY_BUF) len = cmd-data_length else len =

Re: [PATCH] virtio-scsi: Fix hotcpu_notifier use-after-free with virtscsi_freeze

2013-10-28 Thread Paolo Bonzini
; + + err = register_hotcpu_notifier(vscsi-nb); + if (err) + vdev-config-del_vqs(vdev); - return virtscsi_init(vdev, vscsi); + return err; } #endif Reviewed-by: Paolo Bonzini pbonz...@redhat.com Cc: sta...@vger.kernel.org -- To unsubscribe from this list

Re: SCSI's heuristics for enabling WRITE SAME still need work [was: dm mpath: disable WRITE SAME if it fails]

2013-09-24 Thread Paolo Bonzini
Il 21/09/2013 00:03, Martin K. Petersen ha scritto: The major headache here of course is that WRITE SAME is inherently destructive. We can't just fire off one during discovery and see if it works. For WRITE you can issue a command with a transfer length of 0 to see if things work. But

Re: [PATCH] SCSI: Fix potential out-of-bounds access in drivers/scsi/sd.c

2013-09-06 Thread Paolo Bonzini
be present, but the buffer size is limited to 512 bytes. Signed-off-by: Alan Stern st...@rowland.harvard.edu Reported-by: Dmitry Vyukov dvyu...@google.com CC: sta...@vger.kernel.org Reviewed-by: Paolo Bonzini pbonz...@redhat.com --- [as1709] drivers/scsi/sd.c | 11

Re: Potential out-of-bounds access in drivers/scsi/sd.c

2013-09-04 Thread Paolo Bonzini
Il 04/09/2013 16:32, Alan Stern ha scritto: On Wed, 4 Sep 2013, Dmitry Vyukov wrote: Hi, We are working on a memory error detector AddressSanitizer for Linux kernel (https://code.google.com/p/address-sanitizer/wiki/AddressSanitizerForKernel), it can detect use-after-free and

Re: [PATCH] virtio-scsi: Fix virtqueue affinity setup

2013-08-01 Thread Paolo Bonzini
vscsi-num_queues counts the number of request virtqueue which does not include the control and event virtqueue. It is wrong to subtract VIRTIO_SCSI_VQ_BASE from vscsi-num_queues. Reviewed-by: Paolo Bonzini pbonz...@redhat.com This patch fixes the following panic. (qemu) device_del scsi0

Re: [RFC PATCH 0/4] SG_IO filtering via sysfs and minimal whitelist

2013-07-05 Thread Paolo Bonzini
Il 25/06/2013 23:19, Paolo Bonzini ha scritto: Il 27/05/2013 15:50, Paolo Bonzini ha scritto: We've been running in circles for nine months now. Let's restart from the maintainer's suggestion, which was probably dismissed too quickly. This is still not a complete solution, because /dev/sgN

Re: [RFC PATCH 0/4] SG_IO filtering via sysfs and minimal whitelist

2013-06-25 Thread Paolo Bonzini
Il 27/05/2013 15:50, Paolo Bonzini ha scritto: We've been running in circles for nine months now. Let's restart from the maintainer's suggestion, which was probably dismissed too quickly. This is still not a complete solution, because /dev/sgN does not have access to its queue object

Re: [PATCH 4/9] virtio_scsi: Enable new EH timeout handler

2013-06-12 Thread Paolo Bonzini
, .eh_abort_handler = virtscsi_abort, .eh_device_reset_handler = virtscsi_device_reset, + .eh_timed_out = virtscsi_timedout, .can_queue = 1024, .dma_boundary = UINT_MAX, Acked-by: Paolo Bonzini pbonz...@redhat.com -- To unsubscribe from this list: send the line

Re: [PATCH 5/9] virtio-scsi: Implement TMF timeout

2013-06-12 Thread Paolo Bonzini
Il 10/06/2013 03:40, Hannes Reinecke ha scritto: Any TMF might be take longer as expected, or not return at all. So we need to use 'wait_for_completion_timeout' when sending a TMF to protect against these cases. Cc: Paolo Bonzini pbonz...@redhat.com Signed-off-by: Hannes Reinecke h

[RFC PATCH 3/4] block: add back command filter modification via sysfs

2013-05-27 Thread Paolo Bonzini
anyway never really enabled, the different API is not a problem. Cc: linux-scsi@vger.kernel.org Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- Documentation/block/queue-sysfs.txt | 16 ++ block/Kconfig | 10 block/blk-sysfs.c | 41

[RFC PATCH 1/4] block: add back queue-private command filter

2013-05-27 Thread Paolo Bonzini
filtering is desired. This is a partial (and massaged) revert of commit 018e044 (block: get rid of queue-private command filter, 2009-06-26). Cc: linux-scsi@vger.kernel.org Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- block/blk-sysfs.c | 2 ++ block/bsg.c| 2 +- block

[RFC PATCH 2/4] scsi: create an all-zero filter for scanners

2013-05-27 Thread Paolo Bonzini
Using /dev/sg for scanners is blocked from unprivileged users. Reimplement this using customizable command filters, so that the sysfs knobs will work in this case too. Cc: linux-scsi@vger.kernel.org Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- drivers/scsi/scsi_scan.c | 8

[RFC PATCH 0/4] SG_IO filtering via sysfs and minimal whitelist

2013-05-27 Thread Paolo Bonzini
. Paolo Paolo Bonzini (4): block: add back queue-private command filter scsi: create an all-zero filter for scanners block: add back command filter modification via sysfs scsi: lock out SG_IO by default to unprivileged users Documentation/block/queue-sysfs.txt | 16 + block/Kconfig

[RFC PATCH 4/4] scsi: lock out SG_IO by default to unprivileged users

2013-05-27 Thread Paolo Bonzini
be a regression for those who are using Unix permissions, security modules or the device cgroup to confine programs. A meaningful whitelist can then be set by udev, for example. Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- This is not yet usable, because sg devices do not have a link

Re: [PATCH v3 part1 1/4] sg_io: pass request_queue to blk_verify_command

2013-05-25 Thread Paolo Bonzini
Il 25/05/2013 06:14, James Bottomley ha scritto: On Fri, 2013-05-24 at 10:32 +0200, Paolo Bonzini wrote: Il 24/05/2013 10:03, James Bottomley ha scritto: Does anyone in the real world actually care about this bug? Yes, or I would move on and not waste so much time on this. Fine, so produce

Re: PING^7 (was Re: [PATCH v2 00/14] Corrections and customization of the SG_IO command whitelist (CVE-2012-4542))

2013-05-25 Thread Paolo Bonzini
Il 25/05/2013 07:27, Christoph Hellwig ha scritto: On Fri, May 24, 2013 at 09:35:02PM -0700, James Bottomley wrote: I'll go along with this. I'm also wondering what the problem would be if we just allowed all commands on either CAP_SYS_RAWIO or opening the device for write, so we just defer

Re: PING^7 (was Re: [PATCH v2 00/14] Corrections and customization of the SG_IO command whitelist (CVE-2012-4542))

2013-05-25 Thread Paolo Bonzini
Il 25/05/2013 09:11, Christoph Hellwig ha scritto: Linus wanted to keep that for CAP_SYS_RAWIO. We found two uses of SG_IO on partitions: zfs-fuse used SYNCHRONIZE CACHE; some proprietary driver used TEST UNIT READY. Really, the solution is to make the bitmaps configurable in

Re: PING^7 (was Re: [PATCH v2 00/14] Corrections and customization of the SG_IO command whitelist (CVE-2012-4542))

2013-05-25 Thread Paolo Bonzini
Il 25/05/2013 10:37, Tejun Heo ha scritto: Hey, James. On Fri, May 24, 2013 at 09:35:02PM -0700, James Bottomley wrote: Well, I'd actually much prefer disabling CDB whitelisting for all !MMC devices if at all possible. I'll go along with this. I'm also wondering what the problem would be

Re: PING^7 (was Re: [PATCH v2 00/14] Corrections and customization of the SG_IO command whitelist (CVE-2012-4542))

2013-05-25 Thread Paolo Bonzini
Il 25/05/2013 14:48, Tejun Heo ha scritto: * Merge the patch to give out SG_IO access along with write access, so the use cases which want to give out SG_IO access can do so explicitly and be fully responsible for the device. This makes sense to me. If one wants to be allowed to

Re: PING^7 (was Re: [PATCH v2 00/14] Corrections and customization of the SG_IO command whitelist (CVE-2012-4542))

2013-05-24 Thread Paolo Bonzini
Il 24/05/2013 03:44, Tejun Heo ha scritto: On Thu, May 23, 2013 at 11:47:25AM +0200, Paolo Bonzini wrote: No no, I'm not talking about it not working for the users - it's just passing the commands, it of course works. I'm doubting about it being a worthy security isolation layer. cdb

Re: [PATCH v3 part1 1/4] sg_io: pass request_queue to blk_verify_command

2013-05-24 Thread Paolo Bonzini
Il 24/05/2013 09:36, James Bottomley ha scritto: On Thu, 2013-05-23 at 15:58 +0200, Paolo Bonzini wrote: Adjust the blk_verify_command function to let it look at per-queue data. This will be done in the next patch. This is not a bug fix. This is an enabler for your complex and to my mind

Re: [PATCH v3 part1 1/4] sg_io: pass request_queue to blk_verify_command

2013-05-24 Thread Paolo Bonzini
Il 24/05/2013 09:50, James Bottomley ha scritto: On Fri, 2013-05-24 at 09:43 +0200, Paolo Bonzini wrote: Il 24/05/2013 09:36, James Bottomley ha scritto: On Thu, 2013-05-23 at 15:58 +0200, Paolo Bonzini wrote: Adjust the blk_verify_command function to let it look at per-queue data

Re: PING^7 (was Re: [PATCH v2 00/14] Corrections and customization of the SG_IO command whitelist (CVE-2012-4542))

2013-05-24 Thread Paolo Bonzini
Il 24/05/2013 10:02, Tejun Heo ha scritto: On Fri, May 24, 2013 at 4:13 PM, Paolo Bonzini pbonz...@redhat.com wrote: The same filtering table being applied to different classes of hardware is a software bug, but my point is that the practive essentially entrusts non-insignificant part

Re: [PATCH v3 part1 1/4] sg_io: pass request_queue to blk_verify_command

2013-05-24 Thread Paolo Bonzini
Il 24/05/2013 10:03, James Bottomley ha scritto: Does anyone in the real world actually care about this bug? Yes, or I would move on and not waste so much time on this. Fine, so produce a simple fix for this bug which we can discuss that's not tied to this feature.

Re: PING^7 (was Re: [PATCH v2 00/14] Corrections and customization of the SG_IO command whitelist (CVE-2012-4542))

2013-05-24 Thread Paolo Bonzini
Il 24/05/2013 11:07, Tejun Heo ha scritto: On Fri, May 24, 2013 at 5:31 PM, Paolo Bonzini pbonz...@redhat.com wrote: I agree intuition may not count, and it's perfectly possible that firmware writers forgot a break; or put the wrong location in a jump table, so that unimplemented commands give

Re: [PATCH v3 part1 1/4] sg_io: pass request_queue to blk_verify_command

2013-05-24 Thread Paolo Bonzini
Il 24/05/2013 10:32, Paolo Bonzini ha scritto: Il 24/05/2013 10:03, James Bottomley ha scritto: Does anyone in the real world actually care about this bug? Yes, or I would move on and not waste so much time on this. Fine, so produce a simple fix for this bug which we can discuss that's

Re: PING^7 (was Re: [PATCH v2 00/14] Corrections and customization of the SG_IO command whitelist (CVE-2012-4542))

2013-05-23 Thread Paolo Bonzini
Il 23/05/2013 00:17, Tejun Heo ha scritto: Then let's make it fit the use case better. I really can't see much point in crafting the cdb filter when you basically have to entrust the device to the user anyway. Let's either trust the user with the device or not. I'm very doubtful that the

Re: PING^7 (was Re: [PATCH v2 00/14] Corrections and customization of the SG_IO command whitelist (CVE-2012-4542))

2013-05-23 Thread Paolo Bonzini
Il 23/05/2013 11:02, Tejun Heo ha scritto: On Thu, May 23, 2013 at 09:45:42AM +0200, Paolo Bonzini wrote: Il 23/05/2013 00:17, Tejun Heo ha scritto: Then let's make it fit the use case better. I really can't see much point in crafting the cdb filter when you basically have to entrust

[PATCH v3 part1 3/4] sg_io: use different default filters for each device class

2013-05-23 Thread Paolo Bonzini
are still the same, so there is no semantic change in this patch. Cc: sta...@gnu.org Cc: James E.J. Bottomley jbottom...@parallels.com Cc: linux-scsi@vger.kernel.org Cc: Jens Axboe ax...@kernel.dk Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- block/scsi_ioctl.c | 14

<    1   2   3   4   >