linux-block.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Nilay Shroff <nilay@linux.ibm.com>
To: linux-block@vger.kernel.org
Cc: hch@lst.de, ming.lei@redhat.com, dlemoal@kernel.org,
	axboe@kernel.dk, gjoyce@ibm.com
Subject: [RFC PATCHv3 0/2] block: remove q->sysfs_dir_lock and fix race updating nr_hw_queue
Date: Tue, 28 Jan 2025 20:04:12 +0530	[thread overview]
Message-ID: <20250128143436.874357-1-nilay@linux.ibm.com> (raw)

Hi,

There're two patches in this patchest.
The first patch removes redundant q->sysfs_dir_lock.
The second patch fixes nr_hw_queue update racing with disk addition/
removal.

In the current implementation we use q->sysfs_dir_lock for protecting
kobject addition/deletion while we register/unregister blk-mq with
sysfs. However the sysfs/kernfs internal implementation already protects
against the simultaneous addtion/deletion of kobjects. So in that sense
use of q->sysfs_dir_lock appears redundant.

Furthermore, there're few other callsites in the current code where we
use q->sysfs_dir_lock along with q->sysfs_lock while addition/deletion
of independent access ranges for a disk under sysfs. Please refer, disk_
register_independent_access_ranges() and disk_unregister_independent_
access_ranges(). Here as well we could easily remove use of q->sysfs_dir_
lock.

The only thing which q->syfs_dir_lock appears to protect is the use of
variable q->mq_sysfs_init_done. However this could be solved by
converting q->mq_sysfs_init_done to an atomic variable.

In past few days, we have seen many lockdep splat in block layer and
getting rid of this one might help reduce some contention as well we
need to worry less about lock ordering wrt to q->sysfs_dir_lock. The
first patch helps fix this.

The second patch addresses a potential race between nr_hw_queue update
and disk addition/removal. The __blk_mq_update_nr_hw_queues function
removes and then adds hctx sysfs files. Similarly the disk addition/
removal code also adds or remove the hctx sysfs files. So it's quite
possible that disk addition/removal code could race with __blk_mq_update_
nr_hw_queues() while it adds/deletes hctx sysfs files. Apparently,
__blk_mq_update_nr_hw_queues() holds q->tag_list_lock while it runs,
and so to avoid race between __blk_mq_update_nr_hw_queues() and disk
addition/removal code, we should hold the same q->tag_list_lock while
we add/delete hctx sysfs files while registering/uregistering disk queue.
So the second patch in the series helps fix this race condition which may
manifests while we add/remove hctx sysfs files.

I ran blktests and xfstests with lockdep configured to ensure that above 
changes don't generate lockdep spalt.

Nilay Shroff (2):
  block: get rid of request queue ->sysfs_dir_lock
  block: fix nr_hw_queue update racing with disk addition/removal

Changes from v2:
  - few nitpicking fixes in the second patch (hch)
  - Link to v2: https://lore.kernel.org/all/20250123174124.24554-1-nilay@linux.ibm.com/

Changes from v1:
  - remove q->sysfs_init_done and replace it with registered queue
    flag (hch)
  - fix nr_hw_queue update racing with disk addition/removal (hch)
  - Link to v1: https://lore.kernel.org/all/20250120130413.789737-1-nilay@linux.ibm.com/

Nilay Shroff (2):
  block: get rid of request queue ->sysfs_dir_lock
  block: fix nr_hw_queue update racing with disk addition/removal

 block/blk-core.c       |  1 -
 block/blk-ia-ranges.c  |  4 ----
 block/blk-mq-sysfs.c   | 40 ++++++++++++++--------------------------
 block/blk-sysfs.c      |  5 -----
 include/linux/blkdev.h |  3 ---
 5 files changed, 14 insertions(+), 39 deletions(-)

-- 
2.47.1


             reply	other threads:[~2025-01-28 14:34 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-28 14:34 Nilay Shroff [this message]
2025-01-28 14:34 ` [RFC PATCHv3 1/2] block: get rid of request queue ->sysfs_dir_lock Nilay Shroff
2025-01-29  6:47   ` Hannes Reinecke
2025-01-28 14:34 ` [RFC PATCHv3 2/2] block: fix nr_hw_queue update racing with disk addition/removal Nilay Shroff
2025-01-29  6:08   ` Christoph Hellwig
2025-01-29  6:50   ` Hannes Reinecke
2025-01-29 14:17 ` [RFC PATCHv3 0/2] block: remove q->sysfs_dir_lock and fix race updating nr_hw_queue Jens Axboe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250128143436.874357-1-nilay@linux.ibm.com \
    --to=nilay@linux.ibm.com \
    --cc=axboe@kernel.dk \
    --cc=dlemoal@kernel.org \
    --cc=gjoyce@ibm.com \
    --cc=hch@lst.de \
    --cc=linux-block@vger.kernel.org \
    --cc=ming.lei@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).