On 9/11/18 2:16 PM, Matthew Wilcox wrote:
On Wed, Sep 12, 2018 at 04:58:11AM +0800, Yang Shi wrote:
mm/mmap.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
I really think you're going about this the wrong way by duplicating
vm_munmap().
If we don't duplicate vm_munmap() or do_munmap(), we need pass an extra
parameter to them to tell when it is fine to downgrade write lock or if
the lock has been acquired outside it (i.e. in mmap()/mremap()), right?
But, vm_munmap() or do_munmap() is called not only by mmap-related, but
also some other places, like arch-specific places, which don't need
downgrade write lock or are not safe to do so.
Actually, I did this way in the v1 patches, but it got pushed back by
tglx who suggested duplicate the code so that the change could be done
in mm only without touching other files, i.e. arch-specific stuff. I
didn't have strong argument to convince him.
And, Michal prefers have VM_HUGETLB and VM_PFNMAP handled separately for
safe and bisectable sake, which needs call the regular do_munmap().
In addition to this, I just found mpx code may call do_munmap()
recursively when I was looking into the mpx code.
We might be able to handle these by the extra parameter, but it sounds
it make the code hard to understand and error prone.
Thanks,
Yang
