From: Eric Sandeen <sandeen@sandeen.net>
To: Dave Chinner <david@fromorbit.com>
Cc: Eric Sandeen <sandeen@redhat.com>,
ext4 development <linux-ext4@vger.kernel.org>,
Eryu Guan <eguan@redhat.com>, xfs-oss <xfs@oss.sgi.com>
Subject: Re: [PATCH V2] xfstests: make 275 pass
Date: Tue, 17 Jan 2012 10:33:20 -0600 [thread overview]
Message-ID: <4F15A2D0.2090900@sandeen.net> (raw)
In-Reply-To: <20120105003928.GC24466@dastard>
On 1/4/12 6:39 PM, Dave Chinner wrote:
> On Wed, Jan 04, 2012 at 05:21:00PM -0600, Eric Sandeen wrote:
>> On 1/4/12 5:17 PM, Dave Chinner wrote:
>>> On Wed, Jan 04, 2012 at 02:54:25PM -0600, Eric Sandeen wrote:
>>>> Ok, this is a significant rework of 275, which made too many
>>>> assumptions about details of space usage and failed on several
>>>> filesystems (it passed on xfs, but only by accident).
>>>>
>>>> This new version tries to leave about 256k free, then tries
>>>> a single 1M IO, and fails only if 0 bytes are written.
>>>>
>>>> It also sends a lot more to $seq.full for debugging on failure
>>>> and fixes a few other stylistic things.
>>>>
>>>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>>>
>>> I just had another thought about this, Eric....
>>>
>>>> +# And at least some of it should succeed.
>>>> +_filesize=`du $SCRATCH_MNT/tmp1 | awk '{print $1}'`
>>>> +[ $_filesize -eq 0 ] && _fail "write file err: Partial write until enospc failed; wrote 0 bytes."
>>>
>>> The question that just came to mind was this assumes that allocation
>>> succeeded so therefore the partial write succeeded. But that's not
>>> necessary the case. The partial write might not succeed leaving the
>>> file size as zero, but the underlying FS might not remove all the
>>> blocks it allocated (nothing says that it has to). Hence to
>>> determine if a partial write succeeded, we also need to check that
>>> the file size itself is greater than zero....
>>
>> Probably need to read up on what posix says it should do. I think
>> what you're saying is that it might leave blocks allocated past EOF?
>> That'd be surprising to me, but maybe I misunderstand?
>
> There's no guarantee that du is even reporting blocks on disk. e.g
> for XFS du will also report reserved (in-memory) delalloc space on
> the inode and that includes speculative allocation beyond EOF. We
> don't have to remove specultive delalloc ranges when a partial write
> occurs, so effectively checking du output to see if a partial write
> succeeded is not a sufficient test to determine if the partial write
> succeeded or not.
>
> However, if the partial write did succeed then the file size *must*
> change to reflect what was written. Hence I suspect all we actually
> need here is a file size check...
Ok; so would you be happy with just this?
# And at least some of it should succeed.
_filesize=`ls -l $SCRATCH_MNT/tmp1 | awk '{print $5}'`
[ $_filesize -eq 0 ] && _fail "write file err: Partial write until enospc failed; wrote 0 bytes."
If so I'll put your reviewed-by on it and get this pushed, ok?
-Eric
> Cheers,
>
> Dave.
WARNING: multiple messages have this Message-ID (diff)
From: Eric Sandeen <sandeen@sandeen.net>
To: Dave Chinner <david@fromorbit.com>
Cc: Eric Sandeen <sandeen@redhat.com>,
ext4 development <linux-ext4@vger.kernel.org>,
Eryu Guan <eguan@redhat.com>, xfs-oss <xfs@oss.sgi.com>
Subject: Re: [PATCH V2] xfstests: make 275 pass
Date: Tue, 17 Jan 2012 10:33:20 -0600 [thread overview]
Message-ID: <4F15A2D0.2090900@sandeen.net> (raw)
In-Reply-To: <20120105003928.GC24466@dastard>
On 1/4/12 6:39 PM, Dave Chinner wrote:
> On Wed, Jan 04, 2012 at 05:21:00PM -0600, Eric Sandeen wrote:
>> On 1/4/12 5:17 PM, Dave Chinner wrote:
>>> On Wed, Jan 04, 2012 at 02:54:25PM -0600, Eric Sandeen wrote:
>>>> Ok, this is a significant rework of 275, which made too many
>>>> assumptions about details of space usage and failed on several
>>>> filesystems (it passed on xfs, but only by accident).
>>>>
>>>> This new version tries to leave about 256k free, then tries
>>>> a single 1M IO, and fails only if 0 bytes are written.
>>>>
>>>> It also sends a lot more to $seq.full for debugging on failure
>>>> and fixes a few other stylistic things.
>>>>
>>>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>>>
>>> I just had another thought about this, Eric....
>>>
>>>> +# And at least some of it should succeed.
>>>> +_filesize=`du $SCRATCH_MNT/tmp1 | awk '{print $1}'`
>>>> +[ $_filesize -eq 0 ] && _fail "write file err: Partial write until enospc failed; wrote 0 bytes."
>>>
>>> The question that just came to mind was this assumes that allocation
>>> succeeded so therefore the partial write succeeded. But that's not
>>> necessary the case. The partial write might not succeed leaving the
>>> file size as zero, but the underlying FS might not remove all the
>>> blocks it allocated (nothing says that it has to). Hence to
>>> determine if a partial write succeeded, we also need to check that
>>> the file size itself is greater than zero....
>>
>> Probably need to read up on what posix says it should do. I think
>> what you're saying is that it might leave blocks allocated past EOF?
>> That'd be surprising to me, but maybe I misunderstand?
>
> There's no guarantee that du is even reporting blocks on disk. e.g
> for XFS du will also report reserved (in-memory) delalloc space on
> the inode and that includes speculative allocation beyond EOF. We
> don't have to remove specultive delalloc ranges when a partial write
> occurs, so effectively checking du output to see if a partial write
> succeeded is not a sufficient test to determine if the partial write
> succeeded or not.
>
> However, if the partial write did succeed then the file size *must*
> change to reflect what was written. Hence I suspect all we actually
> need here is a file size check...
Ok; so would you be happy with just this?
# And at least some of it should succeed.
_filesize=`ls -l $SCRATCH_MNT/tmp1 | awk '{print $5}'`
[ $_filesize -eq 0 ] && _fail "write file err: Partial write until enospc failed; wrote 0 bytes."
If so I'll put your reviewed-by on it and get this pushed, ok?
-Eric
> Cheers,
>
> Dave.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
next prev parent reply other threads:[~2012-01-17 16:33 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-01-04 19:22 [PATCH] xfstests: make 275 pass on ext4 Eric Sandeen
2012-01-04 19:22 ` Eric Sandeen
2012-01-04 19:37 ` Eric Sandeen
2012-01-04 19:37 ` Eric Sandeen
2012-01-04 20:54 ` [PATCH V2] xfstests: make 275 pass Eric Sandeen
2012-01-04 20:54 ` Eric Sandeen
2012-01-04 23:17 ` Dave Chinner
2012-01-04 23:17 ` Dave Chinner
2012-01-04 23:21 ` Eric Sandeen
2012-01-04 23:21 ` Eric Sandeen
2012-01-05 0:39 ` Dave Chinner
2012-01-05 0:39 ` Dave Chinner
2012-01-17 16:33 ` Eric Sandeen [this message]
2012-01-17 16:33 ` Eric Sandeen
2012-01-25 21:13 ` [PATCH V3] " Eric Sandeen
2012-01-25 21:13 ` Eric Sandeen
2012-09-07 19:56 ` Eric Sandeen
2012-09-07 19:56 ` Eric Sandeen
2012-09-08 0:09 ` Dave Chinner
2012-09-08 0:09 ` Dave Chinner
2012-01-18 2:42 ` [PATCH V2] " Liu Bo
2012-01-18 2:42 ` Liu Bo
2012-01-18 4:03 ` Eric Sandeen
2012-01-18 4:03 ` Eric Sandeen
2012-01-18 4:42 ` Liu Bo
2012-01-18 4:42 ` Liu Bo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4F15A2D0.2090900@sandeen.net \
--to=sandeen@sandeen.net \
--cc=david@fromorbit.com \
--cc=eguan@redhat.com \
--cc=linux-ext4@vger.kernel.org \
--cc=sandeen@redhat.com \
--cc=xfs@oss.sgi.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.