Messages in this thread | | | From | "Rafael J. Wysocki" <> | Subject | Re: [PATCH 1/7] Freezer: Read PF_BORROWED_MM in a nonracy way | Date | Sat, 12 May 2007 02:01:41 +0200 |
| |
On Saturday, 12 May 2007 01:29, Linus Torvalds wrote: > > On Sat, 12 May 2007, Rafael J. Wysocki wrote: > > > > We use this function (ie. kernel/power/process.c:is_user_space()) to > > distinguish kernel threads from user space processes. Therefore we make it > > always return true for user space processes and always return false for kernel > > threads. In the latter case we need to use the task_lock() to ensure that the > > result is as desired (ie. false), because otherwise it might be racing with > > either fs/aio.c:use_mm() or fs/aio.c:unuse_mm(). > > But there is no race protection in the *caller*, so if it can ever return > one or the other, what protects it from changing once the caller returns? > > And if the value can change (because some thread uses "use_mm()"), then > the caller cannot rely on the value that got returned.
The value cannot change because of that. There only is a small window inside unuse_mm() (or use_mm()) in which the value may be wrong. Namely:
static void unuse_mm(struct mm_struct *mm) { struct task_struct *tsk = current;
task_lock(tsk); tsk->flags &= ~PF_BORROWED_MM; --- --- If is_user_space() without the task_lock() is called right here, it will --- return 'true', although it should return 'false'. --- tsk->mm = NULL; /* active_mm is still 'mm' */ enter_lazy_tlb(mm, tsk); task_unlock(tsk); }
IOW, quoting Andrew, "is_user_space() requires that the state of p->mm and p->flags be consistent".
> So you migt as well not return any value at all, since the returned value > is apparently meaningless once the lock has been released.
No, it is not meaningless.
Rafael - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |