All of lore.kernel.org
 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: [PATCHv2 2/6] blk-sysfs: acquire q->limits_lock while reading attributes
Date: Tue, 18 Feb 2025 13:58:55 +0530	[thread overview]
Message-ID: <20250218082908.265283-3-nilay@linux.ibm.com> (raw)
In-Reply-To: <20250218082908.265283-1-nilay@linux.ibm.com>

There're few sysfs attributes(RW) whose store method is protected with
q->limits_lock, however the corresponding show method of such attributes
runs with holding q->sysfs_lock and that doesn't make sense. Hence
update the show method of these sysfs attributes so that reading of
these attributes acquire q->limits_lock instead of q->sysfs_lock.

Similarly, there're few sysfs attributes(RO) whose show method is
currently protected with q->sysfs_lock however updates to these 
attributes could occur using atomic limit update APIs such as queue_
limits_start_update() and queue_limits_commit_update() which run 
holding q->limits_lock. So that means that reading such attributes 
holding q->sysfs_lock doesn't make sense. Hence update the show method 
of these sysfs attributes(RO) such that show method of these attributes 
runs with holding q->limits_lock instead of q->sysfs_lock. 

We have defined a new macro QUEUE_LIM_RO_ENTRY() which uses new show_
limit() method and it runs holding q->limits_lock. All sysfs existing 
attributes(RO) which needs protection using q->limits_lock while 
reading the entry have been now moved to use this new macro for 
attribute initialization.

Similarly, the existing QUEUE_LIM_RW_ENTRY() is updated to use new
show_limit() method for reading attributes instead of existing show()
method. As show_limit() runs holding q->limits_lock the existing sysfs
attributes(RW) whose read/show method needs protection using q->limits_
lock are now inherently protected. 

Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
---
 block/blk-sysfs.c | 100 ++++++++++++++++++++++++++--------------------
 1 file changed, 57 insertions(+), 43 deletions(-)

diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index 0c9be7c7ecc1..7e22ec96f2b3 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -24,6 +24,7 @@ struct queue_sysfs_entry {
 	struct attribute attr;
 	ssize_t (*show)(struct gendisk *disk, char *page);
 	ssize_t (*show_nolock)(struct gendisk *disk, char *page);
+	ssize_t (*show_limit)(struct gendisk *disk, char *page);
 
 	ssize_t (*store)(struct gendisk *disk, const char *page, size_t count);
 	ssize_t (*store_nolock)(struct gendisk *disk,
@@ -436,10 +437,16 @@ static struct queue_sysfs_entry _prefix##_entry = {	\
 	.store	= _prefix##_store,			\
 };
 
+#define QUEUE_LIM_RO_ENTRY(_prefix, _name)			\
+static struct queue_sysfs_entry _prefix##_entry = {		\
+	.attr		= { .name = _name, .mode = 0644 },	\
+	.show_limit	= _prefix##_show,			\
+}
+
 #define QUEUE_LIM_RW_ENTRY(_prefix, _name)			\
-static struct queue_sysfs_entry _prefix##_entry = {	\
+static struct queue_sysfs_entry _prefix##_entry = {		\
 	.attr		= { .name = _name, .mode = 0644 },	\
-	.show		= _prefix##_show,			\
+	.show_limit	= _prefix##_show,			\
 	.store_limit	= _prefix##_store,			\
 }
 
@@ -454,39 +461,39 @@ static struct queue_sysfs_entry _prefix##_entry = {		\
 QUEUE_RW_ENTRY(queue_requests, "nr_requests");
 QUEUE_RW_ENTRY(queue_ra, "read_ahead_kb");
 QUEUE_LIM_RW_ENTRY(queue_max_sectors, "max_sectors_kb");
-QUEUE_RO_ENTRY(queue_max_hw_sectors, "max_hw_sectors_kb");
-QUEUE_RO_ENTRY(queue_max_segments, "max_segments");
-QUEUE_RO_ENTRY(queue_max_integrity_segments, "max_integrity_segments");
-QUEUE_RO_ENTRY(queue_max_segment_size, "max_segment_size");
+QUEUE_LIM_RO_ENTRY(queue_max_hw_sectors, "max_hw_sectors_kb");
+QUEUE_LIM_RO_ENTRY(queue_max_segments, "max_segments");
+QUEUE_LIM_RO_ENTRY(queue_max_integrity_segments, "max_integrity_segments");
+QUEUE_LIM_RO_ENTRY(queue_max_segment_size, "max_segment_size");
 QUEUE_RW_LOAD_MODULE_ENTRY(elv_iosched, "scheduler");
 
-QUEUE_RO_ENTRY(queue_logical_block_size, "logical_block_size");
-QUEUE_RO_ENTRY(queue_physical_block_size, "physical_block_size");
-QUEUE_RO_ENTRY(queue_chunk_sectors, "chunk_sectors");
-QUEUE_RO_ENTRY(queue_io_min, "minimum_io_size");
-QUEUE_RO_ENTRY(queue_io_opt, "optimal_io_size");
+QUEUE_LIM_RO_ENTRY(queue_logical_block_size, "logical_block_size");
+QUEUE_LIM_RO_ENTRY(queue_physical_block_size, "physical_block_size");
+QUEUE_LIM_RO_ENTRY(queue_chunk_sectors, "chunk_sectors");
+QUEUE_LIM_RO_ENTRY(queue_io_min, "minimum_io_size");
+QUEUE_LIM_RO_ENTRY(queue_io_opt, "optimal_io_size");
 
-QUEUE_RO_ENTRY(queue_max_discard_segments, "max_discard_segments");
-QUEUE_RO_ENTRY(queue_discard_granularity, "discard_granularity");
-QUEUE_RO_ENTRY(queue_max_hw_discard_sectors, "discard_max_hw_bytes");
+QUEUE_LIM_RO_ENTRY(queue_max_discard_segments, "max_discard_segments");
+QUEUE_LIM_RO_ENTRY(queue_discard_granularity, "discard_granularity");
+QUEUE_LIM_RO_ENTRY(queue_max_hw_discard_sectors, "discard_max_hw_bytes");
 QUEUE_LIM_RW_ENTRY(queue_max_discard_sectors, "discard_max_bytes");
 QUEUE_RO_ENTRY_NOLOCK(queue_discard_zeroes_data, "discard_zeroes_data");
 
-QUEUE_RO_ENTRY(queue_atomic_write_max_sectors, "atomic_write_max_bytes");
-QUEUE_RO_ENTRY(queue_atomic_write_boundary_sectors,
+QUEUE_LIM_RO_ENTRY(queue_atomic_write_max_sectors, "atomic_write_max_bytes");
+QUEUE_LIM_RO_ENTRY(queue_atomic_write_boundary_sectors,
 		"atomic_write_boundary_bytes");
-QUEUE_RO_ENTRY(queue_atomic_write_unit_max, "atomic_write_unit_max_bytes");
-QUEUE_RO_ENTRY(queue_atomic_write_unit_min, "atomic_write_unit_min_bytes");
+QUEUE_LIM_RO_ENTRY(queue_atomic_write_unit_max, "atomic_write_unit_max_bytes");
+QUEUE_LIM_RO_ENTRY(queue_atomic_write_unit_min, "atomic_write_unit_min_bytes");
 
 QUEUE_RO_ENTRY_NOLOCK(queue_write_same_max, "write_same_max_bytes");
-QUEUE_RO_ENTRY(queue_max_write_zeroes_sectors, "write_zeroes_max_bytes");
-QUEUE_RO_ENTRY(queue_max_zone_append_sectors, "zone_append_max_bytes");
-QUEUE_RO_ENTRY(queue_zone_write_granularity, "zone_write_granularity");
+QUEUE_LIM_RO_ENTRY(queue_max_write_zeroes_sectors, "write_zeroes_max_bytes");
+QUEUE_LIM_RO_ENTRY(queue_max_zone_append_sectors, "zone_append_max_bytes");
+QUEUE_LIM_RO_ENTRY(queue_zone_write_granularity, "zone_write_granularity");
 
-QUEUE_RO_ENTRY(queue_zoned, "zoned");
+QUEUE_LIM_RO_ENTRY(queue_zoned, "zoned");
 QUEUE_RO_ENTRY_NOLOCK(queue_nr_zones, "nr_zones");
-QUEUE_RO_ENTRY(queue_max_open_zones, "max_open_zones");
-QUEUE_RO_ENTRY(queue_max_active_zones, "max_active_zones");
+QUEUE_LIM_RO_ENTRY(queue_max_open_zones, "max_open_zones");
+QUEUE_LIM_RO_ENTRY(queue_max_active_zones, "max_active_zones");
 
 QUEUE_RW_ENTRY_NOLOCK(queue_nomerges, "nomerges");
 QUEUE_LIM_RW_ENTRY(queue_iostats_passthrough, "iostats_passthrough");
@@ -494,16 +501,16 @@ QUEUE_RW_ENTRY_NOLOCK(queue_rq_affinity, "rq_affinity");
 QUEUE_RW_ENTRY_NOLOCK(queue_poll, "io_poll");
 QUEUE_RW_ENTRY_NOLOCK(queue_poll_delay, "io_poll_delay");
 QUEUE_LIM_RW_ENTRY(queue_wc, "write_cache");
-QUEUE_RO_ENTRY(queue_fua, "fua");
-QUEUE_RO_ENTRY(queue_dax, "dax");
+QUEUE_LIM_RO_ENTRY(queue_fua, "fua");
+QUEUE_LIM_RO_ENTRY(queue_dax, "dax");
 QUEUE_RW_ENTRY_NOLOCK(queue_io_timeout, "io_timeout");
-QUEUE_RO_ENTRY(queue_virt_boundary_mask, "virt_boundary_mask");
-QUEUE_RO_ENTRY(queue_dma_alignment, "dma_alignment");
+QUEUE_LIM_RO_ENTRY(queue_virt_boundary_mask, "virt_boundary_mask");
+QUEUE_LIM_RO_ENTRY(queue_dma_alignment, "dma_alignment");
 
 /* legacy alias for logical_block_size: */
 static struct queue_sysfs_entry queue_hw_sector_size_entry = {
-	.attr = {.name = "hw_sector_size", .mode = 0444 },
-	.show = queue_logical_block_size_show,
+	.attr		= {.name = "hw_sector_size", .mode = 0444 },
+	.show_limit	= queue_logical_block_size_show,
 };
 
 QUEUE_LIM_RW_ENTRY(queue_rotational, "rotational");
@@ -589,6 +596,18 @@ static struct attribute *queue_attrs[] = {
 	 * attributes protected with q->sysfs_lock
 	 */
 	&queue_ra_entry.attr,
+
+	/*
+	 * attributes protected with q->limits_lock
+	 */
+	&queue_max_sectors_entry.attr,
+	&queue_max_discard_sectors_entry.attr,
+	&queue_rotational_entry.attr,
+	&queue_iostats_passthrough_entry.attr,
+	&queue_iostats_entry.attr,
+	&queue_stable_writes_entry.attr,
+	&queue_add_random_entry.attr,
+	&queue_wc_entry.attr,
 	&queue_max_hw_sectors_entry.attr,
 	&queue_max_segments_entry.attr,
 	&queue_max_discard_segments_entry.attr,
@@ -617,18 +636,6 @@ static struct attribute *queue_attrs[] = {
 	&queue_virt_boundary_mask_entry.attr,
 	&queue_dma_alignment_entry.attr,
 
-	/*
-	 * attributes protected with q->limits_lock
-	 */
-	&queue_max_sectors_entry.attr,
-	&queue_max_discard_sectors_entry.attr,
-	&queue_rotational_entry.attr,
-	&queue_iostats_passthrough_entry.attr,
-	&queue_iostats_entry.attr,
-	&queue_stable_writes_entry.attr,
-	&queue_add_random_entry.attr,
-	&queue_wc_entry.attr,
-
 	/*
 	 * attributes which don't require locking
 	 */
@@ -709,12 +716,19 @@ queue_attr_show(struct kobject *kobj, struct attribute *attr, char *page)
 	struct gendisk *disk = container_of(kobj, struct gendisk, queue_kobj);
 	ssize_t res;
 
-	if (!entry->show && !entry->show_nolock)
+	if (!entry->show && !entry->show_nolock && !entry->show_limit)
 		return -EIO;
 
 	if (entry->show_nolock)
 		return entry->show_nolock(disk, page);
 
+	if (entry->show_limit) {
+		mutex_lock(&disk->queue->limits_lock);
+		res = entry->show_limit(disk, page);
+		mutex_unlock(&disk->queue->limits_lock);
+		return res;
+	}
+
 	mutex_lock(&disk->queue->sysfs_lock);
 	res = entry->show(disk, page);
 	mutex_unlock(&disk->queue->sysfs_lock);
-- 
2.47.1


  parent reply	other threads:[~2025-02-18  8:29 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-18  8:28 [PATCHv2 0/6] block: fix lock order and remove redundant locking Nilay Shroff
2025-02-18  8:28 ` [PATCHv2 1/6] blk-sysfs: remove q->sysfs_lock for attributes which don't need it Nilay Shroff
2025-02-18  8:46   ` Christoph Hellwig
2025-02-18 11:26     ` Nilay Shroff
2025-02-21 14:02       ` Nilay Shroff
2025-02-22 12:44         ` Ming Lei
2025-02-24 13:09           ` Nilay Shroff
2025-02-24 14:49           ` Christoph Hellwig
2025-02-26 12:09             ` Nilay Shroff
2025-02-24  8:41         ` Hannes Reinecke
2025-02-24 13:12           ` Nilay Shroff
2025-02-18 12:10   ` Ming Lei
2025-02-18 13:11     ` Nilay Shroff
2025-02-18 13:45       ` Ming Lei
2025-02-18 16:29         ` Christoph Hellwig
2025-02-19  3:24           ` Ming Lei
2025-02-19  5:42             ` Christoph Hellwig
2025-02-19  8:34             ` Nilay Shroff
2025-02-19  8:56               ` Nilay Shroff
2025-02-19  9:20                 ` Ming Lei
2025-02-18  8:28 ` Nilay Shroff [this message]
2025-02-18  8:46   ` [PATCHv2 2/6] blk-sysfs: acquire q->limits_lock while reading attributes Christoph Hellwig
2025-02-18  8:28 ` [PATCHv2 3/6] block: Introduce a dedicated lock for protecting queue elevator updates Nilay Shroff
2025-02-18  9:05   ` Christoph Hellwig
2025-02-18 11:14     ` Nilay Shroff
2025-02-18 16:32       ` Christoph Hellwig
2025-02-19  8:41         ` Nilay Shroff
2025-02-18  8:28 ` [PATCHv2 4/6] blk-sysfs: protect nr_requests update using q->elevator_lock Nilay Shroff
2025-02-18  8:28 ` [PATCHv2 5/6] blk-sysfs: protect wbt_lat_usec " Nilay Shroff
2025-02-18  8:28 ` [PATCHv2 6/6] blk-sysfs: protect read_ahead_kb using q->limits_lock Nilay Shroff
2025-02-18  9:12   ` Christoph Hellwig
2025-02-18 11:27     ` Nilay Shroff
2025-02-18  9:21 ` [PATCHv2 0/6] block: fix lock order and remove redundant locking Christoph Hellwig
2025-02-18 12:09   ` Nilay Shroff

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=20250218082908.265283-3-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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.