* Re: [PATCH 7/8] mq-deadline: add blk-mq adaptation of the deadline IO scheduler
From: Jens Axboe @ 2017-02-02 15:30 UTC (permalink / raw)
To: Paolo Valente; +Cc: Jens Axboe, linux-block, Linux-Kernal, osandov
In-Reply-To: <4C7AD847-C02D-47FF-83B7-F0BB8FBED5F5@linaro.org>
On 02/02/2017 02:19 AM, Paolo Valente wrote:
> The scheme is clear. One comment, in case it could make sense and
> avoid more complexity: since put_rq_priv is invoked in two different
> contexts, process or interrupt, I didn't feel so confusing that, when
> put_rq_priv is invoked in the context where the lock cannot be held
> (unless one is willing to pay with irq disabling all the times), the
> lock is not held, while, when invoked in the context where the lock
> can be held, the lock is actually held, or must be taken.
If you grab the same lock from put_rq_priv, yes, you must make it IRQ
disabling in all contexts, and use _irqsave() from put_rq_priv. If it's
just freeing resources, you could potentially wait and do that when
someone else needs them, since that part will come from proces context.
That would need two locks, though.
As I said above, I would not worry about the IRQ disabling lock.
--
Jens Axboe
^ permalink raw reply
* Re: [PATCH v3] scsi, block: fix duplicate bdi name registration crashes
From: Jens Axboe @ 2017-02-02 15:23 UTC (permalink / raw)
To: Dan Williams
Cc: Martin K. Petersen, linux-block, Jan Kara, linux-scsi,
linux-kernel@vger.kernel.org, Christoph Hellwig, James Bottomley,
Bart Van Assche, Omar Sandoval, Omar Sandoval
In-Reply-To: <f3671618-eab3-7597-eebb-f46fefbac12a@kernel.dk>
On 02/01/2017 03:43 PM, Jens Axboe wrote:
> On 02/01/2017 02:40 PM, Dan Williams wrote:
>> On Wed, Feb 1, 2017 at 2:35 PM, Jens Axboe <axboe@kernel.dk> wrote:
>>> On 02/01/2017 02:05 PM, Dan Williams wrote:
>>>> Warnings of the following form occur because scsi reuses a devt number
>>>> while the block layer still has it referenced as the name of the bdi
>>>> [1]:
>>>>
>>>> WARNING: CPU: 1 PID: 93 at fs/sysfs/dir.c:31 sysfs_warn_dup+0x62/0x80
>>>> sysfs: cannot create duplicate filename '/devices/virtual/bdi/8:192'
>>>> [..]
>>>> Call Trace:
>>>> dump_stack+0x86/0xc3
>>>> __warn+0xcb/0xf0
>>>> warn_slowpath_fmt+0x5f/0x80
>>>> ? kernfs_path_from_node+0x4f/0x60
>>>> sysfs_warn_dup+0x62/0x80
>>>> sysfs_create_dir_ns+0x77/0x90
>>>> kobject_add_internal+0xb2/0x350
>>>> kobject_add+0x75/0xd0
>>>> device_add+0x15a/0x650
>>>> device_create_groups_vargs+0xe0/0xf0
>>>> device_create_vargs+0x1c/0x20
>>>> bdi_register+0x90/0x240
>>>> ? lockdep_init_map+0x57/0x200
>>>> bdi_register_owner+0x36/0x60
>>>> device_add_disk+0x1bb/0x4e0
>>>> ? __pm_runtime_use_autosuspend+0x5c/0x70
>>>> sd_probe_async+0x10d/0x1c0
>>>> async_run_entry_fn+0x39/0x170
>>>>
>>>> This is a brute-force fix to pass the devt release information from
>>>> sd_probe() to the locations where we register the bdi,
>>>> device_add_disk(), and unregister the bdi, blk_cleanup_queue().
>>>>
>>>> Thanks to Omar for the quick reproducer script [2]. This patch survives
>>>> where an unmodified kernel fails in a few seconds.
>>>
>>> What is the patch against? Doesn't seem to apply cleanly for me on
>>> master, nor the 4.11 block tree.
>>>
>>
>> I built it on top of Jan's bdi fixes series [1]. I can rebase to
>> block/for-next, just let me know which patches you want to take first.
>>
>> [1]: http://marc.info/?l=linux-block&m=148586843819160&w=2
>
> Ah, no that's fine, we'll just pull Jan's stuff in first. Makes sense.
Both are now applied, thanks Dan.
--
Jens Axboe
^ permalink raw reply
* Re: [PATCH 0/5 v3] BDI lifetime fix
From: Jens Axboe @ 2017-02-02 15:22 UTC (permalink / raw)
To: Jan Kara
Cc: linux-block, Christoph Hellwig, Dan Williams,
Thiago Jung Bauermann, Laurent Dufour
In-Reply-To: <20170202145653.19614-1-jack@suse.cz>
On 02/02/2017 07:56 AM, Jan Kara wrote:
> Hello,
>
> this is the third version of the patch series that attempts to solve the
> problems with the life time of a backing_dev_info structure. Currently it lives
> inside request_queue structure and thus it gets destroyed as soon as request
> queue goes away. However the block device inode still stays around and thus
> inode_to_bdi() call on that inode (e.g. from flusher worker) may happen after
> request queue has been destroyed resulting in oops.
>
> This patch set tries to solve these problems by making backing_dev_info
> independent structure referenced from block device inode. That makes sure
> inode_to_bdi() cannot ever oops. I gave some basic testing to the patches in
> KVM and on a real machine, Dan was running them with libnvdimm test suite which
> was previously triggering the oops and things look good. So patches should be
> reasonably healthy.
>
> Changes since v2:
> * Added Reviewed-by tags
> * Removed slab cache for backing_dev_info
> * Added patch to remove blkdev_get_backing_dev_info()
>
> Changes since v1:
> * Use kref instead of atomic_t for refcount
> * Get rid of free_on_put flag
Added for 4.11, thanks Jan!
--
Jens Axboe
^ permalink raw reply
* Re: [PATCH 0/6] block: fix blk-mq debugfs vs. blktrace
From: Jens Axboe @ 2017-02-02 15:17 UTC (permalink / raw)
To: Greg Kroah-Hartman, Omar Sandoval; +Cc: linux-block, kernel-team
In-Reply-To: <20170202105853.GA27128@kroah.com>
On 02/02/2017 03:58 AM, Greg Kroah-Hartman wrote:
> On Wed, Feb 01, 2017 at 12:31:15AM -0800, Omar Sandoval wrote:
>> On Wed, Feb 01, 2017 at 09:16:08AM +0100, Greg Kroah-Hartman wrote:
>>> On Tue, Jan 31, 2017 at 02:53:16PM -0800, Omar Sandoval wrote:
>>>> From: Omar Sandoval <osandov@fb.com>
>>>>
>>>> When I moved the blk-mq debugging information to debugfs, I didn't
>>>> realize that blktrace also created directories in debugfs that
>>>> conflicted with the blk-mq directories. This series fixes that.
>>>>
>>>> Patch 1 adds a new debugfs helper needed for patch 6. Greg, could I get
>>>> an ack on that if it makes sense? Jens and I went back and forth on this
>>>> for a little while, but patch 6 has more of the rationale on why we
>>>> decided that this approach was the cleanest.
>>>
>>> I can't find patch 6, you only cc:ed me on the first patch :(
>>>
>>> Care to bounce them all to me?
>>>
>>> thanks,
>>>
>>> greg k-h
>>
>> Gah, I forgot --cc-cover to git-send-email. The series is all here:
>> http://marc.info/?l=linux-block&r=1&b=201701&w=2. I can also send the
>> patches directly to you if you prefer that.
>
> I don't understand the problem here. How do you not know if you have
> created the debugfs file or not? You have the structure, with the
> correct name, how could it have been created? Can't you save the dentry
> to the debugfs file in the structure that has the name?
The problem is that blktrace registers a trace name directory, which
can be either whole device or partition, depending on what you trace.
For the blk-mq debug parts, we always just register the whole device
name. There's no way to save the partition dentry, and imho, why even
would you when you can just look it up. It's a file system...
--
Jens Axboe
^ permalink raw reply
* [PATCH 5/5] block: Get rid of blk_get_backing_dev_info()
From: Jan Kara @ 2017-02-02 14:56 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Christoph Hellwig, Dan Williams,
Thiago Jung Bauermann, Laurent Dufour, Jan Kara
In-Reply-To: <20170202145653.19614-1-jack@suse.cz>
blk_get_backing_dev_info() is now a simple dereference. Remove that
function and simplify some code around that.
Signed-off-by: Jan Kara <jack@suse.cz>
---
block/blk-core.c | 14 --------------
block/compat_ioctl.c | 7 ++-----
block/ioctl.c | 7 ++-----
fs/btrfs/disk-io.c | 2 +-
fs/btrfs/volumes.c | 2 +-
fs/xfs/xfs_buf.c | 3 +--
fs/xfs/xfs_buf.h | 1 -
include/linux/backing-dev.h | 2 +-
include/linux/blkdev.h | 1 -
9 files changed, 8 insertions(+), 31 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index 84fabb51714a..47104f6a398b 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -104,20 +104,6 @@ void blk_queue_congestion_threshold(struct request_queue *q)
q->nr_congestion_off = nr;
}
-/**
- * blk_get_backing_dev_info - get the address of a queue's backing_dev_info
- * @bdev: device
- *
- * Locates the passed device's request queue and returns the address of its
- * backing_dev_info. The return value is never NULL however we may return
- * &noop_backing_dev_info if the bdev is not currently open.
- */
-struct backing_dev_info *blk_get_backing_dev_info(struct block_device *bdev)
-{
- return bdev->bd_bdi;
-}
-EXPORT_SYMBOL(blk_get_backing_dev_info);
-
void blk_rq_init(struct request_queue *q, struct request *rq)
{
memset(rq, 0, sizeof(*rq));
diff --git a/block/compat_ioctl.c b/block/compat_ioctl.c
index 556826ac7cb4..570021a0dc1c 100644
--- a/block/compat_ioctl.c
+++ b/block/compat_ioctl.c
@@ -661,7 +661,6 @@ long compat_blkdev_ioctl(struct file *file, unsigned cmd, unsigned long arg)
struct block_device *bdev = inode->i_bdev;
struct gendisk *disk = bdev->bd_disk;
fmode_t mode = file->f_mode;
- struct backing_dev_info *bdi;
loff_t size;
unsigned int max_sectors;
@@ -708,9 +707,8 @@ long compat_blkdev_ioctl(struct file *file, unsigned cmd, unsigned long arg)
case BLKFRAGET:
if (!arg)
return -EINVAL;
- bdi = blk_get_backing_dev_info(bdev);
return compat_put_long(arg,
- (bdi->ra_pages * PAGE_SIZE) / 512);
+ (bdev->bd_bdi->ra_pages * PAGE_SIZE) / 512);
case BLKROGET: /* compatible */
return compat_put_int(arg, bdev_read_only(bdev) != 0);
case BLKBSZGET_32: /* get the logical block size (cf. BLKSSZGET) */
@@ -728,8 +726,7 @@ long compat_blkdev_ioctl(struct file *file, unsigned cmd, unsigned long arg)
case BLKFRASET:
if (!capable(CAP_SYS_ADMIN))
return -EACCES;
- bdi = blk_get_backing_dev_info(bdev);
- bdi->ra_pages = (arg * 512) / PAGE_SIZE;
+ bdev->bd_bdi->ra_pages = (arg * 512) / PAGE_SIZE;
return 0;
case BLKGETSIZE:
size = i_size_read(bdev->bd_inode);
diff --git a/block/ioctl.c b/block/ioctl.c
index be7f4de3eb3c..7b88820b93d9 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -505,7 +505,6 @@ static int blkdev_bszset(struct block_device *bdev, fmode_t mode,
int blkdev_ioctl(struct block_device *bdev, fmode_t mode, unsigned cmd,
unsigned long arg)
{
- struct backing_dev_info *bdi;
void __user *argp = (void __user *)arg;
loff_t size;
unsigned int max_sectors;
@@ -532,8 +531,7 @@ int blkdev_ioctl(struct block_device *bdev, fmode_t mode, unsigned cmd,
case BLKFRAGET:
if (!arg)
return -EINVAL;
- bdi = blk_get_backing_dev_info(bdev);
- return put_long(arg, (bdi->ra_pages * PAGE_SIZE) / 512);
+ return put_long(arg, (bdev->bd_bdi->ra_pages*PAGE_SIZE) / 512);
case BLKROGET:
return put_int(arg, bdev_read_only(bdev) != 0);
case BLKBSZGET: /* get block device soft block size (cf. BLKSSZGET) */
@@ -560,8 +558,7 @@ int blkdev_ioctl(struct block_device *bdev, fmode_t mode, unsigned cmd,
case BLKFRASET:
if(!capable(CAP_SYS_ADMIN))
return -EACCES;
- bdi = blk_get_backing_dev_info(bdev);
- bdi->ra_pages = (arg * 512) / PAGE_SIZE;
+ bdev->bd_bdi->ra_pages = (arg * 512) / PAGE_SIZE;
return 0;
case BLKBSZSET:
return blkdev_bszset(bdev, mode, argp);
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 18004169552c..37a31b12bb0c 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -1800,7 +1800,7 @@ static int btrfs_congested_fn(void *congested_data, int bdi_bits)
list_for_each_entry_rcu(device, &info->fs_devices->devices, dev_list) {
if (!device->bdev)
continue;
- bdi = blk_get_backing_dev_info(device->bdev);
+ bdi = device->bdev->bd_bdi;
if (bdi_congested(bdi, bdi_bits)) {
ret = 1;
break;
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 3c3c69c0eee4..b2e70073a10d 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -366,7 +366,7 @@ static noinline void run_scheduled_bios(struct btrfs_device *device)
*/
blk_start_plug(&plug);
- bdi = blk_get_backing_dev_info(device->bdev);
+ bdi = device->bdev->bd_bdi;
limit = btrfs_async_submit_limit(fs_info);
limit = limit * 2 / 3;
diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index 7f0a01f7b592..8bbec20aa138 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -757,7 +757,7 @@ xfs_buf_readahead_map(
int nmaps,
const struct xfs_buf_ops *ops)
{
- if (bdi_read_congested(target->bt_bdi))
+ if (bdi_read_congested(target->bt_bdev->bd_bdi))
return;
xfs_buf_read_map(target, map, nmaps,
@@ -1790,7 +1790,6 @@ xfs_alloc_buftarg(
btp->bt_mount = mp;
btp->bt_dev = bdev->bd_dev;
btp->bt_bdev = bdev;
- btp->bt_bdi = blk_get_backing_dev_info(bdev);
if (xfs_setsize_buftarg_early(btp, bdev))
goto error;
diff --git a/fs/xfs/xfs_buf.h b/fs/xfs/xfs_buf.h
index 8a9d3a9599f0..3c867e5a63e1 100644
--- a/fs/xfs/xfs_buf.h
+++ b/fs/xfs/xfs_buf.h
@@ -109,7 +109,6 @@ typedef unsigned int xfs_buf_flags_t;
typedef struct xfs_buftarg {
dev_t bt_dev;
struct block_device *bt_bdev;
- struct backing_dev_info *bt_bdi;
struct xfs_mount *bt_mount;
unsigned int bt_meta_sectorsize;
size_t bt_meta_sectormask;
diff --git a/include/linux/backing-dev.h b/include/linux/backing-dev.h
index efb6ca992d05..c52a48cb9a66 100644
--- a/include/linux/backing-dev.h
+++ b/include/linux/backing-dev.h
@@ -191,7 +191,7 @@ static inline struct backing_dev_info *inode_to_bdi(struct inode *inode)
sb = inode->i_sb;
#ifdef CONFIG_BLOCK
if (sb_is_blkdev_sb(sb))
- return blk_get_backing_dev_info(I_BDEV(inode));
+ return I_BDEV(inode)->bd_bdi;
#endif
return sb->s_bdi;
}
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index de85701cc699..84d367bee0a5 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1166,7 +1166,6 @@ extern void blk_queue_rq_timed_out(struct request_queue *, rq_timed_out_fn *);
extern void blk_queue_rq_timeout(struct request_queue *, unsigned int);
extern void blk_queue_flush_queueable(struct request_queue *q, bool queueable);
extern void blk_queue_write_cache(struct request_queue *q, bool enabled, bool fua);
-extern struct backing_dev_info *blk_get_backing_dev_info(struct block_device *bdev);
static inline unsigned short blk_rq_nr_phys_segments(struct request *rq)
{
--
2.10.2
^ permalink raw reply related
* [PATCH 4/5] block: Make blk_get_backing_dev_info() safe without open bdev
From: Jan Kara @ 2017-02-02 14:56 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Christoph Hellwig, Dan Williams,
Thiago Jung Bauermann, Laurent Dufour, Jan Kara
In-Reply-To: <20170202145653.19614-1-jack@suse.cz>
Currenly blk_get_backing_dev_info() is not safe to be called when the
block device is not open as bdev->bd_disk is NULL in that case. However
inode_to_bdi() uses this function and may be call called from flusher
worker or other writeback related functions without bdev being open
which leads to crashes such as:
[113031.075540] Unable to handle kernel paging request for data at address 0x00000000
[113031.075614] Faulting instruction address: 0xc0000000003692e0
0:mon> t
[c0000000fb65f900] c00000000036cb6c writeback_sb_inodes+0x30c/0x590
[c0000000fb65fa10] c00000000036ced4 __writeback_inodes_wb+0xe4/0x150
[c0000000fb65fa70] c00000000036d33c wb_writeback+0x30c/0x450
[c0000000fb65fb40] c00000000036e198 wb_workfn+0x268/0x580
[c0000000fb65fc50] c0000000000f3470 process_one_work+0x1e0/0x590
[c0000000fb65fce0] c0000000000f38c8 worker_thread+0xa8/0x660
[c0000000fb65fd80] c0000000000fc4b0 kthread+0x110/0x130
[c0000000fb65fe30] c0000000000098f0 ret_from_kernel_thread+0x5c/0x6c
--- Exception: 0 at 0000000000000000
0:mon> e
cpu 0x0: Vector: 300 (Data Access) at [c0000000fb65f620]
pc: c0000000003692e0: locked_inode_to_wb_and_lock_list+0x50/0x290
lr: c00000000036cb6c: writeback_sb_inodes+0x30c/0x590
sp: c0000000fb65f8a0
msr: 800000010280b033
dar: 0
dsisr: 40000000
current = 0xc0000001d69be400
paca = 0xc000000003480000 softe: 0 irq_happened: 0x01
pid = 18689, comm = kworker/u16:10
Fix the problem by grabbing reference to bdi on first open of the block
device and drop the reference only once the inode is evicted from
memory. This pins struct backing_dev_info in memory and thus fixes the
crashes.
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reported-and-tested-by: Dan Williams <dan.j.williams@intel.com>
Reported-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
Signed-off-by: Jan Kara <jack@suse.cz>
---
block/blk-core.c | 8 +++-----
fs/block_dev.c | 7 +++++++
include/linux/fs.h | 1 +
3 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index 545ccb4b96f3..84fabb51714a 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -109,14 +109,12 @@ void blk_queue_congestion_threshold(struct request_queue *q)
* @bdev: device
*
* Locates the passed device's request queue and returns the address of its
- * backing_dev_info. This function can only be called if @bdev is opened
- * and the return value is never NULL.
+ * backing_dev_info. The return value is never NULL however we may return
+ * &noop_backing_dev_info if the bdev is not currently open.
*/
struct backing_dev_info *blk_get_backing_dev_info(struct block_device *bdev)
{
- struct request_queue *q = bdev_get_queue(bdev);
-
- return q->backing_dev_info;
+ return bdev->bd_bdi;
}
EXPORT_SYMBOL(blk_get_backing_dev_info);
diff --git a/fs/block_dev.c b/fs/block_dev.c
index ed6a34be7a1e..601b71b76d7f 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -884,6 +884,8 @@ static void bdev_evict_inode(struct inode *inode)
spin_lock(&bdev_lock);
list_del_init(&bdev->bd_list);
spin_unlock(&bdev_lock);
+ if (bdev->bd_bdi != &noop_backing_dev_info)
+ bdi_put(bdev->bd_bdi);
}
static const struct super_operations bdev_sops = {
@@ -986,6 +988,7 @@ struct block_device *bdget(dev_t dev)
bdev->bd_contains = NULL;
bdev->bd_super = NULL;
bdev->bd_inode = inode;
+ bdev->bd_bdi = &noop_backing_dev_info;
bdev->bd_block_size = (1 << inode->i_blkbits);
bdev->bd_part_count = 0;
bdev->bd_invalidated = 0;
@@ -1542,6 +1545,8 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode, int for_part)
bdev->bd_disk = disk;
bdev->bd_queue = disk->queue;
bdev->bd_contains = bdev;
+ if (bdev->bd_bdi == &noop_backing_dev_info)
+ bdev->bd_bdi = bdi_get(disk->queue->backing_dev_info);
if (!partno) {
ret = -ENXIO;
@@ -1637,6 +1642,8 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode, int for_part)
bdev->bd_disk = NULL;
bdev->bd_part = NULL;
bdev->bd_queue = NULL;
+ bdi_put(bdev->bd_bdi);
+ bdev->bd_bdi = &noop_backing_dev_info;
if (bdev != bdev->bd_contains)
__blkdev_put(bdev->bd_contains, mode, 1);
bdev->bd_contains = NULL;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 702cb6c50194..c930cbc19342 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -423,6 +423,7 @@ struct block_device {
int bd_invalidated;
struct gendisk * bd_disk;
struct request_queue * bd_queue;
+ struct backing_dev_info *bd_bdi;
struct list_head bd_list;
/*
* Private data. You must have bd_claim'ed the block_device
--
2.10.2
^ permalink raw reply related
* [PATCH 3/5] block: Dynamically allocate and refcount backing_dev_info
From: Jan Kara @ 2017-02-02 14:56 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Christoph Hellwig, Dan Williams,
Thiago Jung Bauermann, Laurent Dufour, Jan Kara
In-Reply-To: <20170202145653.19614-1-jack@suse.cz>
Instead of storing backing_dev_info inside struct request_queue,
allocate it dynamically, reference count it, and free it when the last
reference is dropped. Currently only request_queue holds the reference
but in the following patch we add other users referencing
backing_dev_info.
Signed-off-by: Jan Kara <jack@suse.cz>
---
block/blk-core.c | 12 +++++-------
block/blk-sysfs.c | 2 +-
include/linux/backing-dev-defs.h | 2 ++
include/linux/backing-dev.h | 10 +++++++++-
include/linux/blkdev.h | 1 -
mm/backing-dev.c | 34 +++++++++++++++++++++++++++++++++-
6 files changed, 50 insertions(+), 11 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index a9ff1b919ae7..545ccb4b96f3 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -693,7 +693,6 @@ static void blk_rq_timed_out_timer(unsigned long data)
struct request_queue *blk_alloc_queue_node(gfp_t gfp_mask, int node_id)
{
struct request_queue *q;
- int err;
q = kmem_cache_alloc_node(blk_requestq_cachep,
gfp_mask | __GFP_ZERO, node_id);
@@ -708,17 +707,16 @@ struct request_queue *blk_alloc_queue_node(gfp_t gfp_mask, int node_id)
if (!q->bio_split)
goto fail_id;
- q->backing_dev_info = &q->_backing_dev_info;
+ q->backing_dev_info = bdi_alloc_node(gfp_mask, node_id);
+ if (!q->backing_dev_info)
+ goto fail_split;
+
q->backing_dev_info->ra_pages =
(VM_MAX_READAHEAD * 1024) / PAGE_SIZE;
q->backing_dev_info->capabilities = BDI_CAP_CGROUP_WRITEBACK;
q->backing_dev_info->name = "block";
q->node = node_id;
- err = bdi_init(q->backing_dev_info);
- if (err)
- goto fail_split;
-
setup_timer(&q->backing_dev_info->laptop_mode_wb_timer,
laptop_mode_timer_fn, (unsigned long) q);
setup_timer(&q->timeout, blk_rq_timed_out_timer, (unsigned long) q);
@@ -769,7 +767,7 @@ struct request_queue *blk_alloc_queue_node(gfp_t gfp_mask, int node_id)
fail_ref:
percpu_ref_exit(&q->q_usage_counter);
fail_bdi:
- bdi_destroy(q->backing_dev_info);
+ bdi_put(q->backing_dev_info);
fail_split:
bioset_free(q->bio_split);
fail_id:
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index 64fb54c6b41c..4cbaa519ec2d 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -799,7 +799,7 @@ static void blk_release_queue(struct kobject *kobj)
container_of(kobj, struct request_queue, kobj);
wbt_exit(q);
- bdi_exit(q->backing_dev_info);
+ bdi_put(q->backing_dev_info);
blkcg_exit_queue(q);
if (q->elevator) {
diff --git a/include/linux/backing-dev-defs.h b/include/linux/backing-dev-defs.h
index e850e76acaaf..ad955817916d 100644
--- a/include/linux/backing-dev-defs.h
+++ b/include/linux/backing-dev-defs.h
@@ -10,6 +10,7 @@
#include <linux/flex_proportions.h>
#include <linux/timer.h>
#include <linux/workqueue.h>
+#include <linux/kref.h>
struct page;
struct device;
@@ -144,6 +145,7 @@ struct backing_dev_info {
char *name;
+ struct kref refcnt; /* Reference counter for the structure */
unsigned int capabilities; /* Device capabilities */
unsigned int min_ratio;
unsigned int max_ratio, max_prop_frac;
diff --git a/include/linux/backing-dev.h b/include/linux/backing-dev.h
index 43b93a947e61..efb6ca992d05 100644
--- a/include/linux/backing-dev.h
+++ b/include/linux/backing-dev.h
@@ -18,7 +18,14 @@
#include <linux/slab.h>
int __must_check bdi_init(struct backing_dev_info *bdi);
-void bdi_exit(struct backing_dev_info *bdi);
+
+static inline struct backing_dev_info *bdi_get(struct backing_dev_info *bdi)
+{
+ kref_get(&bdi->refcnt);
+ return bdi;
+}
+
+void bdi_put(struct backing_dev_info *bdi);
__printf(3, 4)
int bdi_register(struct backing_dev_info *bdi, struct device *parent,
@@ -29,6 +36,7 @@ void bdi_unregister(struct backing_dev_info *bdi);
int __must_check bdi_setup_and_register(struct backing_dev_info *, char *);
void bdi_destroy(struct backing_dev_info *bdi);
+struct backing_dev_info *bdi_alloc_node(gfp_t gfp_mask, int node_id);
void wb_start_writeback(struct bdi_writeback *wb, long nr_pages,
bool range_cyclic, enum wb_reason reason);
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 069e4a102a73..de85701cc699 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -433,7 +433,6 @@ struct request_queue {
struct delayed_work delay_work;
struct backing_dev_info *backing_dev_info;
- struct backing_dev_info _backing_dev_info;
/*
* The queue owner gets to use this for whatever they like.
diff --git a/mm/backing-dev.c b/mm/backing-dev.c
index 3bfed5ab2475..28ce6cf7b2ff 100644
--- a/mm/backing-dev.c
+++ b/mm/backing-dev.c
@@ -237,6 +237,7 @@ static __init int bdi_class_init(void)
bdi_class->dev_groups = bdi_dev_groups;
bdi_debug_init();
+
return 0;
}
postcore_initcall(bdi_class_init);
@@ -776,6 +777,7 @@ int bdi_init(struct backing_dev_info *bdi)
bdi->dev = NULL;
+ kref_init(&bdi->refcnt);
bdi->min_ratio = 0;
bdi->max_ratio = 100;
bdi->max_prop_frac = FPROP_FRAC_BASE;
@@ -791,6 +793,22 @@ int bdi_init(struct backing_dev_info *bdi)
}
EXPORT_SYMBOL(bdi_init);
+struct backing_dev_info *bdi_alloc_node(gfp_t gfp_mask, int node_id)
+{
+ struct backing_dev_info *bdi;
+
+ bdi = kmalloc_node(sizeof(struct backing_dev_info),
+ gfp_mask | __GFP_ZERO, node_id);
+ if (!bdi)
+ return NULL;
+
+ if (bdi_init(bdi)) {
+ kfree(bdi);
+ return NULL;
+ }
+ return bdi;
+}
+
int bdi_register(struct backing_dev_info *bdi, struct device *parent,
const char *fmt, ...)
{
@@ -871,12 +889,26 @@ void bdi_unregister(struct backing_dev_info *bdi)
}
}
-void bdi_exit(struct backing_dev_info *bdi)
+static void bdi_exit(struct backing_dev_info *bdi)
{
WARN_ON_ONCE(bdi->dev);
wb_exit(&bdi->wb);
}
+static void release_bdi(struct kref *ref)
+{
+ struct backing_dev_info *bdi =
+ container_of(ref, struct backing_dev_info, refcnt);
+
+ bdi_exit(bdi);
+ kfree(bdi);
+}
+
+void bdi_put(struct backing_dev_info *bdi)
+{
+ kref_put(&bdi->refcnt, release_bdi);
+}
+
void bdi_destroy(struct backing_dev_info *bdi)
{
bdi_unregister(bdi);
--
2.10.2
^ permalink raw reply related
* [PATCH 2/5] block: Use pointer to backing_dev_info from request_queue
From: Jan Kara @ 2017-02-02 14:56 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Christoph Hellwig, Dan Williams,
Thiago Jung Bauermann, Laurent Dufour, Jan Kara
In-Reply-To: <20170202145653.19614-1-jack@suse.cz>
We will want to have struct backing_dev_info allocated separately from
struct request_queue. As the first step add pointer to backing_dev_info
to request_queue and convert all users touching it. No functional
changes in this patch.
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Jan Kara <jack@suse.cz>
---
block/blk-cgroup.c | 6 +++---
block/blk-core.c | 27 ++++++++++++++-------------
block/blk-integrity.c | 4 ++--
block/blk-settings.c | 2 +-
block/blk-sysfs.c | 8 ++++----
block/blk-wbt.c | 8 ++++----
block/genhd.c | 2 +-
drivers/block/aoe/aoeblk.c | 4 ++--
drivers/block/drbd/drbd_main.c | 6 +++---
drivers/block/drbd/drbd_nl.c | 12 +++++++-----
drivers/block/drbd/drbd_proc.c | 2 +-
drivers/block/drbd/drbd_req.c | 2 +-
drivers/block/pktcdvd.c | 4 ++--
drivers/block/rbd.c | 2 +-
drivers/md/bcache/request.c | 10 +++++-----
drivers/md/bcache/super.c | 8 ++++----
drivers/md/dm-cache-target.c | 2 +-
drivers/md/dm-era-target.c | 2 +-
drivers/md/dm-table.c | 2 +-
drivers/md/dm-thin.c | 2 +-
drivers/md/dm.c | 6 +++---
drivers/md/linear.c | 2 +-
drivers/md/md.c | 6 +++---
drivers/md/multipath.c | 2 +-
drivers/md/raid0.c | 6 +++---
drivers/md/raid1.c | 4 ++--
drivers/md/raid10.c | 10 +++++-----
drivers/md/raid5.c | 12 ++++++------
fs/gfs2/ops_fstype.c | 2 +-
fs/nilfs2/super.c | 2 +-
fs/super.c | 2 +-
include/linux/blkdev.h | 3 ++-
mm/page-writeback.c | 4 ++--
33 files changed, 90 insertions(+), 86 deletions(-)
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index 8ba0af780e88..d673a69b61b4 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -184,7 +184,7 @@ static struct blkcg_gq *blkg_create(struct blkcg *blkcg,
goto err_free_blkg;
}
- wb_congested = wb_congested_get_create(&q->backing_dev_info,
+ wb_congested = wb_congested_get_create(q->backing_dev_info,
blkcg->css.id,
GFP_NOWAIT | __GFP_NOWARN);
if (!wb_congested) {
@@ -469,8 +469,8 @@ static int blkcg_reset_stats(struct cgroup_subsys_state *css,
const char *blkg_dev_name(struct blkcg_gq *blkg)
{
/* some drivers (floppy) instantiate a queue w/o disk registered */
- if (blkg->q->backing_dev_info.dev)
- return dev_name(blkg->q->backing_dev_info.dev);
+ if (blkg->q->backing_dev_info->dev)
+ return dev_name(blkg->q->backing_dev_info->dev);
return NULL;
}
EXPORT_SYMBOL_GPL(blkg_dev_name);
diff --git a/block/blk-core.c b/block/blk-core.c
index 61ba08c58b64..a9ff1b919ae7 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -74,7 +74,7 @@ static void blk_clear_congested(struct request_list *rl, int sync)
* flip its congestion state for events on other blkcgs.
*/
if (rl == &rl->q->root_rl)
- clear_wb_congested(rl->q->backing_dev_info.wb.congested, sync);
+ clear_wb_congested(rl->q->backing_dev_info->wb.congested, sync);
#endif
}
@@ -85,7 +85,7 @@ static void blk_set_congested(struct request_list *rl, int sync)
#else
/* see blk_clear_congested() */
if (rl == &rl->q->root_rl)
- set_wb_congested(rl->q->backing_dev_info.wb.congested, sync);
+ set_wb_congested(rl->q->backing_dev_info->wb.congested, sync);
#endif
}
@@ -116,7 +116,7 @@ struct backing_dev_info *blk_get_backing_dev_info(struct block_device *bdev)
{
struct request_queue *q = bdev_get_queue(bdev);
- return &q->backing_dev_info;
+ return q->backing_dev_info;
}
EXPORT_SYMBOL(blk_get_backing_dev_info);
@@ -584,7 +584,7 @@ void blk_cleanup_queue(struct request_queue *q)
blk_flush_integrity();
/* @q won't process any more request, flush async actions */
- del_timer_sync(&q->backing_dev_info.laptop_mode_wb_timer);
+ del_timer_sync(&q->backing_dev_info->laptop_mode_wb_timer);
blk_sync_queue(q);
if (q->mq_ops)
@@ -596,7 +596,7 @@ void blk_cleanup_queue(struct request_queue *q)
q->queue_lock = &q->__queue_lock;
spin_unlock_irq(lock);
- bdi_unregister(&q->backing_dev_info);
+ bdi_unregister(q->backing_dev_info);
/* @q is and will stay empty, shutdown and put */
blk_put_queue(q);
@@ -708,17 +708,18 @@ struct request_queue *blk_alloc_queue_node(gfp_t gfp_mask, int node_id)
if (!q->bio_split)
goto fail_id;
- q->backing_dev_info.ra_pages =
+ q->backing_dev_info = &q->_backing_dev_info;
+ q->backing_dev_info->ra_pages =
(VM_MAX_READAHEAD * 1024) / PAGE_SIZE;
- q->backing_dev_info.capabilities = BDI_CAP_CGROUP_WRITEBACK;
- q->backing_dev_info.name = "block";
+ q->backing_dev_info->capabilities = BDI_CAP_CGROUP_WRITEBACK;
+ q->backing_dev_info->name = "block";
q->node = node_id;
- err = bdi_init(&q->backing_dev_info);
+ err = bdi_init(q->backing_dev_info);
if (err)
goto fail_split;
- setup_timer(&q->backing_dev_info.laptop_mode_wb_timer,
+ setup_timer(&q->backing_dev_info->laptop_mode_wb_timer,
laptop_mode_timer_fn, (unsigned long) q);
setup_timer(&q->timeout, blk_rq_timed_out_timer, (unsigned long) q);
INIT_LIST_HEAD(&q->queue_head);
@@ -768,7 +769,7 @@ struct request_queue *blk_alloc_queue_node(gfp_t gfp_mask, int node_id)
fail_ref:
percpu_ref_exit(&q->q_usage_counter);
fail_bdi:
- bdi_destroy(&q->backing_dev_info);
+ bdi_destroy(q->backing_dev_info);
fail_split:
bioset_free(q->bio_split);
fail_id:
@@ -1196,7 +1197,7 @@ static struct request *__get_request(struct request_list *rl, unsigned int op,
* disturb iosched and blkcg but weird is bettern than dead.
*/
printk_ratelimited(KERN_WARNING "%s: dev %s: request aux data allocation failed, iosched may be disturbed\n",
- __func__, dev_name(q->backing_dev_info.dev));
+ __func__, dev_name(q->backing_dev_info->dev));
rq->rq_flags &= ~RQF_ELVPRIV;
rq->elv.icq = NULL;
@@ -2696,7 +2697,7 @@ void blk_finish_request(struct request *req, int error)
BUG_ON(blk_queued_rq(req));
if (unlikely(laptop_mode) && req->cmd_type == REQ_TYPE_FS)
- laptop_io_completion(&req->q->backing_dev_info);
+ laptop_io_completion(req->q->backing_dev_info);
blk_delete_timer(req);
diff --git a/block/blk-integrity.c b/block/blk-integrity.c
index d69c5c79f98e..9f0ff5ba4f84 100644
--- a/block/blk-integrity.c
+++ b/block/blk-integrity.c
@@ -443,10 +443,10 @@ void blk_integrity_revalidate(struct gendisk *disk)
return;
if (bi->profile)
- disk->queue->backing_dev_info.capabilities |=
+ disk->queue->backing_dev_info->capabilities |=
BDI_CAP_STABLE_WRITES;
else
- disk->queue->backing_dev_info.capabilities &=
+ disk->queue->backing_dev_info->capabilities &=
~BDI_CAP_STABLE_WRITES;
}
diff --git a/block/blk-settings.c b/block/blk-settings.c
index 529e55f52a03..6eb19bcbf3cb 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -253,7 +253,7 @@ void blk_queue_max_hw_sectors(struct request_queue *q, unsigned int max_hw_secto
max_sectors = min_not_zero(max_hw_sectors, limits->max_dev_sectors);
max_sectors = min_t(unsigned int, max_sectors, BLK_DEF_MAX_SECTORS);
limits->max_sectors = max_sectors;
- q->backing_dev_info.io_pages = max_sectors >> (PAGE_SHIFT - 9);
+ q->backing_dev_info->io_pages = max_sectors >> (PAGE_SHIFT - 9);
}
EXPORT_SYMBOL(blk_queue_max_hw_sectors);
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index 1dbce057592d..64fb54c6b41c 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -89,7 +89,7 @@ queue_requests_store(struct request_queue *q, const char *page, size_t count)
static ssize_t queue_ra_show(struct request_queue *q, char *page)
{
- unsigned long ra_kb = q->backing_dev_info.ra_pages <<
+ unsigned long ra_kb = q->backing_dev_info->ra_pages <<
(PAGE_SHIFT - 10);
return queue_var_show(ra_kb, (page));
@@ -104,7 +104,7 @@ queue_ra_store(struct request_queue *q, const char *page, size_t count)
if (ret < 0)
return ret;
- q->backing_dev_info.ra_pages = ra_kb >> (PAGE_SHIFT - 10);
+ q->backing_dev_info->ra_pages = ra_kb >> (PAGE_SHIFT - 10);
return ret;
}
@@ -236,7 +236,7 @@ queue_max_sectors_store(struct request_queue *q, const char *page, size_t count)
spin_lock_irq(q->queue_lock);
q->limits.max_sectors = max_sectors_kb << 1;
- q->backing_dev_info.io_pages = max_sectors_kb >> (PAGE_SHIFT - 10);
+ q->backing_dev_info->io_pages = max_sectors_kb >> (PAGE_SHIFT - 10);
spin_unlock_irq(q->queue_lock);
return ret;
@@ -799,7 +799,7 @@ static void blk_release_queue(struct kobject *kobj)
container_of(kobj, struct request_queue, kobj);
wbt_exit(q);
- bdi_exit(&q->backing_dev_info);
+ bdi_exit(q->backing_dev_info);
blkcg_exit_queue(q);
if (q->elevator) {
diff --git a/block/blk-wbt.c b/block/blk-wbt.c
index f0a9c07b4c7a..1aedb1f7ee0c 100644
--- a/block/blk-wbt.c
+++ b/block/blk-wbt.c
@@ -96,7 +96,7 @@ static void wb_timestamp(struct rq_wb *rwb, unsigned long *var)
*/
static bool wb_recent_wait(struct rq_wb *rwb)
{
- struct bdi_writeback *wb = &rwb->queue->backing_dev_info.wb;
+ struct bdi_writeback *wb = &rwb->queue->backing_dev_info->wb;
return time_before(jiffies, wb->dirty_sleep + HZ);
}
@@ -279,7 +279,7 @@ enum {
static int __latency_exceeded(struct rq_wb *rwb, struct blk_rq_stat *stat)
{
- struct backing_dev_info *bdi = &rwb->queue->backing_dev_info;
+ struct backing_dev_info *bdi = rwb->queue->backing_dev_info;
u64 thislat;
/*
@@ -339,7 +339,7 @@ static int latency_exceeded(struct rq_wb *rwb)
static void rwb_trace_step(struct rq_wb *rwb, const char *msg)
{
- struct backing_dev_info *bdi = &rwb->queue->backing_dev_info;
+ struct backing_dev_info *bdi = rwb->queue->backing_dev_info;
trace_wbt_step(bdi, msg, rwb->scale_step, rwb->cur_win_nsec,
rwb->wb_background, rwb->wb_normal, rwb->wb_max);
@@ -423,7 +423,7 @@ static void wb_timer_fn(unsigned long data)
status = latency_exceeded(rwb);
- trace_wbt_timer(&rwb->queue->backing_dev_info, status, rwb->scale_step,
+ trace_wbt_timer(rwb->queue->backing_dev_info, status, rwb->scale_step,
inflight);
/*
diff --git a/block/genhd.c b/block/genhd.c
index f2f22d0e8e14..d9ccd42f3675 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -613,7 +613,7 @@ void device_add_disk(struct device *parent, struct gendisk *disk)
disk_alloc_events(disk);
/* Register BDI before referencing it from bdev */
- bdi = &disk->queue->backing_dev_info;
+ bdi = disk->queue->backing_dev_info;
bdi_register_owner(bdi, disk_to_dev(disk));
blk_register_region(disk_devt(disk), disk->minors, NULL,
diff --git a/drivers/block/aoe/aoeblk.c b/drivers/block/aoe/aoeblk.c
index ec9d8610b25f..027b876370bc 100644
--- a/drivers/block/aoe/aoeblk.c
+++ b/drivers/block/aoe/aoeblk.c
@@ -396,8 +396,8 @@ aoeblk_gdalloc(void *vp)
WARN_ON(d->gd);
WARN_ON(d->flags & DEVFL_UP);
blk_queue_max_hw_sectors(q, BLK_DEF_MAX_SECTORS);
- q->backing_dev_info.name = "aoe";
- q->backing_dev_info.ra_pages = READ_AHEAD / PAGE_SIZE;
+ q->backing_dev_info->name = "aoe";
+ q->backing_dev_info->ra_pages = READ_AHEAD / PAGE_SIZE;
d->bufpool = mp;
d->blkq = gd->queue = q;
q->queuedata = d;
diff --git a/drivers/block/drbd/drbd_main.c b/drivers/block/drbd/drbd_main.c
index 83482721bc01..d305f05be648 100644
--- a/drivers/block/drbd/drbd_main.c
+++ b/drivers/block/drbd/drbd_main.c
@@ -2462,7 +2462,7 @@ static int drbd_congested(void *congested_data, int bdi_bits)
if (get_ldev(device)) {
q = bdev_get_queue(device->ldev->backing_bdev);
- r = bdi_congested(&q->backing_dev_info, bdi_bits);
+ r = bdi_congested(q->backing_dev_info, bdi_bits);
put_ldev(device);
if (r)
reason = 'b';
@@ -2834,8 +2834,8 @@ enum drbd_ret_code drbd_create_device(struct drbd_config_context *adm_ctx, unsig
/* we have no partitions. we contain only ourselves. */
device->this_bdev->bd_contains = device->this_bdev;
- q->backing_dev_info.congested_fn = drbd_congested;
- q->backing_dev_info.congested_data = device;
+ q->backing_dev_info->congested_fn = drbd_congested;
+ q->backing_dev_info->congested_data = device;
blk_queue_make_request(q, drbd_make_request);
blk_queue_write_cache(q, true, true);
diff --git a/drivers/block/drbd/drbd_nl.c b/drivers/block/drbd/drbd_nl.c
index f35db29cac76..908c704e20aa 100644
--- a/drivers/block/drbd/drbd_nl.c
+++ b/drivers/block/drbd/drbd_nl.c
@@ -1328,11 +1328,13 @@ static void drbd_setup_queue_param(struct drbd_device *device, struct drbd_backi
if (b) {
blk_queue_stack_limits(q, b);
- if (q->backing_dev_info.ra_pages != b->backing_dev_info.ra_pages) {
+ if (q->backing_dev_info->ra_pages !=
+ b->backing_dev_info->ra_pages) {
drbd_info(device, "Adjusting my ra_pages to backing device's (%lu -> %lu)\n",
- q->backing_dev_info.ra_pages,
- b->backing_dev_info.ra_pages);
- q->backing_dev_info.ra_pages = b->backing_dev_info.ra_pages;
+ q->backing_dev_info->ra_pages,
+ b->backing_dev_info->ra_pages);
+ q->backing_dev_info->ra_pages =
+ b->backing_dev_info->ra_pages;
}
}
fixup_discard_if_not_supported(q);
@@ -3345,7 +3347,7 @@ static void device_to_statistics(struct device_statistics *s,
s->dev_disk_flags = md->flags;
q = bdev_get_queue(device->ldev->backing_bdev);
s->dev_lower_blocked =
- bdi_congested(&q->backing_dev_info,
+ bdi_congested(q->backing_dev_info,
(1 << WB_async_congested) |
(1 << WB_sync_congested));
put_ldev(device);
diff --git a/drivers/block/drbd/drbd_proc.c b/drivers/block/drbd/drbd_proc.c
index be2b93fd2c11..8378142f7a55 100644
--- a/drivers/block/drbd/drbd_proc.c
+++ b/drivers/block/drbd/drbd_proc.c
@@ -288,7 +288,7 @@ static int drbd_seq_show(struct seq_file *seq, void *v)
seq_printf(seq, "%2d: cs:Unconfigured\n", i);
} else {
/* reset device->congestion_reason */
- bdi_rw_congested(&device->rq_queue->backing_dev_info);
+ bdi_rw_congested(device->rq_queue->backing_dev_info);
nc = rcu_dereference(first_peer_device(device)->connection->net_conf);
wp = nc ? nc->wire_protocol - DRBD_PROT_A + 'A' : ' ';
diff --git a/drivers/block/drbd/drbd_req.c b/drivers/block/drbd/drbd_req.c
index de279fe4e4fd..cb6bdb75d52d 100644
--- a/drivers/block/drbd/drbd_req.c
+++ b/drivers/block/drbd/drbd_req.c
@@ -938,7 +938,7 @@ static bool remote_due_to_read_balancing(struct drbd_device *device, sector_t se
switch (rbm) {
case RB_CONGESTED_REMOTE:
- bdi = &device->ldev->backing_bdev->bd_disk->queue->backing_dev_info;
+ bdi = device->ldev->backing_bdev->bd_disk->queue->backing_dev_info;
return bdi_read_congested(bdi);
case RB_LEAST_PENDING:
return atomic_read(&device->local_cnt) >
diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
index 1b94c1ca5c5f..f735c1e48549 100644
--- a/drivers/block/pktcdvd.c
+++ b/drivers/block/pktcdvd.c
@@ -1243,7 +1243,7 @@ static int pkt_handle_queue(struct pktcdvd_device *pd)
&& pd->bio_queue_size <= pd->write_congestion_off);
spin_unlock(&pd->lock);
if (wakeup) {
- clear_bdi_congested(&pd->disk->queue->backing_dev_info,
+ clear_bdi_congested(pd->disk->queue->backing_dev_info,
BLK_RW_ASYNC);
}
@@ -2370,7 +2370,7 @@ static void pkt_make_request_write(struct request_queue *q, struct bio *bio)
spin_lock(&pd->lock);
if (pd->write_congestion_on > 0
&& pd->bio_queue_size >= pd->write_congestion_on) {
- set_bdi_congested(&q->backing_dev_info, BLK_RW_ASYNC);
+ set_bdi_congested(q->backing_dev_info, BLK_RW_ASYNC);
do {
spin_unlock(&pd->lock);
congestion_wait(BLK_RW_ASYNC, HZ);
diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 36d2b9f4e836..82afc56a5614 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -4524,7 +4524,7 @@ static int rbd_init_disk(struct rbd_device *rbd_dev)
q->limits.discard_zeroes_data = 1;
if (!ceph_test_opt(rbd_dev->rbd_client->client, NOCRC))
- q->backing_dev_info.capabilities |= BDI_CAP_STABLE_WRITES;
+ q->backing_dev_info->capabilities |= BDI_CAP_STABLE_WRITES;
disk->queue = q;
diff --git a/drivers/md/bcache/request.c b/drivers/md/bcache/request.c
index 76d20875503c..15f2a2c58e97 100644
--- a/drivers/md/bcache/request.c
+++ b/drivers/md/bcache/request.c
@@ -1009,7 +1009,7 @@ static int cached_dev_congested(void *data, int bits)
struct request_queue *q = bdev_get_queue(dc->bdev);
int ret = 0;
- if (bdi_congested(&q->backing_dev_info, bits))
+ if (bdi_congested(q->backing_dev_info, bits))
return 1;
if (cached_dev_get(dc)) {
@@ -1018,7 +1018,7 @@ static int cached_dev_congested(void *data, int bits)
for_each_cache(ca, d->c, i) {
q = bdev_get_queue(ca->bdev);
- ret |= bdi_congested(&q->backing_dev_info, bits);
+ ret |= bdi_congested(q->backing_dev_info, bits);
}
cached_dev_put(dc);
@@ -1032,7 +1032,7 @@ void bch_cached_dev_request_init(struct cached_dev *dc)
struct gendisk *g = dc->disk.disk;
g->queue->make_request_fn = cached_dev_make_request;
- g->queue->backing_dev_info.congested_fn = cached_dev_congested;
+ g->queue->backing_dev_info->congested_fn = cached_dev_congested;
dc->disk.cache_miss = cached_dev_cache_miss;
dc->disk.ioctl = cached_dev_ioctl;
}
@@ -1125,7 +1125,7 @@ static int flash_dev_congested(void *data, int bits)
for_each_cache(ca, d->c, i) {
q = bdev_get_queue(ca->bdev);
- ret |= bdi_congested(&q->backing_dev_info, bits);
+ ret |= bdi_congested(q->backing_dev_info, bits);
}
return ret;
@@ -1136,7 +1136,7 @@ void bch_flash_dev_request_init(struct bcache_device *d)
struct gendisk *g = d->disk;
g->queue->make_request_fn = flash_dev_make_request;
- g->queue->backing_dev_info.congested_fn = flash_dev_congested;
+ g->queue->backing_dev_info->congested_fn = flash_dev_congested;
d->cache_miss = flash_dev_cache_miss;
d->ioctl = flash_dev_ioctl;
}
diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
index 3a19cbc8b230..85e3f21c2514 100644
--- a/drivers/md/bcache/super.c
+++ b/drivers/md/bcache/super.c
@@ -807,7 +807,7 @@ static int bcache_device_init(struct bcache_device *d, unsigned block_size,
blk_queue_make_request(q, NULL);
d->disk->queue = q;
q->queuedata = d;
- q->backing_dev_info.congested_data = d;
+ q->backing_dev_info->congested_data = d;
q->limits.max_hw_sectors = UINT_MAX;
q->limits.max_sectors = UINT_MAX;
q->limits.max_segment_size = UINT_MAX;
@@ -1132,9 +1132,9 @@ static int cached_dev_init(struct cached_dev *dc, unsigned block_size)
set_capacity(dc->disk.disk,
dc->bdev->bd_part->nr_sects - dc->sb.data_offset);
- dc->disk.disk->queue->backing_dev_info.ra_pages =
- max(dc->disk.disk->queue->backing_dev_info.ra_pages,
- q->backing_dev_info.ra_pages);
+ dc->disk.disk->queue->backing_dev_info->ra_pages =
+ max(dc->disk.disk->queue->backing_dev_info->ra_pages,
+ q->backing_dev_info->ra_pages);
bch_cached_dev_request_init(dc);
bch_cached_dev_writeback_init(dc);
diff --git a/drivers/md/dm-cache-target.c b/drivers/md/dm-cache-target.c
index e04c61e0839e..fa255d66ca2f 100644
--- a/drivers/md/dm-cache-target.c
+++ b/drivers/md/dm-cache-target.c
@@ -2291,7 +2291,7 @@ static void do_waker(struct work_struct *ws)
static int is_congested(struct dm_dev *dev, int bdi_bits)
{
struct request_queue *q = bdev_get_queue(dev->bdev);
- return bdi_congested(&q->backing_dev_info, bdi_bits);
+ return bdi_congested(q->backing_dev_info, bdi_bits);
}
static int cache_is_congested(struct dm_target_callbacks *cb, int bdi_bits)
diff --git a/drivers/md/dm-era-target.c b/drivers/md/dm-era-target.c
index bf2b2676cb8a..9fab33b113c4 100644
--- a/drivers/md/dm-era-target.c
+++ b/drivers/md/dm-era-target.c
@@ -1379,7 +1379,7 @@ static void stop_worker(struct era *era)
static int dev_is_congested(struct dm_dev *dev, int bdi_bits)
{
struct request_queue *q = bdev_get_queue(dev->bdev);
- return bdi_congested(&q->backing_dev_info, bdi_bits);
+ return bdi_congested(q->backing_dev_info, bdi_bits);
}
static int era_is_congested(struct dm_target_callbacks *cb, int bdi_bits)
diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index 0a427de23ed2..3ad16d9c9d5a 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -1750,7 +1750,7 @@ int dm_table_any_congested(struct dm_table *t, int bdi_bits)
char b[BDEVNAME_SIZE];
if (likely(q))
- r |= bdi_congested(&q->backing_dev_info, bdi_bits);
+ r |= bdi_congested(q->backing_dev_info, bdi_bits);
else
DMWARN_LIMIT("%s: any_congested: nonexistent device %s",
dm_device_name(t->md),
diff --git a/drivers/md/dm-thin.c b/drivers/md/dm-thin.c
index d1c05c12a9db..95dfa3904975 100644
--- a/drivers/md/dm-thin.c
+++ b/drivers/md/dm-thin.c
@@ -2714,7 +2714,7 @@ static int pool_is_congested(struct dm_target_callbacks *cb, int bdi_bits)
return 1;
q = bdev_get_queue(pt->data_dev->bdev);
- return bdi_congested(&q->backing_dev_info, bdi_bits);
+ return bdi_congested(q->backing_dev_info, bdi_bits);
}
static void requeue_bios(struct pool *pool)
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 3086da5664f3..db934b1dba9d 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -1314,7 +1314,7 @@ static int dm_any_congested(void *congested_data, int bdi_bits)
* With request-based DM we only need to check the
* top-level queue for congestion.
*/
- r = md->queue->backing_dev_info.wb.state & bdi_bits;
+ r = md->queue->backing_dev_info->wb.state & bdi_bits;
} else {
map = dm_get_live_table_fast(md);
if (map)
@@ -1397,7 +1397,7 @@ void dm_init_md_queue(struct mapped_device *md)
* - must do so here (in alloc_dev callchain) before queue is used
*/
md->queue->queuedata = md;
- md->queue->backing_dev_info.congested_data = md;
+ md->queue->backing_dev_info->congested_data = md;
}
void dm_init_normal_md_queue(struct mapped_device *md)
@@ -1408,7 +1408,7 @@ void dm_init_normal_md_queue(struct mapped_device *md)
/*
* Initialize aspects of queue that aren't relevant for blk-mq
*/
- md->queue->backing_dev_info.congested_fn = dm_any_congested;
+ md->queue->backing_dev_info->congested_fn = dm_any_congested;
blk_queue_bounce_limit(md->queue, BLK_BOUNCE_ANY);
}
diff --git a/drivers/md/linear.c b/drivers/md/linear.c
index 5975c9915684..f1c7bbac31a5 100644
--- a/drivers/md/linear.c
+++ b/drivers/md/linear.c
@@ -62,7 +62,7 @@ static int linear_congested(struct mddev *mddev, int bits)
for (i = 0; i < mddev->raid_disks && !ret ; i++) {
struct request_queue *q = bdev_get_queue(conf->disks[i].rdev->bdev);
- ret |= bdi_congested(&q->backing_dev_info, bits);
+ ret |= bdi_congested(q->backing_dev_info, bits);
}
return ret;
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 82821ee0d57f..ede3b2ad45db 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -5341,8 +5341,8 @@ int md_run(struct mddev *mddev)
queue_flag_set_unlocked(QUEUE_FLAG_NONROT, mddev->queue);
else
queue_flag_clear_unlocked(QUEUE_FLAG_NONROT, mddev->queue);
- mddev->queue->backing_dev_info.congested_data = mddev;
- mddev->queue->backing_dev_info.congested_fn = md_congested;
+ mddev->queue->backing_dev_info->congested_data = mddev;
+ mddev->queue->backing_dev_info->congested_fn = md_congested;
}
if (pers->sync_request) {
if (mddev->kobj.sd &&
@@ -5699,7 +5699,7 @@ static int do_md_stop(struct mddev *mddev, int mode,
__md_stop_writes(mddev);
__md_stop(mddev);
- mddev->queue->backing_dev_info.congested_fn = NULL;
+ mddev->queue->backing_dev_info->congested_fn = NULL;
/* tell userspace to handle 'inactive' */
sysfs_notify_dirent_safe(mddev->sysfs_state);
diff --git a/drivers/md/multipath.c b/drivers/md/multipath.c
index aa8c4e5c1ee2..d457afa672d5 100644
--- a/drivers/md/multipath.c
+++ b/drivers/md/multipath.c
@@ -169,7 +169,7 @@ static int multipath_congested(struct mddev *mddev, int bits)
if (rdev && !test_bit(Faulty, &rdev->flags)) {
struct request_queue *q = bdev_get_queue(rdev->bdev);
- ret |= bdi_congested(&q->backing_dev_info, bits);
+ ret |= bdi_congested(q->backing_dev_info, bits);
/* Just like multipath_map, we just check the
* first available device
*/
diff --git a/drivers/md/raid0.c b/drivers/md/raid0.c
index a162fedeb51a..1ec22cdac8cb 100644
--- a/drivers/md/raid0.c
+++ b/drivers/md/raid0.c
@@ -36,7 +36,7 @@ static int raid0_congested(struct mddev *mddev, int bits)
for (i = 0; i < raid_disks && !ret ; i++) {
struct request_queue *q = bdev_get_queue(devlist[i]->bdev);
- ret |= bdi_congested(&q->backing_dev_info, bits);
+ ret |= bdi_congested(q->backing_dev_info, bits);
}
return ret;
}
@@ -415,8 +415,8 @@ static int raid0_run(struct mddev *mddev)
*/
int stripe = mddev->raid_disks *
(mddev->chunk_sectors << 9) / PAGE_SIZE;
- if (mddev->queue->backing_dev_info.ra_pages < 2* stripe)
- mddev->queue->backing_dev_info.ra_pages = 2* stripe;
+ if (mddev->queue->backing_dev_info->ra_pages < 2* stripe)
+ mddev->queue->backing_dev_info->ra_pages = 2* stripe;
}
dump_zones(mddev);
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index a1f3fbed9100..c5a128ec03fb 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -740,9 +740,9 @@ static int raid1_congested(struct mddev *mddev, int bits)
* non-congested targets, it can be removed
*/
if ((bits & (1 << WB_async_congested)) || 1)
- ret |= bdi_congested(&q->backing_dev_info, bits);
+ ret |= bdi_congested(q->backing_dev_info, bits);
else
- ret &= bdi_congested(&q->backing_dev_info, bits);
+ ret &= bdi_congested(q->backing_dev_info, bits);
}
}
rcu_read_unlock();
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index ab5e86209322..d15028f63095 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -860,7 +860,7 @@ static int raid10_congested(struct mddev *mddev, int bits)
if (rdev && !test_bit(Faulty, &rdev->flags)) {
struct request_queue *q = bdev_get_queue(rdev->bdev);
- ret |= bdi_congested(&q->backing_dev_info, bits);
+ ret |= bdi_congested(q->backing_dev_info, bits);
}
}
rcu_read_unlock();
@@ -3806,8 +3806,8 @@ static int raid10_run(struct mddev *mddev)
* maybe...
*/
stripe /= conf->geo.near_copies;
- if (mddev->queue->backing_dev_info.ra_pages < 2 * stripe)
- mddev->queue->backing_dev_info.ra_pages = 2 * stripe;
+ if (mddev->queue->backing_dev_info->ra_pages < 2 * stripe)
+ mddev->queue->backing_dev_info->ra_pages = 2 * stripe;
}
if (md_integrity_register(mddev))
@@ -4608,8 +4608,8 @@ static void end_reshape(struct r10conf *conf)
int stripe = conf->geo.raid_disks *
((conf->mddev->chunk_sectors << 9) / PAGE_SIZE);
stripe /= conf->geo.near_copies;
- if (conf->mddev->queue->backing_dev_info.ra_pages < 2 * stripe)
- conf->mddev->queue->backing_dev_info.ra_pages = 2 * stripe;
+ if (conf->mddev->queue->backing_dev_info->ra_pages < 2 * stripe)
+ conf->mddev->queue->backing_dev_info->ra_pages = 2 * stripe;
}
conf->fullsync = 0;
}
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 06d7279bdd04..e075cdae5b5d 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -6262,10 +6262,10 @@ raid5_store_skip_copy(struct mddev *mddev, const char *page, size_t len)
mddev_suspend(mddev);
conf->skip_copy = new;
if (new)
- mddev->queue->backing_dev_info.capabilities |=
+ mddev->queue->backing_dev_info->capabilities |=
BDI_CAP_STABLE_WRITES;
else
- mddev->queue->backing_dev_info.capabilities &=
+ mddev->queue->backing_dev_info->capabilities &=
~BDI_CAP_STABLE_WRITES;
mddev_resume(mddev);
}
@@ -7084,8 +7084,8 @@ static int raid5_run(struct mddev *mddev)
int data_disks = conf->previous_raid_disks - conf->max_degraded;
int stripe = data_disks *
((mddev->chunk_sectors << 9) / PAGE_SIZE);
- if (mddev->queue->backing_dev_info.ra_pages < 2 * stripe)
- mddev->queue->backing_dev_info.ra_pages = 2 * stripe;
+ if (mddev->queue->backing_dev_info->ra_pages < 2 * stripe)
+ mddev->queue->backing_dev_info->ra_pages = 2 * stripe;
chunk_size = mddev->chunk_sectors << 9;
blk_queue_io_min(mddev->queue, chunk_size);
@@ -7694,8 +7694,8 @@ static void end_reshape(struct r5conf *conf)
int data_disks = conf->raid_disks - conf->max_degraded;
int stripe = data_disks * ((conf->chunk_sectors << 9)
/ PAGE_SIZE);
- if (conf->mddev->queue->backing_dev_info.ra_pages < 2 * stripe)
- conf->mddev->queue->backing_dev_info.ra_pages = 2 * stripe;
+ if (conf->mddev->queue->backing_dev_info->ra_pages < 2 * stripe)
+ conf->mddev->queue->backing_dev_info->ra_pages = 2 * stripe;
}
}
}
diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index a34308df927f..c0e5b9a8bf5f 100644
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -1226,7 +1226,7 @@ static int set_gfs2_super(struct super_block *s, void *data)
* We set the bdi here to the queue backing, file systems can
* overwrite this in ->fill_super()
*/
- s->s_bdi = &bdev_get_queue(s->s_bdev)->backing_dev_info;
+ s->s_bdi = bdev_get_queue(s->s_bdev)->backing_dev_info;
return 0;
}
diff --git a/fs/nilfs2/super.c b/fs/nilfs2/super.c
index 12eeae62a2b1..e1872f36147f 100644
--- a/fs/nilfs2/super.c
+++ b/fs/nilfs2/super.c
@@ -1068,7 +1068,7 @@ nilfs_fill_super(struct super_block *sb, void *data, int silent)
sb->s_time_gran = 1;
sb->s_max_links = NILFS_LINK_MAX;
- sb->s_bdi = &bdev_get_queue(sb->s_bdev)->backing_dev_info;
+ sb->s_bdi = bdev_get_queue(sb->s_bdev)->backing_dev_info;
err = load_nilfs(nilfs, sb);
if (err)
diff --git a/fs/super.c b/fs/super.c
index 1709ed029a2c..ea662b0e5e78 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -1047,7 +1047,7 @@ static int set_bdev_super(struct super_block *s, void *data)
* We set the bdi here to the queue backing, file systems can
* overwrite this in ->fill_super()
*/
- s->s_bdi = &bdev_get_queue(s->s_bdev)->backing_dev_info;
+ s->s_bdi = bdev_get_queue(s->s_bdev)->backing_dev_info;
return 0;
}
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 83695641bd5e..069e4a102a73 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -432,7 +432,8 @@ struct request_queue {
*/
struct delayed_work delay_work;
- struct backing_dev_info backing_dev_info;
+ struct backing_dev_info *backing_dev_info;
+ struct backing_dev_info _backing_dev_info;
/*
* The queue owner gets to use this for whatever they like.
diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index 290e8b7d3181..216449825859 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -1988,11 +1988,11 @@ void laptop_mode_timer_fn(unsigned long data)
* We want to write everything out, not just down to the dirty
* threshold
*/
- if (!bdi_has_dirty_io(&q->backing_dev_info))
+ if (!bdi_has_dirty_io(q->backing_dev_info))
return;
rcu_read_lock();
- list_for_each_entry_rcu(wb, &q->backing_dev_info.wb_list, bdi_node)
+ list_for_each_entry_rcu(wb, &q->backing_dev_info->wb_list, bdi_node)
if (wb_has_dirty_io(wb))
wb_start_writeback(wb, nr_pages, true,
WB_REASON_LAPTOP_TIMER);
--
2.10.2
^ permalink raw reply related
* [PATCH 1/5] block: Unhash block device inodes on gendisk destruction
From: Jan Kara @ 2017-02-02 14:56 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Christoph Hellwig, Dan Williams,
Thiago Jung Bauermann, Laurent Dufour, Jan Kara
In-Reply-To: <20170202145653.19614-1-jack@suse.cz>
Currently, block device inodes stay around after corresponding gendisk
hash died until memory reclaim finds them and frees them. Since we will
make block device inode pin the bdi, we want to free the block device
inode as soon as the device goes away so that bdi does not stay around
unnecessarily. Furthermore we need to avoid issues when new device with
the same major,minor pair gets created since reusing the bdi structure
would be rather difficult in this case.
Unhashing block device inode on gendisk destruction nicely deals with
these problems. Once last block device inode reference is dropped (which
may be directly in del_gendisk()), the inode gets evicted. Furthermore if
the major,minor pair gets reallocated, we are guaranteed to get new
block device inode even if old block device inode is not yet evicted and
thus we avoid issues with possible reuse of bdi.
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Jan Kara <jack@suse.cz>
---
block/genhd.c | 2 ++
fs/block_dev.c | 15 +++++++++++++++
include/linux/fs.h | 1 +
3 files changed, 18 insertions(+)
diff --git a/block/genhd.c b/block/genhd.c
index fcd6d4fae657..f2f22d0e8e14 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -648,6 +648,8 @@ void del_gendisk(struct gendisk *disk)
disk_part_iter_init(&piter, disk,
DISK_PITER_INCL_EMPTY | DISK_PITER_REVERSE);
while ((part = disk_part_iter_next(&piter))) {
+ bdev_unhash_inode(MKDEV(disk->major,
+ disk->first_minor + part->partno));
invalidate_partition(disk, part->partno);
delete_partition(disk, part->partno);
}
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 5db5d1340d69..ed6a34be7a1e 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -954,6 +954,21 @@ static int bdev_set(struct inode *inode, void *data)
static LIST_HEAD(all_bdevs);
+/*
+ * If there is a bdev inode for this device, unhash it so that it gets evicted
+ * as soon as last inode reference is dropped.
+ */
+void bdev_unhash_inode(dev_t dev)
+{
+ struct inode *inode;
+
+ inode = ilookup5(blockdev_superblock, hash(dev), bdev_test, &dev);
+ if (inode) {
+ remove_inode_hash(inode);
+ iput(inode);
+ }
+}
+
struct block_device *bdget(dev_t dev)
{
struct block_device *bdev;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 2ba074328894..702cb6c50194 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2342,6 +2342,7 @@ extern struct kmem_cache *names_cachep;
#ifdef CONFIG_BLOCK
extern int register_blkdev(unsigned int, const char *);
extern void unregister_blkdev(unsigned int, const char *);
+extern void bdev_unhash_inode(dev_t dev);
extern struct block_device *bdget(dev_t);
extern struct block_device *bdgrab(struct block_device *bdev);
extern void bd_set_size(struct block_device *, loff_t size);
--
2.10.2
^ permalink raw reply related
* [PATCH 0/5 v3] BDI lifetime fix
From: Jan Kara @ 2017-02-02 14:56 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Christoph Hellwig, Dan Williams,
Thiago Jung Bauermann, Laurent Dufour, Jan Kara
Hello,
this is the third version of the patch series that attempts to solve the
problems with the life time of a backing_dev_info structure. Currently it lives
inside request_queue structure and thus it gets destroyed as soon as request
queue goes away. However the block device inode still stays around and thus
inode_to_bdi() call on that inode (e.g. from flusher worker) may happen after
request queue has been destroyed resulting in oops.
This patch set tries to solve these problems by making backing_dev_info
independent structure referenced from block device inode. That makes sure
inode_to_bdi() cannot ever oops. I gave some basic testing to the patches in
KVM and on a real machine, Dan was running them with libnvdimm test suite which
was previously triggering the oops and things look good. So patches should be
reasonably healthy.
Changes since v2:
* Added Reviewed-by tags
* Removed slab cache for backing_dev_info
* Added patch to remove blkdev_get_backing_dev_info()
Changes since v1:
* Use kref instead of atomic_t for refcount
* Get rid of free_on_put flag
Honza
^ permalink raw reply
* Re: [PATCH 3/4] block: Dynamically allocate and refcount backing_dev_info
From: Jan Kara @ 2017-02-02 13:32 UTC (permalink / raw)
To: Jens Axboe
Cc: Jan Kara, Christoph Hellwig, linux-block, Dan Williams,
Thiago Jung Bauermann, Laurent Dufour
In-Reply-To: <53f2bb8c-d4e6-7af9-2250-cccdf520660b@kernel.dk>
On Wed 01-02-17 14:45:20, Jens Axboe wrote:
> On 02/01/2017 04:22 AM, Jan Kara wrote:
> > On Wed 01-02-17 01:50:07, Christoph Hellwig wrote:
> >> On Tue, Jan 31, 2017 at 01:54:28PM +0100, Jan Kara wrote:
> >>> Instead of storing backing_dev_info inside struct request_queue,
> >>> allocate it dynamically, reference count it, and free it when the last
> >>> reference is dropped. Currently only request_queue holds the reference
> >>> but in the following patch we add other users referencing
> >>> backing_dev_info.
> >>
> >> Do we really need the separate slab cache? Otherwise this looks
> >> fine to me.
> >
> > Yeah, probably it is not worth it. I'll remove it.
>
> I agree on that, it should not be a hot path. Will you respin the series
> after making this change? Would be great to get this queued up.
Yes, will send it later today. I was just waiting whether someone else does
not have more comments to the series.
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH 0/6] block: fix blk-mq debugfs vs. blktrace
From: Greg Kroah-Hartman @ 2017-02-02 10:58 UTC (permalink / raw)
To: Omar Sandoval; +Cc: Jens Axboe, linux-block, kernel-team
In-Reply-To: <20170201083115.GA16278@vader>
On Wed, Feb 01, 2017 at 12:31:15AM -0800, Omar Sandoval wrote:
> On Wed, Feb 01, 2017 at 09:16:08AM +0100, Greg Kroah-Hartman wrote:
> > On Tue, Jan 31, 2017 at 02:53:16PM -0800, Omar Sandoval wrote:
> > > From: Omar Sandoval <osandov@fb.com>
> > >
> > > When I moved the blk-mq debugging information to debugfs, I didn't
> > > realize that blktrace also created directories in debugfs that
> > > conflicted with the blk-mq directories. This series fixes that.
> > >
> > > Patch 1 adds a new debugfs helper needed for patch 6. Greg, could I get
> > > an ack on that if it makes sense? Jens and I went back and forth on this
> > > for a little while, but patch 6 has more of the rationale on why we
> > > decided that this approach was the cleanest.
> >
> > I can't find patch 6, you only cc:ed me on the first patch :(
> >
> > Care to bounce them all to me?
> >
> > thanks,
> >
> > greg k-h
>
> Gah, I forgot --cc-cover to git-send-email. The series is all here:
> http://marc.info/?l=linux-block&r=1&b=201701&w=2. I can also send the
> patches directly to you if you prefer that.
I don't understand the problem here. How do you not know if you have
created the debugfs file or not? You have the structure, with the
correct name, how could it have been created? Can't you save the dentry
to the debugfs file in the structure that has the name?
thanks,
greg k-h
^ permalink raw reply
* Re: Doubt related to immutable biovecs
From: Suraj Choudhari @ 2017-02-02 9:35 UTC (permalink / raw)
To: Ming Lei; +Cc: linux-block
In-Reply-To: <CACVXFVNwB48b5WGoR3p_akdsCc9e74TordZ89OaHNC+bdtBP3Q@mail.gmail.com>
Thanks very much Ming and all !
I referred to multipage bvec patches link and it was indeed good & insightful.
I have few more query related to immutable bvecs & multipage bvecs, in
particular related to 4.4 kernel --
1) My understanding is bi_vcnt can't be used for a cloned BIO because
'bio_clone_fast' does NOT update the bi_vcnt field in the cloned BIO.
Is my understanding correct ?
2) For a block device driver, is it be safe to use the 'bi_vcnt' of a
BIO that's received from the upper layers, in order to access the
bvecs array, particularly in the '4.4' kernel ?
- I am thinking it may be safe to access bi_vcnt as long as the
block driver has NOT received a CLONE bio from upper layer. The
reason I think can be bvecs are not multipage in 4.4. Is my
understanding right ?
Thanks & Regards,
Suraj
On Fri, Jan 20, 2017 at 9:27 PM, Ming Lei <tom.leiming@gmail.com> wrote:
> Hi,
>
> On Fri, Jan 20, 2017 at 9:41 PM, Suraj Choudhari
> <surajschoudhari@gmail.com> wrote:
>> Hello,
>>
>> I've some queries related to accessing the 'bio_vec' and 'bi_vcnt'
>> members in the BIO structure after the kernel changes implemented for
>> the 'immutable biovecs' -
>>
>> Background -
>> - From the changes done for the immutable biovecs, I understand that
>> the driver code now no longer needs to reference the 'bi_vcnt' and
>> 'bi_io_vec' fields directly. Instead we can use the 'bvec_iter'
>> iterator in order to to access the bio_vec.
>> - We've iterator functions bio_iter_iovec() and bio_advance_iter() as
>> well, which return us literal 'struct biovecs' taking into account the
>> bi_bvec_done and bi_size values.
>>
>> Doubts -
>> 1) Few functions in the block layer and some drivers still directly
>> refer to the 'bi_vcnt' and 'bio_vec' members of the BIO structure
>> [instead of accessing bio_vecs using the bvec_iter iterator].
>> Would there be some changes in the functions to compulsorily use the
>> 'bvec_iter' in order to access bio_vecs? If yes, are such changes
>
> Generally speaking, yes, it is always better to use iterator helpers.
>
>> related to the 'multipage biovec' feature ?
>
> Yes, it is a must for supporting multipage bvec.
>
>>
>> 2) Can there be some problem if a driver directly accesses the
>> bi_io_vec [bio_vec] in the bio structure based on the values of bi_idx
>> and bi_vcnt ? [instead of using the bvec_iter iterator].
>
> The direct access can't work any more once switching to multipage bvec,
> and you can find some hint in the following link:
>
> https://lkml.org/lkml/2017/1/15/266
>
> Also .bi_vcnt can't be used for BIO_CLONED bio, and it is very easy
> to make mistakes to access the table directly if someone don't understand
> the iterator details enough.
>
>
> Thanks,
> Ming Lei
^ permalink raw reply
* Re: [PATCH 7/8] mq-deadline: add blk-mq adaptation of the deadline IO scheduler
From: Paolo Valente @ 2017-02-02 9:19 UTC (permalink / raw)
To: Jens Axboe; +Cc: Jens Axboe, linux-block, Linux-Kernal, osandov
In-Reply-To: <21ff7888-ec08-0dab-a997-872ad0027fe2@fb.com>
> Il giorno 02 feb 2017, alle ore 06:19, Jens Axboe <axboe@fb.com> ha =
scritto:
>=20
> On 02/01/2017 04:11 AM, Paolo Valente wrote:
>>> +static bool dd_bio_merge(struct blk_mq_hw_ctx *hctx, struct bio =
*bio)
>>> +{
>>> + struct request_queue *q =3D hctx->queue;
>>> + struct deadline_data *dd =3D q->elevator->elevator_data;
>>> + int ret;
>>> +
>>> + spin_lock(&dd->lock);
>>> + ret =3D blk_mq_sched_try_merge(q, bio);
>>> + spin_unlock(&dd->lock);
>>> +
>>=20
>> Hi Jens,
>> first, good news, bfq is passing my first sanity checks. Still, I
>> need a little more help for the following issue. There is a case =
that
>> would be impossible to handle without modifying code outside bfq. =
But
>> so far such a case never occurred, and I hope that it can never =
occur.
>> I'll try to briefly list all relevant details on this concern of =
mine,
>> so that you can quickly confirm my hope, or highlight where or what I
>> am missing.
>=20
> Remember my earlier advice - it's not a problem to change anything in
> the core, in fact I would be surprised if you did not need to. My
> foresight isn't THAT good! It's much better to fix up an inconsistency
> there, rather than work around it in the consumer of that API.
>=20
>> First, as done above for mq-deadline, invoking blk_mq_sched_try_merge
>> with the scheduler lock held is of course necessary (for example, to
>> protect q->last_merge). This may lead to put_rq_private invoked
>> with the lock held, in case of successful merge.
>=20
> Right, or some other lock with the same scope, as per my other email.
>=20
>> As a consequence, put_rq_private may be invoked:
>> (1) in IRQ context, no scheduler lock held, because of a completion:
>> can be handled by deferring work and lock grabbing, because the
>> completed request is not queued in the scheduler any more;
>> (2) in process context, scheduler lock held, because of the above
>> successful merge: must be handled immediately, for consistency,
>> because the request is still queued in the scheduler;
>> (3) in process context, no scheduler lock held, for some other =
reason:
>> some path apparently may lead to this case, although I've never seen
>> it to happen. Immediate handling, and hence locking, may be needed,
>> depending on whether the request is still queued in the scheduler.
>>=20
>> So, my main question is: is case (3) actually impossible? Should it
>> be possible, I guess we would have a problem, because of the
>> different lock state with respect to (2).
>=20
> I agree, there's some inconsistency there, if you potentially need to
> grab the lock in your put_rq_private handler. The problem case is #2,
> when we have the merge. I would probably suggest that the best way to
> handle that is to pass back the dropped request so we can put it =
outside
> of holding the lock.
>=20
> Let me see if I can come up with a good solution for this. We have to =
be
> consistent in how we invoke the scheduler functions, we can't have =
hooks
> that are called in unknown lock states. I also don't want you to have =
to
> add defer work handling in that kind of path, that will impact your
> performance and overhead.
>=20
I'll try to learn from your solution, because, as of now, I don't see
how to avoid deferred work for the case where put_rq_private is
invoked in interrupt context. In fact, for this case, we cannot grab
the lock, unless we turn all spin_lock into spin_lock_irq*.
>> Finally, I hope that it is certainly impossible to have a case (4): =
in
>> IRQ context, no lock held, but with the request in the scheduler.
>=20
> That should not be possible.
>=20
> Edit: since I'm on a flight and email won't send, I had a few minutes =
to
> hack this up. Totally untested, but something like the below should do
> it. Not super pretty... I'll play with this a bit more tomorrow.
>=20
>=20
The scheme is clear. One comment, in case it could make sense and
avoid more complexity: since put_rq_priv is invoked in two different
contexts, process or interrupt, I didn't feel so confusing that, when
put_rq_priv is invoked in the context where the lock cannot be held
(unless one is willing to pay with irq disabling all the times), the
lock is not held, while, when invoked in the context where the lock
can be held, the lock is actually held, or must be taken.
Thanks,
Paolo
> diff --git a/block/blk-core.c b/block/blk-core.c
> index c142de090c41..530a9a3f60c9 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -1609,7 +1609,7 @@ static blk_qc_t blk_queue_bio(struct =
request_queue *q, struct bio *bio)
> {
> struct blk_plug *plug;
> int el_ret, where =3D ELEVATOR_INSERT_SORT;
> - struct request *req;
> + struct request *req, *free;
> unsigned int request_count =3D 0;
> unsigned int wb_acct;
>=20
> @@ -1650,15 +1650,21 @@ static blk_qc_t blk_queue_bio(struct =
request_queue *q, struct bio *bio)
> if (el_ret =3D=3D ELEVATOR_BACK_MERGE) {
> if (bio_attempt_back_merge(q, req, bio)) {
> elv_bio_merged(q, req, bio);
> - if (!attempt_back_merge(q, req))
> + free =3D attempt_back_merge(q, req);
> + if (!free)
> elv_merged_request(q, req, el_ret);
> + else
> + __blk_put_request(q, free);
> goto out_unlock;
> }
> } else if (el_ret =3D=3D ELEVATOR_FRONT_MERGE) {
> if (bio_attempt_front_merge(q, req, bio)) {
> elv_bio_merged(q, req, bio);
> - if (!attempt_front_merge(q, req))
> + free =3D attempt_front_merge(q, req);
> + if (!free)
> elv_merged_request(q, req, el_ret);
> + else
> + __blk_put_request(q, free);
> goto out_unlock;
> }
> }
> diff --git a/block/blk-merge.c b/block/blk-merge.c
> index 6aa43dec5af4..011b1c6e3cb4 100644
> --- a/block/blk-merge.c
> +++ b/block/blk-merge.c
> @@ -661,29 +661,29 @@ static void blk_account_io_merge(struct request =
*req)
> /*
> * Has to be called with the request spinlock acquired
> */
> -static int attempt_merge(struct request_queue *q, struct request =
*req,
> - struct request *next)
> +static struct request *attempt_merge(struct request_queue *q,
> + struct request *req, struct request =
*next)
> {
> if (!rq_mergeable(req) || !rq_mergeable(next))
> - return 0;
> + return NULL;
>=20
> if (req_op(req) !=3D req_op(next))
> - return 0;
> + return NULL;
>=20
> /*
> * not contiguous
> */
> if (blk_rq_pos(req) + blk_rq_sectors(req) !=3D blk_rq_pos(next))
> - return 0;
> + return NULL;
>=20
> if (rq_data_dir(req) !=3D rq_data_dir(next)
> || req->rq_disk !=3D next->rq_disk
> || req_no_special_merge(next))
> - return 0;
> + return NULL;
>=20
> if (req_op(req) =3D=3D REQ_OP_WRITE_SAME &&
> !blk_write_same_mergeable(req->bio, next->bio))
> - return 0;
> + return NULL;
>=20
> /*
> * If we are allowed to merge, then append bio list
> @@ -692,7 +692,7 @@ static int attempt_merge(struct request_queue *q, =
struct request *req,
> * counts here.
> */
> if (!ll_merge_requests_fn(q, req, next))
> - return 0;
> + return NULL;
>=20
> /*
> * If failfast settings disagree or any of the two is already
> @@ -732,30 +732,32 @@ static int attempt_merge(struct request_queue =
*q, struct request *req,
> if (blk_rq_cpu_valid(next))
> req->cpu =3D next->cpu;
>=20
> - /* owner-ship of bio passed from next to req */
> + /*
> + * owner-ship of bio passed from next to req, return 'next' for
> + * the caller to free
> + */
> next->bio =3D NULL;
> - __blk_put_request(q, next);
> - return 1;
> + return next;
> }
>=20
> -int attempt_back_merge(struct request_queue *q, struct request *rq)
> +struct request *attempt_back_merge(struct request_queue *q, struct =
request *rq)
> {
> struct request *next =3D elv_latter_request(q, rq);
>=20
> if (next)
> return attempt_merge(q, rq, next);
>=20
> - return 0;
> + return NULL;
> }
>=20
> -int attempt_front_merge(struct request_queue *q, struct request *rq)
> +struct request *attempt_front_merge(struct request_queue *q, struct =
request *rq)
> {
> struct request *prev =3D elv_former_request(q, rq);
>=20
> if (prev)
> return attempt_merge(q, prev, rq);
>=20
> - return 0;
> + return NULL;
> }
>=20
> int blk_attempt_req_merge(struct request_queue *q, struct request *rq,
> @@ -767,7 +769,12 @@ int blk_attempt_req_merge(struct request_queue =
*q, struct request *rq,
> if (!e->type->ops.sq.elevator_allow_rq_merge_fn(q, rq, =
next))
> return 0;
>=20
> - return attempt_merge(q, rq, next);
> + if (attempt_merge(q, rq, next)) {
> + __blk_put_request(q, next);
> + return 1;
> + }
> +
> + return 0;
> }
>=20
> bool blk_rq_merge_ok(struct request *rq, struct bio *bio)
> diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
> index 114814ec3d49..d93b56d53c4e 100644
> --- a/block/blk-mq-sched.c
> +++ b/block/blk-mq-sched.c
> @@ -234,7 +234,8 @@ void blk_mq_sched_move_to_dispatch(struct =
blk_mq_hw_ctx *hctx,
> }
> EXPORT_SYMBOL_GPL(blk_mq_sched_move_to_dispatch);
>=20
> -bool blk_mq_sched_try_merge(struct request_queue *q, struct bio *bio)
> +bool blk_mq_sched_try_merge(struct request_queue *q, struct bio *bio,
> + struct request **merged_request)
> {
> struct request *rq;
> int ret;
> @@ -244,7 +245,8 @@ bool blk_mq_sched_try_merge(struct request_queue =
*q, struct bio *bio)
> if (!blk_mq_sched_allow_merge(q, rq, bio))
> return false;
> if (bio_attempt_back_merge(q, rq, bio)) {
> - if (!attempt_back_merge(q, rq))
> + *merged_request =3D attempt_back_merge(q, rq);
> + if (!*merged_request)
> elv_merged_request(q, rq, ret);
> return true;
> }
> @@ -252,7 +254,8 @@ bool blk_mq_sched_try_merge(struct request_queue =
*q, struct bio *bio)
> if (!blk_mq_sched_allow_merge(q, rq, bio))
> return false;
> if (bio_attempt_front_merge(q, rq, bio)) {
> - if (!attempt_front_merge(q, rq))
> + *merged_request =3D attempt_front_merge(q, rq);
> + if (!*merged_request)
> elv_merged_request(q, rq, ret);
> return true;
> }
> diff --git a/block/blk-mq-sched.h b/block/blk-mq-sched.h
> index 9478aaeb48c5..3643686a54b8 100644
> --- a/block/blk-mq-sched.h
> +++ b/block/blk-mq-sched.h
> @@ -16,7 +16,8 @@ void blk_mq_sched_put_request(struct request *rq);
>=20
> void blk_mq_sched_request_inserted(struct request *rq);
> bool blk_mq_sched_bypass_insert(struct blk_mq_hw_ctx *hctx, struct =
request *rq);
> -bool blk_mq_sched_try_merge(struct request_queue *q, struct bio =
*bio);
> +bool blk_mq_sched_try_merge(struct request_queue *q, struct bio *bio,
> + struct request **merged_request);
> bool __blk_mq_sched_bio_merge(struct request_queue *q, struct bio =
*bio);
> bool blk_mq_sched_try_insert_merge(struct request_queue *q, struct =
request *rq);
> void blk_mq_sched_restart_queues(struct blk_mq_hw_ctx *hctx);
> diff --git a/block/blk.h b/block/blk.h
> index c1bd4bf9e645..918cea38d51e 100644
> --- a/block/blk.h
> +++ b/block/blk.h
> @@ -204,8 +204,8 @@ int ll_back_merge_fn(struct request_queue *q, =
struct request *req,
> struct bio *bio);
> int ll_front_merge_fn(struct request_queue *q, struct request *req,=20
> struct bio *bio);
> -int attempt_back_merge(struct request_queue *q, struct request *rq);
> -int attempt_front_merge(struct request_queue *q, struct request *rq);
> +struct request *attempt_back_merge(struct request_queue *q, struct =
request *rq);
> +struct request *attempt_front_merge(struct request_queue *q, struct =
request *rq);
> int blk_attempt_req_merge(struct request_queue *q, struct request *rq,
> struct request *next);
> void blk_recalc_rq_segments(struct request *rq);
> diff --git a/block/mq-deadline.c b/block/mq-deadline.c
> index 49583536698c..682fa64f55ff 100644
> --- a/block/mq-deadline.c
> +++ b/block/mq-deadline.c
> @@ -371,12 +371,16 @@ static bool dd_bio_merge(struct blk_mq_hw_ctx =
*hctx, struct bio *bio)
> {
> struct request_queue *q =3D hctx->queue;
> struct deadline_data *dd =3D q->elevator->elevator_data;
> - int ret;
> + struct request *free =3D NULL;
> + bool ret;
>=20
> spin_lock(&dd->lock);
> - ret =3D blk_mq_sched_try_merge(q, bio);
> + ret =3D blk_mq_sched_try_merge(q, bio, &free);
> spin_unlock(&dd->lock);
>=20
> + if (free)
> + blk_mq_free_request(free);
> +
> return ret;
> }
>=20
>=20
> --=20
> Jens Axboe
>=20
^ permalink raw reply
* Re: [PATCH v3] scsi, block: fix duplicate bdi name registration crashes
From: Christoph Hellwig @ 2017-02-02 8:14 UTC (permalink / raw)
To: Dan Williams
Cc: martin.petersen, Jens Axboe, linux-block, Jan Kara, linux-scsi,
linux-kernel, Christoph Hellwig, James Bottomley, Bart Van Assche,
Omar Sandoval, Omar Sandoval
In-Reply-To: <148598661794.11527.10024529085859239448.stgit@dwillia2-desk3.amr.corp.intel.com>
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: [PATCH 7/8] mq-deadline: add blk-mq adaptation of the deadline IO scheduler
From: Jens Axboe @ 2017-02-02 5:20 UTC (permalink / raw)
To: Paolo Valente; +Cc: Jens Axboe, linux-block, Linux-Kernal, osandov
In-Reply-To: <26ED11F4-2507-4A5D-ACB9-C501AA9CBB29@linaro.org>
On 02/01/2017 04:56 AM, Paolo Valente wrote:
>> +/*
>> + * add rq to rbtree and fifo
>> + */
>> +static void dd_insert_request(struct blk_mq_hw_ctx *hctx, struct request *rq,
>> + bool at_head)
>> +{
>> + struct request_queue *q = hctx->queue;
>> + struct deadline_data *dd = q->elevator->elevator_data;
>> + const int data_dir = rq_data_dir(rq);
>> +
>> + if (blk_mq_sched_try_insert_merge(q, rq))
>> + return;
>> +
>
> A related doubt: shouldn't blk_mq_sched_try_insert_merge be invoked
> with the scheduler lock held too, as blk_mq_sched_try_merge, to
> protect (at least) q->last_merge?
>
> In bfq this function is invoked with the lock held.
It doesn't matter which lock you use, as long as:
1) You use the same one consistently
2) It has the same scope as the queue lock (the one you call the
scheduler lock)
mq-deadline sets up a per-queue structure, deadline_data, and it has a
lock embedded in that structure. This is what mq-deadline uses to
serialize access to its data structures, as well as those in the queue
(like last_merge).
--
Jens Axboe
^ permalink raw reply
* Re: [PATCH 7/8] mq-deadline: add blk-mq adaptation of the deadline IO scheduler
From: Jens Axboe @ 2017-02-02 5:19 UTC (permalink / raw)
To: Paolo Valente; +Cc: Jens Axboe, linux-block, Linux-Kernal, osandov
In-Reply-To: <84DA10A3-5055-4D48-A990-6CB5A5D34F0C@linaro.org>
On 02/01/2017 04:11 AM, Paolo Valente wrote:
>> +static bool dd_bio_merge(struct blk_mq_hw_ctx *hctx, struct bio *bio)
>> +{
>> + struct request_queue *q = hctx->queue;
>> + struct deadline_data *dd = q->elevator->elevator_data;
>> + int ret;
>> +
>> + spin_lock(&dd->lock);
>> + ret = blk_mq_sched_try_merge(q, bio);
>> + spin_unlock(&dd->lock);
>> +
>
> Hi Jens,
> first, good news, bfq is passing my first sanity checks. Still, I
> need a little more help for the following issue. There is a case that
> would be impossible to handle without modifying code outside bfq. But
> so far such a case never occurred, and I hope that it can never occur.
> I'll try to briefly list all relevant details on this concern of mine,
> so that you can quickly confirm my hope, or highlight where or what I
> am missing.
Remember my earlier advice - it's not a problem to change anything in
the core, in fact I would be surprised if you did not need to. My
foresight isn't THAT good! It's much better to fix up an inconsistency
there, rather than work around it in the consumer of that API.
> First, as done above for mq-deadline, invoking blk_mq_sched_try_merge
> with the scheduler lock held is of course necessary (for example, to
> protect q->last_merge). This may lead to put_rq_private invoked
> with the lock held, in case of successful merge.
Right, or some other lock with the same scope, as per my other email.
> As a consequence, put_rq_private may be invoked:
> (1) in IRQ context, no scheduler lock held, because of a completion:
> can be handled by deferring work and lock grabbing, because the
> completed request is not queued in the scheduler any more;
> (2) in process context, scheduler lock held, because of the above
> successful merge: must be handled immediately, for consistency,
> because the request is still queued in the scheduler;
> (3) in process context, no scheduler lock held, for some other reason:
> some path apparently may lead to this case, although I've never seen
> it to happen. Immediate handling, and hence locking, may be needed,
> depending on whether the request is still queued in the scheduler.
>
> So, my main question is: is case (3) actually impossible? Should it
> be possible, I guess we would have a problem, because of the
> different lock state with respect to (2).
I agree, there's some inconsistency there, if you potentially need to
grab the lock in your put_rq_private handler. The problem case is #2,
when we have the merge. I would probably suggest that the best way to
handle that is to pass back the dropped request so we can put it outside
of holding the lock.
Let me see if I can come up with a good solution for this. We have to be
consistent in how we invoke the scheduler functions, we can't have hooks
that are called in unknown lock states. I also don't want you to have to
add defer work handling in that kind of path, that will impact your
performance and overhead.
> Finally, I hope that it is certainly impossible to have a case (4): in
> IRQ context, no lock held, but with the request in the scheduler.
That should not be possible.
Edit: since I'm on a flight and email won't send, I had a few minutes to
hack this up. Totally untested, but something like the below should do
it. Not super pretty... I'll play with this a bit more tomorrow.
diff --git a/block/blk-core.c b/block/blk-core.c
index c142de090c41..530a9a3f60c9 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1609,7 +1609,7 @@ static blk_qc_t blk_queue_bio(struct request_queue *q, struct bio *bio)
{
struct blk_plug *plug;
int el_ret, where = ELEVATOR_INSERT_SORT;
- struct request *req;
+ struct request *req, *free;
unsigned int request_count = 0;
unsigned int wb_acct;
@@ -1650,15 +1650,21 @@ static blk_qc_t blk_queue_bio(struct request_queue *q, struct bio *bio)
if (el_ret == ELEVATOR_BACK_MERGE) {
if (bio_attempt_back_merge(q, req, bio)) {
elv_bio_merged(q, req, bio);
- if (!attempt_back_merge(q, req))
+ free = attempt_back_merge(q, req);
+ if (!free)
elv_merged_request(q, req, el_ret);
+ else
+ __blk_put_request(q, free);
goto out_unlock;
}
} else if (el_ret == ELEVATOR_FRONT_MERGE) {
if (bio_attempt_front_merge(q, req, bio)) {
elv_bio_merged(q, req, bio);
- if (!attempt_front_merge(q, req))
+ free = attempt_front_merge(q, req);
+ if (!free)
elv_merged_request(q, req, el_ret);
+ else
+ __blk_put_request(q, free);
goto out_unlock;
}
}
diff --git a/block/blk-merge.c b/block/blk-merge.c
index 6aa43dec5af4..011b1c6e3cb4 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -661,29 +661,29 @@ static void blk_account_io_merge(struct request *req)
/*
* Has to be called with the request spinlock acquired
*/
-static int attempt_merge(struct request_queue *q, struct request *req,
- struct request *next)
+static struct request *attempt_merge(struct request_queue *q,
+ struct request *req, struct request *next)
{
if (!rq_mergeable(req) || !rq_mergeable(next))
- return 0;
+ return NULL;
if (req_op(req) != req_op(next))
- return 0;
+ return NULL;
/*
* not contiguous
*/
if (blk_rq_pos(req) + blk_rq_sectors(req) != blk_rq_pos(next))
- return 0;
+ return NULL;
if (rq_data_dir(req) != rq_data_dir(next)
|| req->rq_disk != next->rq_disk
|| req_no_special_merge(next))
- return 0;
+ return NULL;
if (req_op(req) == REQ_OP_WRITE_SAME &&
!blk_write_same_mergeable(req->bio, next->bio))
- return 0;
+ return NULL;
/*
* If we are allowed to merge, then append bio list
@@ -692,7 +692,7 @@ static int attempt_merge(struct request_queue *q, struct request *req,
* counts here.
*/
if (!ll_merge_requests_fn(q, req, next))
- return 0;
+ return NULL;
/*
* If failfast settings disagree or any of the two is already
@@ -732,30 +732,32 @@ static int attempt_merge(struct request_queue *q, struct request *req,
if (blk_rq_cpu_valid(next))
req->cpu = next->cpu;
- /* owner-ship of bio passed from next to req */
+ /*
+ * owner-ship of bio passed from next to req, return 'next' for
+ * the caller to free
+ */
next->bio = NULL;
- __blk_put_request(q, next);
- return 1;
+ return next;
}
-int attempt_back_merge(struct request_queue *q, struct request *rq)
+struct request *attempt_back_merge(struct request_queue *q, struct request *rq)
{
struct request *next = elv_latter_request(q, rq);
if (next)
return attempt_merge(q, rq, next);
- return 0;
+ return NULL;
}
-int attempt_front_merge(struct request_queue *q, struct request *rq)
+struct request *attempt_front_merge(struct request_queue *q, struct request *rq)
{
struct request *prev = elv_former_request(q, rq);
if (prev)
return attempt_merge(q, prev, rq);
- return 0;
+ return NULL;
}
int blk_attempt_req_merge(struct request_queue *q, struct request *rq,
@@ -767,7 +769,12 @@ int blk_attempt_req_merge(struct request_queue *q, struct request *rq,
if (!e->type->ops.sq.elevator_allow_rq_merge_fn(q, rq, next))
return 0;
- return attempt_merge(q, rq, next);
+ if (attempt_merge(q, rq, next)) {
+ __blk_put_request(q, next);
+ return 1;
+ }
+
+ return 0;
}
bool blk_rq_merge_ok(struct request *rq, struct bio *bio)
diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
index 114814ec3d49..d93b56d53c4e 100644
--- a/block/blk-mq-sched.c
+++ b/block/blk-mq-sched.c
@@ -234,7 +234,8 @@ void blk_mq_sched_move_to_dispatch(struct blk_mq_hw_ctx *hctx,
}
EXPORT_SYMBOL_GPL(blk_mq_sched_move_to_dispatch);
-bool blk_mq_sched_try_merge(struct request_queue *q, struct bio *bio)
+bool blk_mq_sched_try_merge(struct request_queue *q, struct bio *bio,
+ struct request **merged_request)
{
struct request *rq;
int ret;
@@ -244,7 +245,8 @@ bool blk_mq_sched_try_merge(struct request_queue *q, struct bio *bio)
if (!blk_mq_sched_allow_merge(q, rq, bio))
return false;
if (bio_attempt_back_merge(q, rq, bio)) {
- if (!attempt_back_merge(q, rq))
+ *merged_request = attempt_back_merge(q, rq);
+ if (!*merged_request)
elv_merged_request(q, rq, ret);
return true;
}
@@ -252,7 +254,8 @@ bool blk_mq_sched_try_merge(struct request_queue *q, struct bio *bio)
if (!blk_mq_sched_allow_merge(q, rq, bio))
return false;
if (bio_attempt_front_merge(q, rq, bio)) {
- if (!attempt_front_merge(q, rq))
+ *merged_request = attempt_front_merge(q, rq);
+ if (!*merged_request)
elv_merged_request(q, rq, ret);
return true;
}
diff --git a/block/blk-mq-sched.h b/block/blk-mq-sched.h
index 9478aaeb48c5..3643686a54b8 100644
--- a/block/blk-mq-sched.h
+++ b/block/blk-mq-sched.h
@@ -16,7 +16,8 @@ void blk_mq_sched_put_request(struct request *rq);
void blk_mq_sched_request_inserted(struct request *rq);
bool blk_mq_sched_bypass_insert(struct blk_mq_hw_ctx *hctx, struct request *rq);
-bool blk_mq_sched_try_merge(struct request_queue *q, struct bio *bio);
+bool blk_mq_sched_try_merge(struct request_queue *q, struct bio *bio,
+ struct request **merged_request);
bool __blk_mq_sched_bio_merge(struct request_queue *q, struct bio *bio);
bool blk_mq_sched_try_insert_merge(struct request_queue *q, struct request *rq);
void blk_mq_sched_restart_queues(struct blk_mq_hw_ctx *hctx);
diff --git a/block/blk.h b/block/blk.h
index c1bd4bf9e645..918cea38d51e 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -204,8 +204,8 @@ int ll_back_merge_fn(struct request_queue *q, struct request *req,
struct bio *bio);
int ll_front_merge_fn(struct request_queue *q, struct request *req,
struct bio *bio);
-int attempt_back_merge(struct request_queue *q, struct request *rq);
-int attempt_front_merge(struct request_queue *q, struct request *rq);
+struct request *attempt_back_merge(struct request_queue *q, struct request *rq);
+struct request *attempt_front_merge(struct request_queue *q, struct request *rq);
int blk_attempt_req_merge(struct request_queue *q, struct request *rq,
struct request *next);
void blk_recalc_rq_segments(struct request *rq);
diff --git a/block/mq-deadline.c b/block/mq-deadline.c
index 49583536698c..682fa64f55ff 100644
--- a/block/mq-deadline.c
+++ b/block/mq-deadline.c
@@ -371,12 +371,16 @@ static bool dd_bio_merge(struct blk_mq_hw_ctx *hctx, struct bio *bio)
{
struct request_queue *q = hctx->queue;
struct deadline_data *dd = q->elevator->elevator_data;
- int ret;
+ struct request *free = NULL;
+ bool ret;
spin_lock(&dd->lock);
- ret = blk_mq_sched_try_merge(q, bio);
+ ret = blk_mq_sched_try_merge(q, bio, &free);
spin_unlock(&dd->lock);
+ if (free)
+ blk_mq_free_request(free);
+
return ret;
}
--
Jens Axboe
^ permalink raw reply related
* Re: [PATCH] nbd: use an idr to keep track of nbd devices
From: Jens Axboe @ 2017-02-02 1:36 UTC (permalink / raw)
To: Josef Bacik, axboe, linux-block, kernel-team
In-Reply-To: <1485983500-3830-1-git-send-email-jbacik@fb.com>
On 02/01/2017 01:11 PM, Josef Bacik wrote:
> To prepare for dynamically adding new nbd devices to the system switch
> from using an array for the nbd devices and instead use an idr. This
> copies what loop does for keeping track of its devices.
Applied for 4.11, thanks.
--
Jens Axboe
^ permalink raw reply
* Re: [PATCH] nbd: use our own workqueue for recv threads
From: Jens Axboe @ 2017-02-02 1:35 UTC (permalink / raw)
To: Josef Bacik, axboe, linux-block, kernel-team
In-Reply-To: <1485983471-3717-1-git-send-email-jbacik@fb.com>
On 02/01/2017 01:11 PM, Josef Bacik wrote:
> Since we are in the memory reclaim path we need our recv work to be on a
> workqueue that has WQ_MEM_RECLAIM set so we can avoid deadlocks. Also
> set WQ_HIGHPRI since we are in the completion path for IO.
Applied for 4.11.
--
Jens Axboe
^ permalink raw reply
* Re: [PATCH 3/4] block: Dynamically allocate and refcount backing_dev_info
From: Jens Axboe @ 2017-02-01 22:45 UTC (permalink / raw)
To: Jan Kara, Christoph Hellwig
Cc: linux-block, Dan Williams, Thiago Jung Bauermann, Laurent Dufour
In-Reply-To: <20170201122257.GE11567@quack2.suse.cz>
On 02/01/2017 04:22 AM, Jan Kara wrote:
> On Wed 01-02-17 01:50:07, Christoph Hellwig wrote:
>> On Tue, Jan 31, 2017 at 01:54:28PM +0100, Jan Kara wrote:
>>> Instead of storing backing_dev_info inside struct request_queue,
>>> allocate it dynamically, reference count it, and free it when the last
>>> reference is dropped. Currently only request_queue holds the reference
>>> but in the following patch we add other users referencing
>>> backing_dev_info.
>>
>> Do we really need the separate slab cache? Otherwise this looks
>> fine to me.
>
> Yeah, probably it is not worth it. I'll remove it.
I agree on that, it should not be a hot path. Will you respin the series
after making this change? Would be great to get this queued up.
--
Jens Axboe
^ permalink raw reply
* Re: [PATCH v3] scsi, block: fix duplicate bdi name registration crashes
From: Jens Axboe @ 2017-02-01 22:43 UTC (permalink / raw)
To: Dan Williams
Cc: Martin K. Petersen, linux-block, Jan Kara, linux-scsi,
linux-kernel@vger.kernel.org, Christoph Hellwig, James Bottomley,
Bart Van Assche, Omar Sandoval, Omar Sandoval
In-Reply-To: <CAPcyv4jC5mVednFh8crsa0-GGFyPPEBKHUz5itCQ_4Tc1tX6Ew@mail.gmail.com>
On 02/01/2017 02:40 PM, Dan Williams wrote:
> On Wed, Feb 1, 2017 at 2:35 PM, Jens Axboe <axboe@kernel.dk> wrote:
>> On 02/01/2017 02:05 PM, Dan Williams wrote:
>>> Warnings of the following form occur because scsi reuses a devt number
>>> while the block layer still has it referenced as the name of the bdi
>>> [1]:
>>>
>>> WARNING: CPU: 1 PID: 93 at fs/sysfs/dir.c:31 sysfs_warn_dup+0x62/0x80
>>> sysfs: cannot create duplicate filename '/devices/virtual/bdi/8:192'
>>> [..]
>>> Call Trace:
>>> dump_stack+0x86/0xc3
>>> __warn+0xcb/0xf0
>>> warn_slowpath_fmt+0x5f/0x80
>>> ? kernfs_path_from_node+0x4f/0x60
>>> sysfs_warn_dup+0x62/0x80
>>> sysfs_create_dir_ns+0x77/0x90
>>> kobject_add_internal+0xb2/0x350
>>> kobject_add+0x75/0xd0
>>> device_add+0x15a/0x650
>>> device_create_groups_vargs+0xe0/0xf0
>>> device_create_vargs+0x1c/0x20
>>> bdi_register+0x90/0x240
>>> ? lockdep_init_map+0x57/0x200
>>> bdi_register_owner+0x36/0x60
>>> device_add_disk+0x1bb/0x4e0
>>> ? __pm_runtime_use_autosuspend+0x5c/0x70
>>> sd_probe_async+0x10d/0x1c0
>>> async_run_entry_fn+0x39/0x170
>>>
>>> This is a brute-force fix to pass the devt release information from
>>> sd_probe() to the locations where we register the bdi,
>>> device_add_disk(), and unregister the bdi, blk_cleanup_queue().
>>>
>>> Thanks to Omar for the quick reproducer script [2]. This patch survives
>>> where an unmodified kernel fails in a few seconds.
>>
>> What is the patch against? Doesn't seem to apply cleanly for me on
>> master, nor the 4.11 block tree.
>>
>
> I built it on top of Jan's bdi fixes series [1]. I can rebase to
> block/for-next, just let me know which patches you want to take first.
>
> [1]: http://marc.info/?l=linux-block&m=148586843819160&w=2
Ah, no that's fine, we'll just pull Jan's stuff in first. Makes sense.
--
Jens Axboe
^ permalink raw reply
* Re: [PATCH v3] scsi, block: fix duplicate bdi name registration crashes
From: Dan Williams @ 2017-02-01 22:40 UTC (permalink / raw)
To: Jens Axboe
Cc: Martin K. Petersen, linux-block, Jan Kara, linux-scsi,
linux-kernel@vger.kernel.org, Christoph Hellwig, James Bottomley,
Bart Van Assche, Omar Sandoval, Omar Sandoval
In-Reply-To: <e331cf91-37a2-d886-1136-4f33279e65ad@kernel.dk>
On Wed, Feb 1, 2017 at 2:35 PM, Jens Axboe <axboe@kernel.dk> wrote:
> On 02/01/2017 02:05 PM, Dan Williams wrote:
>> Warnings of the following form occur because scsi reuses a devt number
>> while the block layer still has it referenced as the name of the bdi
>> [1]:
>>
>> WARNING: CPU: 1 PID: 93 at fs/sysfs/dir.c:31 sysfs_warn_dup+0x62/0x80
>> sysfs: cannot create duplicate filename '/devices/virtual/bdi/8:192'
>> [..]
>> Call Trace:
>> dump_stack+0x86/0xc3
>> __warn+0xcb/0xf0
>> warn_slowpath_fmt+0x5f/0x80
>> ? kernfs_path_from_node+0x4f/0x60
>> sysfs_warn_dup+0x62/0x80
>> sysfs_create_dir_ns+0x77/0x90
>> kobject_add_internal+0xb2/0x350
>> kobject_add+0x75/0xd0
>> device_add+0x15a/0x650
>> device_create_groups_vargs+0xe0/0xf0
>> device_create_vargs+0x1c/0x20
>> bdi_register+0x90/0x240
>> ? lockdep_init_map+0x57/0x200
>> bdi_register_owner+0x36/0x60
>> device_add_disk+0x1bb/0x4e0
>> ? __pm_runtime_use_autosuspend+0x5c/0x70
>> sd_probe_async+0x10d/0x1c0
>> async_run_entry_fn+0x39/0x170
>>
>> This is a brute-force fix to pass the devt release information from
>> sd_probe() to the locations where we register the bdi,
>> device_add_disk(), and unregister the bdi, blk_cleanup_queue().
>>
>> Thanks to Omar for the quick reproducer script [2]. This patch survives
>> where an unmodified kernel fails in a few seconds.
>
> What is the patch against? Doesn't seem to apply cleanly for me on
> master, nor the 4.11 block tree.
>
I built it on top of Jan's bdi fixes series [1]. I can rebase to
block/for-next, just let me know which patches you want to take first.
[1]: http://marc.info/?l=linux-block&m=148586843819160&w=2
^ permalink raw reply
* Re: [PATCH v3] scsi, block: fix duplicate bdi name registration crashes
From: Jens Axboe @ 2017-02-01 22:35 UTC (permalink / raw)
To: Dan Williams, martin.petersen
Cc: linux-block, Jan Kara, linux-scsi, linux-kernel,
Christoph Hellwig, James Bottomley, Bart Van Assche,
Omar Sandoval, Omar Sandoval
In-Reply-To: <148598661794.11527.10024529085859239448.stgit@dwillia2-desk3.amr.corp.intel.com>
On 02/01/2017 02:05 PM, Dan Williams wrote:
> Warnings of the following form occur because scsi reuses a devt number
> while the block layer still has it referenced as the name of the bdi
> [1]:
>
> WARNING: CPU: 1 PID: 93 at fs/sysfs/dir.c:31 sysfs_warn_dup+0x62/0x80
> sysfs: cannot create duplicate filename '/devices/virtual/bdi/8:192'
> [..]
> Call Trace:
> dump_stack+0x86/0xc3
> __warn+0xcb/0xf0
> warn_slowpath_fmt+0x5f/0x80
> ? kernfs_path_from_node+0x4f/0x60
> sysfs_warn_dup+0x62/0x80
> sysfs_create_dir_ns+0x77/0x90
> kobject_add_internal+0xb2/0x350
> kobject_add+0x75/0xd0
> device_add+0x15a/0x650
> device_create_groups_vargs+0xe0/0xf0
> device_create_vargs+0x1c/0x20
> bdi_register+0x90/0x240
> ? lockdep_init_map+0x57/0x200
> bdi_register_owner+0x36/0x60
> device_add_disk+0x1bb/0x4e0
> ? __pm_runtime_use_autosuspend+0x5c/0x70
> sd_probe_async+0x10d/0x1c0
> async_run_entry_fn+0x39/0x170
>
> This is a brute-force fix to pass the devt release information from
> sd_probe() to the locations where we register the bdi,
> device_add_disk(), and unregister the bdi, blk_cleanup_queue().
>
> Thanks to Omar for the quick reproducer script [2]. This patch survives
> where an unmodified kernel fails in a few seconds.
What is the patch against? Doesn't seem to apply cleanly for me on
master, nor the 4.11 block tree.
--
Jens Axboe
^ permalink raw reply
* Re: [PATCH] block: queue lock must be acquired when iterating over rls
From: Jens Axboe @ 2017-02-01 22:31 UTC (permalink / raw)
To: Tahsin Erdogan, linux-block; +Cc: linux-kernel
In-Reply-To: <20170201063650.15060-1-tahsin@google.com>
On 01/31/2017 10:36 PM, Tahsin Erdogan wrote:
> blk_set_queue_dying() does not acquire queue lock before it calls
> blk_queue_for_each_rl(). This allows a racing blkg_destroy() to
> remove blkg->q_node from the linked list and have
> blk_queue_for_each_rl() loop infitely over the removed blkg->q_node
> list node.
Thanks, applied.
--
Jens Axboe
^ permalink raw reply
* Re: [PATCH v3] scsi, block: fix duplicate bdi name registration crashes
From: Bart Van Assche @ 2017-02-01 22:23 UTC (permalink / raw)
To: dan.j.williams@intel.com, martin.petersen@oracle.com
Cc: linux-kernel@vger.kernel.org, linux-block@vger.kernel.org,
hch@lst.de, axboe@kernel.dk, linux-scsi@vger.kernel.org,
osandov@osandov.com, James.Bottomley@hansenpartnership.com,
osandov@fb.com, jack@suse.cz
In-Reply-To: <148598661794.11527.10024529085859239448.stgit@dwillia2-desk3.amr.corp.intel.com>
On Wed, 2017-02-01 at 14:05 -0800, Dan Williams wrote:
> Warnings of the following form occur because scsi reuses a devt number
> while the block layer still has it referenced as the name of the bdi
Thanks!
Reviewed-by: Bart Van Assche <bart.vanassche@sandisk.com>=
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox