On Fri, Apr 25, 2014 at 5:01 AM, Hugh Dickins <hughd@xxxxxxxxxx> wrote:
>
> Two, Ben said earlier that he's more worried about users of
> unmap_mapping_range() than concurrent munmap(); and you said
> earlier that you would almost prefer to have some special lock
> to serialize with page_mkclean().
>
> Er, i_mmap_mutex.
>
> That's what unmap_mapping_range(), and page_mkclean()'s rmap_walk,
> take to iterate over the file vmas. So perhaps there's no race at all
> in the unmap_mapping_range() case. And easy (I imagine) to fix the
> race in Dave's racewrite.c use of MADV_DONTNEED: untested patch below.
Hmm. unmap_mapping_range() is just abotu the only thing that _does_
take i_mmap_mutex. unmap_single_vma() does it for
is_vm_hugetlb_page(), which is a bit confusing. And normally we only
take it for the actual final vma link/unlink, not for the actual
traversal. So we'd have to change that all quite radically (or we'd
have to drop and re-take it).
So I'm not quite convinced. Your simple patch looks simple and should
certainly fix DaveH's test-case, but then leaves munmap/exit as a
separate thing to fix. And I don't see how to do that cleanly (it
really looks like "we'll just have to take that semaphore again
separately).
i_mmap_mutex is likely not contended, but we *do* take it for private
mappings too (and for read-only ones), so this lock is actually much
more common than the dirty shared mapping.
So I think I prefer my patch, even if that may be partly due to just
it being mine ;)
Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-arch" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
