[PATCH] xfs: remove incorrect log write optimization

[PATCH] xfs: remove incorrect log write optimization

[Christoph Hellwig]

We do need a barrier for the first buffer of a split log write.  Otherwise
we might incorrectly stamp the tail LSN into transactions in the first
block, or not flush data I/O before updating the inode size.

Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: xfs-dev/fs/xfs/xfs_log.c
===================================================================
--- xfs-dev.orig/fs/xfs/xfs_log.c	2010-06-28 11:51:30.423004228 +0200
+++ xfs-dev/fs/xfs/xfs_log.c	2010-06-28 11:52:52.559255519 +0200
@@ -1423,11 +1423,8 @@ xlog_sync(xlog_t		*log,
 	XFS_BUF_BUSY(bp);
 	XFS_BUF_ASYNC(bp);
 	bp->b_flags |= XBF_LOG_BUFFER;
-	/*
-	 * Do an ordered write for the log block.
-	 * Its unnecessary to flush the first split block in the log wrap case.
-	 */
-	if (!split && (log->l_mp->m_flags & XFS_MOUNT_BARRIER))
+
+	if (log->l_mp->m_flags & XFS_MOUNT_BARRIER)
 		XFS_BUF_ORDERED(bp);
 
 	ASSERT(XFS_BUF_ADDR(bp) <= log->l_logBBsize-1);

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs: remove incorrect log write optimization

[Christoph Hellwig]

ping?

On Mon, Jun 28, 2010 at 10:34:34AM -0400, Christoph Hellwig wrote:
> We do need a barrier for the first buffer of a split log write.  Otherwise
> we might incorrectly stamp the tail LSN into transactions in the first
> block, or not flush data I/O before updating the inode size.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> Index: xfs-dev/fs/xfs/xfs_log.c
> ===================================================================
> --- xfs-dev.orig/fs/xfs/xfs_log.c	2010-06-28 11:51:30.423004228 +0200
> +++ xfs-dev/fs/xfs/xfs_log.c	2010-06-28 11:52:52.559255519 +0200
> @@ -1423,11 +1423,8 @@ xlog_sync(xlog_t		*log,
>  	XFS_BUF_BUSY(bp);
>  	XFS_BUF_ASYNC(bp);
>  	bp->b_flags |= XBF_LOG_BUFFER;
> -	/*
> -	 * Do an ordered write for the log block.
> -	 * Its unnecessary to flush the first split block in the log wrap case.
> -	 */
> -	if (!split && (log->l_mp->m_flags & XFS_MOUNT_BARRIER))
> +
> +	if (log->l_mp->m_flags & XFS_MOUNT_BARRIER)
>  		XFS_BUF_ORDERED(bp);
>  
>  	ASSERT(XFS_BUF_ADDR(bp) <= log->l_logBBsize-1);
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
---end quoted text---

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs: remove incorrect log write optimization

[Alex Elder]

On Mon, 2010-06-28 at 10:34 -0400, Christoph Hellwig wrote:
> We do need a barrier for the first buffer of a split log write.  Otherwise
> we might incorrectly stamp the tail LSN into transactions in the first
> block, or not flush data I/O before updating the inode size.

I'm not sure whether "first block" means the one at the beginning
of the log, or the one at the beginning of a split log write.  In
any case recovery handles the situation correctly if the write at
the end of the log didn't make it to disk before the start of the
log.  I think you're right about ordering with respect to data I/O
though.

This looks good to me.

Reviewed-by: Alex Elder <aelder@sgi.com>

> Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> Index: xfs-dev/fs/xfs/xfs_log.c
> ===================================================================
> --- xfs-dev.orig/fs/xfs/xfs_log.c	2010-06-28 11:51:30.423004228 +0200
> +++ xfs-dev/fs/xfs/xfs_log.c	2010-06-28 11:52:52.559255519 +0200
> @@ -1423,11 +1423,8 @@ xlog_sync(xlog_t		*log,
>  	XFS_BUF_BUSY(bp);
>  	XFS_BUF_ASYNC(bp);
>  	bp->b_flags |= XBF_LOG_BUFFER;
> -	/*
> -	 * Do an ordered write for the log block.
> -	 * Its unnecessary to flush the first split block in the log wrap case.
> -	 */
> -	if (!split && (log->l_mp->m_flags & XFS_MOUNT_BARRIER))
> +
> +	if (log->l_mp->m_flags & XFS_MOUNT_BARRIER)
>  		XFS_BUF_ORDERED(bp);
>  
>  	ASSERT(XFS_BUF_ADDR(bp) <= log->l_logBBsize-1);
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs



_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs: remove incorrect log write optimization

[Christoph Hellwig]

On Sun, Jul 11, 2010 at 10:30:00PM -0500, Alex Elder wrote:
> On Mon, 2010-06-28 at 10:34 -0400, Christoph Hellwig wrote:
> > We do need a barrier for the first buffer of a split log write.  Otherwise
> > we might incorrectly stamp the tail LSN into transactions in the first
> > block, or not flush data I/O before updating the inode size.
> 
> I'm not sure whether "first block" means the one at the beginning
> of the log, or the one at the beginning of a split log write.

It means beginning of the split write.  I can fix up the commit message
if you want.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs: remove incorrect log write optimization

[Alex Elder]

On Mon, 2010-07-12 at 12:04 -0400, Christoph Hellwig wrote:
> On Sun, Jul 11, 2010 at 10:30:00PM -0500, Alex Elder wrote:
> > On Mon, 2010-06-28 at 10:34 -0400, Christoph Hellwig wrote:
> > > We do need a barrier for the first buffer of a split log write.  Otherwise
> > > we might incorrectly stamp the tail LSN into transactions in the first
> > > block, or not flush data I/O before updating the inode size.
> > 
> > I'm not sure whether "first block" means the one at the beginning
> > of the log, or the one at the beginning of a split log write.
> 
> It means beginning of the split write.  I can fix up the commit message
> if you want.
> 

Don't bother, I will change it to:

We do need a barrier for the first buffer of a split log write.
Otherwise we might incorrectly stamp the tail LSN into transactions
in the first part of the split write, or not flush data I/O before
updating the inode size.


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs