Re: [PATCH v6 1/2] blktrace: Fix potentail deadlock between delete & sysfs ops

2017-09-19 Thread Christoph Hellwig
On Tue, Sep 19, 2017 at 11:58:34AM -0400, Waiman Long wrote: > I was trying not to add a new mutex to a structure just for blktrace as > it is an optional feature that is enabled only if the > CONFIG_BLK_DEV_IO_TRACE config option is defined and it will only need > to be taken occasionally. So?

[PATCHv2] bcache: option for allow stale data on read failure

2017-09-19 Thread Coly Li
When bcache does read I/Os, for example in writeback or writethrough mode, if a read request on cache device is failed, bcache will try to recovery the request by reading from cached device. If the data on cached device is not synced with cache device, then requester will get a stale data. For

Re: [PATCH V4 00/14] blk-mq-sched: improve SCSI-MQ performance

2017-09-19 Thread Ming Lei
On Tue, Sep 19, 2017 at 12:25:15PM -0700, Omar Sandoval wrote: > On Sat, Sep 02, 2017 at 11:17:15PM +0800, Ming Lei wrote: > > Hi, > > > > In Red Hat internal storage test wrt. blk-mq scheduler, we > > found that I/O performance is much bad with mq-deadline, especially > > about sequential I/O on

Re: [PATCH 1/5] block: don't call blk_mq_delay_run_hw_queue() in case of BLK_STS_RESOURCE

2017-09-19 Thread Ming Lei
Hi Mike, On Tue, Sep 19, 2017 at 07:50:06PM -0400, Mike Snitzer wrote: > On Tue, Sep 19 2017 at 7:25pm -0400, > Bart Van Assche wrote: > > > On Wed, 2017-09-20 at 06:44 +0800, Ming Lei wrote: > > > For this issue, it isn't same between SCSI and dm-rq. > > > > > > We

Re: [PATCH] block: move sanity checking ahead of bi_front/back_seg_size updating

2017-09-19 Thread jianchao.wang
On 09/19/2017 10:36 PM, Christoph Hellwig wrote: > On Tue, Sep 19, 2017 at 08:55:59AM +0800, jianchao.wang wrote: >>> But can you elaborate a little more on how this found and if there >>> is a way to easily reproduce it, say for a blktests test case? >>> >> It is found when I made the patch of

Re: [PATCH V4 05/14] blk-mq-sched: improve dispatching from sw queue

2017-09-19 Thread Ming Lei
Hi Jens, Thanks for your comment! On Tue, Sep 19, 2017 at 02:37:50PM -0600, Jens Axboe wrote: > On 09/02/2017 09:17 AM, Ming Lei wrote: > > @@ -142,18 +178,31 @@ void blk_mq_sched_dispatch_requests(struct > > blk_mq_hw_ctx *hctx) > > if (!list_empty(_list)) { > >

Re: [PATCH V4 06/14] blk-mq-sched: don't dequeue request until all in ->dispatch are flushed

2017-09-19 Thread Ming Lei
On Tue, Sep 19, 2017 at 12:11:05PM -0700, Omar Sandoval wrote: > On Sat, Sep 02, 2017 at 11:17:21PM +0800, Ming Lei wrote: > > During dispatching, we moved all requests from hctx->dispatch to > > one temporary list, then dispatch them one by one from this list. > > Unfortunately during this

Re: [PATCH 1/5] block: don't call blk_mq_delay_run_hw_queue() in case of BLK_STS_RESOURCE

2017-09-19 Thread Ming Lei
On Tue, Sep 19, 2017 at 04:49:15PM +, Bart Van Assche wrote: > On Wed, 2017-09-20 at 00:04 +0800, Ming Lei wrote: > > Run queue at end_io is definitely wrong, because blk-mq has SCHED_RESTART > > to do that already. > > Sorry but I disagree. If SCHED_RESTART is set that causes the blk-mq core

Re: [PATCH 1/5] block: don't call blk_mq_delay_run_hw_queue() in case of BLK_STS_RESOURCE

2017-09-19 Thread Ming Lei
On Tue, Sep 19, 2017 at 06:42:30PM +, Bart Van Assche wrote: > On Wed, 2017-09-20 at 00:55 +0800, Ming Lei wrote: > > On Wed, Sep 20, 2017 at 12:49 AM, Bart Van Assche > > wrote: > > > On Wed, 2017-09-20 at 00:04 +0800, Ming Lei wrote: > > > > Run queue at end_io is

Re: [PATCH 1/5] block: don't call blk_mq_delay_run_hw_queue() in case of BLK_STS_RESOURCE

2017-09-19 Thread Bart Van Assche
On Wed, 2017-09-20 at 06:44 +0800, Ming Lei wrote: > For this issue, it isn't same between SCSI and dm-rq. > > We don't need to run queue in .end_io of dm, and the theory is > simple, otherwise it isn't performance issue, and should be I/O hang. > > 1) every dm-rq's request is 1:1 mapped to

Re: [PATCH V4 04/14] blk-mq-sched: move actual dispatching into one helper

2017-09-19 Thread Omar Sandoval
On Sat, Sep 02, 2017 at 11:17:19PM +0800, Ming Lei wrote: > So that it becomes easy to support to dispatch from > sw queue in the following patch. > > No functional change. > > Reviewed-by: Bart Van Assche > Signed-off-by: Ming Lei > --- >

Re: [PATCH] scsi: ensure the header peeked does not change in the actual message

2017-09-19 Thread Christoph Hellwig
On Tue, Sep 19, 2017 at 12:15:57PM -0400, Meng Xu wrote: > Hi Christoph, > > By saying not copying the byte twice, did you mean > copy_from_user(req->cmd, sic->data + sizeof(opcode), cmdlen - > sizeof(opcode)) ? > > Does it affect the how req->cmd will be used later? > If no, I'll submit another

[PATCH] bcache: check ca->alloc_thread initialized before wake up it

2017-09-19 Thread Coly Li
In bcache code, sysfs entries are created before all resources get allocated, e.g. allocation thread of a cache set. There is posibility for NULL pointer deference if a resource is accessed but which is not initialized yet. Indeed Jorg Bornschein catches one on cache set allocation thread and

Re: [PATCH 1/5] block: don't call blk_mq_delay_run_hw_queue() in case of BLK_STS_RESOURCE

2017-09-19 Thread Mike Snitzer
On Tue, Sep 19 2017 at 7:25pm -0400, Bart Van Assche wrote: > On Wed, 2017-09-20 at 06:44 +0800, Ming Lei wrote: > > For this issue, it isn't same between SCSI and dm-rq. > > > > We don't need to run queue in .end_io of dm, and the theory is > > simple, otherwise it

Re: [PATCH 9/9] nvme: implement multipath access to nvme subsystems

2017-09-19 Thread Tony Yang
Hi , Christoph I use the above code to recompile the kernel. The following error occurred. I can't find the blk_steal_bios function. What's the reason for that? Hope to get your help, thank you [root@scst1 nvme-nvme-4.13]# make CHK include/config/kernel.release CHK

Re: [PATCH V4 00/14] blk-mq-sched: improve SCSI-MQ performance

2017-09-19 Thread Omar Sandoval
On Sat, Sep 02, 2017 at 11:17:15PM +0800, Ming Lei wrote: > Hi, > > In Red Hat internal storage test wrt. blk-mq scheduler, we > found that I/O performance is much bad with mq-deadline, especially > about sequential I/O on some multi-queue SCSI devcies(lpfc, qla2xxx, > SRP...) > > Turns out one

Re: [PATCH] block: cope with WRITE SAME failing in blkdev_issue_zeroout()

2017-09-19 Thread Christoph Hellwig
On Wed, Sep 06, 2017 at 07:38:10PM +0200, Ilya Dryomov wrote: > sd_config_write_same() ignores ->max_ws_blocks == 0 and resets it to > permit trying WRITE SAME on older SCSI devices, unless ->no_write_same > is set. This means blkdev_issue_zeroout() must cope with WRITE SAME > failing with

Re: [PATCH v6 1/2] blktrace: Fix potentail deadlock between delete & sysfs ops

2017-09-19 Thread Steven Rostedt
On Tue, 19 Sep 2017 13:41:35 -0700 Christoph Hellwig wrote: > Call it blk_trace mutex and move it right next to the blk_trace > structure: > > ifdef CONFIG_BLK_DEV_IO_TRACE > struct blk_trace*blk_trace; > struct mutexblk_trace_mutex; >

[PATCH] scsi: skip message header in next fetch

2017-09-19 Thread Meng Xu
In sg_scsi_ioctl(), the header of the userspace buffer, sic->data, is fetched twice from the userspace. The first fetch is used to peek at the opcode and derive cmdlen while the second fetch copies the whole message. However, the userspace memory backing opcode can change across fetches which

Re: [PATCH V4 06/14] blk-mq-sched: don't dequeue request until all in ->dispatch are flushed

2017-09-19 Thread Omar Sandoval
On Sat, Sep 02, 2017 at 11:17:21PM +0800, Ming Lei wrote: > During dispatching, we moved all requests from hctx->dispatch to > one temporary list, then dispatch them one by one from this list. > Unfortunately during this period, run queue from other contexts > may think the queue is idle, then

Re: [PATCH V4 05/14] blk-mq-sched: improve dispatching from sw queue

2017-09-19 Thread Jens Axboe
On 09/02/2017 09:17 AM, Ming Lei wrote: > @@ -142,18 +178,31 @@ void blk_mq_sched_dispatch_requests(struct > blk_mq_hw_ctx *hctx) > if (!list_empty(_list)) { > blk_mq_sched_mark_restart_hctx(hctx); > do_sched_dispatch = blk_mq_dispatch_rq_list(q, _list); > -

blkdiscard apparently failing to fully discard a SATA SSD

2017-09-19 Thread Lennert Buytenhek
Hello! When I use blkdiscard to trim a SATA SSD in its entirety, it seems that not all sectors are getting trimmed. I initially ran into this on a Sandisk SDSSDA240G, but I reproduced it on a Samsung 850 EVO M.2 250GB and a few other SATA SSDs. This happens on Fedora kernel

Re: [PATCH] block: cope with WRITE SAME failing in blkdev_issue_zeroout()

2017-09-19 Thread Ilya Dryomov
On Wed, Sep 13, 2017 at 11:50 AM, Ilya Dryomov wrote: > On Wed, Sep 6, 2017 at 7:38 PM, Ilya Dryomov wrote: >> sd_config_write_same() ignores ->max_ws_blocks == 0 and resets it to >> permit trying WRITE SAME on older SCSI devices, unless ->no_write_same >>

Re: [PATCH v6 2/2] block_dev: Rename bd_fsfreeze_mutex

2017-09-19 Thread Waiman Long
On 09/18/2017 07:47 PM, Christoph Hellwig wrote: > Don't rename it to a way to long name. Either add a separate mutex > for your purpose (unless there is interaction between freezing and > blktrace, which I doubt), or properly comment the usage. I would agree with you if the long name causes the

Re: [PATCH v6 1/2] blktrace: Fix potentail deadlock between delete & sysfs ops

2017-09-19 Thread Waiman Long
On 09/18/2017 08:01 PM, Christoph Hellwig wrote: > Taking a look at this it seems like using a lock in struct block_device > isn't the right thing to do anyway - all the action is on fields in > struct blk_trace, so having a lock inside that would make a lot more > sense. > > It would also help to

Re: [PATCH] block,bfq: Disable writeback throttling

2017-09-19 Thread Paolo Valente
> Il giorno 14 set 2017, alle ore 09:14, oleksa...@natalenko.name ha scritto: > > Tested-by: Oleksandr Natalenko > >> Similarly to CFQ, BFQ has its write-throttling heuristics, and it >> is better not to combine them with further write-throttling >> heuristics of a

Re: [PATCH 1/5] block: don't call blk_mq_delay_run_hw_queue() in case of BLK_STS_RESOURCE

2017-09-19 Thread Bart Van Assche
On Wed, 2017-09-20 at 00:04 +0800, Ming Lei wrote: > Run queue at end_io is definitely wrong, because blk-mq has SCHED_RESTART > to do that already. Sorry but I disagree. If SCHED_RESTART is set that causes the blk-mq core to reexamine the software queues and the hctx dispatch list but not the

Re: [PATCH 1/5] block: don't call blk_mq_delay_run_hw_queue() in case of BLK_STS_RESOURCE

2017-09-19 Thread Ming Lei
On Wed, Sep 20, 2017 at 12:49 AM, Bart Van Assche wrote: > On Wed, 2017-09-20 at 00:04 +0800, Ming Lei wrote: >> Run queue at end_io is definitely wrong, because blk-mq has SCHED_RESTART >> to do that already. > > Sorry but I disagree. If SCHED_RESTART is set that causes

Re: [PATCH 2/5] dm-mpath: return DM_MAPIO_REQUEUE in case of rq allocation failure

2017-09-19 Thread Ming Lei
On Tue, Sep 19, 2017 at 10:41:30AM -0400, Mike Snitzer wrote: > On Sun, Sep 17 2017 at 9:23am -0400, > Ming Lei wrote: > > > On Fri, Sep 15, 2017 at 04:06:55PM -0400, Mike Snitzer wrote: > > > On Fri, Sep 15 2017 at 1:29pm -0400, > > > Bart Van Assche

Re: [PATCH 1/5] block: don't call blk_mq_delay_run_hw_queue() in case of BLK_STS_RESOURCE

2017-09-19 Thread Ming Lei
On Tue, Sep 19, 2017 at 11:56:03AM -0400, Mike Snitzer wrote: > On Tue, Sep 19 2017 at 11:36am -0400, > Bart Van Assche wrote: > > > On Tue, 2017-09-19 at 13:43 +0800, Ming Lei wrote: > > > On Mon, Sep 18, 2017 at 03:18:16PM +, Bart Van Assche wrote: > > > > If you

Re: [PATCH 1/5] block: don't call blk_mq_delay_run_hw_queue() in case of BLK_STS_RESOURCE

2017-09-19 Thread Ming Lei
On Tue, Sep 19, 2017 at 11:48:23AM -0400, Mike Snitzer wrote: > On Tue, Sep 19 2017 at 1:43am -0400, > Ming Lei wrote: > > > On Mon, Sep 18, 2017 at 03:18:16PM +, Bart Van Assche wrote: > > > On Sun, 2017-09-17 at 20:40 +0800, Ming Lei wrote: > > > > "if no request has

Re: [PATCH v6 1/2] blktrace: Fix potentail deadlock between delete & sysfs ops

2017-09-19 Thread Christoph Hellwig
On Tue, Sep 19, 2017 at 08:49:12AM -0400, Waiman Long wrote: > On 09/18/2017 08:01 PM, Christoph Hellwig wrote: > > Taking a look at this it seems like using a lock in struct block_device > > isn't the right thing to do anyway - all the action is on fields in > > struct blk_trace, so having a lock

Re: [PATCH 2/5] dm-mpath: return DM_MAPIO_REQUEUE in case of rq allocation failure

2017-09-19 Thread Mike Snitzer
On Sun, Sep 17 2017 at 9:23am -0400, Ming Lei wrote: > On Fri, Sep 15, 2017 at 04:06:55PM -0400, Mike Snitzer wrote: > > On Fri, Sep 15 2017 at 1:29pm -0400, > > Bart Van Assche wrote: > > > > > On Sat, 2017-09-16 at 00:44 +0800, Ming Lei wrote: >

Re: [PATCH 1/5] block: don't call blk_mq_delay_run_hw_queue() in case of BLK_STS_RESOURCE

2017-09-19 Thread Bart Van Assche
On Tue, 2017-09-19 at 11:48 -0400, Mike Snitzer wrote: > This thread proves that it is definitely brittle to be relying on fixed > delays like this: > https://patchwork.kernel.org/patch/9703249/ Hello Mike, Sorry but I think that's a misinterpretation of my patch. I came up with that patch

Re: [PATCH v6 1/2] blktrace: Fix potentail deadlock between delete & sysfs ops

2017-09-19 Thread Waiman Long
On 09/19/2017 10:38 AM, Christoph Hellwig wrote: > On Tue, Sep 19, 2017 at 08:49:12AM -0400, Waiman Long wrote: >> On 09/18/2017 08:01 PM, Christoph Hellwig wrote: >>> Taking a look at this it seems like using a lock in struct block_device >>> isn't the right thing to do anyway - all the action is

Re: [PATCH 1/5] block: don't call blk_mq_delay_run_hw_queue() in case of BLK_STS_RESOURCE

2017-09-19 Thread Mike Snitzer
On Tue, Sep 19 2017 at 11:36am -0400, Bart Van Assche wrote: > On Tue, 2017-09-19 at 13:43 +0800, Ming Lei wrote: > > On Mon, Sep 18, 2017 at 03:18:16PM +, Bart Van Assche wrote: > > > If you are still looking at removing the blk_mq_delay_run_hw_queue() calls > > >

Re: [PATCH] scsi: ensure the header peeked does not change in the actual message

2017-09-19 Thread Christoph Hellwig
> diff --git a/block/scsi_ioctl.c b/block/scsi_ioctl.c > index 7440de4..971044d 100644 > --- a/block/scsi_ioctl.c > +++ b/block/scsi_ioctl.c > @@ -466,6 +466,12 @@ int sg_scsi_ioctl(struct request_queue *q, struct > gendisk *disk, fmode_t mode, > if (copy_from_user(req->cmd, sic->data,

Re: [PATCH 1/5] block: don't call blk_mq_delay_run_hw_queue() in case of BLK_STS_RESOURCE

2017-09-19 Thread Mike Snitzer
On Tue, Sep 19 2017 at 11:52am -0400, Bart Van Assche wrote: > On Tue, 2017-09-19 at 11:48 -0400, Mike Snitzer wrote: > > This thread proves that it is definitely brittle to be relying on fixed > > delays like this: > > https://patchwork.kernel.org/patch/9703249/ > >

Re: [PATCH] scsi: ensure the header peeked does not change in the actual message

2017-09-19 Thread Meng Xu
Hi Christoph, By saying not copying the byte twice, did you mean copy_from_user(req->cmd, sic->data + sizeof(opcode), cmdlen - sizeof(opcode)) ? Does it affect the how req->cmd will be used later? If no, I'll submit another patch as instructed. Best Regards, Meng On 09/19/2017 12:01 PM,

Re: [RFC PATCH] blk-mq: Fix lost request during timeout

2017-09-19 Thread Keith Busch
On Tue, Sep 19, 2017 at 03:18:45PM +, Bart Van Assche wrote: > On Tue, 2017-09-19 at 11:07 -0400, Keith Busch wrote: > > The problem is when blk-mq's timeout handler prevents the request from > > completing, and doesn't leave any indication the driver requested to > > complete it. Who is

Re: [RFC PATCH] blk-mq: Fix lost request during timeout

2017-09-19 Thread Bart Van Assche
On Mon, 2017-09-18 at 21:55 -0400, Keith Busch wrote: > The only way to complete that request now is if the timeout > handler returns BLK_EH_HANDLED, but the scsi-mq abort path returns > BLK_EH_NOT_HANDLED on success (very few drivers actually return > BLK_EH_HANDLED). > > After the timeout

Re: [RFC PATCH] blk-mq: Fix lost request during timeout

2017-09-19 Thread Ming Lei
On Tue, Sep 19, 2017 at 11:07 PM, Keith Busch wrote: > On Tue, Sep 19, 2017 at 12:16:31PM +0800, Ming Lei wrote: >> On Tue, Sep 19, 2017 at 7:08 AM, Keith Busch wrote: >> > >> > Indeed that prevents .complete from running concurrently with the >> >

Re: [PATCH 1/5] block: don't call blk_mq_delay_run_hw_queue() in case of BLK_STS_RESOURCE

2017-09-19 Thread Mike Snitzer
On Tue, Sep 19 2017 at 1:43am -0400, Ming Lei wrote: > On Mon, Sep 18, 2017 at 03:18:16PM +, Bart Van Assche wrote: > > On Sun, 2017-09-17 at 20:40 +0800, Ming Lei wrote: > > > "if no request has completed before the delay has expired" can't be a > > > reason to rerun

Re: [PATCH] block: move sanity checking ahead of bi_front/back_seg_size updating

2017-09-19 Thread Christoph Hellwig
On Tue, Sep 19, 2017 at 08:55:59AM +0800, jianchao.wang wrote: > > But can you elaborate a little more on how this found and if there > > is a way to easily reproduce it, say for a blktests test case? > > > It is found when I made the patch of > 'block: consider merge of segments when merge bio

Is the possible cross-talking between unrelated file-descriptors on bsg-device by design?

2017-09-19 Thread Benjamin Block
Hello linux-block, I wrote some tests recently to test patches against bsg.c and bsg-lib.c, and while writing those I noticed something strange: When you use the write() and read() call on multiple file-descriptors for a single bsg-device (FC or SCSI), it is possible that you get cross-talk

Re: [RFC PATCH] blk-mq: Fix lost request during timeout

2017-09-19 Thread Bart Van Assche
On Tue, 2017-09-19 at 11:07 -0400, Keith Busch wrote: > The problem is when blk-mq's timeout handler prevents the request from > completing, and doesn't leave any indication the driver requested to > complete it. Who is responsible for completing that request now? Hello Keith, My understanding

[PATCH] scsi: ensure the header peeked does not change in the actual message

2017-09-19 Thread Meng Xu
In sg_scsi_ioctl(), the header of the userspace buffer, sic->data, is fetched twice from the userspace. The first fetch is used to peek at the opcode and derive cmdlen while the second fetch copies the whole message. However, the userspace memory backing opcode can change across fetches which

Re: [RFC PATCH] blk-mq: Fix lost request during timeout

2017-09-19 Thread Keith Busch
On Tue, Sep 19, 2017 at 12:16:31PM +0800, Ming Lei wrote: > On Tue, Sep 19, 2017 at 7:08 AM, Keith Busch wrote: > > > > Indeed that prevents .complete from running concurrently with the > > timeout handler, but scsi_mq_done and nvme_handle_cqe are not .complete > >

Re: [PATCH 1/5] block: don't call blk_mq_delay_run_hw_queue() in case of BLK_STS_RESOURCE

2017-09-19 Thread Bart Van Assche
On Tue, 2017-09-19 at 13:43 +0800, Ming Lei wrote: > On Mon, Sep 18, 2017 at 03:18:16PM +, Bart Van Assche wrote: > > If you are still looking at removing the blk_mq_delay_run_hw_queue() calls > > then I think you are looking in the wrong direction. What kind of problem > > are you trying to

Re: [RFC PATCH] blk-mq: Fix lost request during timeout

2017-09-19 Thread Keith Busch
On Tue, Sep 19, 2017 at 11:22:20PM +0800, Ming Lei wrote: > On Tue, Sep 19, 2017 at 11:07 PM, Keith Busch wrote: > > On Tue, Sep 19, 2017 at 12:16:31PM +0800, Ming Lei wrote: > >> On Tue, Sep 19, 2017 at 7:08 AM, Keith Busch wrote: > >> > > >> >

Re: Is the possible cross-talking between unrelated file-descriptors on bsg-device by design?

2017-09-19 Thread Douglas Gilbert
On 2017-09-19 10:56 AM, Benjamin Block wrote: Hello linux-block, I wrote some tests recently to test patches against bsg.c and bsg-lib.c, and while writing those I noticed something strange: When you use the write() and read() call on multiple file-descriptors for a single bsg-device (FC or

Re: [PATCH 1/5] block: don't call blk_mq_delay_run_hw_queue() in case of BLK_STS_RESOURCE

2017-09-19 Thread Bart Van Assche
On Wed, 2017-09-20 at 00:55 +0800, Ming Lei wrote: > On Wed, Sep 20, 2017 at 12:49 AM, Bart Van Assche > wrote: > > On Wed, 2017-09-20 at 00:04 +0800, Ming Lei wrote: > > > Run queue at end_io is definitely wrong, because blk-mq has SCHED_RESTART > > > to do that already.