lkml.org 
[lkml]   [2008]   [Jan]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH -v8 3/4] Enable the MS_ASYNC functionality in sys_msync()
    2008/1/23, Peter Zijlstra <a.p.zijlstra@chello.nl>:
    >
    > On Wed, 2008-01-23 at 02:21 +0300, Anton Salikhmetov wrote:
    >
    > > +static void vma_wrprotect_pmd_range(struct vm_area_struct *vma, pmd_t *pmd,
    > > + unsigned long start, unsigned long end)
    > > +{
    > > + while (start < end) {
    > > + spinlock_t *ptl;
    > > + pte_t *pte = pte_offset_map_lock(vma->vm_mm, pmd, start, &ptl);
    > > +
    > > + if (pte_dirty(*pte) && pte_write(*pte)) {
    > > + pte_t entry = ptep_clear_flush(vma, start, pte);
    > > +
    > > + entry = pte_wrprotect(entry);
    > > + set_pte_at(vma->vm_mm, start, pte, entry);
    > > + }
    > > +
    > > + pte_unmap_unlock(pte, ptl);
    > > + start += PAGE_SIZE;
    > > + }
    > > +}
    >
    > You've had two examples on how to write this loop, one from git commit
    > 204ec841fbea3e5138168edbc3a76d46747cc987, and one from my draft, but
    > this one looks like neither and is much less efficient. Take the lock
    > only once per pmd, not once per pte please.
    >
    > > +static void vma_wrprotect_pud_range(struct vm_area_struct *vma, pud_t *pud,
    > > + unsigned long start, unsigned long end)
    > > +{
    > > + pmd_t *pmd = pmd_offset(pud, start);
    > > +
    > > + while (start < end) {
    > > + unsigned long next = pmd_addr_end(start, end);
    > > +
    > > + if (!pmd_none_or_clear_bad(pmd))
    > > + vma_wrprotect_pmd_range(vma, pmd, start, next);
    > > +
    > > + ++pmd;
    > > + start = next;
    > > + }
    > > +}
    > > +
    > > +static void vma_wrprotect_pgd_range(struct vm_area_struct *vma, pgd_t *pgd,
    > > + unsigned long start, unsigned long end)
    > > +{
    > > + pud_t *pud = pud_offset(pgd, start);
    > > +
    > > + while (start < end) {
    > > + unsigned long next = pud_addr_end(start, end);
    > > +
    > > + if (!pud_none_or_clear_bad(pud))
    > > + vma_wrprotect_pud_range(vma, pud, start, next);
    > > +
    > > + ++pud;
    > > + start = next;
    > > + }
    > > +}
    > > +
    > > +static void vma_wrprotect(struct vm_area_struct *vma)
    > > +{
    > > + unsigned long addr = vma->vm_start;
    > > + pgd_t *pgd = pgd_offset(vma->vm_mm, addr);
    > > +
    > > + while (addr < vma->vm_end) {
    > > + unsigned long next = pgd_addr_end(addr, vma->vm_end);
    > > +
    > > + if (!pgd_none_or_clear_bad(pgd))
    > > + vma_wrprotect_pgd_range(vma, pgd, addr, next);
    > > +
    > > + ++pgd;
    > > + addr = next;
    > > + }
    > > +}
    >
    > I think you want to pass start, end here too, you might not need to
    > sweep the whole vma.

    Thanks for you feedback, Peter!

    I will redesign the vma_wrprotect_pmd_range() routine the way it
    acquires the spinlock outside of the loop. I will also rewrite the
    vma_wrprotect() function to process only the specified range.

    >
    >
    >


    \
     
     \ /
      Last update: 2008-01-23 13:57    [W:4.082 / U:0.192 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site