linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 1/2] iomap: add a swapfile activation function
@ 2018-05-03 17:46 Darrick J. Wong
  2018-05-03 17:49 ` [PATCH v3 2/2] generic: test swapfile creation, activation, and deactivation Darrick J. Wong
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Darrick J. Wong @ 2018-05-03 17:46 UTC (permalink / raw)
  To: xfs, linux-fsdevel, linux-mm; +Cc: hch, cyberax, jack, osandov, Eryu Guan

From: Darrick J. Wong <darrick.wong@oracle.com>

Add a new iomap_swapfile_activate function so that filesystems can
activate swap files without having to use the obsolete and slow bmap
function.  This enables XFS to support fallocate'd swap files and
swap files on realtime devices.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
v3: catch null iomap addr, fix too-short extent detection
v2: document the swap file layout requirements, combine adjacent
    real/unwritten extents, align reported swap extents to physical page
    size boundaries, fix compiler errors when swap disabled
---
 fs/iomap.c            |  162 +++++++++++++++++++++++++++++++++++++++++++++++++
 fs/xfs/xfs_aops.c     |   12 ++++
 include/linux/iomap.h |   11 +++
 3 files changed, 185 insertions(+)

diff --git a/fs/iomap.c b/fs/iomap.c
index afd163586aa0..ac7115492366 100644
--- a/fs/iomap.c
+++ b/fs/iomap.c
@@ -27,6 +27,7 @@
 #include <linux/task_io_accounting_ops.h>
 #include <linux/dax.h>
 #include <linux/sched/signal.h>
+#include <linux/swap.h>
 
 #include "internal.h"
 
@@ -1089,3 +1090,164 @@ iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
 	return ret;
 }
 EXPORT_SYMBOL_GPL(iomap_dio_rw);
+
+/* Swapfile activation */
+
+#ifdef CONFIG_SWAP
+struct iomap_swapfile_info {
+	struct iomap iomap;		/* accumulated iomap */
+	struct swap_info_struct *sis;
+	uint64_t lowest_ppage;		/* lowest physical addr seen (pages) */
+	uint64_t highest_ppage;		/* highest physical addr seen (pages) */
+	unsigned long nr_pages;		/* number of pages collected */
+	int nr_extents;			/* extent count */
+};
+
+/*
+ * Collect physical extents for this swap file.  Physical extents reported to
+ * the swap code must be trimmed to align to a page boundary.  The logical
+ * offset within the file is irrelevant since the swapfile code maps logical
+ * page numbers of the swap device to the physical page-aligned extents.
+ */
+static int iomap_swapfile_add_extent(struct iomap_swapfile_info *isi)
+{
+	struct iomap *iomap = &isi->iomap;
+	unsigned long nr_pages;
+	uint64_t first_ppage;
+	uint64_t first_ppage_reported;
+	uint64_t last_ppage;
+	int error;
+
+	/*
+	 * Round the start up and the end down so that the physical
+	 * extent aligns to a page boundary.
+	 */
+	first_ppage = ALIGN(iomap->addr, PAGE_SIZE) >> PAGE_SHIFT;
+	last_ppage = (ALIGN_DOWN(iomap->addr + iomap->length, PAGE_SIZE) >>
+			PAGE_SHIFT) - 1;
+
+	/* Skip too-short physical extents. */
+	if (first_ppage > last_ppage)
+		return 0;
+	nr_pages = last_ppage - first_ppage + 1;
+
+	/*
+	 * Calculate how much swap space we're adding; the first page contains
+	 * the swap header and doesn't count.  The mm still wants that first
+	 * page fed to add_swap_extent, however.
+	 */
+	first_ppage_reported = first_ppage;
+	if (iomap->offset == 0)
+		first_ppage_reported++;
+	if (isi->lowest_ppage > first_ppage_reported)
+		isi->lowest_ppage = first_ppage_reported;
+	if (isi->highest_ppage < last_ppage)
+		isi->highest_ppage = last_ppage;
+
+	/* Add extent, set up for the next call. */
+	error = add_swap_extent(isi->sis, isi->nr_pages, nr_pages, first_ppage);
+	if (error < 0)
+		return error;
+	isi->nr_extents += error;
+	isi->nr_pages += nr_pages;
+	return 0;
+}
+
+/*
+ * Accumulate iomaps for this swap file.  We have to accumulate iomaps because
+ * swap only cares about contiguous page-aligned physical extents and makes no
+ * distinction between written and unwritten extents.
+ */
+static loff_t iomap_swapfile_activate_actor(struct inode *inode, loff_t pos,
+		loff_t count, void *data, struct iomap *iomap)
+{
+	struct iomap_swapfile_info *isi = data;
+	int error;
+
+	/* Skip holes. */
+	if (iomap->type == IOMAP_HOLE)
+		goto out;
+
+	/* Only one bdev per swap file. */
+	if (iomap->bdev != isi->sis->bdev)
+		goto err;
+
+	/* Only real or unwritten extents. */
+	if (iomap->type != IOMAP_MAPPED && iomap->type != IOMAP_UNWRITTEN)
+		goto err;
+
+	/* No uncommitted metadata or shared blocks or inline data. */
+	if (iomap->flags & (IOMAP_F_DIRTY | IOMAP_F_SHARED |
+			    IOMAP_F_DATA_INLINE))
+		goto err;
+
+	/* No null physical addresses. */
+	if (iomap->addr == IOMAP_NULL_ADDR)
+		goto err;
+
+	if (isi->iomap.length == 0) {
+		/* No accumulated extent, so just store it. */
+		memcpy(&isi->iomap, iomap, sizeof(isi->iomap));
+	} else if (isi->iomap.addr + isi->iomap.length == iomap->addr) {
+		/* Append this to the accumulated extent. */
+		isi->iomap.length += iomap->length;
+	} else {
+		/* Otherwise, add the retained iomap and store this one. */
+		error = iomap_swapfile_add_extent(isi);
+		if (error)
+			return error;
+		memcpy(&isi->iomap, iomap, sizeof(isi->iomap));
+	}
+out:
+	return count;
+err:
+	pr_err("swapon: file cannot be used for swap\n");
+	return -EINVAL;
+}
+
+/*
+ * Iterate a swap file's iomaps to construct physical extents that can be
+ * passed to the swapfile subsystem.
+ */
+int iomap_swapfile_activate(struct swap_info_struct *sis,
+		struct file *swap_file, sector_t *pagespan,
+		const struct iomap_ops *ops)
+{
+	struct iomap_swapfile_info isi = {
+		.sis = sis,
+		.lowest_ppage = (sector_t)-1ULL,
+	};
+	struct address_space *mapping = swap_file->f_mapping;
+	struct inode *inode = mapping->host;
+	loff_t pos = 0;
+	loff_t len = ALIGN_DOWN(i_size_read(inode), PAGE_SIZE);
+	loff_t ret;
+
+	ret = filemap_write_and_wait(inode->i_mapping);
+	if (ret)
+		return ret;
+
+	while (len > 0) {
+		ret = iomap_apply(inode, pos, len, IOMAP_REPORT,
+				ops, &isi, iomap_swapfile_activate_actor);
+		if (ret <= 0)
+			return ret;
+
+		pos += ret;
+		len -= ret;
+	}
+
+	if (isi.iomap.length) {
+		ret = iomap_swapfile_add_extent(&isi);
+		if (ret)
+			return ret;
+	}
+
+	*pagespan = 1 + isi.highest_ppage - isi.lowest_ppage;
+	sis->max = isi.nr_pages;
+	sis->pages = isi.nr_pages - 1;
+	sis->highest_bit = isi.nr_pages - 1;
+	return isi.nr_extents;
+}
+EXPORT_SYMBOL_GPL(iomap_swapfile_activate);
+#endif /* CONFIG_SWAP */
diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
index 0ab824f574ed..80de476cecf8 100644
--- a/fs/xfs/xfs_aops.c
+++ b/fs/xfs/xfs_aops.c
@@ -1475,6 +1475,16 @@ xfs_vm_set_page_dirty(
 	return newly_dirty;
 }
 
+static int
+xfs_iomap_swapfile_activate(
+	struct swap_info_struct		*sis,
+	struct file			*swap_file,
+	sector_t			*span)
+{
+	sis->bdev = xfs_find_bdev_for_inode(file_inode(swap_file));
+	return iomap_swapfile_activate(sis, swap_file, span, &xfs_iomap_ops);
+}
+
 const struct address_space_operations xfs_address_space_operations = {
 	.readpage		= xfs_vm_readpage,
 	.readpages		= xfs_vm_readpages,
@@ -1488,6 +1498,7 @@ const struct address_space_operations xfs_address_space_operations = {
 	.migratepage		= buffer_migrate_page,
 	.is_partially_uptodate  = block_is_partially_uptodate,
 	.error_remove_page	= generic_error_remove_page,
+	.swap_activate		= xfs_iomap_swapfile_activate,
 };
 
 const struct address_space_operations xfs_dax_aops = {
@@ -1495,4 +1506,5 @@ const struct address_space_operations xfs_dax_aops = {
 	.direct_IO		= noop_direct_IO,
 	.set_page_dirty		= noop_set_page_dirty,
 	.invalidatepage		= noop_invalidatepage,
+	.swap_activate		= xfs_iomap_swapfile_activate,
 };
diff --git a/include/linux/iomap.h b/include/linux/iomap.h
index 19a07de28212..4bd87294219a 100644
--- a/include/linux/iomap.h
+++ b/include/linux/iomap.h
@@ -106,4 +106,15 @@ typedef int (iomap_dio_end_io_t)(struct kiocb *iocb, ssize_t ret,
 ssize_t iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
 		const struct iomap_ops *ops, iomap_dio_end_io_t end_io);
 
+#ifdef CONFIG_SWAP
+struct file;
+struct swap_info_struct;
+
+int iomap_swapfile_activate(struct swap_info_struct *sis,
+		struct file *swap_file, sector_t *pagespan,
+		const struct iomap_ops *ops);
+#else
+# define iomap_swapfile_activate(sis, swapfile, pagespan, ops)	(-EIO)
+#endif /* CONFIG_SWAP */
+
 #endif /* LINUX_IOMAP_H */

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

* [PATCH v3 2/2] generic: test swapfile creation, activation, and deactivation
  2018-05-03 17:46 [PATCH v3 1/2] iomap: add a swapfile activation function Darrick J. Wong
@ 2018-05-03 17:49 ` Darrick J. Wong
  2018-05-03 20:58 ` [PATCH v3 1/2] iomap: add a swapfile activation function Omar Sandoval
  2018-05-09 15:20 ` Jan Kara
  2 siblings, 0 replies; 7+ messages in thread
From: Darrick J. Wong @ 2018-05-03 17:49 UTC (permalink / raw)
  To: xfs, linux-fsdevel, linux-mm
  Cc: hch, cyberax, jack, osandov, Eryu Guan, fstests

From: Darrick J. Wong <darrick.wong@oracle.com>

Test swapfile activation and deactivation.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 tests/generic/708     |  150 +++++++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/708.out |   10 +++
 tests/generic/group   |    1 
 3 files changed, 161 insertions(+)
 create mode 100755 tests/generic/708
 create mode 100644 tests/generic/708.out

diff --git a/tests/generic/708 b/tests/generic/708
new file mode 100755
index 00000000..1c576b39
--- /dev/null
+++ b/tests/generic/708
@@ -0,0 +1,150 @@
+#! /bin/bash
+# FS QA Test No. 708
+#
+# Test various swapfile activation oddities.
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2018 Oracle.  All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#
+#-----------------------------------------------------------------------
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -f $testfile
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+# real QA test starts here
+_supported_fs generic
+_supported_os Linux
+_require_scratch_swapfile
+
+rm -f $seqres.full
+_scratch_mkfs >>$seqres.full 2>&1
+_scratch_mount >>$seqres.full 2>&1
+
+swapfile=$SCRATCH_MNT/swap
+len=$((2 * 1048576))
+
+swapfile_cycle() {
+	local swapfile="$1"
+
+	mkswap $swapfile >> $seqres.full
+	filefrag -v $swapfile >> $seqres.full
+	swapon $swapfile 2>&1 | _filter_scratch
+	swapon -v --bytes >> $seqres.full
+	swapoff $swapfile 2>> $seeqres.full
+	rm -f $swapfile
+}
+
+test_can_falloc_swap() {
+	local test_swapfile=$TEST_DIR/swapfile
+
+	echo "can we fallocate swap?"
+	$XFS_IO_PROG -f -c "falloc 0 64k" $test_swapfile
+	test -f $test_swapfile || return 1
+	mkswap $test_swapfile
+	swapon $test_swapfile
+	res=$?
+	swapoff $test_swapfile
+	rm -f $test_swapfile
+	return $res
+}
+
+unset can_falloc_swap
+test_can_falloc_swap >> $seqres.full 2>&1 && can_falloc_swap=yes
+page_size=$(get_page_size)
+
+# Create a sparse swap file
+echo "sparse swap" | tee -a $seqres.full
+$XFS_IO_PROG -f -c "truncate $len" $swapfile >> $seqres.full
+swapfile_cycle $swapfile
+
+# Create a regular swap file
+echo "regular swap" | tee -a $seqres.full
+_pwrite_byte 0x58 0 $len $swapfile >> $seqres.full
+swapfile_cycle $swapfile
+
+# Create a fallocated swap file
+echo "fallocate swap" | tee -a $seqres.full
+if [ -n "$can_falloc_swap" ]; then
+	$XFS_IO_PROG -f -c "falloc 0 $len" $swapfile >> $seqres.full
+	swapfile_cycle $swapfile
+fi
+
+# Create a swap file with a little too much junk on the end
+echo "too long swap" | tee -a $seqres.full
+_pwrite_byte 0x58 0 $((len + 3)) $swapfile >> $seqres.full
+swapfile_cycle $swapfile
+
+# Create a swap file with a large discontiguous range(?)
+echo "large discontig swap" | tee -a $seqres.full
+_pwrite_byte 0x58 0 $((len * 2)) $swapfile >> $seqres.full
+old_sz="$(stat -c '%s' $swapfile)"
+$XFS_IO_PROG -c "fcollapse $((len / 2)) $len" $swapfile >> $seqres.full 2>&1
+new_sz="$(stat -c '%s' $swapfile)"
+if [ $old_sz -gt $new_sz ]; then
+	swapfile_cycle $swapfile
+fi
+rm -f $swapfile
+
+# Create a swap file with a small discontiguous range(?)
+echo "small discontig swap" | tee -a $seqres.full
+_pwrite_byte 0x58 0 $((len + 1024)) $swapfile >> $seqres.full
+old_sz="$(stat -c '%s' $swapfile)"
+$XFS_IO_PROG -c "fcollapse 66560 1024" $swapfile >> $seqres.full 2>&1
+new_sz="$(stat -c '%s' $swapfile)"
+if [ $old_sz -gt $new_sz ]; then
+	swapfile_cycle $swapfile
+fi
+rm -f $swapfile
+
+# Create a fallocated swap file and touch every other $PAGE_SIZE to create
+# a mess of written/unwritten extent records
+echo "mixed swap" | tee -a $seqres.full
+if [ -n "$can_falloc_swap" ]; then
+	$XFS_IO_PROG -f -c "falloc 0 $len" $swapfile >> $seqres.full
+	seq $page_size $((page_size * 2)) $len | while read offset; do
+		_pwrite_byte 0x58 $offset 1 $swapfile >> $seqres.full
+	done
+	swapfile_cycle $swapfile
+fi
+
+# Create a ridiculously small swap file; mkswap says the minimum is 40k.
+echo "tiny swap" | tee -a $seqres.full
+tiny_len=40960
+if [ "$page_size" -gt "$tiny_len" ]; then
+	tiny_len=$page_size
+fi
+_pwrite_byte 0x58 0 $tiny_len $swapfile >> $seqres.full
+swapfile_cycle $swapfile
+
+status=0
+exit
diff --git a/tests/generic/708.out b/tests/generic/708.out
new file mode 100644
index 00000000..d6199b99
--- /dev/null
+++ b/tests/generic/708.out
@@ -0,0 +1,10 @@
+QA output created by 708
+sparse swap
+swapon: SCRATCH_MNT/swap: skipping - it appears to have holes.
+regular swap
+fallocate swap
+too long swap
+large discontig swap
+small discontig swap
+mixed swap
+tiny swap
diff --git a/tests/generic/group b/tests/generic/group
index 49f5cbe1..94cbcee9 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -489,3 +489,4 @@
 484 auto quick
 600 auto quick insert
 706 auto quick attr
+708 auto quick swapfile

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

* Re: [PATCH v3 1/2] iomap: add a swapfile activation function
  2018-05-03 17:46 [PATCH v3 1/2] iomap: add a swapfile activation function Darrick J. Wong
  2018-05-03 17:49 ` [PATCH v3 2/2] generic: test swapfile creation, activation, and deactivation Darrick J. Wong
@ 2018-05-03 20:58 ` Omar Sandoval
  2018-05-03 21:24   ` Darrick J. Wong
  2018-05-09 15:20 ` Jan Kara
  2 siblings, 1 reply; 7+ messages in thread
From: Omar Sandoval @ 2018-05-03 20:58 UTC (permalink / raw)
  To: Darrick J. Wong
  Cc: xfs, linux-fsdevel, linux-mm, hch, cyberax, jack, Eryu Guan

On Thu, May 03, 2018 at 10:46:59AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Add a new iomap_swapfile_activate function so that filesystems can
> activate swap files without having to use the obsolete and slow bmap
> function.  This enables XFS to support fallocate'd swap files and
> swap files on realtime devices.
> 

Shouldn't we also prevent the extents of an active swapfile from
becoming shared? If I swapon(a) and reflink(a, b), swapout to a now has
to break the reflink or corrupt b! In my old Btrfs swapfile series [1] I
just forbid all reflink operations on active swapfiles. 

One thing to note is that then this will need a matching
->swap_deactivate(), which currently isn't called if ->swap_activate()
returned > 0.

1: https://github.com/osandov/linux/tree/btrfs-swap

> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> v3: catch null iomap addr, fix too-short extent detection
> v2: document the swap file layout requirements, combine adjacent
>     real/unwritten extents, align reported swap extents to physical page
>     size boundaries, fix compiler errors when swap disabled

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

* Re: [PATCH v3 1/2] iomap: add a swapfile activation function
  2018-05-03 20:58 ` [PATCH v3 1/2] iomap: add a swapfile activation function Omar Sandoval
@ 2018-05-03 21:24   ` Darrick J. Wong
  2018-05-03 21:26     ` Omar Sandoval
  0 siblings, 1 reply; 7+ messages in thread
From: Darrick J. Wong @ 2018-05-03 21:24 UTC (permalink / raw)
  To: Omar Sandoval; +Cc: xfs, linux-fsdevel, linux-mm, hch, cyberax, jack, Eryu Guan

On Thu, May 03, 2018 at 01:58:03PM -0700, Omar Sandoval wrote:
> On Thu, May 03, 2018 at 10:46:59AM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Add a new iomap_swapfile_activate function so that filesystems can
> > activate swap files without having to use the obsolete and slow bmap
> > function.  This enables XFS to support fallocate'd swap files and
> > swap files on realtime devices.
> > 
> 
> Shouldn't we also prevent the extents of an active swapfile from
> becoming shared? If I swapon(a) and reflink(a, b), swapout to a now has
> to break the reflink or corrupt b! In my old Btrfs swapfile series [1] I
> just forbid all reflink operations on active swapfiles. 

xfs already does this in its reflink handler: it takes the inode lock &
bails out if IS_SWAPFILE().  swapon calls claim_swapfile to take the
inode lock and sets S_SWAPFILE (if successful) so the two are
effectively locked out from each other...

> One thing to note is that then this will need a matching
> ->swap_deactivate(), which currently isn't called if ->swap_activate()
> returned > 0.

...so there shouldn't be any state to undo if the
iomap_swapfile_activate fails.

--D

> 
> 1: https://github.com/osandov/linux/tree/btrfs-swap
> 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> > v3: catch null iomap addr, fix too-short extent detection
> > v2: document the swap file layout requirements, combine adjacent
> >     real/unwritten extents, align reported swap extents to physical page
> >     size boundaries, fix compiler errors when swap disabled

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

* Re: [PATCH v3 1/2] iomap: add a swapfile activation function
  2018-05-03 21:24   ` Darrick J. Wong
@ 2018-05-03 21:26     ` Omar Sandoval
  0 siblings, 0 replies; 7+ messages in thread
From: Omar Sandoval @ 2018-05-03 21:26 UTC (permalink / raw)
  To: Darrick J. Wong
  Cc: xfs, linux-fsdevel, linux-mm, hch, cyberax, jack, Eryu Guan

On Thu, May 03, 2018 at 02:24:47PM -0700, Darrick J. Wong wrote:
> On Thu, May 03, 2018 at 01:58:03PM -0700, Omar Sandoval wrote:
> > On Thu, May 03, 2018 at 10:46:59AM -0700, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <darrick.wong@oracle.com>
> > > 
> > > Add a new iomap_swapfile_activate function so that filesystems can
> > > activate swap files without having to use the obsolete and slow bmap
> > > function.  This enables XFS to support fallocate'd swap files and
> > > swap files on realtime devices.
> > > 
> > 
> > Shouldn't we also prevent the extents of an active swapfile from
> > becoming shared? If I swapon(a) and reflink(a, b), swapout to a now has
> > to break the reflink or corrupt b! In my old Btrfs swapfile series [1] I
> > just forbid all reflink operations on active swapfiles. 
> 
> xfs already does this in its reflink handler: it takes the inode lock &
> bails out if IS_SWAPFILE().  swapon calls claim_swapfile to take the
> inode lock and sets S_SWAPFILE (if successful) so the two are
> effectively locked out from each other...
> 
> > One thing to note is that then this will need a matching
> > ->swap_deactivate(), which currently isn't called if ->swap_activate()
> > returned > 0.
> 
> ...so there shouldn't be any state to undo if the
> iomap_swapfile_activate fails.

Makes sense, thanks.

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

* Re: [PATCH v3 1/2] iomap: add a swapfile activation function
  2018-05-03 17:46 [PATCH v3 1/2] iomap: add a swapfile activation function Darrick J. Wong
  2018-05-03 17:49 ` [PATCH v3 2/2] generic: test swapfile creation, activation, and deactivation Darrick J. Wong
  2018-05-03 20:58 ` [PATCH v3 1/2] iomap: add a swapfile activation function Omar Sandoval
@ 2018-05-09 15:20 ` Jan Kara
  2018-05-09 17:11   ` Darrick J. Wong
  2 siblings, 1 reply; 7+ messages in thread
From: Jan Kara @ 2018-05-09 15:20 UTC (permalink / raw)
  To: Darrick J. Wong
  Cc: xfs, linux-fsdevel, linux-mm, hch, cyberax, jack, osandov, Eryu Guan

On Thu 03-05-18 10:46:59, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Add a new iomap_swapfile_activate function so that filesystems can
> activate swap files without having to use the obsolete and slow bmap
> function.  This enables XFS to support fallocate'd swap files and
> swap files on realtime devices.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> v3: catch null iomap addr, fix too-short extent detection
> v2: document the swap file layout requirements, combine adjacent
>     real/unwritten extents, align reported swap extents to physical page
>     size boundaries, fix compiler errors when swap disabled
> ---
>  fs/iomap.c            |  162 +++++++++++++++++++++++++++++++++++++++++++++++++
>  fs/xfs/xfs_aops.c     |   12 ++++
>  include/linux/iomap.h |   11 +++
>  3 files changed, 185 insertions(+)
> 
> diff --git a/fs/iomap.c b/fs/iomap.c
> index afd163586aa0..ac7115492366 100644
> --- a/fs/iomap.c
> +++ b/fs/iomap.c
> @@ -27,6 +27,7 @@
>  #include <linux/task_io_accounting_ops.h>
>  #include <linux/dax.h>
>  #include <linux/sched/signal.h>
> +#include <linux/swap.h>
>  
>  #include "internal.h"
>  
> @@ -1089,3 +1090,164 @@ iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
>  	return ret;
>  }
>  EXPORT_SYMBOL_GPL(iomap_dio_rw);
> +
> +/* Swapfile activation */
> +
> +#ifdef CONFIG_SWAP
> +struct iomap_swapfile_info {
> +	struct iomap iomap;		/* accumulated iomap */
> +	struct swap_info_struct *sis;
> +	uint64_t lowest_ppage;		/* lowest physical addr seen (pages) */
> +	uint64_t highest_ppage;		/* highest physical addr seen (pages) */
> +	unsigned long nr_pages;		/* number of pages collected */
> +	int nr_extents;			/* extent count */
> +};
> +
> +/*
> + * Collect physical extents for this swap file.  Physical extents reported to
> + * the swap code must be trimmed to align to a page boundary.  The logical
> + * offset within the file is irrelevant since the swapfile code maps logical
> + * page numbers of the swap device to the physical page-aligned extents.
> + */
> +static int iomap_swapfile_add_extent(struct iomap_swapfile_info *isi)
> +{
> +	struct iomap *iomap = &isi->iomap;
> +	unsigned long nr_pages;
> +	uint64_t first_ppage;
> +	uint64_t first_ppage_reported;
> +	uint64_t last_ppage;
> +	int error;
> +
> +	/*
> +	 * Round the start up and the end down so that the physical
> +	 * extent aligns to a page boundary.
> +	 */
> +	first_ppage = ALIGN(iomap->addr, PAGE_SIZE) >> PAGE_SHIFT;
> +	last_ppage = (ALIGN_DOWN(iomap->addr + iomap->length, PAGE_SIZE) >>
> +			PAGE_SHIFT) - 1;

But this can still end up underflowing last_ppage to (unsigned long)-1 and
the following test won't trigger?

> +
> +	/* Skip too-short physical extents. */
> +	if (first_ppage > last_ppage)
> +		return 0;

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

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

* Re: [PATCH v3 1/2] iomap: add a swapfile activation function
  2018-05-09 15:20 ` Jan Kara
@ 2018-05-09 17:11   ` Darrick J. Wong
  0 siblings, 0 replies; 7+ messages in thread
From: Darrick J. Wong @ 2018-05-09 17:11 UTC (permalink / raw)
  To: Jan Kara; +Cc: xfs, linux-fsdevel, linux-mm, hch, cyberax, osandov, Eryu Guan

On Wed, May 09, 2018 at 05:20:02PM +0200, Jan Kara wrote:
> On Thu 03-05-18 10:46:59, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Add a new iomap_swapfile_activate function so that filesystems can
> > activate swap files without having to use the obsolete and slow bmap
> > function.  This enables XFS to support fallocate'd swap files and
> > swap files on realtime devices.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> > v3: catch null iomap addr, fix too-short extent detection
> > v2: document the swap file layout requirements, combine adjacent
> >     real/unwritten extents, align reported swap extents to physical page
> >     size boundaries, fix compiler errors when swap disabled
> > ---
> >  fs/iomap.c            |  162 +++++++++++++++++++++++++++++++++++++++++++++++++
> >  fs/xfs/xfs_aops.c     |   12 ++++
> >  include/linux/iomap.h |   11 +++
> >  3 files changed, 185 insertions(+)
> > 
> > diff --git a/fs/iomap.c b/fs/iomap.c
> > index afd163586aa0..ac7115492366 100644
> > --- a/fs/iomap.c
> > +++ b/fs/iomap.c
> > @@ -27,6 +27,7 @@
> >  #include <linux/task_io_accounting_ops.h>
> >  #include <linux/dax.h>
> >  #include <linux/sched/signal.h>
> > +#include <linux/swap.h>
> >  
> >  #include "internal.h"
> >  
> > @@ -1089,3 +1090,164 @@ iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
> >  	return ret;
> >  }
> >  EXPORT_SYMBOL_GPL(iomap_dio_rw);
> > +
> > +/* Swapfile activation */
> > +
> > +#ifdef CONFIG_SWAP
> > +struct iomap_swapfile_info {
> > +	struct iomap iomap;		/* accumulated iomap */
> > +	struct swap_info_struct *sis;
> > +	uint64_t lowest_ppage;		/* lowest physical addr seen (pages) */
> > +	uint64_t highest_ppage;		/* highest physical addr seen (pages) */
> > +	unsigned long nr_pages;		/* number of pages collected */
> > +	int nr_extents;			/* extent count */
> > +};
> > +
> > +/*
> > + * Collect physical extents for this swap file.  Physical extents reported to
> > + * the swap code must be trimmed to align to a page boundary.  The logical
> > + * offset within the file is irrelevant since the swapfile code maps logical
> > + * page numbers of the swap device to the physical page-aligned extents.
> > + */
> > +static int iomap_swapfile_add_extent(struct iomap_swapfile_info *isi)
> > +{
> > +	struct iomap *iomap = &isi->iomap;
> > +	unsigned long nr_pages;
> > +	uint64_t first_ppage;
> > +	uint64_t first_ppage_reported;
> > +	uint64_t last_ppage;
> > +	int error;
> > +
> > +	/*
> > +	 * Round the start up and the end down so that the physical
> > +	 * extent aligns to a page boundary.
> > +	 */
> > +	first_ppage = ALIGN(iomap->addr, PAGE_SIZE) >> PAGE_SHIFT;
> > +	last_ppage = (ALIGN_DOWN(iomap->addr + iomap->length, PAGE_SIZE) >>
> > +			PAGE_SHIFT) - 1;
> 
> But this can still end up underflowing last_ppage to (unsigned long)-1 and
> the following test won't trigger?

Yeah, I'll fix it and resubmit.  Thx for catching this.

--D

> > +
> > +	/* Skip too-short physical extents. */
> > +	if (first_ppage > last_ppage)
> > +		return 0;
> 
> 								Honza
> -- 
> Jan Kara <jack@suse.com>
> SUSE Labs, CR

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

end of thread, other threads:[~2018-05-09 17:11 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-03 17:46 [PATCH v3 1/2] iomap: add a swapfile activation function Darrick J. Wong
2018-05-03 17:49 ` [PATCH v3 2/2] generic: test swapfile creation, activation, and deactivation Darrick J. Wong
2018-05-03 20:58 ` [PATCH v3 1/2] iomap: add a swapfile activation function Omar Sandoval
2018-05-03 21:24   ` Darrick J. Wong
2018-05-03 21:26     ` Omar Sandoval
2018-05-09 15:20 ` Jan Kara
2018-05-09 17:11   ` Darrick J. Wong

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).