Re: move the bdi from the request_queue to the gendisk
From: Jan Kara <jack@suse.cz>
Date: 2021-08-11 12:47:16
Also in:
cgroups, linux-block, linux-fsdevel
On Wed 11-08-21 13:51:47, Christoph Hellwig wrote:
On Wed, Aug 11, 2021 at 01:25:14PM +0200, Jan Kara wrote:quoted
Well, non-default bdi_writeback structures do hold bdi reference - see wb_exit() which drops the reference. I think the problem rather was that a block device's inode->i_wb was pointing to the default bdi_writeback structure and that got freed after bdi_put() before block device inode was shutdown through bdput()... So what I think we need is that if the inode references the default writeback structure, it actually holds a reference to the bdi.Qian, can you test the patch below instead of the one I sent yesterday?
Sadly the patch below will not work because the bdi refcount will never drop to 0. wb_exit() for the default writeback structure is called only from release_bdi() (i.e., after the bdi refcount is 0). That is why I wrote above that references to default wb from inodes would hold the ref, not the default wb structure itself. So we would need to explicitely hack this into __inode_attach_wb() and inode_detach_wb(). Somewhat cleaner approach might be to modify wb_get(), wb_tryget(), wb_put() to get reference to bdi instead of doing nothing for the default wb. And drop a lot of special-casing of the default wb from various functions. But I guess the special cases are there to avoid the performance overhead for the common case because getting wb ref is common. Also that's why wbs use percpu refcount and we would need something similar for bdis. I guess this needs more thinking and your quick workaround is OK for now. Honza
quoted hunk ↗ jump to hunk
diff --git a/mm/backing-dev.c b/mm/backing-dev.c index cd06dca232c3..edfb7ce2cc93 100644 --- a/mm/backing-dev.c +++ b/mm/backing-dev.c@@ -283,8 +283,7 @@ static int wb_init(struct bdi_writeback *wb, struct backing_dev_info *bdi, memset(wb, 0, sizeof(*wb)); - if (wb != &bdi->wb) - bdi_get(bdi); + bdi_get(bdi); wb->bdi = bdi; wb->last_old_flush = jiffies; INIT_LIST_HEAD(&wb->b_dirty);@@ -362,8 +361,7 @@ static void wb_exit(struct bdi_writeback *wb) percpu_counter_destroy(&wb->stat[i]); fprop_local_destroy_percpu(&wb->completions); - if (wb != &wb->bdi->wb) - bdi_put(wb->bdi); + bdi_put(wb->bdi); } #ifdef CONFIG_CGROUP_WRITEBACK
-- Jan Kara [off-list ref] SUSE Labs, CR