xfs
[Top] [All Lists]

Re: [PATCH 1/3] xfs: pass inode instead of file to xfs_reflink_dirty_ran

To: Christoph Hellwig <hch@xxxxxx>
Subject: Re: [PATCH 1/3] xfs: pass inode instead of file to xfs_reflink_dirty_range
From: "Darrick J. Wong" <darrick.wong@xxxxxxxxxx>
Date: Mon, 4 Jan 2016 17:23:09 -0800
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1451822873-12969-2-git-send-email-hch@xxxxxx>
References: <1451822873-12969-1-git-send-email-hch@xxxxxx> <1451822873-12969-2-git-send-email-hch@xxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Sun, Jan 03, 2016 at 01:07:51PM +0100, Christoph Hellwig wrote:
> We don't actually need a file for write_begin/end, those can be passed
> as NULL for disk based file systems.  This is important as we won't

I wasn't aware that one could /do/ that. :)

Looks good, though.  I think I can clean the filp arguments out of the reflink
code entirely, too.

--D

> even have a file pointer during a truncate operation, which gives a
> guaranteed NULL pointer dererference with the current code.
> 
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
>  fs/xfs/xfs_reflink.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> index da4a715..4a3f0ee 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -1316,7 +1316,7 @@ out_error:
>   */
>  STATIC int
>  xfs_reflink_dirty_range(
> -     struct file             *filp,
> +     struct inode            *inode,
>       xfs_off_t               pos,
>       xfs_off_t               len)
>  {
> @@ -1330,14 +1330,14 @@ xfs_reflink_dirty_range(
>       unsigned long           bytes;  /* Bytes to write to page */
>       void                    *fsdata;
>  
> -     mapping = filp->f_mapping;
> +     mapping = inode->i_mapping;
>       a_ops = mapping->a_ops;
>       flags = AOP_FLAG_UNINTERRUPTIBLE;
>       do {
>  
>               offset = (pos & (PAGE_CACHE_SIZE - 1));
>               bytes = min_t(unsigned long, len, PAGE_CACHE_SIZE) - offset;
> -             rpage = xfs_get_page(file_inode(filp), pos);
> +             rpage = xfs_get_page(inode, pos);
>               if (IS_ERR(rpage)) {
>                       error = PTR_ERR(rpage);
>                       break;
> @@ -1346,24 +1346,24 @@ xfs_reflink_dirty_range(
>                       break;
>               }
>  
> -             error = a_ops->write_begin(filp, mapping, pos, bytes, flags,
> +             error = a_ops->write_begin(NULL, mapping, pos, bytes, flags,
>                                          &page, &fsdata);
>               page_cache_release(rpage);
>               if (error < 0)
>                       break;
>  
> -             trace_xfs_reflink_unshare_page(file_inode(filp), page,
> +             trace_xfs_reflink_unshare_page(inode, page,
>                               pos, bytes);
>  
>               if (!PageUptodate(page)) {
> -                     pr_err("%s: STALE? ino=%lu pos=%llu\n",
> -                             __func__, filp->f_inode->i_ino, pos);
> +                     pr_err("%s: STALE? ino=%llu pos=%llu\n",
> +                             __func__, XFS_I(inode)->i_ino, pos);
>                       WARN_ON(1);
>               }
>               if (mapping_writably_mapped(mapping))
>                       flush_dcache_page(page);
>  
> -             error = a_ops->write_end(filp, mapping, pos, bytes, bytes,
> +             error = a_ops->write_end(NULL, mapping, pos, bytes, bytes,
>                                        page, fsdata);
>               if (error < 0)
>                       break;
> @@ -1454,7 +1454,7 @@ xfs_reflink_dirty_extents(
>                       flen = XFS_FSB_TO_B(mp, rlen);
>                       if (fpos + flen > isize)
>                               flen = isize - fpos;
> -                     error = xfs_reflink_dirty_range(filp, fpos, flen);
> +                     error = xfs_reflink_dirty_range(VFS_I(ip), fpos, flen);
>                       xfs_ilock(ip, XFS_ILOCK_EXCL);
>                       if (error)
>                               goto out;
> -- 
> 1.9.1
> 
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs

<Prev in Thread] Current Thread [Next in Thread>