Messages in this thread Patch in this message | | | Date | Wed, 31 May 2006 16:38:09 -0400 | From | Peter Staubach <> | Subject | Re: [PATCH] memory mapped files not updating timestamps |
| |
Eric Dumazet wrote:
> Peter Staubach a écrit : > >> --- linux-2.6.16.i686/mm/msync.c.org >> +++ linux-2.6.16.i686/mm/msync.c >> @@ -206,12 +206,16 @@ asmlinkage long sys_msync(unsigned long >> file = vma->vm_file; >> start = vma->vm_end; >> if ((flags & MS_ASYNC) && file && nr_pages_dirtied) { >> + struct address_space *mapping = file->f_mapping; >> + >> get_file(file); >> up_read(¤t->mm->mmap_sem); >> - balance_dirty_pages_ratelimited_nr(file->f_mapping, >> + balance_dirty_pages_ratelimited_nr(mapping, >> nr_pages_dirtied); >> fput(file); > > > <here>, another thread can perform an munmap(), and the file can be > totally dismantled. > >> down_read(¤t->mm->mmap_sem); > > > So referencing 'mapping' is *buggy* here. > I believe that you have to move 'fput(file);' *after* the folloging > two lines. > >> + if (test_and_clear_bit(AS_MCTIME, &mapping->flags)) >> + inode_update_time(mapping->host); >> vma = find_vma(current->mm, start); >> } else if ((flags & MS_SYNC) && file && >> (vma->vm_flags & VM_SHARED)) { > > > > Eric
Yes, sorry, you mentioned that before and I meant to address that. I almost reordered those operations originally because I think that it is cleaner when acquires and releases are done in the opposite order. However, I was also trying to change as little as possible too.
Anyway, attached is an updated patch.
Thanx...
ps
Signed-off-by: Peter Staubach <staubach@redhat.com> --- linux-2.6.16.x86_64/fs/inode.c.org +++ linux-2.6.16.x86_64/fs/inode.c @@ -1211,8 +1211,8 @@ void touch_atime(struct vfsmount *mnt, s EXPORT_SYMBOL(touch_atime); /** - * file_update_time - update mtime and ctime time - * @file: file accessed + * inode_update_time - update mtime and ctime time + * @inode: file accessed * * Update the mtime and ctime members of an inode and mark the inode * for writeback. Note that this function is meant exclusively for @@ -1222,9 +1222,8 @@ EXPORT_SYMBOL(touch_atime); * timestamps are handled by the server. */ -void file_update_time(struct file *file) +void inode_update_time(struct inode *inode) { - struct inode *inode = file->f_dentry->d_inode; struct timespec now; int sync_it = 0; @@ -1246,7 +1245,7 @@ void file_update_time(struct file *file) mark_inode_dirty_sync(inode); } -EXPORT_SYMBOL(file_update_time); +EXPORT_SYMBOL(inode_update_time); int inode_needs_sync(struct inode *inode) { --- linux-2.6.16.x86_64/fs/fs-writeback.c.org +++ linux-2.6.16.x86_64/fs/fs-writeback.c @@ -168,6 +168,9 @@ __sync_single_inode(struct inode *inode, spin_unlock(&inode_lock); + if (test_and_clear_bit(AS_MCTIME, &mapping->flags)) + inode_update_time(inode); + ret = do_writepages(mapping, wbc); /* Don't write the inode if only I_DIRTY_PAGES was set */ --- linux-2.6.16.x86_64/fs/buffer.c.org +++ linux-2.6.16.x86_64/fs/buffer.c @@ -347,6 +347,10 @@ long do_fsync(struct file *file, int dat if (!ret) ret = err; current->flags &= ~PF_SYNCWRITE; + + if (test_and_clear_bit(AS_MCTIME, &mapping->flags)) + inode_update_time(mapping->host); + out: return ret; } @@ -837,6 +841,7 @@ EXPORT_SYMBOL(mark_buffer_dirty_inode); int __set_page_dirty_buffers(struct page *page) { struct address_space * const mapping = page->mapping; + int ret = 0; spin_lock(&mapping->private_lock); if (page_has_buffers(page)) { @@ -861,9 +866,13 @@ int __set_page_dirty_buffers(struct page } write_unlock_irq(&mapping->tree_lock); __mark_inode_dirty(mapping->host, I_DIRTY_PAGES); - return 1; + ret = 1; } - return 0; + + if (page_mapped(page)) + set_bit(AS_MCTIME, &mapping->flags); + + return ret; } EXPORT_SYMBOL(__set_page_dirty_buffers); --- linux-2.6.16.x86_64/include/linux/fs.h.org +++ linux-2.6.16.x86_64/include/linux/fs.h @@ -1778,7 +1778,12 @@ extern int buffer_migrate_page(struct pa extern int inode_change_ok(struct inode *, struct iattr *); extern int __must_check inode_setattr(struct inode *, struct iattr *); -extern void file_update_time(struct file *file); +extern void inode_update_time(struct inode *); + +static inline void file_update_time(struct file *file) +{ + inode_update_time(file->f_dentry->d_inode); +} static inline ino_t parent_ino(struct dentry *dentry) { --- linux-2.6.16.x86_64/include/linux/pagemap.h.org +++ linux-2.6.16.x86_64/include/linux/pagemap.h @@ -16,8 +16,9 @@ * Bits in mapping->flags. The lower __GFP_BITS_SHIFT bits are the page * allocation mode flags. */ -#define AS_EIO (__GFP_BITS_SHIFT + 0) /* IO error on async write */ +#define AS_EIO (__GFP_BITS_SHIFT + 0) /* IO error on async write */ #define AS_ENOSPC (__GFP_BITS_SHIFT + 1) /* ENOSPC on async write */ +#define AS_MCTIME (__GFP_BITS_SHIFT + 2) /* need m/ctime change */ static inline gfp_t mapping_gfp_mask(struct address_space * mapping) { --- linux-2.6.16.x86_64/mm/page-writeback.c.org +++ linux-2.6.16.x86_64/mm/page-writeback.c @@ -627,8 +627,10 @@ EXPORT_SYMBOL(write_one_page); */ int __set_page_dirty_nobuffers(struct page *page) { + struct address_space *mapping = page_mapping(page); + int ret = 0; + if (!TestSetPageDirty(page)) { - struct address_space *mapping = page_mapping(page); struct address_space *mapping2; if (mapping) { @@ -648,9 +650,11 @@ int __set_page_dirty_nobuffers(struct pa I_DIRTY_PAGES); } } - return 1; + ret = 1; } - return 0; + if (page_mapped(page)) + set_bit(AS_MCTIME, &mapping->flags); + return ret; } EXPORT_SYMBOL(__set_page_dirty_nobuffers); --- linux-2.6.16.x86_64/mm/msync.c.org +++ linux-2.6.16.x86_64/mm/msync.c @@ -206,12 +206,16 @@ asmlinkage long sys_msync(unsigned long file = vma->vm_file; start = vma->vm_end; if ((flags & MS_ASYNC) && file && nr_pages_dirtied) { + struct address_space *mapping = file->f_mapping; + get_file(file); up_read(¤t->mm->mmap_sem); - balance_dirty_pages_ratelimited_nr(file->f_mapping, + balance_dirty_pages_ratelimited_nr(mapping, nr_pages_dirtied); - fput(file); down_read(¤t->mm->mmap_sem); + if (test_and_clear_bit(AS_MCTIME, &mapping->flags)) + inode_update_time(mapping->host); + fput(file); vma = find_vma(current->mm, start); } else if ((flags & MS_SYNC) && file && (vma->vm_flags & VM_SHARED)) { --- linux-2.6.16.x86_64/mm/mmap.c.org +++ linux-2.6.16.x86_64/mm/mmap.c @@ -203,6 +203,8 @@ void unlink_file_vma(struct vm_area_stru spin_lock(&mapping->i_mmap_lock); __remove_shared_vm_struct(vma, file, mapping); spin_unlock(&mapping->i_mmap_lock); + if (test_and_clear_bit(AS_MCTIME, &mapping->flags)) + inode_update_time(mapping->host); } } | |