[Top] [All Lists]

Re: [PATCH] xfs: inode log reservations are too small

To: Dave Chinner <david@xxxxxxxxxxxxx>, Chris Dunlop <chris@xxxxxxxxxxxx>
Subject: Re: [PATCH] xfs: inode log reservations are too small
From: Ben Myers <bpm@xxxxxxx>
Date: Fri, 30 Aug 2013 13:57:55 -0500
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1377670235-4168-1-git-send-email-david@xxxxxxxxxxxxx>
References: <1377670235-4168-1-git-send-email-david@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.20 (2009-06-14)
On Wed, Aug 28, 2013 at 04:10:35PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
> We've been seeing occasional problems with log space leaks and
> transaction underruns such as this for some time:
>  XFS (dm-0): xlog_write: reservation summary:
>    trans type  = FSYNC_TS (36)
>    unit res    = 2740 bytes
>    current res = -4 bytes
>    total reg   = 0 bytes (o/flow = 0 bytes)
>    ophdrs      = 0 (ophdr space = 0 bytes)
>    ophdr + reg = 0 bytes
>    num regions = 0
> Turns out that xfstests generic/311 is reliably reproducing this
> problem with the test it runs at sequence 16 of it execution. It is
> a 100% reliable reproducer with the mkfs configuration of "-b
> size=1024 -m crc=1" on a 10GB scratch device.
> The problem? Inode forks in btree format are logged in memory
> format, not disk format (i.e. bmbt format, not bmdr format). That
> means there is a btree block header being logged, when such a
> structure is never written to the inode fork in bmdr format. The
> bmdr header in the inode is only 4 bytes, while the bmbt header is
> 24 bytes for v4 filesystems and 72 bytes for v5 filesystems.
> We currently reserve the inode size plus the rounded up overhead of
> a logging a buffer, which is 128 bytes. That means the reservation
> for a 512 byte inode is 640 bytes. What we can actually log is:
>       inode core, data and attr fork = 512 bytes
>       inode log format + log op header = 56 + 12 = 68 bytes
>       data fork bmbt hdr = 24/72 bytes
>       attr fork bmbt hdr = 24/72 bytes
> So, for a v2 inodes we can log at least 628 bytes, but if we split that
> inode over the end of the log across log buffers, we need to also
> another log op header, which takes us to 640 bytes. If there's
> another reservation taken out of this that I haven't taken into
> account (perhaps multiple iclog splits?) or I haven't corectly
> calculated the bmbt format space used (entirely possible), then
> we will overun it.
> For v3 inodes the maximum is actually 724 bytes, and even a
> single maximally sized btree format fork can blow it (652 bytes).
> And that's exactly what is happening with the FSYNC_TS transaction
> in the above output - it's consumed 644 bytes of space after the CIL
> context took the space reserved for it (2100 bytes).
> This problem has always been present in the XFS code - the btree
> format inode forks have always been logged in this manner. Hence
> there has always been the possibility of an overrun with such a
> transaction. The CRC code has just exposed it frequently enough to
> be able to debug and understand the root cause....
> So, let's fix all the inode log space reservations.
> [ I'm so glad we spent the effort to clean up the transaction
>   reservation code. This is an easy fix now. ]
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>

Applied this after cleaning up Chris's 'directory'.  Thanks.

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