Re: [dm-devel] [for-4.16 PATCH v3 3/3] dm: fix awkward and incomplete request_queue initialization

2018-01-10 Thread Hannes Reinecke
On 01/11/2018 03:12 AM, Mike Snitzer wrote: > DM is now no longer prone to having its request_queue be improperly > initialized. > > Summary of changes: > > - defer DM's blk_register_queue() from add_disk()-time until > dm_setup_md_queue() by setting QUEUE_FLAG_DEFER_REG in alloc_dev(). > > -

Re: [dm-devel] [for-4.16 PATCH v3 2/3] block: allow gendisk's request_queue registration to be deferred

2018-01-10 Thread Hannes Reinecke
On 01/11/2018 03:12 AM, Mike Snitzer wrote: > Since I can remember DM has forced the block layer to allow the > allocation and initialization of the request_queue to be distinct > operations. Reason for this is block/genhd.c:add_disk() has requires > that the request_queue (and associated bdi) be

Re: [dm-devel] [for-4.16 PATCH v3 2/3] block: allow gendisk's request_queue registration to be deferred

2018-01-10 Thread Hannes Reinecke
On 01/11/2018 03:12 AM, Mike Snitzer wrote: > Since I can remember DM has forced the block layer to allow the > allocation and initialization of the request_queue to be distinct > operations. Reason for this is block/genhd.c:add_disk() has requires > that the request_queue (and associated bdi) be

Re: [dm-devel] [for-4.16 PATCH v3 1/3] block: only bdi_unregister() in del_gendisk() if !GENHD_FL_HIDDEN

2018-01-10 Thread Hannes Reinecke
On 01/11/2018 03:12 AM, Mike Snitzer wrote: > device_add_disk() will only call bdi_register_owner() if > !GENHD_FL_HIDDEN, so it follows that del_gendisk() should only call > bdi_unregister() if !GENHD_FL_HIDDEN. > > Found with code inspection. bdi_unregister() won't do any harm if > bdi_register

[dm-devel] [PATCH V3 4/5] blk-mq: return dispatch result to caller in blk_mq_try_issue_directly

2018-01-10 Thread Ming Lei
In the following patch, we will use blk_mq_try_issue_directly() for DM to return the dispatch result, and DM need this informatin to improve IO merge. Signed-off-by: Ming Lei --- block/blk-mq.c | 23 ++- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/block/blk

[dm-devel] [PATCH V3 5/5] blk-mq: issue request directly for blk_insert_cloned_request

2018-01-10 Thread Ming Lei
blk_insert_cloned_request() is called in fast path of dm-rq driver, and in this function we append request to hctx->dispatch_list of the underlying queue directly. 1) This way isn't efficient enough because hctx lock is always required 2) With blk_insert_cloned_request(), we bypass underlying que

[dm-devel] [PATCH V3 3/5] blk-mq: move actual issue into one helper

2018-01-10 Thread Ming Lei
No functional change, just to clean up code a bit, so that the following change of using direct issue for blk_mq_request_bypass_insert() which is needed by DM can be easier to do. Signed-off-by: Ming Lei --- block/blk-mq.c | 39 +++ 1 file changed, 27 insertio

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

2018-01-10 Thread Ming Lei
blk-mq will rerun queue via RESTART or dispatch wake after one request is completed, so not necessary to wait random time for requeuing, we should trust blk-mq to do it. More importantly, we need return BLK_STS_RESOURCE to blk-mq so that dequeue from I/O scheduler can be stopped, then I/O merge ge

[dm-devel] [PATCH V3 0/5] dm-rq: improve sequential I/O performance

2018-01-10 Thread Ming Lei
Hi Guys, The 1st patch removes the workaround of blk_mq_delay_run_hw_queue() in case of requeue, this way isn't necessary, and more worse, it makes BLK_MQ_S_SCHED_RESTART not working, and degarde I/O performance. The 2nd patch return DM_MAPIO_REQUEUE to dm-rq if underlying request allocation fail

[dm-devel] [PATCH V3 1/5] dm-mpath: don't call blk_mq_delay_run_hw_queue() in case of BLK_STS_RESOURCE

2018-01-10 Thread Ming Lei
If .queue_rq() returns BLK_STS_RESOURCE, blk-mq will rerun the queue in the three situations: 1) if BLK_MQ_S_SCHED_RESTART is set - queue is rerun after one rq is completed, see blk_mq_sched_restart() which is run from blk_mq_free_request() 2) run out of driver tag - queue is rerun after one tag

Re: [dm-devel] [for-4.16 PATCH v3 3/3] dm: fix awkward and incomplete request_queue initialization

2018-01-10 Thread Ming Lei
On Wed, Jan 10, 2018 at 09:12:56PM -0500, Mike Snitzer wrote: > DM is now no longer prone to having its request_queue be improperly > initialized. > > Summary of changes: > > - defer DM's blk_register_queue() from add_disk()-time until > dm_setup_md_queue() by setting QUEUE_FLAG_DEFER_REG in al

Re: [dm-devel] [for-4.16 PATCH v3 2/3] block: allow gendisk's request_queue registration to be deferred

2018-01-10 Thread Ming Lei
On Wed, Jan 10, 2018 at 09:12:55PM -0500, Mike Snitzer wrote: > Since I can remember DM has forced the block layer to allow the > allocation and initialization of the request_queue to be distinct > operations. Reason for this is block/genhd.c:add_disk() has requires > that the request_queue (and a

Re: [dm-devel] [for-4.16 PATCH v3 1/3] block: only bdi_unregister() in del_gendisk() if !GENHD_FL_HIDDEN

2018-01-10 Thread Ming Lei
On Wed, Jan 10, 2018 at 09:12:54PM -0500, Mike Snitzer wrote: > device_add_disk() will only call bdi_register_owner() if > !GENHD_FL_HIDDEN, so it follows that del_gendisk() should only call > bdi_unregister() if !GENHD_FL_HIDDEN. > > Found with code inspection. bdi_unregister() won't do any harm

[dm-devel] [for-4.16 PATCH v3 0/3] block/dm: allow DM to defer blk_register_queue() until ready

2018-01-10 Thread Mike Snitzer
Hi Jens, I eliminated my implementation that set disk->queue = NULL before calling add_disk(). As we discussed it left way too much potential for NULL pointer crashes and I agree it was too fragile. This v3's approach is much simpler. It adjusts block core so that blk_register_queue() can be de

[dm-devel] [for-4.16 PATCH v3 2/3] block: allow gendisk's request_queue registration to be deferred

2018-01-10 Thread Mike Snitzer
Since I can remember DM has forced the block layer to allow the allocation and initialization of the request_queue to be distinct operations. Reason for this is block/genhd.c:add_disk() has requires that the request_queue (and associated bdi) be tied to the gendisk before add_disk() is called -- b

[dm-devel] [for-4.16 PATCH v3 3/3] dm: fix awkward and incomplete request_queue initialization

2018-01-10 Thread Mike Snitzer
DM is now no longer prone to having its request_queue be improperly initialized. Summary of changes: - defer DM's blk_register_queue() from add_disk()-time until dm_setup_md_queue() by setting QUEUE_FLAG_DEFER_REG in alloc_dev(). - dm_setup_md_queue() is updated to fully initialize DM's reques

[dm-devel] [for-4.16 PATCH v3 1/3] block: only bdi_unregister() in del_gendisk() if !GENHD_FL_HIDDEN

2018-01-10 Thread Mike Snitzer
device_add_disk() will only call bdi_register_owner() if !GENHD_FL_HIDDEN, so it follows that del_gendisk() should only call bdi_unregister() if !GENHD_FL_HIDDEN. Found with code inspection. bdi_unregister() won't do any harm if bdi_register_owner() wasn't used but best to avoid the unnecessary c

Re: [dm-devel] [PATCHv2 0/5] nvme/dm failover unification

2018-01-10 Thread Jens Axboe
On 1/10/18 1:28 AM, Christoph Hellwig wrote: > The whole series looks fine to me: > > Reviewed-by: Christoph Hellwig > > Jens, do you want me to apply this to the nvme tree, or pick it up > directly? I queued it up, thanks. -- Jens Axboe -- dm-devel mailing list dm-devel@redhat.com https://w

Re: [dm-devel] [PATCHv2] Fill NVMe specific path info

2018-01-10 Thread Xose Vazquez Perez
On 02/21/2017 11:12 PM, Keith Busch wrote: > diff --git a/libmultipath/hwtable.c b/libmultipath/hwtable.c > index f5a5f7b..8409261 100644 > --- a/libmultipath/hwtable.c > +++ b/libmultipath/hwtable.c > @@ -1073,6 +1073,16 @@ static struct hwentry default_hw[] = { > .prio_name = P

Re: [dm-devel] dm-integrity: don't store cipher request on the stack (was: [QUESTION] hash import and request initialization)

2018-01-10 Thread Mike Snitzer
On Wed, Jan 10 2018 at 9:32am -0500, Mikulas Patocka wrote: > > > On Wed, 27 Dec 2017, Herbert Xu wrote: > > > On Tue, Dec 26, 2017 at 02:21:53PM +0200, Gilad Ben-Yossef wrote: > > > > > > See how SKCIPHER_REQUEST_ON_STACK is being used with an asymmetric > > > skcipher > > > in drivers/md/

[dm-devel] [PATCH] dm-integrity: don't store cipher request on the stack (was: [QUESTION] hash import and request initialization)

2018-01-10 Thread Mikulas Patocka
On Wed, 27 Dec 2017, Herbert Xu wrote: > On Tue, Dec 26, 2017 at 02:21:53PM +0200, Gilad Ben-Yossef wrote: > > > > See how SKCIPHER_REQUEST_ON_STACK is being used with an asymmetric skcipher > > in drivers/md/dm-integrity.c > > That's just broken. SKCIPHER_REQUEST_ON_STACK is only meant for >

Re: [dm-devel] [for-4.16 PATCH v2 2/3] block: cope with gendisk's 'queue' being added later

2018-01-10 Thread Mike Snitzer
On Wed, Jan 10 2018 at 2:55am -0500, Ming Lei wrote: > On Wed, Jan 10, 2018 at 12:21 PM, Mike Snitzer wrote: > > On Tue, Jan 09 2018 at 10:46pm -0500, > > Ming Lei wrote: > > > >> Another related issue is that blk-mq debugfs can't work yet for dm-mpath. > > > > Not sure how that is a related i

Re: [dm-devel] [for-4.16 PATCH v2 2/3] block: cope with gendisk's 'queue' being added later

2018-01-10 Thread Hannes Reinecke
On 01/10/2018 09:32 AM, Christoph Hellwig wrote: > On Tue, Jan 09, 2018 at 09:41:03PM -0500, Mike Snitzer wrote: >> Since I can remember DM has forced the block layer to allow the >> allocation and initialization of the request_queue to be distinct >> operations. Reason for this was block/genhd.c:

Re: [dm-devel] [for-4.16 PATCH v2 2/3] block: cope with gendisk's 'queue' being added later

2018-01-10 Thread Christoph Hellwig
On Tue, Jan 09, 2018 at 09:41:03PM -0500, Mike Snitzer wrote: > Since I can remember DM has forced the block layer to allow the > allocation and initialization of the request_queue to be distinct > operations. Reason for this was block/genhd.c:add_disk() has required > that the request_queue (and

Re: [dm-devel] [for-4.16 PATCH v2 1/3] block: only bdi_unregister() in del_gendisk() if !GENHD_FL_HIDDEN

2018-01-10 Thread Christoph Hellwig
It's in fact completely harmless :) But not even calling it obviously is just as fine. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCHv2 0/5] nvme/dm failover unification

2018-01-10 Thread Christoph Hellwig
The whole series looks fine to me: Reviewed-by: Christoph Hellwig Jens, do you want me to apply this to the nvme tree, or pick it up directly? -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCHv2 5/5] dm mpath: Use blk_path_error

2018-01-10 Thread Johannes Thumshirn
Looks good, Reviewed-by: Johannes Thumshirn -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG N

Re: [dm-devel] [PATCHv2 4/5] nvme/multipath: Use blk_path_error

2018-01-10 Thread Johannes Thumshirn
Looks good, Reviewed-by: Johannes Thumshirn -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG N

Re: [dm-devel] [PATCHv2 3/5] block: Provide blk_status_t decoding for path errors

2018-01-10 Thread Johannes Thumshirn
Looks good, Reviewed-by: Johannes Thumshirn -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG N

Re: [dm-devel] [PATCHv2 2/5] nvme/multipath: Consult blk_status_t for failover

2018-01-10 Thread Johannes Thumshirn
Looks good, Reviewed-by: Johannes Thumshirn -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG N