xfs
[Top] [All Lists]

Re: [PATCH 3/5] xfs: make inodes dirty before issuing I/O

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH 3/5] xfs: make inodes dirty before issuing I/O
From: Eric Sandeen <sandeen@xxxxxxxxxxx>
Date: Sun, 10 May 2009 13:02:16 -0500
Cc: xfs@xxxxxxxxxxx
In-reply-to: <20090426140707.884922000@xxxxxxxxxxxxxxxxxxxxxx>
References: <20090426140305.113371000@xxxxxxxxxxxxxxxxxxxxxx> <20090426140707.884922000@xxxxxxxxxxxxxxxxxxxxxx>
User-agent: Thunderbird 2.0.0.21 (Macintosh/20090302)
Christoph Hellwig wrote:
> To make sure they get properly waited on in sync when I/O is in flight and
> we latter need to update the inode size.
> 

maybe mention the new helper in the changelog just for completeness...

> 
> Index: linux-2.6/fs/xfs/linux-2.6/xfs_aops.c
> ===================================================================
> --- linux-2.6.orig/fs/xfs/linux-2.6/xfs_aops.c        2009-04-26 
> 10:33:05.556127371 +0200
> +++ linux-2.6/fs/xfs/linux-2.6/xfs_aops.c     2009-04-26 10:37:23.137953826 
> +0200
> @@ -186,19 +186,37 @@ xfs_destroy_ioend(
>  }
>  
>  /*
> + * If the end of the current ioend is beyond the current EOF,
> + * return the new EOF value, otherwise zero.
> + */
> +STATIC xfs_fsize_t
> +xfs_ioend_new_eof(
> +     xfs_ioend_t             *ioend)
> +{
> +     xfs_inode_t             *ip = XFS_I(ioend->io_inode);
> +     xfs_fsize_t             isize;
> +     xfs_fsize_t             bsize;
> +
> +     bsize = ioend->io_offset + ioend->io_size;
> +     isize = MAX(ip->i_size, ip->i_new_size);
> +     isize = MIN(isize, bsize);
> +     return isize > ip->i_d.di_size ? isize : 0;
> +}
> +
> +/*
>   * Update on-disk file size now that data has been written to disk.
>   * The current in-memory file size is i_size.  If a write is beyond
>   * eof i_new_size will be the intended file size until i_size is
>   * updated.  If this write does not extend all the way to the valid
>   * file size then restrict this update to the end of the write.
>   */
> +
>  STATIC void
>  xfs_setfilesize(
>       xfs_ioend_t             *ioend)
>  {
>       xfs_inode_t             *ip = XFS_I(ioend->io_inode);
>       xfs_fsize_t             isize;
> -     xfs_fsize_t             bsize;
>  
>       ASSERT((ip->i_d.di_mode & S_IFMT) == S_IFREG);
>       ASSERT(ioend->io_type != IOMAP_READ);
> @@ -206,14 +224,9 @@ xfs_setfilesize(
>       if (unlikely(ioend->io_error))
>               return;
>  
> -     bsize = ioend->io_offset + ioend->io_size;
> -
>       xfs_ilock(ip, XFS_ILOCK_EXCL);
> -
> -     isize = MAX(ip->i_size, ip->i_new_size);
> -     isize = MIN(isize, bsize);
> -
> -     if (ip->i_d.di_size < isize) {
> +     isize = xfs_ioend_new_eof(ioend);
> +     if (isize) {

It strikes me as a little odd to potentially get back "isize == 0" here
when nothing about the size is 0.  Would it make more sense to rename
this variable to "new_isize" or something?

>               ip->i_d.di_size = isize;
>               ip->i_update_core = 1;
>               ip->i_update_size = 1;
> @@ -405,10 +418,16 @@ xfs_submit_ioend_bio(
>       struct bio      *bio)
>  {
>       atomic_inc(&ioend->io_remaining);
> -
>       bio->bi_private = ioend;
>       bio->bi_end_io = xfs_end_bio;
>  
> +     /*
> +      * if the I/O is beyond EOF we mark the inode dirty immediately
           ^If (uber-nitpick, in akpm-mode today I guess!)

> +      * but don't update the inode size until I/O completion.
> +      */

Maybe extend this comment a bit to say -why- you are doing this, not
just -what- you are doing?

> +     if (xfs_ioend_new_eof(ioend))
> +             xfs_mark_inode_dirty_sync(XFS_I(ioend->io_inode));
> +
>       submit_bio(WRITE, bio);
>       ASSERT(!bio_flagged(bio, BIO_EOPNOTSUPP));
>       bio_put(bio);
> 
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs
> 

<Prev in Thread] Current Thread [Next in Thread>
  • Re: [PATCH 3/5] xfs: make inodes dirty before issuing I/O, Eric Sandeen <=