On 09/09, Mircea CIRJALIU - MELIU wrote:
quoted
From: Oleg Nesterov <oleg@redhat.com>
But why is it safe to drop ->mmap_sem without checking
FAULT_FLAG_ALLOW_RETRY/RETRY_NOWAIT ?
Dropping mmap_sem will have the same effects regardless of FAULT_FLAG_ALLOW_RETRY/RETRY_NOWAIT.
Another thread can unmap the VMA from underneath us, or remap another one in its place.
In the end, the VMA has to be revalidated when re-acquiring the mmap_sem.
Or am I wrong?!
To simplify, lets forget about RETRY_NOWAIT/TRIED.
Again, I can be easily wrong. But iiuc, you simply can't drop mmap_sem
if FAULT_FLAG_ALLOW_RETRY is not set, the caller doesn't expect mmap_sem
can be unlocked.
OTOH, if FAULT_FLAG_ALLOW_RETRY is set and you drop mmap_sem, you can
only return VM_FAULT_RETRY to let the caller know it was dropped.
quoted
quoted
+ * If FAULT_FLAG_ALLOW_RETRY is set, the mmap_sem must be
released
quoted
+ * before returning VM_FAULT_RETRY only if
FAULT_FLAG_RETRY_NOWAIT is
quoted
+ * not set.
Well, iiuc FAULT_FLAG_ALLOW_RETRY means that ->fault() _can_ drop
mmap_sem and return VM_FAULT_RETRY (unless NOWAIT).
That comment is just copied from elsewhere in the code.
My interpretation was that the fault handler _should_ return with mmap_sem
held or not depending on FAULT_FLAG_RETRY_NOWAIT.
Yes, this depends on FAULT_FLAG_RETRY_NOWAIT.
But your comment above looks as if he mmap_sem must be _always_ released
if FAULT_FLAG_ALLOW_RETRY && !FAULT_FLAG_RETRY_NOWAIT. This is not true.
Nevermind. If you ever resend this patch, please CC mm/ experts. I tried
to look at this code again and I feel it has much more problems, but as
I said this is not my area.
And I think you should split this patch, mirror_vm_fault() should come in
a separate patch to simplify the review.
Oleg.