fallocate on ext4 creating holes

fallocate on ext4 creating holes

[Thomas Stringer]

I've found that in recent kernel versions (5.7+) that fallocate creates 
files on ext4 filesystem with holes. Here is a repo:

$ sudo fallocate -l 512MiB /mnt/swapfile
$ sudo filefrag -e /mnt/swapfile
Filesystem type is: ef53
File size of /mnt/swapfile is 536870912 (131072 blocks of 4096 bytes)
  ext:     logical_offset:        physical_offset: length: expected: flags:
    0:        0..   63487:      34816..     98303: 63488:             
unwritten
    1:    63488..  126975:     100352..    163839:  63488: 98304: unwritten
    2:   126976..  131071:     165888..    169983:   4096: 163840: 
last,unwritten,eof
/mnt/swapfile: 3 extents found
$ sudo chmod 0600 /mnt/swapfile
$ sudo mkswap /mnt/swapfile
$ sudo swapon /mnt/swapfile
swapon: /mnt/swapfile: swapon failed: Invalid argument
$ sudo journalctl | grep hole
Oct 15 15:42:59 hostname kernel: swapon: swapfile has holes
Oct 15 15:43:09 hostname kernel: swapon: swapfile has holes
Oct 15 15:43:23 hostname kernel: swapon: swapfile has holes

But in previous versions (4.19 verified) this operation succeeds and 
swapon doesn't indicate holes in the file.

Is the behavior of creating files with fallocate on ext4 filesystems 
with holes expected and by-design?

Thank you in advance for any insight!
Thomas

Re: fallocate on ext4 creating holes

[Theodore Y. Ts'o]

On Fri, Oct 16, 2020 at 02:30:10PM -0400, Thomas Stringer wrote:
> I've found that in recent kernel versions (5.7+) that fallocate creates
> files on ext4 filesystem with holes.

It's not actually that it is creating holes, but rather it was fallouth
from switching ext4 to use iomap for FIBMAP/FIEMAP.

The following patch is queued up to be pushed to Linus shortly, and
will be backported to stable kernels.
 
Apologies for the inconvenience.  This would have been pushed to Linus
before 5.8 was cut, but an unrelated kernel bug that showed up after
5.8-rc1 and was causing very hard-to-reproduce memory corruptions
(although it was happening quite reliably in my test setup) caused my
regression testing to go completely dark for most of the 5.8-rcX
development, and by the time the bug was finally fixed (around -rc6 if
I remember correctly), I decided to wait until the 5.9 merge window.

  	   	       	 	    - Ted

commit 0e6895ba00b7be45f3ab0d2107dda3ef1245f5b4
Author: Ritesh Harjani <riteshh@linux.ibm.com>
Date:   Fri Sep 4 14:46:53 2020 +0530

    ext4: implement swap_activate aops using iomap
    
    After moving ext4's bmap to iomap interface, swapon functionality
    on files created using fallocate (which creates unwritten extents) are
    failing. This is since iomap_bmap interface returns 0 for unwritten
    extents and thus generic_swapfile_activate considers this as holes
    and hence bail out with below kernel msg :-
    
    [340.915835] swapon: swapfile has holes
    
    To fix this we need to implement ->swap_activate aops in ext4
    which will use ext4_iomap_report_ops. Since we only need to return
    the list of extents so ext4_iomap_report_ops should be enough.
    
    Cc: stable@kernel.org
    Reported-by: Yuxuan Shui <yshuiv7@gmail.com>
    Fixes: ac58e4fb03f ("ext4: move ext4 bmap to use iomap infrastructure")
    Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com>
    Link: https://lore.kernel.org/r/20200904091653.1014334-1-riteshh@linux.ibm.com
    Signed-off-by: Theodore Ts'o <tytso@mit.edu>

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index bf596467c234..771ed8b1fadb 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -3601,6 +3601,13 @@ static int ext4_set_page_dirty(struct page *page)
 	return __set_page_dirty_buffers(page);
 }
 
+static int ext4_iomap_swap_activate(struct swap_info_struct *sis,
+				    struct file *file, sector_t *span)
+{
+	return iomap_swapfile_activate(sis, file, span,
+				       &ext4_iomap_report_ops);
+}
+
 static const struct address_space_operations ext4_aops = {
 	.readpage		= ext4_readpage,
 	.readahead		= ext4_readahead,
@@ -3616,6 +3623,7 @@ static const struct address_space_operations ext4_aops = {
 	.migratepage		= buffer_migrate_page,
 	.is_partially_uptodate  = block_is_partially_uptodate,
 	.error_remove_page	= generic_error_remove_page,
+	.swap_activate		= ext4_iomap_swap_activate,
 };
 
 static const struct address_space_operations ext4_journalled_aops = {
@@ -3632,6 +3640,7 @@ static const struct address_space_operations ext4_journalled_aops = {
 	.direct_IO		= noop_direct_IO,
 	.is_partially_uptodate  = block_is_partially_uptodate,
 	.error_remove_page	= generic_error_remove_page,
+	.swap_activate		= ext4_iomap_swap_activate,
 };
 
 static const struct address_space_operations ext4_da_aops = {
@@ -3649,6 +3658,7 @@ static const struct address_space_operations ext4_da_aops = {
 	.migratepage		= buffer_migrate_page,
 	.is_partially_uptodate  = block_is_partially_uptodate,
 	.error_remove_page	= generic_error_remove_page,
+	.swap_activate		= ext4_iomap_swap_activate,
 };
 
 static const struct address_space_operations ext4_dax_aops = {
@@ -3657,6 +3667,7 @@ static const struct address_space_operations ext4_dax_aops = {
 	.set_page_dirty		= noop_set_page_dirty,
 	.bmap			= ext4_bmap,
 	.invalidatepage		= noop_invalidatepage,
+	.swap_activate		= ext4_iomap_swap_activate,
 };
 
 void ext4_set_aops(struct inode *inode)