remove disk_name()

remove disk_name()

[Christoph Hellwig]

Hi Jens,

this series removes the obsolete disk_name helper in favor of using the
%pg format specifier.

Diffstat:
 blk-settings.c    |   12 +++---------
 blk.h             |    1 -
 genhd.c           |   35 +++++++++++++++--------------------
 partitions/core.c |    2 +-
 4 files changed, 19 insertions(+), 31 deletions(-)

[PATCH 1/6] block: reduce stack usage in diskstats_show

[Christoph Hellwig]

From: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>

I have compiled the kernel with a cross compiler "hppa-linux-gnu-" v9.3.0
on x86-64 host machine. I got the following warning:

block/genhd.c: In function ‘diskstats_show’:
block/genhd.c:1227:1: warning: the frame size of 1688 bytes is larger
than 1280 bytes [-Wframe-larger-than=]
 1227  |  }

By Reduced the stack footprint by using the %pg printk specifier instead
of disk_name to remove the need for the on-stack buffer.

Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/genhd.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index af4d2ab4a633..80ad85822419 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1117,7 +1117,6 @@ static int diskstats_show(struct seq_file *seqf, void *v)
 {
 	struct gendisk *gp = v;
 	struct block_device *hd;
-	char buf[BDEVNAME_SIZE];
 	unsigned int inflight;
 	struct disk_stats stat;
 	unsigned long idx;
@@ -1140,15 +1139,14 @@ static int diskstats_show(struct seq_file *seqf, void *v)
 		else
 			inflight = part_in_flight(hd);
 
-		seq_printf(seqf, "%4d %7d %s "
+		seq_printf(seqf, "%4d %7d %pg "
 			   "%lu %lu %lu %u "
 			   "%lu %lu %lu %u "
 			   "%u %u %u "
 			   "%lu %lu %lu %u "
 			   "%lu %u"
 			   "\n",
-			   MAJOR(hd->bd_dev), MINOR(hd->bd_dev),
-			   disk_name(gp, hd->bd_partno, buf),
+			   MAJOR(hd->bd_dev), MINOR(hd->bd_dev), hd,
 			   stat.ios[STAT_READ],
 			   stat.merges[STAT_READ],
 			   stat.sectors[STAT_READ],
-- 
2.30.2

Re: [PATCH 1/6] block: reduce stack usage in diskstats_show

[Hannes Reinecke]

On 7/27/21 8:25 AM, Christoph Hellwig wrote:
> From: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
> 
> I have compiled the kernel with a cross compiler "hppa-linux-gnu-" v9.3.0
> on x86-64 host machine. I got the following warning:
> 
> block/genhd.c: In function ‘diskstats_show’:
> block/genhd.c:1227:1: warning: the frame size of 1688 bytes is larger
> than 1280 bytes [-Wframe-larger-than=]
>  1227  |  }
> 
> By Reduced the stack footprint by using the %pg printk specifier instead
> of disk_name to remove the need for the on-stack buffer.
> 
> Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  block/genhd.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		           Kernel Storage Architect
hare@suse.de			                  +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer

[PATCH 2/6] block: use the %pg format specifier in printk_all_partitions

[Christoph Hellwig]

Simplify printing the partition name by using the %pg format specifier
that is equivalent to a bdevname call.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/genhd.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index 80ad85822419..e07b1b028875 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -683,7 +683,6 @@ void __init printk_all_partitions(void)
 	while ((dev = class_dev_iter_next(&iter))) {
 		struct gendisk *disk = dev_to_disk(dev);
 		struct block_device *part;
-		char name_buf[BDEVNAME_SIZE];
 		char devt_buf[BDEVT_SIZE];
 		unsigned long idx;
 
@@ -703,11 +702,10 @@ void __init printk_all_partitions(void)
 		xa_for_each(&disk->part_tbl, idx, part) {
 			if (!bdev_nr_sectors(part))
 				continue;
-			printk("%s%s %10llu %s %s",
+			printk("%s%s %10llu %pg %s",
 			       bdev_is_partition(part) ? "  " : "",
 			       bdevt_str(part->bd_dev, devt_buf),
-			       bdev_nr_sectors(part) >> 1,
-			       disk_name(disk, part->bd_partno, name_buf),
+			       bdev_nr_sectors(part) >> 1, part,
 			       part->bd_meta_info ?
 					part->bd_meta_info->uuid : "");
 			if (bdev_is_partition(part))
-- 
2.30.2

[PATCH 3/6] block: use the %pg format specifier in show_partition

[Christoph Hellwig]

Simplify printing the partition name by using the %pg format specifier
that is equivalent to a bdevname call.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/genhd.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index e07b1b028875..ffdbdefdea7b 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -783,7 +783,6 @@ static int show_partition(struct seq_file *seqf, void *v)
 	struct gendisk *sgp = v;
 	struct block_device *part;
 	unsigned long idx;
-	char buf[BDEVNAME_SIZE];
 
 	/* Don't show non-partitionable removeable devices or empty devices */
 	if (!get_capacity(sgp) || (!disk_max_parts(sgp) &&
@@ -796,10 +795,9 @@ static int show_partition(struct seq_file *seqf, void *v)
 	xa_for_each(&sgp->part_tbl, idx, part) {
 		if (!bdev_nr_sectors(part))
 			continue;
-		seq_printf(seqf, "%4d  %7d %10llu %s\n",
+		seq_printf(seqf, "%4d  %7d %10llu %pg\n",
 			   MAJOR(part->bd_dev), MINOR(part->bd_dev),
-			   bdev_nr_sectors(part) >> 1,
-			   disk_name(sgp, part->bd_partno, buf));
+			   bdev_nr_sectors(part) >> 1, part);
 	}
 	rcu_read_unlock();
 	return 0;
-- 
2.30.2

[PATCH 4/6] block: simplify printing the device names disk_stack_limits

[Christoph Hellwig]

Printk ->disk_name directly for the disk and use the %pg format specifier
for the block device, which is equivalent to a bdevname call.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-settings.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/block/blk-settings.c b/block/blk-settings.c
index 902c40d67120..109012719aa0 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -661,15 +661,9 @@ void disk_stack_limits(struct gendisk *disk, struct block_device *bdev,
 	struct request_queue *t = disk->queue;
 
 	if (blk_stack_limits(&t->limits, &bdev_get_queue(bdev)->limits,
-			get_start_sect(bdev) + (offset >> 9)) < 0) {
-		char top[BDEVNAME_SIZE], bottom[BDEVNAME_SIZE];
-
-		disk_name(disk, 0, top);
-		bdevname(bdev, bottom);
-
-		printk(KERN_NOTICE "%s: Warning: Device %s is misaligned\n",
-		       top, bottom);
-	}
+			get_start_sect(bdev) + (offset >> 9)) < 0)
+		pr_notice("%s: Warning: Device %pg is misaligned\n",
+			disk->disk_name, bdev);
 
 	blk_queue_update_readahead(disk->queue);
 }
-- 
2.30.2

[PATCH 5/6] block: simplify disk name formatting in check_partition

[Christoph Hellwig]

disk_name for partition 0 just copies out the disk_name field.  Replace
the call to disk_name with a %s format specifier.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/partitions/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/partitions/core.c b/block/partitions/core.c
index 4230d4f71879..48e001aa1930 100644
--- a/block/partitions/core.c
+++ b/block/partitions/core.c
@@ -136,7 +136,7 @@ static struct parsed_partitions *check_partition(struct gendisk *hd)
 	state->pp_buf[0] = '\0';
 
 	state->bdev = hd->part0;
-	disk_name(hd, 0, state->name);
+	snprintf(state->name, BDEVNAME_SIZE, "%s", hd->disk_name);
 	snprintf(state->pp_buf, PAGE_SIZE, " %s:", state->name);
 	if (isdigit(state->name[strlen(state->name)-1]))
 		sprintf(state->name, "p");
-- 
2.30.2

[PATCH 6/6] block: remove disk_name()

[Christoph Hellwig]

Remove the disk_name function now that all users are gone.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk.h   |  1 -
 block/genhd.c | 17 +++++++++--------
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/block/blk.h b/block/blk.h
index 4b885c0f6708..56f33fbcde59 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -344,7 +344,6 @@ static inline void blk_queue_clear_zone_settings(struct request_queue *q) {}
 
 int blk_alloc_ext_minor(void);
 void blk_free_ext_minor(unsigned int minor);
-char *disk_name(struct gendisk *hd, int partno, char *buf);
 #define ADDPART_FLAG_NONE	0
 #define ADDPART_FLAG_RAID	1
 #define ADDPART_FLAG_WHOLEDISK	2
diff --git a/block/genhd.c b/block/genhd.c
index ffdbdefdea7b..966e32fecfa4 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -78,11 +78,17 @@ bool set_capacity_and_notify(struct gendisk *disk, sector_t size)
 EXPORT_SYMBOL_GPL(set_capacity_and_notify);
 
 /*
- * Format the device name of the indicated disk into the supplied buffer and
- * return a pointer to that same buffer for convenience.
+ * Format the device name of the indicated block device into the supplied buffer
+ * and return a pointer to that same buffer for convenience.
+ *
+ * Note: do not use this in new code, use the %pg specifier to sprintf and
+ * printk insted.
  */
-char *disk_name(struct gendisk *hd, int partno, char *buf)
+const char *bdevname(struct block_device *bdev, char *buf)
 {
+	struct gendisk *hd = bdev->bd_disk;
+	int partno = bdev->bd_partno;
+
 	if (!partno)
 		snprintf(buf, BDEVNAME_SIZE, "%s", hd->disk_name);
 	else if (isdigit(hd->disk_name[strlen(hd->disk_name)-1]))
@@ -92,11 +98,6 @@ char *disk_name(struct gendisk *hd, int partno, char *buf)
 
 	return buf;
 }
-
-const char *bdevname(struct block_device *bdev, char *buf)
-{
-	return disk_name(bdev->bd_disk, bdev->bd_partno, buf);
-}
 EXPORT_SYMBOL(bdevname);
 
 static void part_stat_read_all(struct block_device *part,
-- 
2.30.2

Re: remove disk_name()

[Martin K. Petersen]

Christoph,

> this series removes the obsolete disk_name helper in favor of using
> the %pg format specifier.

Looks good to me.

Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: remove disk_name()

[Jens Axboe]

On 7/27/21 12:25 AM, Christoph Hellwig wrote:
> Hi Jens,
> 
> this series removes the obsolete disk_name helper in favor of using the
> %pg format specifier.

Applied, thanks.

-- 
Jens Axboe