On Mon, Jan 08, 2007 at 03:03:09PM +1100, David Chinner wrote:
> With the recent cancel_dirty_page() changes, a warning was
> added if we cancel a dirty page that is still mapped into
> the page tables.
> This happens in XFS from fs_tosspages() and fs_flushinval_pages()
> because they call truncate_inode_pages().
>
> truncate_inode_pages() does not invalidate existing page mappings;
> it is expected taht this is called only when truncating the file
> or destroying the inode and on both these cases there can be
> no mapped ptes. However, we call this when doing direct I/O writes
> to remove pages from the page cache. As a result, we can rip
> a page from the page cache that still has mappings attached.
>
> The correct fix is to use invalidate_inode_pages2_range() instead
> of truncate_inode_pages(). They essentially do the same thing, but
> the former also removes any pte mappings before removing the page
> from the page cache.
>
> Comments?
>
> Cheers,
>
> Dave.
> --
> Dave Chinner
> Principal Engineer
> SGI Australian Software Group
>
>
> ---
> fs/xfs/linux-2.6/xfs_fs_subr.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> Index: 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_fs_subr.c
> ===================================================================
> --- 2.6.x-xfs-new.orig/fs/xfs/linux-2.6/xfs_fs_subr.c 2006-12-12
> 12:05:17.000000000 +1100
> +++ 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_fs_subr.c 2007-01-08
> 09:30:22.056571711 +1100
> @@ -21,6 +21,8 @@ int fs_noerr(void) { return 0; }
> int fs_nosys(void) { return ENOSYS; }
> void fs_noval(void) { return; }
>
> +#define XFS_OFF_TO_PCSIZE(off) \
> + (((off) + PAGE_CACHE_SIZE - 1) >> PAGE_CACHE_SHIFT)
I don't think this is right.
Assuming 4k page size, first = 2k, last = 6k will result in
invalidating page indexes 1 and 2 i.e. offset 4k -> 12k. In fact,
we want to invalidate pages 0 and 1.
IOWs, I think it should be:
+#define XFS_OFF_TO_PCINDEX(off) ((off) >> PAGE_CACHE_SHIFT)
Comments?
Cheers,
Dave.
--
Dave Chinner
Principal Engineer
SGI Australian Software Group
|