[lkml]   [2006]   [Jun]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [PATCH] memory mapped files not updating timestamps
    On Wed, 31 May 2006 13:53:16 -0400
    Peter Staubach <> wrote:


    A few issues here.

    > I embarked on this work due to a bug reported by one of Red Hat's large
    > customers. They are finding that files, which should have been backed
    > up, were not getting backed up. This is due to the mtime on the files
    > not changing and their backup software looking for mtime changes. This
    > is corruption and I need to get it fixed, sooner as opposed to later.
    > While I would like to get this fixed on top of Peter Zijlstra's changes,
    > the process for those is looking long and complicated. I am wondering
    > if we could consider these changes and then add the requirement of
    > maintaining these semantics to those that Peter's work is attempting to
    > address.

    We seem to be having a problem getting a coherent changelog. A changelog
    should describe why the patch exists, what it does and how it does it.
    Please develop and maintain a changelog for each patch and reissue the
    changelog with each reissuing of a patch, thanks.

    This sequence:

    + if (test_and_clear_bit(AS_MCTIME, &mapping->flags))
    + inode_update_time(inode);

    appears all over the place and should be implemented in a helper function.

    The patch should work correctly for mmaps of block devices and I don't
    think it does. Sometimes it updates the timestamp on the kernel-internal
    blockdev inode at mapping->host and sometimes it updates the inode of the
    device node (/dev/hda1) at file->f_dentry->d_inode. It should be updating
    the /dev/hda1 inode.

    The change to unlink_file_vma() is awkward, IMO. It means that this helper
    function "knows" things about what its caller is using it for. I'd suggest
    that this code should be moved up to a higher level where we have a more
    sure semantic context, rather than being implemented in some low-level
    helper function where it happens to be convenient.

    Also, inode_update_time() can sleep.
    mark_inode_dirty_sync()->__mark_inode_dirty()->ext3_dirty_inode(). This is
    despite Documentation/filesystems/Locking saying "must not sleep". But
    unlink_file_vma() (at least) is called from atomic context:


    Which _should_ have triggered warnings if full kernel debugging was enabled
    and sufficient testing was performed. Perhaps a might_sleep() is needed in

    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to
    More majordomo info at
    Please read the FAQ at

     \ /
      Last update: 2006-06-19 06:36    [W:0.022 / U:95.272 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site