block changes to improve device mapper for 4.16

GIT PULL Mike Snitzer 3 participants 5 messages Jan 15, 2018

Subsystems (2)

Block layer Device mapper

lore

Share & tools

lore

Mike Snitzer

author

2018-01-15 02:59 UTC #0 lore raw

Hi Jens,

I prepared this pull request in the hope that it may help you review and stage these changes for 4.16.

I went over Ming's changes again to refine the headers and code comments for clarity to help ease review and inclussion.

I've done extensive testing of the changes in this pull request in combination with all the dm-4.16 changes I'll be sending to Linus, using this tree (which gets pulled into linux-next, I wanted some coverage):
https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/log/?h=for-next
I'll obviously drop these changes from dm's linux-next once you pick them up.

FYI, this dm-4.16 commit addresses the concerns Bart raised last week against Ming's dm-mpath changes:
https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=dm-4.16&id=7a9d12664a183c4a1e2fa86b0a9206006b95138e

The following changes since commit bf9ae8c5325c0070d0ec81a849bba8d156f65993:

blk-mq: fix bad clear of RQF_MQ_INFLIGHT in blk_mq_ct_ctx_init() (2018-01-14 10:46:24 -0700)

are available in the Git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git tags/for-block-4.16/dm

for you to fetch changes up to 0e94dfab56071a7afd0a9de9b8f6a0535148fb36:

blk-mq: issue request directly for blk_insert_cloned_request (2018-01-14 13:00:02 -0500)

Please pull, thanks!
Mike

----------------------------------------------------------------
- Small correctness fix in del_gendisk() if GENHD_FL_HIDDEN is used.

- Cleanup blk_unregister_queue() to more precisely protect against concurrent sysfs changes, blk_mq_unregister_dev() now requires caller to hold q->sysfslock (blk_unregister_queue is only caller).

- Introduce add_disk() variant, add_disk_no_queue_reg(), that allows the gendisk to be registered but the associated disk->queue's blk_register_queue() is left for the driver to do once its request_queue is fully initialized. Fixes long-standing DM request_queue initialization issues.

- Ming's blk-mq improvements to blk_insert_cloned_request(), which is used exclusively by request-based DM's blk-mq mode, that enable substantial dm-mpath sequential IO performance improvements.

----------------------------------------------------------------
Mike Snitzer (4):
block: only bdi_unregister() in del_gendisk() if !GENHD_FL_HIDDEN
block: properly protect the 'queue' kobj in blk_unregister_queue
block: allow gendisk's request_queue registration to be deferred
dm: fix incomplete request_queue initialization

Ming Lei (3):
blk-mq: move actual issue into __blk_mq_issue_req helper
blk-mq: return dispatch result from blk_mq_try_issue_directly
blk-mq: issue request directly for blk_insert_cloned_request

 block/blk-core.c      |  3 +-
 block/blk-mq-sysfs.c  |  9 +-----
 block/blk-mq.c        | 86 +++++++++++++++++++++++++++++++++++++++------------
 block/blk-mq.h        |  3 ++
 block/blk-sysfs.c     | 18 +++++++++--
 block/genhd.c         | 23 +++++++++++---
 drivers/md/dm-rq.c    | 28 ++++++++++-------
 drivers/md/dm.c       | 11 ++++++-
 include/linux/genhd.h |  5 +++
 9 files changed, 136 insertions(+), 50 deletions(-)

Jens Axboe

maintainer

2018-01-15 15:33 UTC #1 lore raw

Quoted 58 lines · Mike Snitzer

Hi Jens,
I prepared this pull request in the hope that it may help you review and stage these changes for 4.16.
I went over Ming's changes again to refine the headers and code comments for clarity to help ease review and inclussion.
I've done extensive testing of the changes in this pull request in combination with all the dm-4.16 changes I'll be sending to Linus, using this tree (which gets pulled into linux-next, I wanted some coverage):
https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/log/?h=for-next
I'll obviously drop these changes from dm's linux-next once you pick them up.
FYI, this dm-4.16 commit addresses the concerns Bart raised last week against Ming's dm-mpath changes:
https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=dm-4.16&id=7a9d12664a183c4a1e2fa86b0a9206006b95138e
The following changes since commit bf9ae8c5325c0070d0ec81a849bba8d156f65993:
blk-mq: fix bad clear of RQF_MQ_INFLIGHT in blk_mq_ct_ctx_init() (2018-01-14 10:46:24 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git tags/for-block-4.16/dm
for you to fetch changes up to 0e94dfab56071a7afd0a9de9b8f6a0535148fb36:
blk-mq: issue request directly for blk_insert_cloned_request (2018-01-14 13:00:02 -0500)
Please pull, thanks!
Mike
----------------------------------------------------------------
- Small correctness fix in del_gendisk() if GENHD_FL_HIDDEN is used.
- Cleanup blk_unregister_queue() to more precisely protect against concurrent sysfs changes, blk_mq_unregister_dev() now requires caller to hold q->sysfslock (blk_unregister_queue is only caller).
- Introduce add_disk() variant, add_disk_no_queue_reg(), that allows the gendisk to be registered but the associated disk->queue's blk_register_queue() is left for the driver to do once its request_queue is fully initialized. Fixes long-standing DM request_queue initialization issues.
- Ming's blk-mq improvements to blk_insert_cloned_request(), which is used exclusively by request-based DM's blk-mq mode, that enable substantial dm-mpath sequential IO performance improvements.
----------------------------------------------------------------
Mike Snitzer (4):
block: only bdi_unregister() in del_gendisk() if !GENHD_FL_HIDDEN
block: properly protect the 'queue' kobj in blk_unregister_queue
block: allow gendisk's request_queue registration to be deferred
dm: fix incomplete request_queue initialization

Quoted 2 lines

Ming Lei (3):
blk-mq: move actual issue into __blk_mq_issue_req helper

I don't like this patch at all - it's a 10 line function (if that) that ends up with three outputs, two of them hidden in passed in pointers. On top of that, a function that is named __blk_mq_issue_req() and returns bool, you would logically expect a 'true' return to mean that it succeeded. This is the opposite.

Not strongly opposed to the rest.

Signature · 1 line

Jens Axboe

Mike Snitzer

author

2018-01-15 15:52 UTC #2 lore raw

Quoted 1 line · Jens Axboe

On 1/14/18 7:59 PM, Mike Snitzer wrote:

...

Quoted 10 lines

Quoted 2 lines
Ming Lei (3):
blk-mq: move actual issue into __blk_mq_issue_req helper
I don't like this patch at all - it's a 10 line function (if that) that ends up with three outputs, two of them hidden in passed in pointers. On top of that, a function that is named __blk_mq_issue_req() and returns bool, you would logically expect a 'true' return to mean that it succeeded. This is the opposite.
Not strongly opposed to the rest.

OK, I'll have a closer look at how to clean it up (and also get with Ming).

In the meantime, you can either cherry-pick my first 4 patches or feel free to use this to pull them in:

The following changes since commit bf9ae8c5325c0070d0ec81a849bba8d156f65993:

blk-mq: fix bad clear of RQF_MQ_INFLIGHT in blk_mq_ct_ctx_init() (2018-01-14 10:46:24 -0700)

are available in the Git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git tags/for-block-4.16/dm-changes-1

for you to fetch changes up to d0cc27da2b04351b3cb52afeb99ceca7b9f91f3b:

dm: fix incomplete request_queue initialization (2018-01-14 12:59:59 -0500)

----------------------------------------------------------------
- Small correctness fix in del_gendisk() if GENHD_FL_HIDDEN is used.

 block/blk-mq-sysfs.c  |  9 +--------
 block/blk-sysfs.c     | 18 +++++++++++++++---
 block/genhd.c         | 23 +++++++++++++++++++----
 drivers/md/dm-rq.c    |  9 ---------
 drivers/md/dm.c       | 11 ++++++++++-
 include/linux/genhd.h |  5 +++++
 6 files changed, 50 insertions(+), 25 deletions(-)

Jens Axboe

maintainer

2018-01-15 15:54 UTC #3 lore raw

Quoted 14 lines · Mike Snitzer

Quoted 1 line · Jens Axboe
On 1/14/18 7:59 PM, Mike Snitzer wrote:
...
Quoted 9 lines
Quoted 2 lines
Ming Lei (3):
blk-mq: move actual issue into __blk_mq_issue_req helper
I don't like this patch at all - it's a 10 line function (if that) that ends up with three outputs, two of them hidden in passed in pointers. On top of that, a function that is named __blk_mq_issue_req() and returns bool, you would logically expect a 'true' return to mean that it succeeded. This is the opposite.

Quoted 7 lines

Quoted 1 line
Not strongly opposed to the rest.
OK, I'll have a closer look at how to clean it up (and also get with Ming).
In the meantime, you can either cherry-pick my first 4 patches or feel free to use this to pull them in:

I took 1-3 initially, added 4 now as well.

Signature · 1 line

Jens Axboe

Ming Lei

2018-01-15 17:00 UTC #4 lore raw

In reply to Jens Axboe

Quoted 59 lines · Jens Axboe

Quoted 58 lines · Mike Snitzer
Hi Jens,
I prepared this pull request in the hope that it may help you review and stage these changes for 4.16.
I went over Ming's changes again to refine the headers and code comments for clarity to help ease review and inclussion.
I've done extensive testing of the changes in this pull request in combination with all the dm-4.16 changes I'll be sending to Linus, using this tree (which gets pulled into linux-next, I wanted some coverage):
https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/log/?h=for-next
I'll obviously drop these changes from dm's linux-next once you pick them up.
FYI, this dm-4.16 commit addresses the concerns Bart raised last week against Ming's dm-mpath changes:
https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=dm-4.16&id=7a9d12664a183c4a1e2fa86b0a9206006b95138e
The following changes since commit bf9ae8c5325c0070d0ec81a849bba8d156f65993:
blk-mq: fix bad clear of RQF_MQ_INFLIGHT in blk_mq_ct_ctx_init() (2018-01-14 10:46:24 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git tags/for-block-4.16/dm
for you to fetch changes up to 0e94dfab56071a7afd0a9de9b8f6a0535148fb36:
blk-mq: issue request directly for blk_insert_cloned_request (2018-01-14 13:00:02 -0500)
Please pull, thanks!
Mike
----------------------------------------------------------------
- Small correctness fix in del_gendisk() if GENHD_FL_HIDDEN is used.
- Cleanup blk_unregister_queue() to more precisely protect against concurrent sysfs changes, blk_mq_unregister_dev() now requires caller to hold q->sysfslock (blk_unregister_queue is only caller).
- Introduce add_disk() variant, add_disk_no_queue_reg(), that allows the gendisk to be registered but the associated disk->queue's blk_register_queue() is left for the driver to do once its request_queue is fully initialized. Fixes long-standing DM request_queue initialization issues.
- Ming's blk-mq improvements to blk_insert_cloned_request(), which is used exclusively by request-based DM's blk-mq mode, that enable substantial dm-mpath sequential IO performance improvements.
----------------------------------------------------------------
Mike Snitzer (4):
block: only bdi_unregister() in del_gendisk() if !GENHD_FL_HIDDEN
block: properly protect the 'queue' kobj in blk_unregister_queue
block: allow gendisk's request_queue registration to be deferred
dm: fix incomplete request_queue initialization

Quoted 8 lines

Quoted 2 lines
Ming Lei (3):
blk-mq: move actual issue into __blk_mq_issue_req helper
I don't like this patch at all - it's a 10 line function (if that) that ends up with three outputs, two of them hidden in passed in pointers. On top of that, a function that is named __blk_mq_issue_req() and returns bool, you would logically expect a 'true' return to mean that it succeeded. This is the opposite.

OK, __blk_mq_issue_req() has been cleaned up in V4, please consider it for V4.16.

Thanks,
Ming