On Mon, Jul 15, 2013 at 09:51:21PM -0400, Rik van Riel wrote: > On 07/15/2013 03:24 AM, David Gibson wrote: > >On Sun, Jul 14, 2013 at 08:16:44PM -0700, Davidlohr Bueso wrote: > > >>>Reading the existing comment, this change looks very suspicious to me. > >>>A per-vma mutex is just not going to provide the necessary exclusion, is > >>>it? (But I recall next to nothing about these regions and > >>>reservations.) > > > >A per-VMA lock is definitely wrong. I think it handles one form of > >the race, between threads sharing a VM on a MAP_PRIVATE mapping. > >However another form of the race can and does occur between different > >MAP_SHARED VMAs in the same or different processes. I think there may > >be edge cases involving mremap() and MAP_PRIVATE that will also be > >missed by a per-VMA lock. > > > >Note that the libhugetlbfs testsuite contains tests for both PRIVATE > >and SHARED variants of the race. > > Can we get away with simply using a mutex in the file? > Say vma->vm_file->mapping->i_mmap_mutex? So I don't know the VM well enough to know if this could conflict with other usages of i_mmap_mutex. But unfortunately, whether or not its otherwise correct that approach won't address the scalability issue at hand here. In the case where the race matters, we're always dealing with the same file. Otherwise, we'd end up with a genuine, rather than spurious, out-of-memory error, regardless of how the race turned out. So in the case with the performance bottleneck we're considering, the i_mmap_mutex approach degenerates to serialization on a single mutex, just as before. In order to improve scalability, we need to consider which page within the file we're instantiating which is what the hash function in my patch does. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Attachment:
pgpWn7s88zJmN.pgp
Description: PGP signature