On Sun, Apr 07, 2024 at 09:40:52PM +0800, Yu Kuai wrote:
> Hi,
>
> 在 2024/04/07 20:59, Ming Lei 写道:
> > Multiple gendisk instances can allocated/added for single request queue
> > in case of disk rebind. blkg may still stay in q->blkg_list when calling
> > blkcg_init_disk() for rebind, then q->blkg_list becomes corrupted.
> >
> > Fix the list corruption issue by:
> >
> > - add blkg_init_queue() to initialize q->blkg_list & q->blkcg_mutex only
> > - move calling blkg_init_queue() into blk_alloc_queue()
> >
> > The list corruption should be started since commit f1c006f1c685 ("blk-cgroup:
> > synchronize pd_free_fn() from blkg_free_workfn() and blkcg_deactivate_policy()")
> > which delays removing blkg from q->blkg_list into blkg_free_workfn().
>
> I'm not familiar with how bind/unbind works yet, however, the patch
> itself looks reasonable to me, the initialization of fields related to
> queue should not be delayed to disk allocation.
>
> Reviewed-by: Yu Kuai <yukuai3@xxxxxxxxxx>
Thanks!
>
> BTW, it looks like the whole blkcg_init_disk() can go away:
> - init of ioprio and blk-throttle can be delayed to the first user
> configuration;
> - root_blkg allocation doesn't rely on disk at all;
>
> Or is there any plan to move the blkcg related field or code path to
> gendisk instead of queue? I might missing some previous discussions.
Christoph worked towards this direction, but not succeed:
a06377c5d01e Revert "blk-cgroup: pin the gendisk in struct blkcg_gq"
9a9c261e6b55 Revert "blk-cgroup: pass a gendisk to blkg_lookup"
1231039db31c Revert "blk-cgroup: move the cgroup information to struct gendisk"
dcb522014351 Revert "blk-cgroup: simplify blkg freeing from initialization failure paths"
Thanks,
Ming
