Re: fallocate on XFS for swap

["Besogonov, Aleksei" <cyberax@amazon.com>]

On 3/9/18, 16:58, "Dave Chinner" <david@fromorbit.com> wrote:
    On Fri, Mar 09, 2018 at 03:44:22PM -0800, Darrick J. Wong wrote:
    > [you really ought to cc the xfs list]
    > 
    > On Fri, Mar 09, 2018 at 10:05:24PM +0000, Besogonov, Aleksei wrote:
    > > Hi!
    > > 
    > > We’re working at Amazon on making XFS our default root filesystem for
    > > the upcoming Amazon Linux 2 (now in prod preview). One of the problems
    > > that we’ve encountered is inability to use fallocated files for swap
    > > on XFS. This is really important for us, since we’re shipping our
    > > current Amazon Linux with hibernation support .
    > 
    > <shudder>
    > 
    > > I’ve traced the problem to bmap(), used in generic_swapfile_activate
    > > call, which returns 0 for blocks inside holes created by fallocate and
    > > Dave Chinner confirmed it in a private email. I’m thinking about ways
    > > to fix it, so far I see the following possibilities:
    > > 
    > > 1. Change bmap() to not return zeroes for blocks inside holes. But
    > > this is an ABI change and it likely will break some obscure userspace
    > > utility somewhere.
    > 
    > bmap is a horrible interface, let's leave it to wither and eventually go
    > away.
    > 
    > > 2. Change generic_swap_activate to use a more modern interface, by
    > > adding fiemap-like operation to address_space_operations with fallback
    > > on bmap().
    > 
    > Probably the best idea, but see fs/iomap.c since we're basically leasing
    > a chunk of file space to the kernel.  Leasing space to a user that wants
    > direct access is becoming rather common (rdma, map_sync, etc.)
    
    thing is, we don't want in-kernel users of fiemap. We've got other
    block mapping interfaces that can be used, such as iomap...
    
    > > 3. Add an XFS-specific implementation of swapfile_activate.
    > 
    > Ugh no.
    
    What we want is an iomap-based re-implementation of
    generic_swap_activate(). One of the ways to plumb that in is to
    use ->swapfile_activate() like so:
    
    iomap_swapfile_activate()
    {
    	return iomap_apply(... iomap_swapfile_add_extent, ...)
    }
    
    xfs_vm_swapfile_activate()
    {
    	return iomap_swapfile_activate(xfs_iomap_ops);
    }
    
    	.swapfile_activate = xfs_vm_swapfile_activate()
    
    And massage the swapfile_activate callout be friendly to fragmented
    files. i.e. change the nfs caller to run a
    "add_single_swap_extent()" caller rather than have to do it in the
    generic code on return....

This sounds reasonable, I'll try to implement it this week or so for XFS.
No guarantees about NFS, though.