2015-02-03 21:09:50

by Anna Schumaker

[permalink] [raw]
Subject: [PATCH] fsx: check for filesystem support of FALLOCATE_FL_KEEP_SIZE

The NFS implementation of fallocate() does not support passing the
KEEP_SIZE flag by itself, causing tests to randomly fail.

Signed-off-by: Anna Schumaker <[email protected]>
---
ltp/fsx.c | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/ltp/fsx.c b/ltp/fsx.c
index 3709419..0bb8ae8 100644
--- a/ltp/fsx.c
+++ b/ltp/fsx.c
@@ -142,6 +142,7 @@ int randomoplen = 1; /* -O flag disables it */
int seed = 1; /* -S flag */
int mapped_writes = 1; /* -W flag disables */
int fallocate_calls = 1; /* -F flag disables */
+int keep_size_calls = 1; /* -K flag disables */
int punch_hole_calls = 1; /* -H flag disables */
int zero_range_calls = 1; /* -z flag disables */
int collapse_range_calls = 1; /* -C flag disables */
@@ -907,7 +908,7 @@ do_zero_range(unsigned offset, unsigned length)
{
unsigned end_offset;
int mode = FALLOC_FL_ZERO_RANGE;
- int keep_size;
+ int keep_size = 0;

if (length == 0) {
if (!quiet && testcalls > simulatedopcount)
@@ -916,7 +917,8 @@ do_zero_range(unsigned offset, unsigned length)
return;
}

- keep_size = random() % 2;
+ if (keep_size_calls)
+ keep_size = random() % 2;

end_offset = keep_size ? 0 : offset + length;

@@ -1018,7 +1020,7 @@ void
do_preallocate(unsigned offset, unsigned length)
{
unsigned end_offset;
- int keep_size;
+ int keep_size = 0;

if (length == 0) {
if (!quiet && testcalls > simulatedopcount)
@@ -1027,7 +1029,8 @@ do_preallocate(unsigned offset, unsigned length)
return;
}

- keep_size = random() % 2;
+ if (keep_size_calls)
+ keep_size = random() % 2;

end_offset = keep_size ? 0 : offset + length;

@@ -1493,7 +1496,7 @@ main(int argc, char **argv)

setvbuf(stdout, (char *)0, _IOLBF, 0); /* line buffered stdout */

- while ((ch = getopt(argc, argv, "b:c:dfl:m:no:p:qr:s:t:w:xyAD:FHzCLN:OP:RS:WZ"))
+ while ((ch = getopt(argc, argv, "b:c:dfl:m:no:p:qr:s:t:w:xyAD:FKHzCLN:OP:RS:WZ"))
!= EOF)
switch (ch) {
case 'b':
@@ -1590,6 +1593,9 @@ main(int argc, char **argv)
case 'F':
fallocate_calls = 0;
break;
+ case 'K':
+ keep_size_calls = 0;
+ break;
case 'H':
punch_hole_calls = 0;
break;
@@ -1751,6 +1757,8 @@ main(int argc, char **argv)

if (fallocate_calls)
fallocate_calls = test_fallocate(0);
+ if (keep_size_calls)
+ keep_size_calls = test_fallocate(FALLOC_FL_KEEP_SIZE);
if (punch_hole_calls)
punch_hole_calls = test_fallocate(FALLOC_FL_PUNCH_HOLE |
FALLOC_FL_KEEP_SIZE);
--
2.2.2



2015-02-05 14:06:05

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH] fsx: check for filesystem support of FALLOCATE_FL_KEEP_SIZE

On Tue, Feb 03, 2015 at 04:09:43PM -0500, Anna Schumaker wrote:
> The NFS implementation of fallocate() does not support passing the
> KEEP_SIZE flag by itself, causing tests to randomly fail.
>
> Signed-off-by: Anna Schumaker <[email protected]>

Looks good,

Reviewed-by: Christoph Hellwig <[email protected]>

2015-02-05 14:16:22

by Boaz Harrosh

[permalink] [raw]
Subject: Re: [PATCH] fsx: check for filesystem support of FALLOCATE_FL_KEEP_SIZE

On 02/03/2015 11:09 PM, Anna Schumaker wrote:
> The NFS implementation of fallocate() does not support passing the
> KEEP_SIZE flag by itself, causing tests to randomly fail.
>

This I do not understand please explain. If all the other FSs just ignore
a trivial case, then why should NFS not ignore it the same as all
other filesystems.

Why NFS is allowed to be special. (And apparently break user code xfs
being an example of one)

Thanks
Boaz

> Signed-off-by: Anna Schumaker <[email protected]>
> ---
> ltp/fsx.c | 18 +++++++++++++-----
> 1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/ltp/fsx.c b/ltp/fsx.c
> index 3709419..0bb8ae8 100644
> --- a/ltp/fsx.c
> +++ b/ltp/fsx.c
> @@ -142,6 +142,7 @@ int randomoplen = 1; /* -O flag disables it */
> int seed = 1; /* -S flag */
> int mapped_writes = 1; /* -W flag disables */
> int fallocate_calls = 1; /* -F flag disables */
> +int keep_size_calls = 1; /* -K flag disables */
> int punch_hole_calls = 1; /* -H flag disables */
> int zero_range_calls = 1; /* -z flag disables */
> int collapse_range_calls = 1; /* -C flag disables */
> @@ -907,7 +908,7 @@ do_zero_range(unsigned offset, unsigned length)
> {
> unsigned end_offset;
> int mode = FALLOC_FL_ZERO_RANGE;
> - int keep_size;
> + int keep_size = 0;
>
> if (length == 0) {
> if (!quiet && testcalls > simulatedopcount)
> @@ -916,7 +917,8 @@ do_zero_range(unsigned offset, unsigned length)
> return;
> }
>
> - keep_size = random() % 2;
> + if (keep_size_calls)
> + keep_size = random() % 2;
>
> end_offset = keep_size ? 0 : offset + length;
>
> @@ -1018,7 +1020,7 @@ void
> do_preallocate(unsigned offset, unsigned length)
> {
> unsigned end_offset;
> - int keep_size;
> + int keep_size = 0;
>
> if (length == 0) {
> if (!quiet && testcalls > simulatedopcount)
> @@ -1027,7 +1029,8 @@ do_preallocate(unsigned offset, unsigned length)
> return;
> }
>
> - keep_size = random() % 2;
> + if (keep_size_calls)
> + keep_size = random() % 2;
>
> end_offset = keep_size ? 0 : offset + length;
>
> @@ -1493,7 +1496,7 @@ main(int argc, char **argv)
>
> setvbuf(stdout, (char *)0, _IOLBF, 0); /* line buffered stdout */
>
> - while ((ch = getopt(argc, argv, "b:c:dfl:m:no:p:qr:s:t:w:xyAD:FHzCLN:OP:RS:WZ"))
> + while ((ch = getopt(argc, argv, "b:c:dfl:m:no:p:qr:s:t:w:xyAD:FKHzCLN:OP:RS:WZ"))
> != EOF)
> switch (ch) {
> case 'b':
> @@ -1590,6 +1593,9 @@ main(int argc, char **argv)
> case 'F':
> fallocate_calls = 0;
> break;
> + case 'K':
> + keep_size_calls = 0;
> + break;
> case 'H':
> punch_hole_calls = 0;
> break;
> @@ -1751,6 +1757,8 @@ main(int argc, char **argv)
>
> if (fallocate_calls)
> fallocate_calls = test_fallocate(0);
> + if (keep_size_calls)
> + keep_size_calls = test_fallocate(FALLOC_FL_KEEP_SIZE);
> if (punch_hole_calls)
> punch_hole_calls = test_fallocate(FALLOC_FL_PUNCH_HOLE |
> FALLOC_FL_KEEP_SIZE);
>


2015-02-05 14:21:28

by Anna Schumaker

[permalink] [raw]
Subject: Re: [PATCH] fsx: check for filesystem support of FALLOCATE_FL_KEEP_SIZE

On 02/05/2015 09:16 AM, Boaz Harrosh wrote:
> On 02/03/2015 11:09 PM, Anna Schumaker wrote:
>> The NFS implementation of fallocate() does not support passing the
>> KEEP_SIZE flag by itself, causing tests to randomly fail.
>>
>
> This I do not understand please explain. If all the other FSs just ignore
> a trivial case, then why should NFS not ignore it the same as all
> other filesystems.
>
> Why NFS is allowed to be special. (And apparently break user code xfs
> being an example of one)

I'm just working with what we have in NFS v4.2: ALLOCATE is fallocate(flags = 0), and DEALLOCATE is fallocate(flags = FALLOCATE_FL_KEEP_SIZE | FALLOCATE_FL_PUNCH_HOLE).

Anna
>
> Thanks
> Boaz
>
>> Signed-off-by: Anna Schumaker <[email protected]>
>> ---
>> ltp/fsx.c | 18 +++++++++++++-----
>> 1 file changed, 13 insertions(+), 5 deletions(-)
>>
>> diff --git a/ltp/fsx.c b/ltp/fsx.c
>> index 3709419..0bb8ae8 100644
>> --- a/ltp/fsx.c
>> +++ b/ltp/fsx.c
>> @@ -142,6 +142,7 @@ int randomoplen = 1; /* -O flag disables it */
>> int seed = 1; /* -S flag */
>> int mapped_writes = 1; /* -W flag disables */
>> int fallocate_calls = 1; /* -F flag disables */
>> +int keep_size_calls = 1; /* -K flag disables */
>> int punch_hole_calls = 1; /* -H flag disables */
>> int zero_range_calls = 1; /* -z flag disables */
>> int collapse_range_calls = 1; /* -C flag disables */
>> @@ -907,7 +908,7 @@ do_zero_range(unsigned offset, unsigned length)
>> {
>> unsigned end_offset;
>> int mode = FALLOC_FL_ZERO_RANGE;
>> - int keep_size;
>> + int keep_size = 0;
>>
>> if (length == 0) {
>> if (!quiet && testcalls > simulatedopcount)
>> @@ -916,7 +917,8 @@ do_zero_range(unsigned offset, unsigned length)
>> return;
>> }
>>
>> - keep_size = random() % 2;
>> + if (keep_size_calls)
>> + keep_size = random() % 2;
>>
>> end_offset = keep_size ? 0 : offset + length;
>>
>> @@ -1018,7 +1020,7 @@ void
>> do_preallocate(unsigned offset, unsigned length)
>> {
>> unsigned end_offset;
>> - int keep_size;
>> + int keep_size = 0;
>>
>> if (length == 0) {
>> if (!quiet && testcalls > simulatedopcount)
>> @@ -1027,7 +1029,8 @@ do_preallocate(unsigned offset, unsigned length)
>> return;
>> }
>>
>> - keep_size = random() % 2;
>> + if (keep_size_calls)
>> + keep_size = random() % 2;
>>
>> end_offset = keep_size ? 0 : offset + length;
>>
>> @@ -1493,7 +1496,7 @@ main(int argc, char **argv)
>>
>> setvbuf(stdout, (char *)0, _IOLBF, 0); /* line buffered stdout */
>>
>> - while ((ch = getopt(argc, argv, "b:c:dfl:m:no:p:qr:s:t:w:xyAD:FHzCLN:OP:RS:WZ"))
>> + while ((ch = getopt(argc, argv, "b:c:dfl:m:no:p:qr:s:t:w:xyAD:FKHzCLN:OP:RS:WZ"))
>> != EOF)
>> switch (ch) {
>> case 'b':
>> @@ -1590,6 +1593,9 @@ main(int argc, char **argv)
>> case 'F':
>> fallocate_calls = 0;
>> break;
>> + case 'K':
>> + keep_size_calls = 0;
>> + break;
>> case 'H':
>> punch_hole_calls = 0;
>> break;
>> @@ -1751,6 +1757,8 @@ main(int argc, char **argv)
>>
>> if (fallocate_calls)
>> fallocate_calls = test_fallocate(0);
>> + if (keep_size_calls)
>> + keep_size_calls = test_fallocate(FALLOC_FL_KEEP_SIZE);
>> if (punch_hole_calls)
>> punch_hole_calls = test_fallocate(FALLOC_FL_PUNCH_HOLE |
>> FALLOC_FL_KEEP_SIZE);
>>
>


2015-02-05 14:22:35

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH] fsx: check for filesystem support of FALLOCATE_FL_KEEP_SIZE

On Thu, Feb 05, 2015 at 04:16:19PM +0200, Boaz Harrosh wrote:
> On 02/03/2015 11:09 PM, Anna Schumaker wrote:
> > The NFS implementation of fallocate() does not support passing the
> > KEEP_SIZE flag by itself, causing tests to randomly fail.
> >
>
> This I do not understand please explain. If all the other FSs just ignore
> a trivial case, then why should NFS not ignore it the same as all
> other filesystems.
>
> Why NFS is allowed to be special. (And apparently break user code xfs
> being an example of one)

fsx is a test tool. We've never guaranteed that a filesystem needs to
support all or a special subset of fallocate options.

The NFSv4.2 protocol doesn't support preallocations beyond the file
size, which is something I decided would be to horrible to refit into
the NFS file abstraction when rewriting the preallocation / reservation
section of the spec.

2015-02-05 14:31:53

by Boaz Harrosh

[permalink] [raw]
Subject: Re: [PATCH] fsx: check for filesystem support of FALLOCATE_FL_KEEP_SIZE

On 02/05/2015 04:22 PM, Christoph Hellwig wrote:
> On Thu, Feb 05, 2015 at 04:16:19PM +0200, Boaz Harrosh wrote:
>> On 02/03/2015 11:09 PM, Anna Schumaker wrote:
>>> The NFS implementation of fallocate() does not support passing the
>>> KEEP_SIZE flag by itself, causing tests to randomly fail.
>>>
>>
>> This I do not understand please explain. If all the other FSs just ignore
>> a trivial case, then why should NFS not ignore it the same as all
>> other filesystems.
>>
>> Why NFS is allowed to be special. (And apparently break user code xfs
>> being an example of one)
>
> fsx is a test tool. We've never guaranteed that a filesystem needs to
> support all or a special subset of fallocate options.
>
> The NFSv4.2 protocol doesn't support preallocations beyond the file
> size, which is something I decided would be to horrible to refit into
> the NFS file abstraction when rewriting the preallocation / reservation
> section of the spec.

Ha OK now I understand. KEEP_SIZE is not supported when allocating blocks.

Pity, it is very useful for exactly those that need fallocte like video files.
Perhaps in the next version, thanks.

Boaz