At Thu, 03 May 2012 15:06:46 +0800, Liu Yuan wrote: > > On 05/03/2012 02:16 PM, tao.peng at emc.com wrote: > > > Hi list, > > > > I was reading sheepdog code and noticed that in update_vnode_info(), > > if one thread calls get_vnode_info() between > > put_vnode_info(current_vnode_info) and current_vnode_info = > > vnode_info, it then will get an invalid pointer. So it looks like a > > race window there? > > > > Best, Tao > > > Oops, it seems that we have brought in a big monster, we don't have any > lock mechanism to protect current_vnode_info, which is referenced and > calculated by worker threads and main thread scattered everywhere in the > code and it is really a dilemma to introduce such a ubiquitous lock > between worker and main threads. > > Should we revert the patch introduce current_vnode_info? The old code also has the completely same problem; worker threads call get_ordered_sd_vnode_list()/free_ordered_sd_vnode_list(). The correct approach is to modify the code so that all get_vnode_info()/put_vnode_info() are called only in the main thread. I'll do the work if no one is trying to fix it now. Thanks, Kazutaka |