lkml.org 
[lkml]   [2006]   [Oct]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRE: Hugepage regression
    On Tue, 10 Oct 2006, Chen, Kenneth W wrote:
    > Hugh Dickins wrote on Tuesday, October 10, 2006 12:18 PM
    >
    > > But again, I protest
    > > the "if (vma->vm_file)" in your unmap_hugepage_range - how would a
    > > hugepage area ever have NULL vma->vm_file?
    >
    > It's coming from do_mmap_pgoff(), file->f_op->mmap can fail with error
    > code (e.g. not enough hugetlb page) and in the error recovery path, it
    > nulls out vma->vm_file first before calls down to unmap_region().

    I stand corrected: thanks.

    > I asked that question before:

    So you did, on Oct 2nd: sorry, that got lost amidst the overload in
    my mailbox, I've just forwarded it to myself again, to check later
    on what else I may have missed. (I am aware that I've still not
    scrutinized the patches you sent out a day or two later, now in -mm.)

    > can we reverse that order (call unmap_region
    > and then nulls out vma->vmfile and fput)?

    I'm pretty sure we cannot: the ordering is quite intentional, that if
    a driver ->mmap failed, then it'd be wrong to call down to driver in
    the unmap_region (if a driver is nicely behaved, that unmap_region
    shouldn't be unnecessary; but some do rely on us clearing ptes there).

    Okay, last refuge of all who've made a fool of themselves:
    may I ask you to add a comment in your unmap_hugepage_range,
    pointing to how the do_mmap_pgoff error case NULLifies vm_file?

    (Or else change hugetlbfs_file_mmap to set VM_HUGETLB only once it's
    succeeded: but that smacks of me refusing to accept I was wrong.)

    Hugh

    >
    > unmap_and_free_vma:
    > if (correct_wcount)
    > atomic_inc(&inode->i_writecount);
    > vma->vm_file = NULL;
    > fput(file);
    >
    > /* Undo any partial mapping done by a device driver. */
    > unmap_region(mm, vma, prev, vma->vm_start, vma->vm_end);
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2006-10-10 22:13    [W:0.024 / U:0.228 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site