On Wed, Nov 08, 2023 at 11:52:52PM +0800, Xu Yilun wrote: > > >> > > >> In fpga_region_get() / fpga_region_put(): call get_device() before > > >> acquiring the mutex and put_device() after having released the mutex > > >> to avoid races. Why do you need another reference count with a lock? You already have that with the calls to get/put_device(). > > > Could you help elaborate more about the race? > > > > > > > I accidentally misused the word race. My concern was that memory might > > be released after the last put_device(), causing mutex_unlock() to be > > called on a mutex that does not exist anymore. It should not happen > > for the moment since the region does not use devres, but I think it > > still makes the code more brittle. > > It makes sense. > > But I dislike the mutex itself. The purpose is to exclusively grab the > device, but a mutex is too much heavy for this. Why "heavy"? Is this a fast-path? Have you measured it? > The lock/unlock of mutex > scattered in different functions is also an uncommon style. Maybe some > atomic count should be enough. Don't make a fake lock with an atomic variable, use real locks if you need it. Or don't even care about module unloading at all! Why does it matter? It can never happen without explicit intervention and it's something that a lot of the time, just will cause problems. Don't do a lot of extra work for something that doesn't matter. thanks, greg k-h
