Hi,
On 2020/2/13 11:48, Tejun Heo wrote:
Hello,
On Thu, Feb 13, 2020 at 10:46:34AM +0800, Yufen Yu wrote:
For each time of register, bdi_register() will try to create a new 'dev'.
bdi_register
bdi_register_va
if (bdi->dev) // if bdi->dev is not NULL, return directly
return 0;
dev = device_create_vargs()...
So, I think freeing bdi->dev until bdi itself does may be a problem
for drivers that supported re-registration bdi, such as:
Ugh, thanks for noticing that. I guess the right thing to do is then
going full RCU. What do you think about expanding your previous patch
so that ->dev has __rcu annotation, users use the RCU accessors and
the device is destroyed asynchronously through call_rcu()?
If we destroy the device asynchronously by call_rcu(), we may need to
add a new member 'rcu_head' into struct backing_dev_info. Right?
The code may be like:
bdi_unregister()
{
...
if (bdi->dev) {
...
device_get(bdi->dev);
device_unregister(bdi->dev);
bdi->dev = NULL; //XXX
bdi_get(bdi); //avoiding bdi to be freed before calling bdi_release_device
call_rcu(&bdi->rcu_head, bdi_release_device);
}
...
}
bdi_release_device()
{
...
put_device(bdi->dev);//XXX
bdi_put(bdi);
}
But, the problem is how do we get 'bdi->dev' in bdi_release_device().
If we do not set bdi->dev as 'NULL', re-registration bdi may cannot work well.
If I get it wrong, please point it out.
Thanks
Yufen
