public inbox for linux-btrfs@vger.kernel.org
 help / color / mirror / Atom feed
* [BUG] fallocate behavior when crossing end-of-file
@ 2009-05-06 13:03 Raskin Michael
  2009-05-06 14:59 ` Edward Shishkin
  0 siblings, 1 reply; 6+ messages in thread
From: Raskin Michael @ 2009-05-06 13:03 UTC (permalink / raw)
  To: linux-btrfs

		Hello.
	I found the following bug in BtrFS:

	1. Create and open an empty file
	2. fallocate (fd, 0, 1)
	
Desired: something (probably, one block) is allocated/reserved for the
file. File length is set to 1 byte.

Actual: A block is allocated. File length is set to 1 block (4096
bytes). The rest of the file is filled with zeros.

	last_byte = min(extent_map_end(em), alloc_end);
	last_byte = (last_byte + mask) & ~mask;
	if (em->block_start == EXTENT_MAP_HOLE) {
		ret = prealloc_file_range(trans, inode, cur_offset,
				last_byte, locked_end + 1,
				alloc_hint, mode);

That part seems strange to me. You make an effort for block size to
divide last_byte. But last_byte should be
max(i_size_read(inode), offset+len)
- without any rounding.

Michael Raskin

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [BUG] fallocate behavior when crossing end-of-file
  2009-05-06 13:03 [BUG] fallocate behavior when crossing end-of-file Raskin Michael
@ 2009-05-06 14:59 ` Edward Shishkin
  2009-05-06 15:10   ` Chris Mason
  2009-05-06 15:12   ` Michael Raskin
  0 siblings, 2 replies; 6+ messages in thread
From: Edward Shishkin @ 2009-05-06 14:59 UTC (permalink / raw)
  To: Raskin Michael; +Cc: linux-btrfs

Raskin Michael wrote:
> 		Hello.
>   

Hello.

> 	I found the following bug in BtrFS:
>
> 	1. Create and open an empty file
> 	2. fallocate (fd, 0, 1)
> 	
> Desired: something (probably, one block) is allocated/reserved for the
> file. File length is set to 1 byte.
>   
             
Where is it documented?

IMHO we should only guarantee that writes
to [0, 1] won't fail because of lack of disk space.
Other things (including file size) are up to implementation.

> Actual: A block is allocated. File length is set to 1 block (4096
> bytes). The rest of the file is filled with zeros.
>
> 	last_byte = min(extent_map_end(em), alloc_end);
> 	last_byte = (last_byte + mask) & ~mask;
> 	if (em->block_start == EXTENT_MAP_HOLE) {
> 		ret = prealloc_file_range(trans, inode, cur_offset,
> 				last_byte, locked_end + 1,
> 				alloc_hint, mode);
>
> That part seems strange to me. You make an effort for block size to
> divide last_byte. But last_byte should be
> max(i_size_read(inode), offset+len)
> - without any rounding.
>
> Michael Raskin
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>   


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [BUG] fallocate behavior when crossing end-of-file
  2009-05-06 14:59 ` Edward Shishkin
@ 2009-05-06 15:10   ` Chris Mason
  2009-05-06 15:12   ` Michael Raskin
  1 sibling, 0 replies; 6+ messages in thread
From: Chris Mason @ 2009-05-06 15:10 UTC (permalink / raw)
  To: Edward Shishkin; +Cc: Raskin Michael, linux-btrfs

On Wed, 2009-05-06 at 16:59 +0200, Edward Shishkin wrote:
> Raskin Michael wrote:
> > 		Hello.
> >   
> 
> Hello.
> 
> > 	I found the following bug in BtrFS:
> >
> > 	1. Create and open an empty file
> > 	2. fallocate (fd, 0, 1)
> > 	
> > Desired: something (probably, one block) is allocated/reserved for the
> > file. File length is set to 1 byte.
> >   
>              
> Where is it documented?
> 
> IMHO we should only guarantee that writes
> to [0, 1] won't fail because of lack of disk space.
> Other things (including file size) are up to implementation.
> 

This one looks like a pretty simple bug in the fallocate call.  It is
rounding up the size to a block boundary.

-chris

> > Actual: A block is allocated. File length is set to 1 block (4096
> > bytes). The rest of the file is filled with zeros.
> >
> > 	last_byte = min(extent_map_end(em), alloc_end);
> > 	last_byte = (last_byte + mask) & ~mask;
> > 	if (em->block_start == EXTENT_MAP_HOLE) {
> > 		ret = prealloc_file_range(trans, inode, cur_offset,
> > 				last_byte, locked_end + 1,
> > 				alloc_hint, mode);
> >
> > That part seems strange to me. You make an effort for block size to
> > divide last_byte. But last_byte should be
> > max(i_size_read(inode), offset+len)
> > - without any rounding.
> >
> > Michael Raskin
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >
> >   
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [BUG] fallocate behavior when crossing end-of-file
  2009-05-06 14:59 ` Edward Shishkin
  2009-05-06 15:10   ` Chris Mason
@ 2009-05-06 15:12   ` Michael Raskin
  2009-05-06 15:36     ` Edward Shishkin
  1 sibling, 1 reply; 6+ messages in thread
From: Michael Raskin @ 2009-05-06 15:12 UTC (permalink / raw)
  To: Edward Shishkin; +Cc: linux-btrfs

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Edward Shishkin wrote:
>> Desired: something (probably, one block) is allocated/reserved for the
>> file. File length is set to 1 byte.
>              
> Where is it documented?
> 
> IMHO we should only guarantee that writes
> to [0, 1] won't fail because of lack of disk space.
> Other things (including file size) are up to implementation.

POSIX, SUS.

http://www.opengroup.org/onlinepubs/009695399/functions/posix_fallocate.html

<<<
If the offset+ len is beyond the current file size, then
posix_fallocate() shall adjust the file size to offset+ len. Otherwise,
the file size shall not be changed.
>>>

Michael Raskin
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iQEcBAEBAgAGBQJKAajDAAoJEE6tnN0aWvw3umYH/Rhcs2m4UosPLOsSylVAiFiB
rRJbeYWX5xaw9wTCV7RW3BkAEM2gJ5De10j4x7g81L7GZQZNz3Pwu6WN+kQOrBnG
gSeCg7u5OpvJUrX6HxeJEeUOCg9VDzuTNyEWY3pUEo7L00MLuYqTQimg2pua3oAv
Ib7bUWVfRguxgQVO7pnw//CV6L+KBY7OP2za+c2x+Ec+KN0QsgAlYATxNP9+hI4o
VJL7nIGoAs5rZXkLuDJpQYgI2sl6yzZy1Jq0Q3+UnjDDLlU945yMQm4LGTEMMHJX
i/Ci58y+SNhiDFrPrW1nRCZXE0RiEk01hGTtVVI2yfZJz/dabZAtSgEbyp0k0/c=
=6Fmf
-----END PGP SIGNATURE-----

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [BUG] fallocate behavior when crossing end-of-file
  2009-05-06 15:12   ` Michael Raskin
@ 2009-05-06 15:36     ` Edward Shishkin
  2009-05-06 21:07       ` Michael Raskin
  0 siblings, 1 reply; 6+ messages in thread
From: Edward Shishkin @ 2009-05-06 15:36 UTC (permalink / raw)
  To: Michael Raskin; +Cc: linux-btrfs

Michael Raskin wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Edward Shishkin wrote:
>   
>>> Desired: something (probably, one block) is allocated/reserved for the
>>> file. File length is set to 1 byte.
>>>       
>>              
>> Where is it documented?
>>
>> IMHO we should only guarantee that writes
>> to [0, 1] won't fail because of lack of disk space.
>> Other things (including file size) are up to implementation.
>>     
>
> POSIX, SUS.
>
> http://www.opengroup.org/onlinepubs/009695399/functions/posix_fallocate.html
>
> <<<
> If the offset+ len is beyond the current file size, then
> posix_fallocate() shall adjust the file size to offset+ len. Otherwise,
> the file size shall not be changed.
>   

fallocate (2) is something different from posix_fallocate:

"...This default behavior closely resembles the behavior
of the posix_fallocate(3) library function, and is intended
as a method of optimally implementing that function."

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [BUG] fallocate behavior when crossing end-of-file
  2009-05-06 15:36     ` Edward Shishkin
@ 2009-05-06 21:07       ` Michael Raskin
  0 siblings, 0 replies; 6+ messages in thread
From: Michael Raskin @ 2009-05-06 21:07 UTC (permalink / raw)
  To: Edward Shishkin; +Cc: linux-btrfs

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Edward Shishkin wrote:
>> <<<
>> If the offset+ len is beyond the current file size, then
>> posix_fallocate() shall adjust the file size to offset+ len. Otherwise,
>> the file size shall not be changed.
>>   
> 
> fallocate (2) is something different from posix_fallocate:
> 
> "...This default behavior closely resembles the behavior
> of the posix_fallocate(3) library function, and is intended
> as a method of optimally implementing that function."

If you overincrement file size counter, the only way for glibc to
correctly implement is to ftruncate after fallocate . It doesn't look
like an optimal implementation of posix_fallocate (2 system calls). It
also doesn't look like something glibc would do. It currently doesn't.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iQEcBAEBAgAGBQJKAfwBAAoJEE6tnN0aWvw3NQAH/2XFOZE/9MBAxySxOyNY9Gft
ttotQ4+g7SuKX47tQzskcWde3qbofEYzrsF/ADebjczEwbiKcZaJwlLzqL1X6siC
IWzBS8hIWbbKkyF2erQJP3QZEJq/IwRCK6bOj8EJckGJ6q6tLCIasXGs+GOAjhrB
1NWjDfprXP5M4s1xvG9KRH4Lo38Bz4/5w7ZGqGuXOy+M21ZHim+o9Tbkp7LaV9KP
OHqxgXV6tpHHv+Ai16cR1cJzY8XAIZ6EsL4lfpw7nZGlgRC+snccNdyoN3MltFOG
fbqIxrcPb8jL+3sAY+F1EK9oOKrCjQhY8VKxksy++M8+uP7OmgRiRkPyNewbYlw=
=fY9t
-----END PGP SIGNATURE-----

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2009-05-06 21:07 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-05-06 13:03 [BUG] fallocate behavior when crossing end-of-file Raskin Michael
2009-05-06 14:59 ` Edward Shishkin
2009-05-06 15:10   ` Chris Mason
2009-05-06 15:12   ` Michael Raskin
2009-05-06 15:36     ` Edward Shishkin
2009-05-06 21:07       ` Michael Raskin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox