2008/1/14, Miklos Szeredi <[EMAIL PROTECTED]>: > > 2008/1/14, Miklos Szeredi <[EMAIL PROTECTED]>: > > > > http://bugzilla.kernel.org/show_bug.cgi?id=2645 > > > > > > > > Changes for updating the ctime and mtime fields for memory-mapped files: > > > > > > > > 1) new flag triggering update of the inode data; > > > > 2) new function to update ctime and mtime for block device files; > > > > 3) new helper function to update ctime and mtime when needed; > > > > 4) updating time stamps for mapped files in sys_msync() and do_fsync(); > > > > 5) implementing the feature of auto-updating ctime and mtime. > > > > > > How exactly is this done? > > > > > > Is this catering for this case: > > > > > > 1 page is dirtied through mapping > > > 2 app calls msync(MS_ASYNC) > > > 3 page is written again through mapping > > > 4 app calls msync(MS_ASYNC) > > > 5 ... > > > 6 page is written back > > > > > > What happens at 4? Do we care about this one at all? > > > > The POSIX standard requires updating the file times every time when msync() > > is called with MS_ASYNC. I.e. the time stamps should be updated even > > when no physical synchronization is being done immediately. > > Yes. However, on linux MS_ASYNC is basically a no-op, and without > doing _something_ with the dirty pages (which afaics your patch > doesn't do), it's impossible to observe later writes to the same page. > > I don't advocate full POSIX conformance anymore, because it's probably > too expensive to do (I've tried). Rather than that, we should > probably find some sane compromise, that just fixes the real life > issue. Here's a pointer to the thread about this: > > http://lkml.org/lkml/2007/3/27/55 > > Your patch may be a good soultion, but you should describe in detail > what it does when pages are dirtied, and when msync/fsync are called, > and what happens with multiple msync calls that I've asked about. > > I suspect your patch is ignoring writes after the first msync, but > then why care about msync at all? What's so special about the _first_ > msync? Is it just that most test programs only check this, and not > what happens if msync is called more than once? That would be a bug > in the test cases. > > > > > +/* > > > > + * Update the ctime and mtime stamps for memory-mapped block device > > > > files. > > > > + */ > > > > +static void bd_inode_update_time(struct inode *inode) > > > > +{ > > > > + struct block_device *bdev = inode->i_bdev; > > > > + struct list_head *p; > > > > + > > > > + if (bdev == NULL) > > > > + return; > > > > + > > > > + mutex_lock(&bdev->bd_mutex); > > > > + list_for_each(p, &bdev->bd_inodes) { > > > > + inode = list_entry(p, struct inode, i_devices); > > > > + inode_update_time(inode); > > > > + } > > > > + mutex_unlock(&bdev->bd_mutex); > > > > +} > > > > > > Umm, why not just call with file->f_dentry->d_inode, so that you don't > > > need to do this ugly search for the physical inode? The file pointer > > > is available in both msync and fsync. > > > > I'm not sure if I undestood your question. I see two possible > > interpretations for this question, and I'm answering both. > > > > The intention was to make the data changes in the block device data > > visible to all device files associated with the block device. Hence > > the search, because the time stamps for all such device files should > > be updated as well. > > Ahh, but it will only update "active" devices, which are currently > open, no? Is that what we want? > > > Not only the sys_msync() and do_fsync() routines call the helper > > function mapping_update_time(). > > Ah yes, __sync_single_inode() calls it as well. Why?
The __sync_single_inode() function calls mapping_update_time() to enable the "auto-updating" feature discussed earlier. > > Miklos > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
