Re: move the bdi from the request_queue to the gendisk

[Jan Kara <jack@suse.cz>]

On Wed 11-08-21 13:51:47, Christoph Hellwig wrote:
> On Wed, Aug 11, 2021 at 01:25:14PM +0200, Jan Kara wrote:
> > 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

> 
> 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 <jack@suse.com>
SUSE Labs, CR