lkml.org 
[lkml]   [2006]   [Jun]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH 0/3] mm: tracking dirty pages -v5
    From
    Date
    On Tue, 2006-06-06 at 21:06 +0100, Hugh Dickins wrote:

    > You tend to use get_page/put_page amidst code using page_cache_get/
    > page_cache_release. Carry on: it sometimes looks odd, but I can't see
    > any way to impose consistency, short of abolishing one or the other
    > throughout the tree. So don't worry about it.

    Noticed that myself too, came to the same conclusion, thanks for the
    confirmation thought.

    > You've got a minor cleanup to install_page, left over from an earlier
    > iteration: the cleanup looked okay, but of no relevance to your patchset
    > now, is it? Just cut mm/fremap.c out of the patchset I think.

    OK, unless we go back to the previous way I'll send this tiny cleanup as
    a separate patch to Andrew.

    > You've taken the simplification of sys_msync a little too far, I believe:
    > you ought to try to reproduce the same errors as before, so MS_ASYNC
    > should be winding through the separate vmas like MS_SYNC, just to
    > report -ENOMEM if it crosses an unmapped area; and MS_INVALIDATE
    > used to say -EBUSY if VM_LOCKED, but that has disappeared. (Perhaps
    > I've missed other such details, please recheck.)

    Ah, yes, I've noticed this too, I just wasn't sure on if this would be
    wanted or not. Is fixed, thanks!

    > Your comment should
    > say "Nor does it mark" instead of "Nor does it just marks".

    Hehe, thanks, please do point out my mistakes with the English language.
    I have good enough control to convey most of what I intent but I'm not a
    native.

    > Your is_shared_writable(vma) in mprotect_fixup is along the right
    > lines, but wrong: because at that point vma->vm_flags is the old one,
    > and may be omitting VM_WRITE when that is about to be added. Perhaps
    > you should move the "vma->vm_flags = newflags" above it, or perhaps
    > you should change is_shared_writable to work on flags rather than vma
    > (as Linus' is_cow_mapping does).

    How odd, I have the distinct recollection of actually moving that
    assignment upwards a few lines, must have been late or something. Thanks
    for pointing this out, would've never found it again; with me thinking
    it was done with.

    <snip: big tiresome story/>

    Well, you got me. I don't know either, the only thing I can come up with
    is making the breakage compile-time (for 3rd-party modules) instead of
    subtle run-time, but its still not pretty.

    /me looks around at assorted VM gurus; any ideas out there?

    If by tomorrow morning CET nobody has spoken up, I'll just go ahead and
    accept Hugh's apology :-), that is revert back to my original way of
    doing it. (I can always go back to this scheme if some smart but slower
    working brain manages a solution)


    Peter

    -
    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-06-07 20:11    [W:0.058 / U:30.864 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site