Hi Willy, Trond, Christoph,
Here's v3 of a change to make reads and writes from the swapfile use async
DIO, adding a new ->swap_rw() address_space method, rather than readpage()
or direct_IO(), as requested by Willy. This allows NFS to bypass the write
checks that prevent swapfiles from working, plus a bunch of other checks
that may or may not be necessary.
Whilst trying to make this work, I found that NFS's support for swapfiles
seems to have been non-functional since Aug 2019 (I think), so the first
patch fixes that. Question is: do we actually *want* to keep this
functionality, given that it seems that no one's tested it with an upstream
kernel in the last couple of years?
There are additional patches to get rid of noop_direct_IO and replace it
with a feature bitmask, to make btrfs, ext4, xfs and raw blockdevs use the
new ->swap_rw method and thence remove the direct BIO submission paths from
swap.
I kept the IOCB_SWAP flag, using it to enable REQ_SWAP. I'm not sure if
that's necessary, but it seems accounting related.
The synchronous DIO I/O code on NFS, raw blockdev, ext4 swapfile and xfs
swapfile all seem to work fine. Btrfs refuses to swapon because the file
might be CoW'd. I've tried doing "chattr +C", but that didn't help.
The async DIO paths fail spectacularly (from I/O errors to ATA failure
messages on the test disk using a normal swapspace); NFS just hangs.
My patches can be found here also:
https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=swap-dio
I tested this using the procedure and program outlined in the NFS patch.
I also encountered occasional instances of the following warning with NFS, so
I'm wondering if there's a scheduling problem somewhere:
BUG: workqueue lockup - pool cpus=0-3 flags=0x5 nice=0 stuck for 34s!
Showing busy workqueues and worker pools:
workqueue events: flags=0x0
pwq 6: cpus=3 node=0 flags=0x0 nice=0 active=1/256 refcnt=2
in-flight: 1565:fill_page_cache_func
workqueue events_highpri: flags=0x10
pwq 3: cpus=1 node=0 flags=0x1 nice=-20 active=1/256 refcnt=2
in-flight: 1547:fill_page_cache_func
pwq 1: cpus=0 node=0 flags=0x0 nice=-20 active=1/256 refcnt=2
in-flight: 1811:fill_page_cache_func
workqueue events_unbound: flags=0x2
pwq 8: cpus=0-3 flags=0x5 nice=0 active=3/512 refcnt=5
pending: fsnotify_connector_destroy_workfn, fsnotify_mark_destroy_workfn, cleanup_offline_cgwbs_workfn
workqueue events_power_efficient: flags=0x82
pwq 8: cpus=0-3 flags=0x5 nice=0 active=4/256 refcnt=6
pending: neigh_periodic_work, neigh_periodic_work, check_lifetime, do_cache_clean
workqueue writeback: flags=0x4a
pwq 8: cpus=0-3 flags=0x5 nice=0 active=1/256 refcnt=4
in-flight: 433(RESCUER):wb_workfn
workqueue rpciod: flags=0xa
pwq 8: cpus=0-3 flags=0x5 nice=0 active=38/256 refcnt=40
in-flight: 7:rpc_async_schedule, 1609:rpc_async_schedule, 1610:rpc_async_schedule, 912:rpc_async_schedule, 1613:rpc_async_schedule, 1631:rpc_async_schedule, 34:rpc_async_schedule, 44:rpc_async_schedule
pending: rpc_async_schedule, rpc_async_schedule, rpc_async_schedule, rpc_async_schedule, rpc_async_schedule, rpc_async_schedule, rpc_async_schedule, rpc_async_schedule, rpc_async_schedule, rpc_async_schedule, rpc_async_schedule, rpc_async_schedule, rpc_async_schedule, rpc_async_schedule, rpc_async_schedule, rpc_async_schedule, rpc_async_schedule, rpc_async_schedule, rpc_async_schedule, rpc_async_schedule, rpc_async_schedule, rpc_async_schedule, rpc_async_schedule, rpc_async_schedule, rpc_async_schedule, rpc_async_schedule, rpc_async_schedule, rpc_async_schedule, rpc_async_schedule, rpc_async_schedule
workqueue ext4-rsv-conversion: flags=0x2000a
pool 1: cpus=0 node=0 flags=0x0 nice=-20 hung=59s workers=2 idle: 6
pool 3: cpus=1 node=0 flags=0x1 nice=-20 hung=43s workers=2 manager: 20
pool 6: cpus=3 node=0 flags=0x0 nice=0 hung=0s workers=3 idle: 498 29
pool 8: cpus=0-3 flags=0x5 nice=0 hung=34s workers=9 manager: 1623
pool 9: cpus=0-3 flags=0x5 nice=-20 hung=0s workers=2 manager: 5224 idle: 859
Note that this is due to DIO writes to NFS only, as far as I can tell, and
that no reads had happened yet.
Changes:
========
ver #3:
- Introduced a new ->swap_rw() method.
- Added feature support flags to the address_space_operations struct and
got rid of the checks for ->direct_() and noop_direct_IO() and
similar.
- Implemented swap_rw for nfs, adjusting the direct I/O code paths.
- Implemented swap_rw for blockdev, btrfs, ext4 and xfs.
- Got rid of the return value on swap_readpage() as it's never checked.
ver #2:
- Remove the callback param to __swap_writepage() as it's invariant.
- Allocate the kiocb on the stack in sync mode.
- Do an async DIO write if WB_SYNC_ALL isn't set.
- Try to remove the BIO submission paths.
David
Link: https://lore.kernel.org/r/162876946134.3068428.15475611190876694695.stgit@warthog.procyon.org.uk/ # v1
Link: https://lore.kernel.org/r/162879971699.3306668.8977537647318498651.stgit@warthog.procyon.org.uk/ # v2
---
David Howells (9):
mm: Remove the callback func argument from __swap_writepage()
mm: Add 'supports' field to the address_space_operations to list features
mm: Make swap_readpage() void
Introduce IOCB_SWAP kiocb flag to trigger REQ_SWAP
mm: Make swap_readpage() for SWP_FS_OPS use ->swap_rw() not ->readpage()
mm: Make __swap_writepage() do async DIO if asked for it
nfs: Fix write to swapfile failure due to generic_write_checks()
block, btrfs, ext4, xfs: Implement swap_rw
mm: Remove swap BIO paths and only use DIO paths
Documentation/filesystems/vfs.rst | 8 +
block/fops.c | 2 +
drivers/block/loop.c | 6 +-
fs/9p/vfs_addr.c | 1 +
fs/affs/file.c | 1 +
fs/btrfs/inode.c | 14 +-
fs/ceph/addr.c | 13 +-
fs/cifs/file.c | 21 +-
fs/direct-io.c | 2 +
fs/erofs/data.c | 2 +-
fs/exfat/inode.c | 1 +
fs/ext2/inode.c | 4 +-
fs/ext4/inode.c | 17 +-
fs/f2fs/data.c | 1 +
fs/fat/inode.c | 1 +
fs/fcntl.c | 2 +-
fs/fuse/dax.c | 2 +-
fs/fuse/file.c | 1 +
fs/gfs2/aops.c | 2 +-
fs/hfs/inode.c | 1 +
fs/hfsplus/inode.c | 1 +
fs/jfs/inode.c | 1 +
fs/libfs.c | 12 -
fs/nfs/direct.c | 28 +--
fs/nfs/file.c | 15 +-
fs/nilfs2/inode.c | 1 +
fs/ntfs3/inode.c | 1 +
fs/ocfs2/aops.c | 1 +
fs/open.c | 3 +-
fs/orangefs/inode.c | 1 +
fs/overlayfs/file.c | 2 +-
fs/overlayfs/inode.c | 3 +-
fs/reiserfs/inode.c | 1 +
fs/udf/file.c | 1 +
fs/udf/inode.c | 1 +
fs/xfs/xfs_aops.c | 13 +-
fs/zonefs/super.c | 2 +-
include/linux/bio.h | 2 +
include/linux/fs.h | 7 +-
include/linux/nfs_fs.h | 2 +-
include/linux/swap.h | 2 +-
mm/page_io.c | 356 +++++++++++++++---------------
mm/swapfile.c | 4 +-
43 files changed, 275 insertions(+), 287 deletions(-)
Implement swap_rw for block devices, btrfs, ext4 and xfs. This allows the
the page swapping code to use direct-IO rather than direct bio submission,
whilst skipping the checks going via read/write_iter would entail.
Signed-off-by: David Howells <[email protected]>
cc: Matthew Wilcox <[email protected]>
cc: Christoph Hellwig <[email protected]>
cc: Jens Axboe <[email protected]>
cc: Chris Mason <[email protected]>
cc: Josef Bacik <[email protected]>
cc: David Sterba <[email protected]>
cc: "Theodore Ts'o" <[email protected]>
cc: Andreas Dilger <[email protected]>
cc: Darrick J. Wong <[email protected]>
cc: [email protected]
cc: [email protected]
cc: [email protected]
cc: [email protected]
cc: [email protected]
cc: [email protected]
---
block/fops.c | 1 +
fs/btrfs/inode.c | 12 +++++-------
fs/ext4/inode.c | 9 +++++++++
fs/xfs/xfs_aops.c | 9 +++++++++
4 files changed, 24 insertions(+), 7 deletions(-)
diff --git a/block/fops.c b/block/fops.c
index 84c64d814d0d..7ba37dfafae2 100644
--- a/block/fops.c
+++ b/block/fops.c
@@ -382,6 +382,7 @@ const struct address_space_operations def_blk_aops = {
.write_end = blkdev_write_end,
.writepages = blkdev_writepages,
.direct_IO = blkdev_direct_IO,
+ .swap_rw = blkdev_direct_IO,
.migratepage = buffer_migrate_page_norefs,
.is_dirty_writeback = buffer_check_dirty_writeback,
.supports = AS_SUPPORTS_DIRECT_IO,
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index b479c97e42fc..9ffcefecb3bb 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -10852,15 +10852,10 @@ static int btrfs_swap_activate(struct swap_info_struct *sis, struct file *file,
sis->highest_bit = bsi.nr_pages - 1;
return bsi.nr_extents;
}
-#else
-static void btrfs_swap_deactivate(struct file *file)
-{
-}
-static int btrfs_swap_activate(struct swap_info_struct *sis, struct file *file,
- sector_t *span)
+static ssize_t btrfs_swap_rw(struct kiocb *iocb, struct iov_iter *iter)
{
- return -EOPNOTSUPP;
+ return iomap_dio_rw(iocb, iter, &btrfs_dio_iomap_ops, NULL, 0);
}
#endif
@@ -10944,8 +10939,11 @@ static const struct address_space_operations btrfs_aops = {
#endif
.set_page_dirty = btrfs_set_page_dirty,
.error_remove_page = generic_error_remove_page,
+#ifdef CONFIG_SWAP
.swap_activate = btrfs_swap_activate,
.swap_deactivate = btrfs_swap_deactivate,
+ .swap_rw = btrfs_swap_rw,
+#endif
.supports = AS_SUPPORTS_DIRECT_IO,
};
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 08d3541d8daa..3c14724d58a8 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -3651,6 +3651,11 @@ static int ext4_iomap_swap_activate(struct swap_info_struct *sis,
&ext4_iomap_report_ops);
}
+static ssize_t ext4_swap_rw(struct kiocb *iocb, struct iov_iter *iter)
+{
+ return iomap_dio_rw(iocb, iter, &ext4_iomap_ops, NULL, 0);
+}
+
static const struct address_space_operations ext4_aops = {
.readpage = ext4_readpage,
.readahead = ext4_readahead,
@@ -3666,6 +3671,7 @@ static const struct address_space_operations ext4_aops = {
.is_partially_uptodate = block_is_partially_uptodate,
.error_remove_page = generic_error_remove_page,
.swap_activate = ext4_iomap_swap_activate,
+ .swap_rw = ext4_swap_rw,
.supports = AS_SUPPORTS_DIRECT_IO,
};
@@ -3683,6 +3689,7 @@ static const struct address_space_operations ext4_journalled_aops = {
.is_partially_uptodate = block_is_partially_uptodate,
.error_remove_page = generic_error_remove_page,
.swap_activate = ext4_iomap_swap_activate,
+ .swap_rw = ext4_swap_rw,
.supports = AS_SUPPORTS_DIRECT_IO,
};
@@ -3701,6 +3708,7 @@ static const struct address_space_operations ext4_da_aops = {
.is_partially_uptodate = block_is_partially_uptodate,
.error_remove_page = generic_error_remove_page,
.swap_activate = ext4_iomap_swap_activate,
+ .swap_rw = ext4_swap_rw,
.supports = AS_SUPPORTS_DIRECT_IO,
};
@@ -3710,6 +3718,7 @@ static const struct address_space_operations ext4_dax_aops = {
.bmap = ext4_bmap,
.invalidatepage = noop_invalidatepage,
.swap_activate = ext4_iomap_swap_activate,
+ .swap_rw = ext4_swap_rw,
.supports = AS_SUPPORTS_DIRECT_IO,
};
diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
index 2a4570516591..23ade2cc8241 100644
--- a/fs/xfs/xfs_aops.c
+++ b/fs/xfs/xfs_aops.c
@@ -540,6 +540,13 @@ xfs_iomap_swapfile_activate(
&xfs_read_iomap_ops);
}
+static ssize_t xfs_swap_rw(struct kiocb *iocb, struct iov_iter *iter)
+{
+ if (iocb->ki_flags & IOCB_WRITE)
+ return iomap_dio_rw(iocb, iter, &xfs_direct_write_iomap_ops, NULL, 0);
+ return iomap_dio_rw(iocb, iter, &xfs_read_iomap_ops, NULL, 0);
+}
+
const struct address_space_operations xfs_address_space_operations = {
.readpage = xfs_vm_readpage,
.readahead = xfs_vm_readahead,
@@ -552,6 +559,7 @@ const struct address_space_operations xfs_address_space_operations = {
.is_partially_uptodate = iomap_is_partially_uptodate,
.error_remove_page = generic_error_remove_page,
.swap_activate = xfs_iomap_swapfile_activate,
+ .swap_rw = xfs_swap_rw,
.supports = AS_SUPPORTS_DIRECT_IO,
};
@@ -560,5 +568,6 @@ const struct address_space_operations xfs_dax_aops = {
.set_page_dirty = __set_page_dirty_no_writeback,
.invalidatepage = noop_invalidatepage,
.swap_activate = xfs_iomap_swapfile_activate,
+ .swap_rw = xfs_swap_rw,
.supports = AS_SUPPORTS_DIRECT_IO,
};
On Fri, Sep 24, 2021 at 06:17:52PM +0100, David Howells wrote:
>
> Hi Willy, Trond, Christoph,
>
> Here's v3 of a change to make reads and writes from the swapfile use async
> DIO, adding a new ->swap_rw() address_space method, rather than readpage()
> or direct_IO(), as requested by Willy. This allows NFS to bypass the write
> checks that prevent swapfiles from working, plus a bunch of other checks
> that may or may not be necessary.
>
> Whilst trying to make this work, I found that NFS's support for swapfiles
> seems to have been non-functional since Aug 2019 (I think), so the first
> patch fixes that. Question is: do we actually *want* to keep this
> functionality, given that it seems that no one's tested it with an upstream
> kernel in the last couple of years?
>
> There are additional patches to get rid of noop_direct_IO and replace it
> with a feature bitmask, to make btrfs, ext4, xfs and raw blockdevs use the
> new ->swap_rw method and thence remove the direct BIO submission paths from
> swap.
>
> I kept the IOCB_SWAP flag, using it to enable REQ_SWAP. I'm not sure if
> that's necessary, but it seems accounting related.
>
> The synchronous DIO I/O code on NFS, raw blockdev, ext4 swapfile and xfs
> swapfile all seem to work fine. Btrfs refuses to swapon because the file
> might be CoW'd. I've tried doing "chattr +C", but that didn't help.
Ok, so if the filesystem is doing block mapping in the IO path now,
why does the swap file still need to map the file into a private
block mapping now? i.e all the work that iomap_swapfile_activate()
does for filesystems like XFS and ext4 - it's this completely
redundant now that we are doing block mapping during swap file IO
via iomap_dio_rw()?
Actually, that path does all the "can we use this file as a swap
file" checking. So the extent iteration can't go away, just the swap
file mapping part (iomap_swapfile_add_extent()). This is necessary
to ensure there aren't any holes in the file, and we still need that
because the DIO write path will allocate into holes, which leads
me to my main concern here.
Using the DIO path opens up the possibility that the filesystem
could want to run transactions are part of the DIO. Right now we
support unwritten extents for swap files (so they don't have to be
written to allocate the backing store before activation) and that
means we'll be doing DIO to unwritten extents. IO completion of a
DIO write to an unwritten extent will run a transaction to convert
that extent to written. A similar problem with sparse files exists,
because allocation of blocks can be done from the DIO path, and that
requires transactions. File extension is another potential
transaction path we open up by using DIO writes dor swap.
The problem is that a transaction run in swap IO context will will
deadlock the filesystem. Either through the unbound memory demand of
metadata modification, or from needing log space that can't be freed
up because the metadata IO that will free the log space is waiting
on memory allocation that is waiting on swap IO...
I think some more thought needs to be put into controlling the
behaviour/semantics of the DIO path so that it can be safely used
by swap IO, because it's not a direct 1:1 behavioural mapping with
existing DIO and there are potential deadlock vectors we need to
avoid.
Cheers,
Dave.
--
Dave Chinner
[email protected]
On Sun, Sep 26, 2021 at 09:42:43AM +1000, Dave Chinner wrote:
> Ok, so if the filesystem is doing block mapping in the IO path now,
> why does the swap file still need to map the file into a private
> block mapping now? i.e all the work that iomap_swapfile_activate()
> does for filesystems like XFS and ext4 - it's this completely
> redundant now that we are doing block mapping during swap file IO
> via iomap_dio_rw()?
Hi Dave,
Thanks for bringing up all these points. I think they all deserve to go
into the documentation as "things to consider" for people implementing
->swap_rw for their filesystem.
Something I don't think David perhaps made sufficiently clear is that
regular DIO from userspace gets handled by ->read_iter and ->write_iter.
This ->swap_rw op is used exclusive for, as the name suggests, swap DIO.
So filesystems don't have to handle swap DIO and regular DIO the same
way, and can split the allocation work between ->swap_activate and the
iomap callback as they see fit (as long as they can guarantee the lack
of deadlocks under memory pressure).
There are several advantages to using the DIO infrastructure for
swap:
- unify block & net swap paths
- allow filesystems to _see_ swap IOs instead of being bypassed
- get rid of the swap extent rbtree
- allow writing compound pages to swap files instead of splitting
them
- allow ->readpage to be synchronous for better error reporting
- remove page_file_mapping() and page_file_offset()
I suspect there are several problems with this patchset, but I'm not
likely to have a chance to read it closely for a few days. If you
have time to give the XFS parts a good look, that would be fantastic.
On Sun, Sep 26, 2021 at 04:10:43AM +0100, Matthew Wilcox wrote:
> On Sun, Sep 26, 2021 at 09:42:43AM +1000, Dave Chinner wrote:
> > Ok, so if the filesystem is doing block mapping in the IO path now,
> > why does the swap file still need to map the file into a private
> > block mapping now? i.e all the work that iomap_swapfile_activate()
> > does for filesystems like XFS and ext4 - it's this completely
> > redundant now that we are doing block mapping during swap file IO
> > via iomap_dio_rw()?
>
> Hi Dave,
>
> Thanks for bringing up all these points. I think they all deserve to go
> into the documentation as "things to consider" for people implementing
> ->swap_rw for their filesystem.
>
> Something I don't think David perhaps made sufficiently clear is that
> regular DIO from userspace gets handled by ->read_iter and ->write_iter.
> This ->swap_rw op is used exclusive for, as the name suggests, swap DIO.
> So filesystems don't have to handle swap DIO and regular DIO the same
> way, and can split the allocation work between ->swap_activate and the
> iomap callback as they see fit (as long as they can guarantee the lack
> of deadlocks under memory pressure).
I understand this completely.
The point is that the implementation of ->swap_rw is to call
iomap_dio_rw() with the same ops as the normal DIO read/write path
uses. IOWs, apart from the IOCB_SWAP flag, there is no practical
difference between the "swap DIO" and "normal DIO" I/O paths.
> There are several advantages to using the DIO infrastructure for
> swap:
>
> - unify block & net swap paths
> - allow filesystems to _see_ swap IOs instead of being bypassed
> - get rid of the swap extent rbtree
> - allow writing compound pages to swap files instead of splitting
> them
> - allow ->readpage to be synchronous for better error reporting
> - remove page_file_mapping() and page_file_offset()
>
> I suspect there are several problems with this patchset, but I'm not
> likely to have a chance to read it closely for a few days. If you
> have time to give the XFS parts a good look, that would be fantastic.
That's what I've already done, and all the questions I've raised are
from asking a simple question: what happens if a transaction is
required to complete the iomap_dio_rw() swap write operation?
I mean, this is similar to the problems with IOCB_NOWAIT - we're
supposed to return -EAGAIN if we might block during IO submission,
and one of those situations we have to consider is "do we need to
run a transaction". If we get it wrong (and we do!), then the worst
thing that happens is that there is a long latency for IO
submission. It's a minor performance issue, not the end of the
world.
The difference with IOCB_SWAP is that "don't do transactions during
iomap_dio_rw()" is a _hard requirement_ on both IO submission and
completion. That means, from now and forever, we will have to
guarantee a path through iomap_dio_rw() that will never run
transactions on an IO. That requirement needs to be enforced in
every block mapping callback into each filesystem, as this is
something the iomap infrastructure cannot enforce. Hence we'll have
to plumb IOCB_SWAP into a new IOMAP_SWAP iterator flag to pass to
the ->iomap_begin() DIO methods to ensure they do the right thing.
And then the question becomes: what happens if the filesystem cannot
do the right thing? Can the swap code handle an error? e.g. the
first thing that xfs_direct_write_iomap_begin() and
xfs_read_iomap_begin() do is check if the filesystem is shut down
and returns -EIO in that case. IOWs, we've now got normal filesystem
"reject all IO" corruption protection mechanisms in play. Using
iomap_dio_rw() as it stands means that _all swapfile IO will fail_
if the filesystem shuts down.
Right now the swap file IO can keep going blissfully unaware of the
filesystem failure status. The open swapfile will prevent the
filesystem from being unmounted. Hence to unmount the shutdown
filesystem to correct the problem, first the swap file has to be
turned off, which means we have a fail-safe behaviour. Using the
iomap_dio_rw() path means that swapfile IO _can and will fail_.
AFAICT, swap IO errors are pretty much thrown away by the mm code;
the swap_writepage() return value is ignored or placed on the swap
cache address space and ignored. And it looks like the new read path
just sets PageError() and leaves it to callers to detect and deal
with a swapin failure because swap_readpage() is now void...
So it seems like there's a whole new set of failure cases using the
DIO path introduces into the swap IO path that haven't been
considered here. I can't see why we wouldn't be able to solve them,
but these considerations lead me to think that use of the DIO is
based on an incorrect assumption - DIO is not a "simple low level
IO" interface.
Hence I suspect that we'd be much better off with a new
iomap_swap_rw() implementation that just does what swap needs
without any of the complexity of the DIO API. Internally iomap can
share what it needs to share with the DIO path, but at this point
I'm not sure we should be overloading the iomap_dio_rw() path with
the semantics required by swap.
e.g. we limit iomap_swap_rw() to only accept written or unwritten
block mappings within file size on inodes with clean metadata (i.e.
pure overwrite to guarantee no modification transactions), and then
the fs provided ->iomap_begin callback can ignore shutdown state,
elide inode level locking, do read-only mappings, etc without adding
extra overhead to the existing DIO code path...
Cheers,
Dave.
--
Dave Chinner
[email protected]
On Fri, Sep 24, 2021 at 06:17:52PM +0100, David Howells wrote:
>
> Hi Willy, Trond, Christoph,
>
> Here's v3 of a change to make reads and writes from the swapfile use async
> DIO, adding a new ->swap_rw() address_space method, rather than readpage()
> or direct_IO(), as requested by Willy. This allows NFS to bypass the write
> checks that prevent swapfiles from working, plus a bunch of other checks
> that may or may not be necessary.
>
> Whilst trying to make this work, I found that NFS's support for swapfiles
> seems to have been non-functional since Aug 2019 (I think), so the first
> patch fixes that. Question is: do we actually *want* to keep this
> functionality, given that it seems that no one's tested it with an upstream
> kernel in the last couple of years?
>
> There are additional patches to get rid of noop_direct_IO and replace it
> with a feature bitmask, to make btrfs, ext4, xfs and raw blockdevs use the
> new ->swap_rw method and thence remove the direct BIO submission paths from
> swap.
>
> I kept the IOCB_SWAP flag, using it to enable REQ_SWAP. I'm not sure if
> that's necessary, but it seems accounting related.
>
> The synchronous DIO I/O code on NFS, raw blockdev, ext4 swapfile and xfs
> swapfile all seem to work fine. Btrfs refuses to swapon because the file
> might be CoW'd. I've tried doing "chattr +C", but that didn't help.
There was probably some step missing. The file must not have holes, so
either do 'dd' to the right size or use fallocate (which is recommended
in manual page btrfs(5) SWAPFILE SUPPORT). There are some fstests
exercising swapfile (grep -l _format_swapfile tests/generic/*) so you
could try that without having to set up the swapfile manually.
On Sat, 25 Sep 2021, David Howells wrote:
> Whilst trying to make this work, I found that NFS's support for swapfiles
> seems to have been non-functional since Aug 2019 (I think), so the first
> patch fixes that. Question is: do we actually *want* to keep this
> functionality, given that it seems that no one's tested it with an upstream
> kernel in the last couple of years?
SUSE definitely want to keep this functionality. We have customers
using it.
I agree it would be good if it was being tested somewhere....
Thanks,
NeilBrown
David Sterba <[email protected]> wrote:
> > There are additional patches to get rid of noop_direct_IO and replace it
> > with a feature bitmask, to make btrfs, ext4, xfs and raw blockdevs use the
> > new ->swap_rw method and thence remove the direct BIO submission paths from
> > swap.
> >
> > I kept the IOCB_SWAP flag, using it to enable REQ_SWAP. I'm not sure if
> > that's necessary, but it seems accounting related.
>
> There was probably some step missing. The file must not have holes, so
> either do 'dd' to the right size or use fallocate (which is recommended
> in manual page btrfs(5) SWAPFILE SUPPORT). There are some fstests
> exercising swapfile (grep -l _format_swapfile tests/generic/*) so you
> could try that without having to set up the swapfile manually.
Yeah. As advised elsewhere, I removed the file and recreated it, doing the
chattr before extending the file. At that point swapon worked. It didn't
work though, and various userspace programs started dying. I'm guessing my
btrfs_swap_rw() is wrong somehow.
David