Re: [PATCH V2] block: rename 'q->debugfs_dir' and 'q->blk_trace->dir' in blk_unregister_queue()

[yukuai (C)]

On 2020/2/29 7:15, Ming Lei wrote:
> If blk_trace->dir isn't same with .debugfs_dir, you will just rename
> blk_trace->dir, this way can't avoid the failure in step3, can it?
Thank you for your response!
It's true that the problem still exist if they are not the same（I
thougt they can't both exist）. Perhaps I can do the renaming for both.
> I understand that we just need to rename .debugfs_dir, meantime making
> sure blktrace code removes correct debugfs dir, is that enough for fixing
> this issue?
>
> > +	if (old == NULL)
> > +		return;
> > +	while (new == NULL) {
> > +		sprintf(name, "ready_to_remove_%s_%d",
> > +				kobject_name(q->kobj.parent), i++);
> > +		new = debugfs_rename(blk_debugfs_root, *old,
> > +				     blk_debugfs_root, name);
> > +	}
> The above code can be run concurrently with blktrace shutdown, so race might
> exit between the two code paths, then bt->dir may has been renamed or being
> renamed in debugfs_remove(bt->dir), can this function work as expected or
> correct?
To avoid the race, I think we can take the lock 'blk_trace_mutex' while
renaming 'bt->dir'.
> And there is dead loop risk, so suggest to not rename this way.
The deap loop will exist if 'debugfs_rename' keep failing for some
reason. I thougt about setting a limit for max loop count, however, It's
probably not a good solution.

Thanks!
Yu Kuai
>