Hello, On Tue, Nov 12, 2019 at 09:38:57AM +0800, Jiufei Xue wrote: > > Hmm... I'm not sure this description makes sense. > > > Should I change the description to something like this? > "we should treat the leaf nodes as root while the parent are already activated". Hmm... this is addressing an obvious bug. The intention of the code was checking whether all the ancestors and self have already been activated but it just failed to do so, so I think the patch description should reflect that. > > But there's an obvious bug there as it's checking the same active_list > > over and over again. Shouldn't it be sth like the following instead? > > > > if (!list_empty(&iocg->active_list)) > > goto succeed_unlock; > > iocg has already checked before, do you mean we should check it again > after ioc->lock? Yes, that part of the code is correct. It needs to check it again as someone could have changed it since the previous lockless opportunistic checking. Thanks. -- tejun