Hello Zhong, On Wed, Mar 06, 2019 at 09:07:00PM +0800, zhong jiang wrote: > The patch use call_rcu to delay free the task_struct, but It is possible to free the task_struct > ahead of get_mem_cgroup_from_mm. is it right? Yes it is possible to free before get_mem_cgroup_from_mm, but if it's freed before get_mem_cgroup_from_mm rcu_read_lock, rcu_dereference(mm->owner) will return NULL in such case and there will be no problem. The simple fix also clears the mm->owner of the failed-fork-mm before doing the call_rcu. The call_rcu delays the freeing after no other CPU runs in between rcu_read_lock/unlock anymore. That guarantees that those critical section will see mm->owner == NULL if the freeing of the task strut already happened. The solution Mike suggested for this and that we were wondering as ideal in the past for the signal issue too, is to move the uffd delivery at a point where fork is guaranteed to succeed. We should probably try that too to see how it looks like and if it can be done in a not intrusive way, but the simple fix that uses RCU should work too. Rolling back in case of errors inside fork itself isn't easily doable: the moment we push the uffd ctx to the other side of the uffd pipe there's no coming back as that information can reach the userland of the uffd monitor/reader thread immediately after. The rolling back is really the other thread failing at mmget_not_zero eventually. It's the userland that has to rollback in such case when it gets a -ESRCH retval. Note that this fork feature is only ever needed in the non-cooperative case, these things never need to happen when userfaultfd is used by an app (or a lib) that is aware that it is using userfaultfd. Thanks, Andrea
