LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 17/26] nvdimm-pmem: convert to blk_alloc_disk/blk_cleanup_disk
From: Christoph Hellwig @ 2021-05-21  5:51 UTC (permalink / raw)
  To: Jens Axboe, Geert Uytterhoeven, Chris Zankel, Max Filippov,
	Philipp Reisner, Lars Ellenberg, Jim Paris, Joshua Morris,
	Philip Kelleher, Minchan Kim, Nitin Gupta, Matias Bjorling,
	Coly Li, Mike Snitzer, Song Liu, Maxim Levitsky, Alex Dubov,
	Ulf Hansson, Dan Williams, Vishal Verma, Dave Jiang,
	Heiko Carstens, Vasily Gorbik, Christian Borntraeger
  Cc: linux-bcache, linux-xtensa, linux-raid, nvdimm, linux-s390,
	linux-mmc, linux-m68k, linux-nvme, linux-block, dm-devel,
	linuxppc-dev, drbd-dev
In-Reply-To: <20210521055116.1053587-1-hch@lst.de>

Convert the nvdimm-pmem driver to use the blk_alloc_disk and
blk_cleanup_disk helpers to simplify gendisk and request_queue
allocation.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/nvdimm/pmem.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index 968b8483c763..9fcd05084564 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -338,7 +338,7 @@ static void pmem_pagemap_cleanup(struct dev_pagemap *pgmap)
 	struct request_queue *q =
 		container_of(pgmap->ref, struct request_queue, q_usage_counter);
 
-	blk_cleanup_queue(q);
+	blk_cleanup_disk(queue_to_disk(q));
 }
 
 static void pmem_release_queue(void *pgmap)
@@ -361,7 +361,6 @@ static void pmem_release_disk(void *__pmem)
 	kill_dax(pmem->dax_dev);
 	put_dax(pmem->dax_dev);
 	del_gendisk(pmem->disk);
-	put_disk(pmem->disk);
 }
 
 static const struct dev_pagemap_ops fsdax_pagemap_ops = {
@@ -422,10 +421,12 @@ static int pmem_attach_disk(struct device *dev,
 		return -EBUSY;
 	}
 
-	q = blk_alloc_queue(dev_to_node(dev));
-	if (!q)
+	disk = blk_alloc_disk(nid);
+	if (!disk)
 		return -ENOMEM;
+	q = disk->queue;
 
+	pmem->disk = disk;
 	pmem->pfn_flags = PFN_DEV;
 	pmem->pgmap.ref = &q->q_usage_counter;
 	if (is_nd_pfn(dev)) {
@@ -470,11 +471,6 @@ static int pmem_attach_disk(struct device *dev,
 	if (pmem->pfn_flags & PFN_MAP)
 		blk_queue_flag_set(QUEUE_FLAG_DAX, q);
 
-	disk = alloc_disk_node(0, nid);
-	if (!disk)
-		return -ENOMEM;
-	pmem->disk = disk;
-
 	disk->fops		= &pmem_fops;
 	disk->queue		= q;
 	disk->private_data	= pmem;
@@ -490,7 +486,6 @@ static int pmem_attach_disk(struct device *dev,
 		flags = DAXDEV_F_SYNC;
 	dax_dev = alloc_dax(pmem, disk->disk_name, &pmem_dax_ops, flags);
 	if (IS_ERR(dax_dev)) {
-		put_disk(disk);
 		return PTR_ERR(dax_dev);
 	}
 	dax_write_cache(dax_dev, nvdimm_has_cache(nd_region));
-- 
2.30.2


^ permalink raw reply related

* [PATCH 18/26] nvme-multipath: convert to blk_alloc_disk/blk_cleanup_disk
From: Christoph Hellwig @ 2021-05-21  5:51 UTC (permalink / raw)
  To: Jens Axboe, Geert Uytterhoeven, Chris Zankel, Max Filippov,
	Philipp Reisner, Lars Ellenberg, Jim Paris, Joshua Morris,
	Philip Kelleher, Minchan Kim, Nitin Gupta, Matias Bjorling,
	Coly Li, Mike Snitzer, Song Liu, Maxim Levitsky, Alex Dubov,
	Ulf Hansson, Dan Williams, Vishal Verma, Dave Jiang,
	Heiko Carstens, Vasily Gorbik, Christian Borntraeger
  Cc: linux-bcache, linux-xtensa, linux-raid, nvdimm, linux-s390,
	linux-mmc, linux-m68k, linux-nvme, linux-block, dm-devel,
	linuxppc-dev, drbd-dev
In-Reply-To: <20210521055116.1053587-1-hch@lst.de>

Convert the nvme-multipath driver to use the blk_alloc_disk and
blk_cleanup_disk helpers to simplify gendisk and request_queue
allocation.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/nvdimm/pmem.c         |  1 -
 drivers/nvme/host/multipath.c | 45 ++++++++++-------------------------
 2 files changed, 13 insertions(+), 33 deletions(-)

diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index 9fcd05084564..31f3c4bd6f72 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -472,7 +472,6 @@ static int pmem_attach_disk(struct device *dev,
 		blk_queue_flag_set(QUEUE_FLAG_DAX, q);
 
 	disk->fops		= &pmem_fops;
-	disk->queue		= q;
 	disk->private_data	= pmem;
 	nvdimm_namespace_disk_name(ndns, disk->disk_name);
 	set_capacity(disk, (pmem->size - pmem->pfn_pad - pmem->data_offset)
diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index a5d02f236cca..b5fbdb416022 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -427,7 +427,6 @@ static void nvme_requeue_work(struct work_struct *work)
 
 int nvme_mpath_alloc_disk(struct nvme_ctrl *ctrl, struct nvme_ns_head *head)
 {
-	struct request_queue *q;
 	bool vwc = false;
 
 	mutex_init(&head->lock);
@@ -443,33 +442,24 @@ int nvme_mpath_alloc_disk(struct nvme_ctrl *ctrl, struct nvme_ns_head *head)
 	if (!(ctrl->subsys->cmic & NVME_CTRL_CMIC_MULTI_CTRL) || !multipath)
 		return 0;
 
-	q = blk_alloc_queue(ctrl->numa_node);
-	if (!q)
-		goto out;
-	blk_queue_flag_set(QUEUE_FLAG_NONROT, q);
-	/* set to a default value for 512 until disk is validated */
-	blk_queue_logical_block_size(q, 512);
-	blk_set_stacking_limits(&q->limits);
-
-	/* we need to propagate up the VMC settings */
-	if (ctrl->vwc & NVME_CTRL_VWC_PRESENT)
-		vwc = true;
-	blk_queue_write_cache(q, vwc, vwc);
-
-	head->disk = alloc_disk(0);
+	head->disk = blk_alloc_disk(ctrl->numa_node);
 	if (!head->disk)
-		goto out_cleanup_queue;
+		return -ENOMEM;
 	head->disk->fops = &nvme_ns_head_ops;
 	head->disk->private_data = head;
-	head->disk->queue = q;
 	sprintf(head->disk->disk_name, "nvme%dn%d",
 			ctrl->subsys->instance, head->instance);
-	return 0;
 
-out_cleanup_queue:
-	blk_cleanup_queue(q);
-out:
-	return -ENOMEM;
+	blk_queue_flag_set(QUEUE_FLAG_NONROT, head->disk->queue);
+	/* set to a default value of 512 until the disk is validated */
+	blk_queue_logical_block_size(head->disk->queue, 512);
+	blk_set_stacking_limits(&head->disk->queue->limits);
+
+	/* we need to propagate up the VMC settings */
+	if (ctrl->vwc & NVME_CTRL_VWC_PRESENT)
+		vwc = true;
+	blk_queue_write_cache(head->disk->queue, vwc, vwc);
+	return 0;
 }
 
 static void nvme_mpath_set_live(struct nvme_ns *ns)
@@ -768,16 +758,7 @@ void nvme_mpath_remove_disk(struct nvme_ns_head *head)
 	/* make sure all pending bios are cleaned up */
 	kblockd_schedule_work(&head->requeue_work);
 	flush_work(&head->requeue_work);
-	blk_cleanup_queue(head->disk->queue);
-	if (!test_bit(NVME_NSHEAD_DISK_LIVE, &head->flags)) {
-		/*
-		 * if device_add_disk wasn't called, prevent
-		 * disk release to put a bogus reference on the
-		 * request queue
-		 */
-		head->disk->queue = NULL;
-	}
-	put_disk(head->disk);
+	blk_cleanup_disk(head->disk);
 }
 
 void nvme_mpath_init_ctrl(struct nvme_ctrl *ctrl)
-- 
2.30.2


^ permalink raw reply related

* [PATCH 19/26] nfblock: convert to blk_alloc_disk/blk_cleanup_disk
From: Christoph Hellwig @ 2021-05-21  5:51 UTC (permalink / raw)
  To: Jens Axboe, Geert Uytterhoeven, Chris Zankel, Max Filippov,
	Philipp Reisner, Lars Ellenberg, Jim Paris, Joshua Morris,
	Philip Kelleher, Minchan Kim, Nitin Gupta, Matias Bjorling,
	Coly Li, Mike Snitzer, Song Liu, Maxim Levitsky, Alex Dubov,
	Ulf Hansson, Dan Williams, Vishal Verma, Dave Jiang,
	Heiko Carstens, Vasily Gorbik, Christian Borntraeger
  Cc: linux-bcache, linux-xtensa, linux-raid, nvdimm, linux-s390,
	linux-mmc, linux-m68k, linux-nvme, linux-block, dm-devel,
	linuxppc-dev, drbd-dev
In-Reply-To: <20210521055116.1053587-1-hch@lst.de>

Convert the nfblock driver to use the blk_alloc_disk and blk_cleanup_disk
helpers to simplify gendisk and request_queue allocation.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/m68k/emu/nfblock.c | 20 +++++---------------
 1 file changed, 5 insertions(+), 15 deletions(-)

diff --git a/arch/m68k/emu/nfblock.c b/arch/m68k/emu/nfblock.c
index ba808543161a..9a8394e96388 100644
--- a/arch/m68k/emu/nfblock.c
+++ b/arch/m68k/emu/nfblock.c
@@ -55,7 +55,6 @@ struct nfhd_device {
 	int id;
 	u32 blocks, bsize;
 	int bshift;
-	struct request_queue *queue;
 	struct gendisk *disk;
 };
 
@@ -119,32 +118,24 @@ static int __init nfhd_init_one(int id, u32 blocks, u32 bsize)
 	dev->bsize = bsize;
 	dev->bshift = ffs(bsize) - 10;
 
-	dev->queue = blk_alloc_queue(NUMA_NO_NODE);
-	if (dev->queue == NULL)
-		goto free_dev;
-
-	blk_queue_logical_block_size(dev->queue, bsize);
-
-	dev->disk = alloc_disk(16);
+	dev->disk = blk_alloc_disk(NUMA_NO_NODE);
 	if (!dev->disk)
-		goto free_queue;
+		goto free_dev;
 
 	dev->disk->major = major_num;
 	dev->disk->first_minor = dev_id * 16;
+	dev->disk->minors = 16;
 	dev->disk->fops = &nfhd_ops;
 	dev->disk->private_data = dev;
 	sprintf(dev->disk->disk_name, "nfhd%u", dev_id);
 	set_capacity(dev->disk, (sector_t)blocks * (bsize / 512));
-	dev->disk->queue = dev->queue;
-
+	blk_queue_logical_block_size(dev->disk->queue, bsize);
 	add_disk(dev->disk);
 
 	list_add_tail(&dev->list, &nfhd_list);
 
 	return 0;
 
-free_queue:
-	blk_cleanup_queue(dev->queue);
 free_dev:
 	kfree(dev);
 out:
@@ -186,8 +177,7 @@ static void __exit nfhd_exit(void)
 	list_for_each_entry_safe(dev, next, &nfhd_list, list) {
 		list_del(&dev->list);
 		del_gendisk(dev->disk);
-		put_disk(dev->disk);
-		blk_cleanup_queue(dev->queue);
+		blk_cleanup_disk(dev->disk);
 		kfree(dev);
 	}
 	unregister_blkdev(major_num, "nfhd");
-- 
2.30.2


^ permalink raw reply related

* [PATCH 20/26] simdisk: convert to blk_alloc_disk/blk_cleanup_disk
From: Christoph Hellwig @ 2021-05-21  5:51 UTC (permalink / raw)
  To: Jens Axboe, Geert Uytterhoeven, Chris Zankel, Max Filippov,
	Philipp Reisner, Lars Ellenberg, Jim Paris, Joshua Morris,
	Philip Kelleher, Minchan Kim, Nitin Gupta, Matias Bjorling,
	Coly Li, Mike Snitzer, Song Liu, Maxim Levitsky, Alex Dubov,
	Ulf Hansson, Dan Williams, Vishal Verma, Dave Jiang,
	Heiko Carstens, Vasily Gorbik, Christian Borntraeger
  Cc: linux-bcache, linux-xtensa, linux-raid, nvdimm, linux-s390,
	linux-mmc, linux-m68k, linux-nvme, linux-block, dm-devel,
	linuxppc-dev, drbd-dev
In-Reply-To: <20210521055116.1053587-1-hch@lst.de>

Convert the simdisk driver to use the blk_alloc_disk and blk_cleanup_disk
helpers to simplify gendisk and request_queue allocation.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/xtensa/platforms/iss/simdisk.c | 29 +++++++----------------------
 1 file changed, 7 insertions(+), 22 deletions(-)

diff --git a/arch/xtensa/platforms/iss/simdisk.c b/arch/xtensa/platforms/iss/simdisk.c
index fc09be7b1347..3cdfa00738e0 100644
--- a/arch/xtensa/platforms/iss/simdisk.c
+++ b/arch/xtensa/platforms/iss/simdisk.c
@@ -27,7 +27,6 @@
 struct simdisk {
 	const char *filename;
 	spinlock_t lock;
-	struct request_queue *queue;
 	struct gendisk *gd;
 	struct proc_dir_entry *procfile;
 	int users;
@@ -266,21 +265,13 @@ static int __init simdisk_setup(struct simdisk *dev, int which,
 	spin_lock_init(&dev->lock);
 	dev->users = 0;
 
-	dev->queue = blk_alloc_queue(NUMA_NO_NODE);
-	if (dev->queue == NULL) {
-		pr_err("blk_alloc_queue failed\n");
-		goto out_alloc_queue;
-	}
-
-	dev->gd = alloc_disk(SIMDISK_MINORS);
-	if (dev->gd == NULL) {
-		pr_err("alloc_disk failed\n");
-		goto out_alloc_disk;
-	}
+	dev->gd = blk_alloc_disk(NUMA_NO_NODE);
+	if (!dev->gd)
+		return -ENOMEM;
 	dev->gd->major = simdisk_major;
 	dev->gd->first_minor = which;
+	dev->gd->minors = SIMDISK_MINORS;
 	dev->gd->fops = &simdisk_ops;
-	dev->gd->queue = dev->queue;
 	dev->gd->private_data = dev;
 	snprintf(dev->gd->disk_name, 32, "simdisk%d", which);
 	set_capacity(dev->gd, 0);
@@ -288,12 +279,6 @@ static int __init simdisk_setup(struct simdisk *dev, int which,
 
 	dev->procfile = proc_create_data(tmp, 0644, procdir, &simdisk_proc_ops, dev);
 	return 0;
-
-out_alloc_disk:
-	blk_cleanup_queue(dev->queue);
-	dev->queue = NULL;
-out_alloc_queue:
-	return -ENOMEM;
 }
 
 static int __init simdisk_init(void)
@@ -343,10 +328,10 @@ static void simdisk_teardown(struct simdisk *dev, int which,
 	char tmp[2] = { '0' + which, 0 };
 
 	simdisk_detach(dev);
-	if (dev->gd)
+	if (dev->gd) {
 		del_gendisk(dev->gd);
-	if (dev->queue)
-		blk_cleanup_queue(dev->queue);
+		blk_cleanup_disk(dev->gd);
+	}
 	remove_proc_entry(tmp, procdir);
 }
 
-- 
2.30.2


^ permalink raw reply related

* [PATCH 21/26] n64cart: convert to blk_alloc_disk
From: Christoph Hellwig @ 2021-05-21  5:51 UTC (permalink / raw)
  To: Jens Axboe, Geert Uytterhoeven, Chris Zankel, Max Filippov,
	Philipp Reisner, Lars Ellenberg, Jim Paris, Joshua Morris,
	Philip Kelleher, Minchan Kim, Nitin Gupta, Matias Bjorling,
	Coly Li, Mike Snitzer, Song Liu, Maxim Levitsky, Alex Dubov,
	Ulf Hansson, Dan Williams, Vishal Verma, Dave Jiang,
	Heiko Carstens, Vasily Gorbik, Christian Borntraeger
  Cc: linux-bcache, linux-xtensa, linux-raid, nvdimm, linux-s390,
	linux-mmc, linux-m68k, linux-nvme, linux-block, dm-devel,
	linuxppc-dev, drbd-dev
In-Reply-To: <20210521055116.1053587-1-hch@lst.de>

Convert the n64cart driver to use the blk_alloc_disk helper to simplify
gendisk and request_queue allocation.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/block/n64cart.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/block/n64cart.c b/drivers/block/n64cart.c
index 3dae4b631dea..7b4dd10af9ec 100644
--- a/drivers/block/n64cart.c
+++ b/drivers/block/n64cart.c
@@ -132,14 +132,10 @@ static int __init n64cart_probe(struct platform_device *pdev)
 	if (!reg_base)
 		return -EINVAL;
 
-	disk = alloc_disk(0);
+	disk = blk_alloc_disk(NUMA_NO_NODE);
 	if (!disk)
 		return -ENOMEM;
 
-	disk->queue = blk_alloc_queue(NUMA_NO_NODE);
-	if (!disk->queue)
-		return -ENOMEM;
-
 	disk->first_minor = 0;
 	disk->flags = GENHD_FL_NO_PART_SCAN;
 	disk->fops = &n64cart_fops;
-- 
2.30.2


^ permalink raw reply related

* [PATCH 22/26] ps3vram: convert to blk_alloc_disk/blk_cleanup_disk
From: Christoph Hellwig @ 2021-05-21  5:51 UTC (permalink / raw)
  To: Jens Axboe, Geert Uytterhoeven, Chris Zankel, Max Filippov,
	Philipp Reisner, Lars Ellenberg, Jim Paris, Joshua Morris,
	Philip Kelleher, Minchan Kim, Nitin Gupta, Matias Bjorling,
	Coly Li, Mike Snitzer, Song Liu, Maxim Levitsky, Alex Dubov,
	Ulf Hansson, Dan Williams, Vishal Verma, Dave Jiang,
	Heiko Carstens, Vasily Gorbik, Christian Borntraeger
  Cc: linux-bcache, linux-xtensa, linux-raid, nvdimm, linux-s390,
	linux-mmc, linux-m68k, linux-nvme, linux-block, dm-devel,
	linuxppc-dev, drbd-dev
In-Reply-To: <20210521055116.1053587-1-hch@lst.de>

Convert the ps3vram driver to use the blk_alloc_disk and blk_cleanup_disk
helpers to simplify gendisk and request_queue allocation.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/block/ps3vram.c | 31 ++++++++-----------------------
 1 file changed, 8 insertions(+), 23 deletions(-)

diff --git a/drivers/block/ps3vram.c b/drivers/block/ps3vram.c
index 1d738999fb69..7fbf469651c4 100644
--- a/drivers/block/ps3vram.c
+++ b/drivers/block/ps3vram.c
@@ -67,7 +67,6 @@ struct ps3vram_cache {
 };
 
 struct ps3vram_priv {
-	struct request_queue *queue;
 	struct gendisk *gendisk;
 
 	u64 size;
@@ -613,7 +612,6 @@ static int ps3vram_probe(struct ps3_system_bus_device *dev)
 {
 	struct ps3vram_priv *priv;
 	int error, status;
-	struct request_queue *queue;
 	struct gendisk *gendisk;
 	u64 ddr_size, ddr_lpar, ctrl_lpar, info_lpar, reports_lpar,
 	    reports_size, xdr_lpar;
@@ -736,33 +734,23 @@ static int ps3vram_probe(struct ps3_system_bus_device *dev)
 
 	ps3vram_proc_init(dev);
 
-	queue = blk_alloc_queue(NUMA_NO_NODE);
-	if (!queue) {
-		dev_err(&dev->core, "blk_alloc_queue failed\n");
-		error = -ENOMEM;
-		goto out_cache_cleanup;
-	}
-
-	priv->queue = queue;
-	blk_queue_max_segments(queue, BLK_MAX_SEGMENTS);
-	blk_queue_max_segment_size(queue, BLK_MAX_SEGMENT_SIZE);
-	blk_queue_max_hw_sectors(queue, BLK_SAFE_MAX_SECTORS);
-
-	gendisk = alloc_disk(1);
+	gendisk = blk_alloc_disk(NUMA_NO_NODE);
 	if (!gendisk) {
-		dev_err(&dev->core, "alloc_disk failed\n");
+		dev_err(&dev->core, "blk_alloc_disk failed\n");
 		error = -ENOMEM;
-		goto fail_cleanup_queue;
+		goto out_cache_cleanup;
 	}
 
 	priv->gendisk = gendisk;
 	gendisk->major = ps3vram_major;
-	gendisk->first_minor = 0;
+	gendisk->minors = 1;
 	gendisk->fops = &ps3vram_fops;
-	gendisk->queue = queue;
 	gendisk->private_data = dev;
 	strlcpy(gendisk->disk_name, DEVICE_NAME, sizeof(gendisk->disk_name));
 	set_capacity(gendisk, priv->size >> 9);
+	blk_queue_max_segments(gendisk->queue, BLK_MAX_SEGMENTS);
+	blk_queue_max_segment_size(gendisk->queue, BLK_MAX_SEGMENT_SIZE);
+	blk_queue_max_hw_sectors(gendisk->queue, BLK_SAFE_MAX_SECTORS);
 
 	dev_info(&dev->core, "%s: Using %llu MiB of GPU memory\n",
 		 gendisk->disk_name, get_capacity(gendisk) >> 11);
@@ -770,8 +758,6 @@ static int ps3vram_probe(struct ps3_system_bus_device *dev)
 	device_add_disk(&dev->core, gendisk, NULL);
 	return 0;
 
-fail_cleanup_queue:
-	blk_cleanup_queue(queue);
 out_cache_cleanup:
 	remove_proc_entry(DEVICE_NAME, NULL);
 	ps3vram_cache_cleanup(dev);
@@ -802,8 +788,7 @@ static void ps3vram_remove(struct ps3_system_bus_device *dev)
 	struct ps3vram_priv *priv = ps3_system_bus_get_drvdata(dev);
 
 	del_gendisk(priv->gendisk);
-	put_disk(priv->gendisk);
-	blk_cleanup_queue(priv->queue);
+	blk_cleanup_disk(priv->gendisk);
 	remove_proc_entry(DEVICE_NAME, NULL);
 	ps3vram_cache_cleanup(dev);
 	iounmap(priv->reports);
-- 
2.30.2


^ permalink raw reply related

* [PATCH 23/26] dcssblk: convert to blk_alloc_disk/blk_cleanup_disk
From: Christoph Hellwig @ 2021-05-21  5:51 UTC (permalink / raw)
  To: Jens Axboe, Geert Uytterhoeven, Chris Zankel, Max Filippov,
	Philipp Reisner, Lars Ellenberg, Jim Paris, Joshua Morris,
	Philip Kelleher, Minchan Kim, Nitin Gupta, Matias Bjorling,
	Coly Li, Mike Snitzer, Song Liu, Maxim Levitsky, Alex Dubov,
	Ulf Hansson, Dan Williams, Vishal Verma, Dave Jiang,
	Heiko Carstens, Vasily Gorbik, Christian Borntraeger
  Cc: linux-bcache, linux-xtensa, linux-raid, nvdimm, linux-s390,
	linux-mmc, linux-m68k, linux-nvme, linux-block, dm-devel,
	linuxppc-dev, drbd-dev
In-Reply-To: <20210521055116.1053587-1-hch@lst.de>

Convert the dcssblk driver to use the blk_alloc_disk and blk_cleanup_disk
helpers to simplify gendisk and request_queue allocation.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/s390/block/dcssblk.c | 26 ++++++++------------------
 1 file changed, 8 insertions(+), 18 deletions(-)

diff --git a/drivers/s390/block/dcssblk.c b/drivers/s390/block/dcssblk.c
index da33cb4cba28..7faa56399999 100644
--- a/drivers/s390/block/dcssblk.c
+++ b/drivers/s390/block/dcssblk.c
@@ -90,7 +90,6 @@ struct dcssblk_dev_info {
 	int segment_type;
 	unsigned char save_pending;
 	unsigned char is_shared;
-	struct request_queue *dcssblk_queue;
 	int num_of_segments;
 	struct list_head seg_list;
 	struct dax_device *dax_dev;
@@ -429,9 +428,7 @@ dcssblk_shared_store(struct device *dev, struct device_attribute *attr, const ch
 	kill_dax(dev_info->dax_dev);
 	put_dax(dev_info->dax_dev);
 	del_gendisk(dev_info->gd);
-	blk_cleanup_queue(dev_info->dcssblk_queue);
-	dev_info->gd->queue = NULL;
-	put_disk(dev_info->gd);
+	blk_cleanup_disk(dev_info->gd);
 	up_write(&dcssblk_devices_sem);
 
 	if (device_remove_file_self(dev, attr)) {
@@ -644,18 +641,17 @@ dcssblk_add_store(struct device *dev, struct device_attribute *attr, const char
 	dev_info->dev.release = dcssblk_release_segment;
 	dev_info->dev.groups = dcssblk_dev_attr_groups;
 	INIT_LIST_HEAD(&dev_info->lh);
-	dev_info->gd = alloc_disk(DCSSBLK_MINORS_PER_DISK);
+	dev_info->gd = blk_alloc_disk(NUMA_NO_NODE);
 	if (dev_info->gd == NULL) {
 		rc = -ENOMEM;
 		goto seg_list_del;
 	}
 	dev_info->gd->major = dcssblk_major;
+	dev_info->gd->minors = DCSSBLK_MINORS_PER_DISK;
 	dev_info->gd->fops = &dcssblk_devops;
-	dev_info->dcssblk_queue = blk_alloc_queue(NUMA_NO_NODE);
-	dev_info->gd->queue = dev_info->dcssblk_queue;
 	dev_info->gd->private_data = dev_info;
-	blk_queue_logical_block_size(dev_info->dcssblk_queue, 4096);
-	blk_queue_flag_set(QUEUE_FLAG_DAX, dev_info->dcssblk_queue);
+	blk_queue_logical_block_size(dev_info->gd->queue, 4096);
+	blk_queue_flag_set(QUEUE_FLAG_DAX, dev_info->gd->queue);
 
 	seg_byte_size = (dev_info->end - dev_info->start + 1);
 	set_capacity(dev_info->gd, seg_byte_size >> 9); // size in sectors
@@ -719,9 +715,7 @@ dcssblk_add_store(struct device *dev, struct device_attribute *attr, const char
 
 put_dev:
 	list_del(&dev_info->lh);
-	blk_cleanup_queue(dev_info->dcssblk_queue);
-	dev_info->gd->queue = NULL;
-	put_disk(dev_info->gd);
+	blk_cleanup_disk(dev_info->gd);
 	list_for_each_entry(seg_info, &dev_info->seg_list, lh) {
 		segment_unload(seg_info->segment_name);
 	}
@@ -731,9 +725,7 @@ dcssblk_add_store(struct device *dev, struct device_attribute *attr, const char
 dev_list_del:
 	list_del(&dev_info->lh);
 release_gd:
-	blk_cleanup_queue(dev_info->dcssblk_queue);
-	dev_info->gd->queue = NULL;
-	put_disk(dev_info->gd);
+	blk_cleanup_disk(dev_info->gd);
 	up_write(&dcssblk_devices_sem);
 seg_list_del:
 	if (dev_info == NULL)
@@ -801,9 +793,7 @@ dcssblk_remove_store(struct device *dev, struct device_attribute *attr, const ch
 	kill_dax(dev_info->dax_dev);
 	put_dax(dev_info->dax_dev);
 	del_gendisk(dev_info->gd);
-	blk_cleanup_queue(dev_info->dcssblk_queue);
-	dev_info->gd->queue = NULL;
-	put_disk(dev_info->gd);
+	blk_cleanup_disk(dev_info->gd);
 
 	/* unload all related segments */
 	list_for_each_entry(entry, &dev_info->seg_list, lh)
-- 
2.30.2


^ permalink raw reply related

* [PATCH 24/26] xpram: convert to blk_alloc_disk/blk_cleanup_disk
From: Christoph Hellwig @ 2021-05-21  5:51 UTC (permalink / raw)
  To: Jens Axboe, Geert Uytterhoeven, Chris Zankel, Max Filippov,
	Philipp Reisner, Lars Ellenberg, Jim Paris, Joshua Morris,
	Philip Kelleher, Minchan Kim, Nitin Gupta, Matias Bjorling,
	Coly Li, Mike Snitzer, Song Liu, Maxim Levitsky, Alex Dubov,
	Ulf Hansson, Dan Williams, Vishal Verma, Dave Jiang,
	Heiko Carstens, Vasily Gorbik, Christian Borntraeger
  Cc: linux-bcache, linux-xtensa, linux-raid, nvdimm, linux-s390,
	linux-mmc, linux-m68k, linux-nvme, linux-block, dm-devel,
	linuxppc-dev, drbd-dev
In-Reply-To: <20210521055116.1053587-1-hch@lst.de>

Convert the xpram driver to use the blk_alloc_disk and blk_cleanup_disk
helpers to simplify gendisk and request_queue allocation.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/s390/block/xpram.c | 26 +++++++++-----------------
 1 file changed, 9 insertions(+), 17 deletions(-)

diff --git a/drivers/s390/block/xpram.c b/drivers/s390/block/xpram.c
index d1ed39162943..91ef710edfd2 100644
--- a/drivers/s390/block/xpram.c
+++ b/drivers/s390/block/xpram.c
@@ -56,7 +56,6 @@ typedef struct {
 static xpram_device_t xpram_devices[XPRAM_MAX_DEVS];
 static unsigned int xpram_sizes[XPRAM_MAX_DEVS];
 static struct gendisk *xpram_disks[XPRAM_MAX_DEVS];
-static struct request_queue *xpram_queues[XPRAM_MAX_DEVS];
 static unsigned int xpram_pages;
 static int xpram_devs;
 
@@ -341,17 +340,13 @@ static int __init xpram_setup_blkdev(void)
 	int i, rc = -ENOMEM;
 
 	for (i = 0; i < xpram_devs; i++) {
-		xpram_disks[i] = alloc_disk(1);
+		xpram_disks[i] = blk_alloc_disk(NUMA_NO_NODE);
 		if (!xpram_disks[i])
 			goto out;
-		xpram_queues[i] = blk_alloc_queue(NUMA_NO_NODE);
-		if (!xpram_queues[i]) {
-			put_disk(xpram_disks[i]);
-			goto out;
-		}
-		blk_queue_flag_set(QUEUE_FLAG_NONROT, xpram_queues[i]);
-		blk_queue_flag_clear(QUEUE_FLAG_ADD_RANDOM, xpram_queues[i]);
-		blk_queue_logical_block_size(xpram_queues[i], 4096);
+		blk_queue_flag_set(QUEUE_FLAG_NONROT, xpram_disks[i]->queue);
+		blk_queue_flag_clear(QUEUE_FLAG_ADD_RANDOM,
+				xpram_disks[i]->queue);
+		blk_queue_logical_block_size(xpram_disks[i]->queue, 4096);
 	}
 
 	/*
@@ -373,9 +368,9 @@ static int __init xpram_setup_blkdev(void)
 		offset += xpram_devices[i].size;
 		disk->major = XPRAM_MAJOR;
 		disk->first_minor = i;
+		disk->minors = 1;
 		disk->fops = &xpram_devops;
 		disk->private_data = &xpram_devices[i];
-		disk->queue = xpram_queues[i];
 		sprintf(disk->disk_name, "slram%d", i);
 		set_capacity(disk, xpram_sizes[i] << 1);
 		add_disk(disk);
@@ -383,10 +378,8 @@ static int __init xpram_setup_blkdev(void)
 
 	return 0;
 out:
-	while (i--) {
-		blk_cleanup_queue(xpram_queues[i]);
-		put_disk(xpram_disks[i]);
-	}
+	while (i--)
+		blk_cleanup_disk(xpram_disks[i]);
 	return rc;
 }
 
@@ -434,8 +427,7 @@ static void __exit xpram_exit(void)
 	int i;
 	for (i = 0; i < xpram_devs; i++) {
 		del_gendisk(xpram_disks[i]);
-		blk_cleanup_queue(xpram_queues[i]);
-		put_disk(xpram_disks[i]);
+		blk_cleanup_disk(xpram_disks[i]);
 	}
 	unregister_blkdev(XPRAM_MAJOR, XPRAM_NAME);
 	platform_device_unregister(xpram_pdev);
-- 
2.30.2


^ permalink raw reply related

* [PATCH 25/26] null_blk: convert to blk_alloc_disk/blk_cleanup_disk
From: Christoph Hellwig @ 2021-05-21  5:51 UTC (permalink / raw)
  To: Jens Axboe, Geert Uytterhoeven, Chris Zankel, Max Filippov,
	Philipp Reisner, Lars Ellenberg, Jim Paris, Joshua Morris,
	Philip Kelleher, Minchan Kim, Nitin Gupta, Matias Bjorling,
	Coly Li, Mike Snitzer, Song Liu, Maxim Levitsky, Alex Dubov,
	Ulf Hansson, Dan Williams, Vishal Verma, Dave Jiang,
	Heiko Carstens, Vasily Gorbik, Christian Borntraeger
  Cc: linux-bcache, linux-xtensa, linux-raid, nvdimm, linux-s390,
	linux-mmc, linux-m68k, linux-nvme, linux-block, dm-devel,
	linuxppc-dev, drbd-dev
In-Reply-To: <20210521055116.1053587-1-hch@lst.de>

Convert the null_blk driver to use the blk_alloc_disk and blk_cleanup_disk
helpers to simplify gendisk and request_queue allocation.  Note that the
blk-mq mode is left with its own allocations scheme, to be handled later.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/block/null_blk/main.c | 38 +++++++++++++++++------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index 5f006d9e1472..d8e098f1e5b5 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -1597,11 +1597,10 @@ static void null_del_dev(struct nullb *nullb)
 		null_restart_queue_async(nullb);
 	}
 
-	blk_cleanup_queue(nullb->q);
+	blk_cleanup_disk(nullb->disk);
 	if (dev->queue_mode == NULL_Q_MQ &&
 	    nullb->tag_set == &nullb->__tag_set)
 		blk_mq_free_tag_set(nullb->tag_set);
-	put_disk(nullb->disk);
 	cleanup_queues(nullb);
 	if (null_cache_active(nullb))
 		null_free_device_storage(nullb->dev, true);
@@ -1700,22 +1699,19 @@ static int init_driver_queues(struct nullb *nullb)
 static int null_gendisk_register(struct nullb *nullb)
 {
 	sector_t size = ((sector_t)nullb->dev->size * SZ_1M) >> SECTOR_SHIFT;
-	struct gendisk *disk;
+	struct gendisk *disk = nullb->disk;
 
-	disk = nullb->disk = alloc_disk_node(1, nullb->dev->home_node);
-	if (!disk)
-		return -ENOMEM;
 	set_capacity(disk, size);
 
 	disk->flags |= GENHD_FL_EXT_DEVT | GENHD_FL_SUPPRESS_PARTITION_INFO;
 	disk->major		= null_major;
 	disk->first_minor	= nullb->index;
+	disk->minors		= 1;
 	if (queue_is_mq(nullb->q))
 		disk->fops		= &null_rq_ops;
 	else
 		disk->fops		= &null_bio_ops;
 	disk->private_data	= nullb;
-	disk->queue		= nullb->q;
 	strncpy(disk->disk_name, nullb->disk_name, DISK_NAME_LEN);
 
 	if (nullb->dev->zoned) {
@@ -1851,23 +1847,27 @@ static int null_add_dev(struct nullb_device *dev)
 			goto out_cleanup_queues;
 
 		if (!null_setup_fault())
-			goto out_cleanup_queues;
+			goto out_cleanup_tags;
 
+		rv = -ENOMEM;
 		nullb->tag_set->timeout = 5 * HZ;
 		nullb->q = blk_mq_init_queue_data(nullb->tag_set, nullb);
-		if (IS_ERR(nullb->q)) {
-			rv = -ENOMEM;
+		if (IS_ERR(nullb->q))
 			goto out_cleanup_tags;
-		}
+		nullb->disk = alloc_disk_node(1, nullb->dev->home_node);
+		if (!nullb->disk)
+			goto out_cleanup_disk;
+		nullb->disk->queue = nullb->q;
 	} else if (dev->queue_mode == NULL_Q_BIO) {
-		nullb->q = blk_alloc_queue(dev->home_node);
-		if (!nullb->q) {
-			rv = -ENOMEM;
+		rv = -ENOMEM;
+		nullb->disk = blk_alloc_disk(nullb->dev->home_node);
+		if (!nullb->disk)
 			goto out_cleanup_queues;
-		}
+
+		nullb->q = nullb->disk->queue;
 		rv = init_driver_queues(nullb);
 		if (rv)
-			goto out_cleanup_blk_queue;
+			goto out_cleanup_disk;
 	}
 
 	if (dev->mbps) {
@@ -1883,7 +1883,7 @@ static int null_add_dev(struct nullb_device *dev)
 	if (dev->zoned) {
 		rv = null_init_zoned_dev(dev, nullb->q);
 		if (rv)
-			goto out_cleanup_blk_queue;
+			goto out_cleanup_disk;
 	}
 
 	nullb->q->queuedata = nullb;
@@ -1921,8 +1921,8 @@ static int null_add_dev(struct nullb_device *dev)
 	return 0;
 out_cleanup_zone:
 	null_free_zoned_dev(dev);
-out_cleanup_blk_queue:
-	blk_cleanup_queue(nullb->q);
+out_cleanup_disk:
+	blk_cleanup_disk(nullb->disk);
 out_cleanup_tags:
 	if (dev->queue_mode == NULL_Q_MQ && nullb->tag_set == &nullb->__tag_set)
 		blk_mq_free_tag_set(nullb->tag_set);
-- 
2.30.2


^ permalink raw reply related

* [PATCH 26/26] block: unexport blk_alloc_queue
From: Christoph Hellwig @ 2021-05-21  5:51 UTC (permalink / raw)
  To: Jens Axboe, Geert Uytterhoeven, Chris Zankel, Max Filippov,
	Philipp Reisner, Lars Ellenberg, Jim Paris, Joshua Morris,
	Philip Kelleher, Minchan Kim, Nitin Gupta, Matias Bjorling,
	Coly Li, Mike Snitzer, Song Liu, Maxim Levitsky, Alex Dubov,
	Ulf Hansson, Dan Williams, Vishal Verma, Dave Jiang,
	Heiko Carstens, Vasily Gorbik, Christian Borntraeger
  Cc: linux-bcache, linux-xtensa, linux-raid, nvdimm, linux-s390,
	linux-mmc, linux-m68k, linux-nvme, linux-block, dm-devel,
	linuxppc-dev, drbd-dev
In-Reply-To: <20210521055116.1053587-1-hch@lst.de>

blk_alloc_queue is just an internal helper now, unexport it and remove
it from the public header.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-core.c       | 1 -
 block/blk.h            | 2 ++
 include/linux/blkdev.h | 1 -
 3 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 689aac2625d2..3515a66022d7 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -599,7 +599,6 @@ struct request_queue *blk_alloc_queue(int node_id)
 	kmem_cache_free(blk_requestq_cachep, q);
 	return NULL;
 }
-EXPORT_SYMBOL(blk_alloc_queue);
 
 /**
  * blk_get_queue - increment the request_queue refcount
diff --git a/block/blk.h b/block/blk.h
index cba3a94aabfa..3440142f029b 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -359,4 +359,6 @@ int bio_add_hw_page(struct request_queue *q, struct bio *bio,
 		struct page *page, unsigned int len, unsigned int offset,
 		unsigned int max_sectors, bool *same_page);
 
+struct request_queue *blk_alloc_queue(int node_id);
+
 #endif /* BLK_INTERNAL_H */
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 2c28577b50f4..d66d0da72529 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1213,7 +1213,6 @@ static inline int blk_rq_map_sg(struct request_queue *q, struct request *rq,
 extern void blk_dump_rq_flags(struct request *, char *);
 
 bool __must_check blk_get_queue(struct request_queue *);
-struct request_queue *blk_alloc_queue(int node_id);
 extern void blk_put_queue(struct request_queue *);
 extern void blk_set_queue_dying(struct request_queue *);
 
-- 
2.30.2


^ permalink raw reply related

* Re: [PATCH v5 7/9] mm/mremap: Move TLB flush outside page table lock
From: Linus Torvalds @ 2021-05-21  6:13 UTC (permalink / raw)
  To: Aneesh Kumar K.V
  Cc: Nick Piggin, Linux-MM, Kalesh Singh, Joel Fernandes,
	Andrew Morton, linuxppc-dev
In-Reply-To: <f676b053-bda4-a1f5-321e-f00fb3de8a40@linux.ibm.com>

On Thu, May 20, 2021 at 5:03 PM Aneesh Kumar K.V
<aneesh.kumar@linux.ibm.com> wrote:
>
> On 5/21/21 8:10 AM, Linus Torvalds wrote:
> >
> > So mremap does need to flush the TLB before releasing the page table
> > lock, because that's the lifetime boundary for the page that got
> > moved.
>
> How will we avoid that happening with
> c49dd340180260c6239e453263a9a244da9a7c85 /
> 2c91bd4a4e2e530582d6fd643ea7b86b27907151 . The commit improves mremap
> performance by moving level3/level2 page table entries. When doing so we
> are not holding level 4 ptl lock (pte_lock()). But rather we are holding
> pmd_lock or pud_lock(). So if we move pages around without holding the
> pte lock, won't the above issue happen even if we do a tlb flush with
> holding pmd lock/pud lock?

Hmm. Interesting.

Your patch (to flush the TLB after clearing the old location, and
before inserting it into the new one) looks like an "obvious" fix.

But I'm putting that "obvious" in quotes, because I'm now wondering if
it actually fixes anything.

Lookie here:

 - CPU1 does a mremap of a pmd or pud.

    It clears the old pmd/pud, flushes the old TLB range, and then
inserts the pmd/pud at the new location.

 - CPU2 does a page shrinker, which calls try_to_unmap, which calls
try_to_unmap_one.

These are entirely asynchronous, because they have no shared lock. The
mremap uses the pmd lock, the try_to_unmap_one() does the rmap walk,
which does the pte lock.

Now, imagine that the following ordering happens with the two
operations above, and a CPU3 that does accesses:

 - CPU2 follows (and sees) the old page tables in the old location and
the took the pte lock

 - the mremap on CPU1 starts - cleared the old pmd, flushed the tlb,
*and* inserts in the new place.

 - a user thread on CPU3 accesses the new location and fills the TLB
of the *new* address

 - only now does CPU2 get to the "pte_get_and_clear()" to remove one page

 - CPU2 does a TLB flush and frees the page

End result:

 - both CPU1 _and_ CPU2 have flushed the TLB.

 - but both flushed the *OLD* address

 - the page is freed

 - CPU3 still has the stale TLB entry pointing to the page that is now
free and might be reused for something else

Am I missing something?

               Linus

^ permalink raw reply

* Re: [PATCH 12/26] bcache: convert to blk_alloc_disk/blk_cleanup_disk
From: Coly Li @ 2021-05-21  6:15 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: nvdimm, Ulf Hansson, Mike Snitzer, linux-nvme, Philipp Reisner,
	Song Liu, dm-devel, Joshua Morris, drbd-dev, linux-s390,
	Dave Jiang, Maxim Levitsky, Vishal Verma, Christian Borntraeger,
	Geert Uytterhoeven, Matias Bjorling, Nitin Gupta, Vasily Gorbik,
	linux-xtensa, Alex Dubov, Heiko Carstens, linux-raid,
	linux-bcache, linux-block, Philip Kelleher, Dan Williams,
	Jens Axboe, Chris Zankel, Max Filippov, linux-mmc, linux-m68k,
	Jim Paris, Minchan Kim, Lars Ellenberg, linuxppc-dev
In-Reply-To: <20210521055116.1053587-13-hch@lst.de>

On 5/21/21 1:51 PM, Christoph Hellwig wrote:
> Convert the bcache driver to use the blk_alloc_disk and blk_cleanup_disk
> helpers to simplify gendisk and request_queue allocation.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/md/bcache/super.c | 15 ++++-----------
>  1 file changed, 4 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
> index bea8c4429ae8..185246a0d855 100644
> --- a/drivers/md/bcache/super.c
> +++ b/drivers/md/bcache/super.c
> @@ -890,13 +890,9 @@ static void bcache_device_free(struct bcache_device *d)
>  		if (disk_added)
>  			del_gendisk(disk);
>  
> -		if (disk->queue)
> -			blk_cleanup_queue(disk->queue);
> -
> +		blk_cleanup_disk(disk);
>  		ida_simple_remove(&bcache_device_idx,
>  				  first_minor_to_idx(disk->first_minor));
> -		if (disk_added)
> -			put_disk(disk);

The  above 2 lines are added on purpose to prevent an refcount
underflow. It is from commit 86da9f736740 ("bcache: fix refcount
underflow in bcache_device_free()").

Maybe add a parameter to blk_cleanup_disk() or checking (disk->flags &
GENHD_FL_UP) inside blk_cleanup_disk() ?

Coly Li


>  	}
>  
>  	bioset_exit(&d->bio_split);
> @@ -946,7 +942,7 @@ static int bcache_device_init(struct bcache_device *d, unsigned int block_size,
>  			BIOSET_NEED_BVECS|BIOSET_NEED_RESCUER))
>  		goto err;
>  
> -	d->disk = alloc_disk(BCACHE_MINORS);
> +	d->disk = blk_alloc_disk(NUMA_NO_NODE);
>  	if (!d->disk)
>  		goto err;
>  
> @@ -955,14 +951,11 @@ static int bcache_device_init(struct bcache_device *d, unsigned int block_size,
>  
>  	d->disk->major		= bcache_major;
>  	d->disk->first_minor	= idx_to_first_minor(idx);
> +	d->disk->minors		= BCACHE_MINORS;
>  	d->disk->fops		= ops;
>  	d->disk->private_data	= d;
>  
> -	q = blk_alloc_queue(NUMA_NO_NODE);
> -	if (!q)
> -		return -ENOMEM;
> -
> -	d->disk->queue			= q;
> +	q = d->disk->queue;
>  	q->limits.max_hw_sectors	= UINT_MAX;
>  	q->limits.max_sectors		= UINT_MAX;
>  	q->limits.max_segment_size	= UINT_MAX;
> 

The rested looks fine to me.

Thanks.

Coly Li

^ permalink raw reply

* Re: [PATCH 12/26] bcache: convert to blk_alloc_disk/blk_cleanup_disk
From: Christoph Hellwig @ 2021-05-21  6:23 UTC (permalink / raw)
  To: Coly Li
  Cc: nvdimm, Ulf Hansson, Mike Snitzer, linux-nvme, Philipp Reisner,
	Max Filippov, dm-devel, Joshua Morris, drbd-dev, linux-s390,
	Dave Jiang, Maxim Levitsky, Vishal Verma, Christoph Hellwig,
	Christian Borntraeger, Geert Uytterhoeven, Matias Bjorling,
	Nitin Gupta, Vasily Gorbik, linux-xtensa, Alex Dubov,
	Heiko Carstens, linux-raid, linux-bcache, linux-block,
	Philip Kelleher, Dan Williams, Jens Axboe, Chris Zankel, Song Liu,
	linux-mmc, linux-m68k, Jim Paris, Minchan Kim, Lars Ellenberg,
	linuxppc-dev
In-Reply-To: <d4f1c005-2ce0-51b5-c861-431f0ffb3dcf@suse.de>

On Fri, May 21, 2021 at 02:15:32PM +0800, Coly Li wrote:
> The  above 2 lines are added on purpose to prevent an refcount
> underflow. It is from commit 86da9f736740 ("bcache: fix refcount
> underflow in bcache_device_free()").
> 
> Maybe add a parameter to blk_cleanup_disk() or checking (disk->flags &
> GENHD_FL_UP) inside blk_cleanup_disk() ?

Please take a look at patch 4 in the series.

^ permalink raw reply

* [RFC PATCH 0/6] powerpc: Stack tracer fixes
From: Naveen N. Rao @ 2021-05-21  6:48 UTC (permalink / raw)
  To: Steven Rostedt, Michael Ellerman, Michal Suchanek, Torsten Duwe
  Cc: linuxppc-dev, linux-kernel

While the stack tracer worked fine with ppc64 ELFv1 ABI, it was never 
ported/enabled to run properly with ppc64 ELFv2 ABI, -mprofile-kernel in 
particular. The primary issue is that the call to ftrace happens before 
a function sets up its own stackframe. Due to this, the traced function 
never shows up in the stack trace. This produces confusing results, 
especially with the stack trace filter and is evident with the selftest 
failure. This is also an issue on ppc32.

The first two commits add support to the stack tracer for powerpc. This 
support utilizes the powerpc ABI to produce reliable stack traces, as 
well as to determine stackframe sizes.

Patches 3, 4 and 6 add support for decoding the traced function name in 
various stack traces and to show the same.

Patch 5 makes this change for livepatching and I am a bit unsure of this 
change. More details in patch 5.

- Naveen


Naveen N. Rao (6):
  trace/stack: Move code to save the stack trace into a separate
    function
  powerpc/trace: Add support for stack tracer
  powerpc: Indicate traced function name in show_stack()
  powerpc/perf: Include traced function in the callchain
  powerpc/stacktrace: Include ftraced function in
    arch_stack_walk_reliable()
  powerpc/stacktrace: Include ftraced function in arch_stack_walk()

 arch/powerpc/include/asm/ftrace.h  | 18 ++++++
 arch/powerpc/kernel/process.c      |  3 +
 arch/powerpc/kernel/stacktrace.c   |  8 +++
 arch/powerpc/kernel/trace/ftrace.c | 70 +++++++++++++++++++++
 arch/powerpc/perf/callchain.c      |  5 ++
 include/linux/ftrace.h             |  8 +++
 kernel/trace/trace_stack.c         | 98 ++++++++++++++++--------------
 7 files changed, 164 insertions(+), 46 deletions(-)


base-commit: 258eb1f3aaa9face35e613c229c1337263491ea0
-- 
2.30.2


^ permalink raw reply

* [RFC PATCH 1/6] trace/stack: Move code to save the stack trace into a separate function
From: Naveen N. Rao @ 2021-05-21  6:48 UTC (permalink / raw)
  To: Steven Rostedt, Michael Ellerman, Michal Suchanek, Torsten Duwe
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1621577151.git.naveen.n.rao@linux.vnet.ibm.com>

In preparation to add support for stack tracer to powerpc, move code to
save stack trace and to calculate the frame sizes into a separate weak
function. Also provide access to some of the data structures used by the
stack trace code so that architectures can update those.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 include/linux/ftrace.h     |  8 ++++
 kernel/trace/trace_stack.c | 98 ++++++++++++++++++++------------------
 2 files changed, 60 insertions(+), 46 deletions(-)

diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index a69f363b61bf73..8263427379f05c 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -368,10 +368,18 @@ static inline void arch_ftrace_set_direct_caller(struct pt_regs *regs,
 
 #ifdef CONFIG_STACK_TRACER
 
+#define STACK_TRACE_ENTRIES 500
+
+extern unsigned long stack_dump_trace[STACK_TRACE_ENTRIES];
+extern unsigned stack_trace_index[STACK_TRACE_ENTRIES];
+extern unsigned int stack_trace_nr_entries;
+extern unsigned long stack_trace_max_size;
 extern int stack_tracer_enabled;
 
 int stack_trace_sysctl(struct ctl_table *table, int write, void *buffer,
 		       size_t *lenp, loff_t *ppos);
+void stack_get_trace(unsigned long traced_ip, unsigned long *stack_ref,
+					unsigned long stack_size, int *tracer_frame);
 
 /* DO NOT MODIFY THIS VARIABLE DIRECTLY! */
 DECLARE_PER_CPU(int, disable_stack_tracer);
diff --git a/kernel/trace/trace_stack.c b/kernel/trace/trace_stack.c
index 63c28504205162..5b63dbd37c8c25 100644
--- a/kernel/trace/trace_stack.c
+++ b/kernel/trace/trace_stack.c
@@ -19,13 +19,11 @@
 
 #include "trace.h"
 
-#define STACK_TRACE_ENTRIES 500
+unsigned long stack_dump_trace[STACK_TRACE_ENTRIES];
+unsigned stack_trace_index[STACK_TRACE_ENTRIES];
 
-static unsigned long stack_dump_trace[STACK_TRACE_ENTRIES];
-static unsigned stack_trace_index[STACK_TRACE_ENTRIES];
-
-static unsigned int stack_trace_nr_entries;
-static unsigned long stack_trace_max_size;
+unsigned int stack_trace_nr_entries;
+unsigned long stack_trace_max_size;
 static arch_spinlock_t stack_trace_max_lock =
 	(arch_spinlock_t)__ARCH_SPIN_LOCK_UNLOCKED;
 
@@ -152,49 +150,19 @@ static void print_max_stack(void)
  * Although the entry function is not displayed, the first function (sys_foo)
  * will still include the stack size of it.
  */
-static void check_stack(unsigned long ip, unsigned long *stack)
+void __weak stack_get_trace(unsigned long traced_ip, unsigned long *stack_ref,
+					unsigned long stack_size, int *tracer_frame)
 {
-	unsigned long this_size, flags; unsigned long *p, *top, *start;
-	static int tracer_frame;
-	int frame_size = READ_ONCE(tracer_frame);
+	unsigned long *p, *top, *start;
 	int i, x;
 
-	this_size = ((unsigned long)stack) & (THREAD_SIZE-1);
-	this_size = THREAD_SIZE - this_size;
-	/* Remove the frame of the tracer */
-	this_size -= frame_size;
-
-	if (this_size <= stack_trace_max_size)
-		return;
-
-	/* we do not handle interrupt stacks yet */
-	if (!object_is_on_stack(stack))
-		return;
-
-	/* Can't do this from NMI context (can cause deadlocks) */
-	if (in_nmi())
-		return;
-
-	local_irq_save(flags);
-	arch_spin_lock(&stack_trace_max_lock);
-
-	/* In case another CPU set the tracer_frame on us */
-	if (unlikely(!frame_size))
-		this_size -= tracer_frame;
-
-	/* a race could have already updated it */
-	if (this_size <= stack_trace_max_size)
-		goto out;
-
-	stack_trace_max_size = this_size;
-
 	stack_trace_nr_entries = stack_trace_save(stack_dump_trace,
 					       ARRAY_SIZE(stack_dump_trace) - 1,
 					       0);
 
 	/* Skip over the overhead of the stack tracer itself */
 	for (i = 0; i < stack_trace_nr_entries; i++) {
-		if (stack_dump_trace[i] == ip)
+		if (stack_dump_trace[i] == traced_ip)
 			break;
 	}
 
@@ -209,7 +177,7 @@ static void check_stack(unsigned long ip, unsigned long *stack)
 	 * Now find where in the stack these are.
 	 */
 	x = 0;
-	start = stack;
+	start = stack_ref;
 	top = (unsigned long *)
 		(((unsigned long)start & ~(THREAD_SIZE-1)) + THREAD_SIZE);
 
@@ -223,7 +191,7 @@ static void check_stack(unsigned long ip, unsigned long *stack)
 	while (i < stack_trace_nr_entries) {
 		int found = 0;
 
-		stack_trace_index[x] = this_size;
+		stack_trace_index[x] = stack_size;
 		p = start;
 
 		for (; p < top && i < stack_trace_nr_entries; p++) {
@@ -233,7 +201,7 @@ static void check_stack(unsigned long ip, unsigned long *stack)
 			 */
 			if ((READ_ONCE_NOCHECK(*p)) == stack_dump_trace[i]) {
 				stack_dump_trace[x] = stack_dump_trace[i++];
-				this_size = stack_trace_index[x++] =
+				stack_size = stack_trace_index[x++] =
 					(top - p) * sizeof(unsigned long);
 				found = 1;
 				/* Start the search from here */
@@ -245,10 +213,10 @@ static void check_stack(unsigned long ip, unsigned long *stack)
 				 * out what that is, then figure it out
 				 * now.
 				 */
-				if (unlikely(!tracer_frame)) {
-					tracer_frame = (p - stack) *
+				if (unlikely(!*tracer_frame)) {
+					*tracer_frame = (p - stack_ref) *
 						sizeof(unsigned long);
-					stack_trace_max_size -= tracer_frame;
+					stack_trace_max_size -= *tracer_frame;
 				}
 			}
 		}
@@ -272,6 +240,44 @@ static void check_stack(unsigned long ip, unsigned long *stack)
 #endif
 
 	stack_trace_nr_entries = x;
+}
+
+static void check_stack(unsigned long ip, unsigned long *stack)
+{
+	unsigned long this_size, flags;
+	static int tracer_frame;
+	int frame_size = READ_ONCE(tracer_frame);
+
+	this_size = ((unsigned long)stack) & (THREAD_SIZE-1);
+	this_size = THREAD_SIZE - this_size;
+	/* Remove the frame of the tracer */
+	this_size -= frame_size;
+
+	if (this_size <= stack_trace_max_size)
+		return;
+
+	/* we do not handle interrupt stacks yet */
+	if (!object_is_on_stack(stack))
+		return;
+
+	/* Can't do this from NMI context (can cause deadlocks) */
+	if (in_nmi())
+		return;
+
+	local_irq_save(flags);
+	arch_spin_lock(&stack_trace_max_lock);
+
+	/* In case another CPU set the tracer_frame on us */
+	if (unlikely(!frame_size))
+		this_size -= tracer_frame;
+
+	/* a race could have already updated it */
+	if (this_size <= stack_trace_max_size)
+		goto out;
+
+	stack_trace_max_size = this_size;
+
+	stack_get_trace(ip, stack, this_size, &tracer_frame);
 
 	if (task_stack_end_corrupted(current)) {
 		print_max_stack();
-- 
2.30.2


^ permalink raw reply related

* [RFC PATCH 2/6] powerpc/trace: Add support for stack tracer
From: Naveen N. Rao @ 2021-05-21  6:48 UTC (permalink / raw)
  To: Steven Rostedt, Michael Ellerman, Michal Suchanek, Torsten Duwe
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1621577151.git.naveen.n.rao@linux.vnet.ibm.com>

With -mprofile-kernel and ppc32, we call into ftrace at function entry
before the function can establish its own stack frame. This breaks the
ABI since functions are expected to setup a stack frame before calling
into another function. As a consequence of this, when walking the stack,
the ftraced function does not show up in the stack trace.

Fix this by checking for ftrace functions (ftrace_[regs_]call+4) in the
stack trace and looking up the stored nip in pt_regs in its stackframe.
Use the back chain from the stack frame headers to accurately determine
the stack frame sizes, except for the ftraced function on
-mprofile-kernel and ppc32 where we set the frame size to 0.

The max stack tracer ftrace selftest (ftrace/func_stack_tracer.tc)
passes on -mprofile-kernel with this patch.

Before this patch, top of a stack trace with the stack tracer:
        Depth    Size   Location    (44 entries)
        -----    ----   --------
  0)     7616     496   ftrace_call+0x4/0x44
  1)     7120      64   __mod_lruvec_page_state+0x90/0x110
  2)     7056      96   test_clear_page_writeback+0xe4/0x480
  3)     6960      48   end_page_writeback+0xa0/0x1c0
  4)     6912     256   ext4_finish_bio+0x2c0/0x350
  5)     6656     176   ext4_end_bio+0x74/0x280
  6)     6480      64   bio_endio+0x1cc/0x240
  7)     6416     176   blk_update_request+0x2b8/0x640
  8)     6240      64   blk_mq_end_request+0x3c/0x1e0
  9)     6176      48   virtblk_request_done+0x48/0xd0
 10)     6128      48   blk_complete_reqs+0x80/0xa0
 11)     6080     240   __do_softirq+0x150/0x408
 12)     5840      32   irq_exit+0x144/0x150
 13)     5808      80   do_IRQ+0xc8/0x140
 14)     5728      32   hardware_interrupt_common_virt+0x1a4/0x1b0
 15)     5696      64   0x0
 16)     5632     768   virtqueue_notify+0x40/0x80
 17)     4864     240   virtio_queue_rq+0x568/0x610
 18)     4624     256   blk_mq_dispatch_rq_list+0x190/0xbc0
 19)     4368     160   __blk_mq_do_dispatch_sched+0x1f0/0x3d0
 20)     4208      96   __blk_mq_sched_dispatch_requests+0x238/0x2c0
 ...

After this patch:
        Depth    Size   Location    (44 entries)
        -----    ----   --------
  0)     7136       0   rcu_read_unlock_strict+0x8/0x10
  1)     7136      64   __mod_lruvec_page_state+0x90/0x110
  2)     7072      96   test_clear_page_writeback+0xe4/0x480
  3)     6976      48   end_page_writeback+0xa0/0x1c0
  4)     6928     256   ext4_finish_bio+0x2c0/0x350
  5)     6672     176   ext4_end_bio+0x74/0x280
  6)     6496      64   bio_endio+0x1cc/0x240
  7)     6432     176   blk_update_request+0x2b8/0x640
  8)     6256      64   blk_mq_end_request+0x3c/0x1e0
  9)     6192      48   virtblk_request_done+0x48/0xd0
 10)     6144      48   blk_complete_reqs+0x80/0xa0
 11)     6096     240   __do_softirq+0x150/0x408
 12)     5856      32   irq_exit+0x144/0x150
 13)     5824      80   do_IRQ+0xc8/0x140
 14)     5744     784   hardware_interrupt_common_virt+0x1a4/0x1b0
 15)     4960      32   0x0
 16)     4928      48   virtqueue_notify+0x40/0x80
 17)     4880     240   virtio_queue_rq+0x568/0x610
 18)     4640     256   blk_mq_dispatch_rq_list+0x190/0xbc0
 19)     4384     160   __blk_mq_do_dispatch_sched+0x1f0/0x3d0
 20)     4224      96   __blk_mq_sched_dispatch_requests+0x238/0x2c0
 ...

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/ftrace.h  | 18 ++++++++
 arch/powerpc/kernel/trace/ftrace.c | 70 ++++++++++++++++++++++++++++++
 2 files changed, 88 insertions(+)

diff --git a/arch/powerpc/include/asm/ftrace.h b/arch/powerpc/include/asm/ftrace.h
index debe8c4f706260..392296df70e96c 100644
--- a/arch/powerpc/include/asm/ftrace.h
+++ b/arch/powerpc/include/asm/ftrace.h
@@ -126,6 +126,24 @@ static inline void this_cpu_enable_ftrace(void) { }
 static inline void this_cpu_set_ftrace_enabled(u8 ftrace_enabled) { }
 static inline u8 this_cpu_get_ftrace_enabled(void) { return 1; }
 #endif /* CONFIG_PPC64 */
+
+#ifdef CONFIG_FUNCTION_TRACER
+/*
+ * With ppc64 -mprofile-kernel and ppc32, mcount call is made before a function
+ * establishes its own stack frame. While unwinding the stack, such functions
+ * do not appear in the trace. This helper returns the traced function if ip in
+ * the stack frame points to ftrace_[regs_]call.
+ *
+ * In ppc64 ELFv1, mcount call is after a function establishes its own
+ * stackframe. So, this always returns 0.
+ */
+unsigned long ftrace_get_traced_func_if_no_stackframe(unsigned long ip, unsigned long *stack);
+#else
+static inline unsigned long ftrace_get_traced_func_if_no_stackframe(unsigned long ip, unsigned long *stack)
+{
+	return 0;
+}
+#endif /* FUNCTION_TRACER */
 #endif /* !__ASSEMBLY__ */
 
 #endif /* _ASM_POWERPC_FTRACE */
diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c
index ffe9537195aa33..ec1072d9a858d0 100644
--- a/arch/powerpc/kernel/trace/ftrace.c
+++ b/arch/powerpc/kernel/trace/ftrace.c
@@ -21,6 +21,7 @@
 #include <linux/percpu.h>
 #include <linux/init.h>
 #include <linux/list.h>
+#include <linux/sched/task_stack.h>
 
 #include <asm/asm-prototypes.h>
 #include <asm/cacheflush.h>
@@ -987,3 +988,72 @@ char *arch_ftrace_match_adjust(char *str, const char *search)
 		return str;
 }
 #endif /* PPC64_ELF_ABI_v1 */
+
+static int is_ftrace_entry(unsigned long ip)
+{
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+	if (ip == (unsigned long)&ftrace_call + 4 || ip == (unsigned long)&ftrace_regs_call + 4)
+#else
+	if (ip == (unsigned long)&ftrace_call + 4)
+#endif
+		return 1;
+
+	return 0;
+}
+
+unsigned long ftrace_get_traced_func_if_no_stackframe(unsigned long ip, unsigned long *stack)
+{
+	if (!is_ftrace_entry(ip))
+		return 0;
+
+	if (IS_ENABLED(CONFIG_PPC32))
+		return stack[11]; /* see MCOUNT_SAVE_FRAME */
+
+	if (!IS_ENABLED(CONFIG_MPROFILE_KERNEL))
+		return 0;
+
+	return stack[(STACK_FRAME_OVERHEAD + offsetof(struct pt_regs, nip)) / sizeof(unsigned long)];
+}
+
+#ifdef CONFIG_STACK_TRACER
+void stack_get_trace(unsigned long traced_ip,
+		     unsigned long *stack_ref __maybe_unused,
+		     unsigned long stack_size __maybe_unused,
+		     int *tracer_frame)
+{
+	unsigned long sp, newsp, top, ip;
+	int ftrace_call_found = 0;
+	unsigned long *stack;
+	int i = 0;
+
+	sp = current_stack_frame();
+	top = (unsigned long)task_stack_page(current) + THREAD_SIZE;
+
+	while (validate_sp(sp, current, STACK_FRAME_OVERHEAD) && i < STACK_TRACE_ENTRIES) {
+		stack = (unsigned long *) sp;
+		newsp = stack[0];
+		ip = stack[STACK_FRAME_LR_SAVE];
+
+		if (ftrace_call_found) {
+			stack_dump_trace[i] = ip;
+			stack_trace_index[i++] = top - sp;
+		}
+
+		if (is_ftrace_entry(ip)) {
+			if (IS_ENABLED(CONFIG_MPROFILE_KERNEL) || IS_ENABLED(CONFIG_PPC32)) {
+				stack_dump_trace[i] = ftrace_get_traced_func_if_no_stackframe(ip, stack);
+				stack_trace_index[i++] = top - newsp;
+			}
+			if (unlikely(!*tracer_frame)) {
+				*tracer_frame = newsp - (unsigned long)stack_ref;
+				stack_trace_max_size -= *tracer_frame;
+			}
+			ftrace_call_found = 1;
+		}
+
+		sp = newsp;
+	}
+
+	stack_trace_nr_entries = i;
+}
+#endif
-- 
2.30.2


^ permalink raw reply related

* [RFC PATCH 3/6] powerpc: Indicate traced function name in show_stack()
From: Naveen N. Rao @ 2021-05-21  6:48 UTC (permalink / raw)
  To: Steven Rostedt, Michael Ellerman, Michal Suchanek, Torsten Duwe
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1621577151.git.naveen.n.rao@linux.vnet.ibm.com>

With -mprofile-kernel and ppc32, the function tracer is invoked before a
function sets up its own stackframe. This results in the traced function
not appearing in stack traces. Fix this by checking for ftrace entry and
including the traced function in the stack trace.

With this patch, stack traces show the function traced next to the
ftrace entry:
    NIP [c0080000003700d4] handler_pre+0x1c/0x5c [kprobe_example]
    LR [c00000000006c118] kprobe_ftrace_handler+0x1b8/0x270
    Call Trace:
    [c00000001ed7fa50] [c00000001ed7faa0] 0xc00000001ed7faa0 (unreliable)
    [c00000001ed7fab0] [c00000000006c118] kprobe_ftrace_handler+0x1b8/0x270
    [c00000001ed7fb00] [c000000000076604] ftrace_regs_call+0x4/0xa4 (kernel_clone+0xc/0x600)
                                                                    ^^^^^^^^^^^^^^^^^^^^^^^^
    [c00000001ed7fcf0] [c000000000139e08] __do_sys_clone+0x88/0xd0
    [c00000001ed7fdb0] [c00000000002b6c4] system_call_exception+0xf4/0x200
    [c00000001ed7fe10] [c00000000000ca5c] system_call_common+0xec/0x278

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/process.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 89e34aa273e21a..9df1d44939bec1 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -2160,6 +2160,9 @@ void show_stack(struct task_struct *tsk, unsigned long *stack,
 						&ftrace_idx, ip, stack);
 			if (ret_addr != ip)
 				pr_cont(" (%pS)", (void *)ret_addr);
+			ip = ftrace_get_traced_func_if_no_stackframe(ip, stack);
+			if (ip)
+				pr_cont(" (%pS)", (void *)ip);
 			if (firstframe)
 				pr_cont(" (unreliable)");
 			pr_cont("\n");
-- 
2.30.2


^ permalink raw reply related

* [RFC PATCH 4/6] powerpc/perf: Include traced function in the callchain
From: Naveen N. Rao @ 2021-05-21  6:48 UTC (permalink / raw)
  To: Steven Rostedt, Michael Ellerman, Michal Suchanek, Torsten Duwe
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1621577151.git.naveen.n.rao@linux.vnet.ibm.com>

With -mprofile-kernel and ppc32, the function tracer is invoked before a
function sets up its own stackframe. This results in the traced function
not appearing in stack traces. Fix this by checking for ftrace entry and
including the traced function in the perf callchain.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 arch/powerpc/perf/callchain.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/powerpc/perf/callchain.c b/arch/powerpc/perf/callchain.c
index 6c028ee513c0d7..ed4dd4ab6621f6 100644
--- a/arch/powerpc/perf/callchain.c
+++ b/arch/powerpc/perf/callchain.c
@@ -93,6 +93,11 @@ perf_callchain_kernel(struct perf_callchain_entry_ctx *entry, struct pt_regs *re
 		}
 
 		perf_callchain_store(entry, next_ip);
+		if (level && next_ip) {
+			next_ip = ftrace_get_traced_func_if_no_stackframe(next_ip, fp);
+			if (next_ip)
+				perf_callchain_store(entry, next_ip);
+		}
 		if (!valid_next_sp(next_sp, sp))
 			return;
 		sp = next_sp;
-- 
2.30.2


^ permalink raw reply related

* [RFC PATCH 5/6] powerpc/stacktrace: Include ftraced function in arch_stack_walk_reliable()
From: Naveen N. Rao @ 2021-05-21  6:48 UTC (permalink / raw)
  To: Steven Rostedt, Michael Ellerman, Michal Suchanek, Torsten Duwe
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1621577151.git.naveen.n.rao@linux.vnet.ibm.com>

With -mprofile-kernel and ppc32, the function tracer is invoked before a
function sets up its own stackframe. This results in the traced function
not appearing in stack traces. Fix this by checking for ftrace entry and
including the traced function in the stack trace.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
While I don't think we expect to see ftrace show up in the trace for a 
non-running task, I think it is good to cover this scenario. I also 
think it is ok to consider such traces to be reliable. But, I'm not sure 
of the implications w.r.t livepatching.

- Naveen


 arch/powerpc/kernel/stacktrace.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/powerpc/kernel/stacktrace.c b/arch/powerpc/kernel/stacktrace.c
index 1deb1bf331ddbd..1e1be297c5d6c7 100644
--- a/arch/powerpc/kernel/stacktrace.c
+++ b/arch/powerpc/kernel/stacktrace.c
@@ -160,6 +160,10 @@ int arch_stack_walk_reliable(stack_trace_consume_fn consume_entry,
 
 		if (!consume_entry(cookie, ip))
 			return -EINVAL;
+
+		ip = ftrace_get_traced_func_if_no_stackframe(ip, stack);
+		if (ip && !consume_entry(cookie, ip))
+			return -EINVAL;
 	}
 	return 0;
 }
-- 
2.30.2


^ permalink raw reply related

* [RFC PATCH 6/6] powerpc/stacktrace: Include ftraced function in arch_stack_walk()
From: Naveen N. Rao @ 2021-05-21  6:48 UTC (permalink / raw)
  To: Steven Rostedt, Michael Ellerman, Michal Suchanek, Torsten Duwe
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1621577151.git.naveen.n.rao@linux.vnet.ibm.com>

With -mprofile-kernel and ppc32, the function tracer is invoked before a
function sets up its own stackframe. This results in the traced function
not appearing in stack traces. Fix this by checking for ftrace entry and
including the traced function in the stack trace.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/stacktrace.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/powerpc/kernel/stacktrace.c b/arch/powerpc/kernel/stacktrace.c
index 1e1be297c5d6c7..f884c5d21fa7e7 100644
--- a/arch/powerpc/kernel/stacktrace.c
+++ b/arch/powerpc/kernel/stacktrace.c
@@ -51,6 +51,10 @@ void arch_stack_walk(stack_trace_consume_fn consume_entry, void *cookie,
 		if (!consume_entry(cookie, ip))
 			return;
 
+		ip = ftrace_get_traced_func_if_no_stackframe(ip, stack);
+		if (ip && !consume_entry(cookie, ip))
+			return;
+
 		sp = newsp;
 	}
 }
-- 
2.30.2


^ permalink raw reply related

* Re: [PATCH] powerpc/kprobes: Pass ppc_inst as a pointer to emulate_step() on ppc32
From: Naveen N. Rao @ 2021-05-21  7:01 UTC (permalink / raw)
  To: Christophe Leroy, Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <cb7d49dc-3980-2d2b-ee4e-480b89a04b0a@csgroup.eu>

Christophe Leroy wrote:
> 
> 
> Le 20/05/2021 à 12:54, Naveen N. Rao a écrit :
>> Christophe Leroy wrote:
>>>
>>>
>>> Le 20/05/2021 à 09:29, Naveen N. Rao a écrit :
>>>> diff --git a/arch/powerpc/kernel/optprobes.c 
>>>> b/arch/powerpc/kernel/optprobes.c
>>>> index cdf87086fa33a0..2bc53fa48a1b33 100644
>>>> --- a/arch/powerpc/kernel/optprobes.c
>>>> +++ b/arch/powerpc/kernel/optprobes.c
>>>> @@ -281,8 +281,12 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe 
>>>> *p)
>>>>       /*
>>>>        * 3. load instruction to be emulated into relevant register, and
>>>>        */
>>>> -    temp = ppc_inst_read((struct ppc_inst *)p->ainsn.insn);
>>>> -    patch_imm_load_insns(ppc_inst_as_ulong(temp), 4, buff + TMPL_INSN_IDX);
>>>> +    if (IS_ENABLED(CONFIG_PPC64)) {
>>>> +        temp = ppc_inst_read((struct ppc_inst *)p->ainsn.insn);
>>>> +        patch_imm_load_insns(ppc_inst_as_ulong(temp), 4, buff + TMPL_INSN_IDX);
>>>> +    } else {
>>>> +        patch_imm_load_insns((unsigned long)p->ainsn.insn, 4, buff + TMPL_INSN_IDX);
>>>> +    }
>>>
>>> It means commit https://github.com/linuxppc/linux/commit/693557ebf407a85ea400a0b501bb97687d8f4856 
>>> was not necessary and may be reverted.
>> 
>> Indeed, I will send a revert for it.
>> 
> 
> I'm not completely sure it is worth reverting, on an other hand it is pointless anyway to have 
> something to convert to a u64 something that cannot be more than 32 bits on a PPC32, so now that we 
> have ppc_inst_as_ulong() it is as good I think.

Sure. If necessary, the revert can go in separately as part of your 
other cleanup series.

- Naveen


^ permalink raw reply

* Re: [PATCH 00/16] Rid W=1 warnings from Char
From: Lee Jones @ 2021-05-21  7:22 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Tyrel Datwyler, Jason A. Donenfeld, David Airlie, Clemens Ladisch,
	C. Scott Ananian, Paul Mackerras, Kylene Hall, Jerome Glisse,
	Peter Huewe, Michael Neuling, Herbert Xu, Dave Safford,
	Harald Welte, Jason Gunthorpe, Kanoj Sarcar, David Woodhouse,
	Bob Picco, Uwe Kleine-König, Thirupathaiah Annapureddy, cs.c,
	Lijun Pan, Reiner Sailer, Matt Mackall, van Doorn,
	Theodore Ts'o, Greg Kroah-Hartman, Linux Kernel Mailing List,
	Paul Fulghum, Jarkko Sakkinen,
	open list:HARDWARE RANDOM NUMBER GENERATOR CORE, linux-integrity,
	linuxppc-dev
In-Reply-To: <CAK8P3a0VujuG8eU_CEVSvzbk4nAJz8fStedM5eMUrLAr9EJxDQ@mail.gmail.com>

On Thu, 20 May 2021, Arnd Bergmann wrote:

> On Thu, May 20, 2021 at 2:13 PM Lee Jones <lee.jones@linaro.org> wrote:
> >
> > This set is part of a larger effort attempting to clean-up W=1
> > kernel builds, which are currently overwhelmingly riddled with
> > niggly little warnings.
> >
> > Lee Jones (16):
> >   char: pcmcia: cm4000_cs: Remove unused variable 'tmp'
> >   char: pcmcia: cm4040_cs: Remove unused variable 'uc'
> >   char: random: Include header containing our prototypes
> >   char: pcmcia: synclink_cs: Fix a bunch of kernel-doc issues
> >   char: pcmcia: synclink_cs: Fix a bunch of kernel-doc issues
> >   char: applicom: Remove 3 unused variables 'ret' and 2 instances of
> >     'byte_reset_it'
> >   char: tpm: tpm1-cmd: Fix a couple of misnamed functions
> >   char: tpm: tpm_ftpm_tee: Fix a couple of kernel-doc misdemeanours
> >   char: agp: backend: Demote some non-conformant kernel-doc headers
> >   char: agp: frontend: Include header file containing our prototypes
> >   char: agp: via-agp: Remove unused variable 'current_size'
> >   char: hpet: Remove unused variable 'm'
> >   char: agp: generic: Place braces around optimised out function in if()
> >   char: agp: uninorth-agp: Remove unused variable 'size'
> >   char: hw_random: pseries-rng: Demote non-conformant kernel-doc header
> >   char: mem: Provide local prototype for non-static function
> 
> Thanks a lot!
> 
> I've looked all the patches now and commented on patches 6 and 16.
> With my comments addressed
> 
> Reviewed-by: Arnd Bergmann <arnd@arndb.de>

Thanks Arnd.

Would it be possible for the remaining 14 patches to be taken in
please?  I will work on the 2 Arnd commented on in due course and
resubmit them independently.

TIA.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* Re: [PATCH 00/16] Rid W=1 warnings from Char
From: Greg Kroah-Hartman @ 2021-05-21  7:42 UTC (permalink / raw)
  To: Lee Jones
  Cc: Tyrel Datwyler, Jason A. Donenfeld, David Airlie, Clemens Ladisch,
	C. Scott Ananian, Paul Mackerras, Kylene Hall, Jerome Glisse,
	Peter Huewe, Michael Neuling, Herbert Xu, Dave Safford,
	Harald Welte, Jason Gunthorpe, Kanoj Sarcar, Bob Picco,
	Arnd Bergmann, Thirupathaiah Annapureddy, cs.c, Lijun Pan,
	Reiner Sailer, Matt Mackall, van Doorn, Theodore Ts'o,
	David Woodhouse, Linux Kernel Mailing List, Paul Fulghum,
	Jarkko Sakkinen, Uwe Kleine-König,
	open list:HARDWARE RANDOM NUMBER GENERATOR CORE, linux-integrity,
	linuxppc-dev
In-Reply-To: <20210521072236.GX2549456@dell>

On Fri, May 21, 2021 at 08:22:36AM +0100, Lee Jones wrote:
> On Thu, 20 May 2021, Arnd Bergmann wrote:
> 
> > On Thu, May 20, 2021 at 2:13 PM Lee Jones <lee.jones@linaro.org> wrote:
> > >
> > > This set is part of a larger effort attempting to clean-up W=1
> > > kernel builds, which are currently overwhelmingly riddled with
> > > niggly little warnings.
> > >
> > > Lee Jones (16):
> > >   char: pcmcia: cm4000_cs: Remove unused variable 'tmp'
> > >   char: pcmcia: cm4040_cs: Remove unused variable 'uc'
> > >   char: random: Include header containing our prototypes
> > >   char: pcmcia: synclink_cs: Fix a bunch of kernel-doc issues
> > >   char: pcmcia: synclink_cs: Fix a bunch of kernel-doc issues
> > >   char: applicom: Remove 3 unused variables 'ret' and 2 instances of
> > >     'byte_reset_it'
> > >   char: tpm: tpm1-cmd: Fix a couple of misnamed functions
> > >   char: tpm: tpm_ftpm_tee: Fix a couple of kernel-doc misdemeanours
> > >   char: agp: backend: Demote some non-conformant kernel-doc headers
> > >   char: agp: frontend: Include header file containing our prototypes
> > >   char: agp: via-agp: Remove unused variable 'current_size'
> > >   char: hpet: Remove unused variable 'm'
> > >   char: agp: generic: Place braces around optimised out function in if()
> > >   char: agp: uninorth-agp: Remove unused variable 'size'
> > >   char: hw_random: pseries-rng: Demote non-conformant kernel-doc header
> > >   char: mem: Provide local prototype for non-static function
> > 
> > Thanks a lot!
> > 
> > I've looked all the patches now and commented on patches 6 and 16.
> > With my comments addressed
> > 
> > Reviewed-by: Arnd Bergmann <arnd@arndb.de>
> 
> Thanks Arnd.
> 
> Would it be possible for the remaining 14 patches to be taken in
> please?  I will work on the 2 Arnd commented on in due course and
> resubmit them independently.

Yes, I can queue them up, thanks.

greg k-h

^ permalink raw reply

* Re: [PATCH 00/16] Rid W=1 warnings from Char
From: Greg Kroah-Hartman @ 2021-05-21  7:44 UTC (permalink / raw)
  To: Lee Jones
  Cc: Tyrel Datwyler, Jason A. Donenfeld, David Airlie, Clemens Ladisch,
	C. Scott Ananian, Paul Mackerras, Kylene Hall, Jerome Glisse,
	Peter Huewe, Michael Neuling, Herbert Xu, Dave Safford,
	Harald Welte, Jason Gunthorpe, Kanoj Sarcar, Bob Picco,
	Arnd Bergmann, Thirupathaiah Annapureddy, cs.c, Lijun Pan,
	Reiner Sailer, Matt Mackall, van Doorn, Theodore Ts'o,
	David Woodhouse, Linux Kernel Mailing List, Paul Fulghum,
	Jarkko Sakkinen, Uwe Kleine-König,
	open list:HARDWARE RANDOM NUMBER GENERATOR CORE, linux-integrity,
	linuxppc-dev
In-Reply-To: <YKdkZdvN+uu6lu0g@kroah.com>

On Fri, May 21, 2021 at 09:42:29AM +0200, Greg Kroah-Hartman wrote:
> On Fri, May 21, 2021 at 08:22:36AM +0100, Lee Jones wrote:
> > On Thu, 20 May 2021, Arnd Bergmann wrote:
> > 
> > > On Thu, May 20, 2021 at 2:13 PM Lee Jones <lee.jones@linaro.org> wrote:
> > > >
> > > > This set is part of a larger effort attempting to clean-up W=1
> > > > kernel builds, which are currently overwhelmingly riddled with
> > > > niggly little warnings.
> > > >
> > > > Lee Jones (16):
> > > >   char: pcmcia: cm4000_cs: Remove unused variable 'tmp'
> > > >   char: pcmcia: cm4040_cs: Remove unused variable 'uc'
> > > >   char: random: Include header containing our prototypes
> > > >   char: pcmcia: synclink_cs: Fix a bunch of kernel-doc issues
> > > >   char: pcmcia: synclink_cs: Fix a bunch of kernel-doc issues
> > > >   char: applicom: Remove 3 unused variables 'ret' and 2 instances of
> > > >     'byte_reset_it'
> > > >   char: tpm: tpm1-cmd: Fix a couple of misnamed functions
> > > >   char: tpm: tpm_ftpm_tee: Fix a couple of kernel-doc misdemeanours
> > > >   char: agp: backend: Demote some non-conformant kernel-doc headers
> > > >   char: agp: frontend: Include header file containing our prototypes
> > > >   char: agp: via-agp: Remove unused variable 'current_size'
> > > >   char: hpet: Remove unused variable 'm'
> > > >   char: agp: generic: Place braces around optimised out function in if()
> > > >   char: agp: uninorth-agp: Remove unused variable 'size'
> > > >   char: hw_random: pseries-rng: Demote non-conformant kernel-doc header
> > > >   char: mem: Provide local prototype for non-static function
> > > 
> > > Thanks a lot!
> > > 
> > > I've looked all the patches now and commented on patches 6 and 16.
> > > With my comments addressed
> > > 
> > > Reviewed-by: Arnd Bergmann <arnd@arndb.de>
> > 
> > Thanks Arnd.
> > 
> > Would it be possible for the remaining 14 patches to be taken in
> > please?  I will work on the 2 Arnd commented on in due course and
> > resubmit them independently.
> 
> Yes, I can queue them up, thanks.

Wait, I can take the ones for code that I maintain, not for others.
drivers/char/ is "fun" in that some subdirs have active
trees/maintainers that are not me and Arnd so they go to Linus through
other ways.  I'll queue up the ones that I am "allowed" to and you will
get the emails from that.

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH 00/16] Rid W=1 warnings from Char
From: Lee Jones @ 2021-05-21  7:56 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Tyrel Datwyler, Jason A. Donenfeld, David Airlie, Clemens Ladisch,
	C. Scott Ananian, Paul Mackerras, Kylene Hall, Jerome Glisse,
	Peter Huewe, Michael Neuling, Herbert Xu, Dave Safford,
	Harald Welte, Jason Gunthorpe, Kanoj Sarcar, Bob Picco,
	Arnd Bergmann, Thirupathaiah Annapureddy, cs.c, Lijun Pan,
	Reiner Sailer, Matt Mackall, van Doorn, Theodore Ts'o,
	David Woodhouse, Linux Kernel Mailing List, Paul Fulghum,
	Jarkko Sakkinen, Uwe Kleine-König,
	open list:HARDWARE RANDOM NUMBER GENERATOR CORE, linux-integrity,
	linuxppc-dev
In-Reply-To: <YKdk2NYHysgTGlRt@kroah.com>

On Fri, 21 May 2021, Greg Kroah-Hartman wrote:

> On Fri, May 21, 2021 at 09:42:29AM +0200, Greg Kroah-Hartman wrote:
> > On Fri, May 21, 2021 at 08:22:36AM +0100, Lee Jones wrote:
> > > On Thu, 20 May 2021, Arnd Bergmann wrote:
> > > 
> > > > On Thu, May 20, 2021 at 2:13 PM Lee Jones <lee.jones@linaro.org> wrote:
> > > > >
> > > > > This set is part of a larger effort attempting to clean-up W=1
> > > > > kernel builds, which are currently overwhelmingly riddled with
> > > > > niggly little warnings.
> > > > >
> > > > > Lee Jones (16):
> > > > >   char: pcmcia: cm4000_cs: Remove unused variable 'tmp'
> > > > >   char: pcmcia: cm4040_cs: Remove unused variable 'uc'
> > > > >   char: random: Include header containing our prototypes
> > > > >   char: pcmcia: synclink_cs: Fix a bunch of kernel-doc issues
> > > > >   char: pcmcia: synclink_cs: Fix a bunch of kernel-doc issues
> > > > >   char: applicom: Remove 3 unused variables 'ret' and 2 instances of
> > > > >     'byte_reset_it'
> > > > >   char: tpm: tpm1-cmd: Fix a couple of misnamed functions
> > > > >   char: tpm: tpm_ftpm_tee: Fix a couple of kernel-doc misdemeanours
> > > > >   char: agp: backend: Demote some non-conformant kernel-doc headers
> > > > >   char: agp: frontend: Include header file containing our prototypes
> > > > >   char: agp: via-agp: Remove unused variable 'current_size'
> > > > >   char: hpet: Remove unused variable 'm'
> > > > >   char: agp: generic: Place braces around optimised out function in if()
> > > > >   char: agp: uninorth-agp: Remove unused variable 'size'
> > > > >   char: hw_random: pseries-rng: Demote non-conformant kernel-doc header
> > > > >   char: mem: Provide local prototype for non-static function
> > > > 
> > > > Thanks a lot!
> > > > 
> > > > I've looked all the patches now and commented on patches 6 and 16.
> > > > With my comments addressed
> > > > 
> > > > Reviewed-by: Arnd Bergmann <arnd@arndb.de>
> > > 
> > > Thanks Arnd.
> > > 
> > > Would it be possible for the remaining 14 patches to be taken in
> > > please?  I will work on the 2 Arnd commented on in due course and
> > > resubmit them independently.
> > 
> > Yes, I can queue them up, thanks.
> 
> Wait, I can take the ones for code that I maintain, not for others.
> drivers/char/ is "fun" in that some subdirs have active
> trees/maintainers that are not me and Arnd so they go to Linus through
> other ways.  I'll queue up the ones that I am "allowed" to and you will
> get the emails from that.

Understood, thanks.

Hopefully the other maintainers will be as on the ball as you are.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox