swapon: commit swapfile caches to disk
PATCH 1/1 swapon: commit swapfile caches to disk
Testcase include swapon(8) fails on mainline kernel-v4.17+ as:
swapon01 1 TFAIL : swapon01.c:47: Failed to turn on swapfile: TEST_ERRNO=EINVAL(22): Invalid argument swapon02 4 TFAIL : swapon02.c:91: swapon(2) failed to produce expected error: 16, errno: EBUSY and got 22. swapon03 1 TFAIL : swapon03.c:243: Failed to setup swaps swapoff01 1 TBROK : swapoff01.c:64: Failed to turn on the swap file, skipping test iteration
And dmesg shows that:
[ 128.698981] swapon: file is not committed
The error located in IOMAP_F_DIRTY(linux/fs/iomap.c) checking, which means this swapfile have uncommitted metadata in caches. After adding sync() to the end of all mkswap opertaion, these errors were gone.
linux/fs/iomap.c:
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;
...
/* No uncommitted metadata or shared blocks. */
if (iomap->flags & IOMAP_F_DIRTY) {
pr_err("swapon: file is not committed\n");
return -EINVAL;
}
...
}
}---
Notes:
The new changes about swapfile activation function were merged in kernel-4.17 recently, so we didn't hit this failures before.
testcases/kernel/syscalls/swapoff/swapoff01.c | 2 ++ testcases/kernel/syscalls/swapoff/swapoff02.c | 2 ++ testcases/kernel/syscalls/swapon/libswapon.c | 2 ++ 3 files changed, 6 insertions(+)
testcases/kernel/syscalls/swapoff/swapoff01.c +2 -0
@@ -105,6 +105,8 @@ static void setup(void)if (system ("mkswap swapfile01 > tmpfile 2>&1" ) !=0 )tst_brkm (TBROK , cleanup,"Failed to make swapfile" ); + +sync (); }static void cleanup (void )
testcases/kernel/syscalls/swapoff/swapoff02.c +2 -0
@@ -154,6 +154,8 @@ static void setup(void)if (tst_fill_file ("./swapfile01" ,0x00 ,1024 ,1 ))tst_brkm (TBROK , cleanup,"Failed to create swapfile" ); + +sync (); }static void cleanup (void )
testcases/kernel/syscalls/swapon/libswapon.c +2 -0
@@ -46,4 +46,6 @@ void make_swapfile(void (cleanup)(void), const char *swapfile) argv[2 ] =NULL ;tst_run_cmd (cleanup, argv,"/dev/null" ,"/dev/null" ,0 ); + +sync (); }
Signature · 1 line
1.9.3
PATCH 1/1 swapon: commit swapfile caches to disk
Hi!
Quoted 5 lines
Testcase include swapon(8) fails on mainline kernel-v4.17+ as:
swapon01 1 TFAIL : swapon01.c:47: Failed to turn on swapfile: TEST_ERRNO=EINVAL(22): Invalid argument swapon02 4 TFAIL : swapon02.c:91: swapon(2) failed to produce expected error: 16, errno: EBUSY and got 22. swapon03 1 TFAIL : swapon03.c:243: Failed to setup swaps swapoff01 1 TBROK : swapoff01.c:64: Failed to turn on the swap file, skipping test iteration
4.17+ means that this is yet to be release kernel, right? In that situation we wait if the change will survive upon the official release before changing the LTP test.
testcases/kernel/syscalls/swapoff/swapoff01.c static void setup(void) · line 109
Quoted 10 lines
testcases/kernel/syscalls/swapoff/swapoff01.c +2
-0@@ -105,6 +105,8 @@ static void setup(void)if (system ("mkswap swapfile01 > tmpfile 2>&1" ) !=0 )tst_brkm (TBROK , cleanup,"Failed to make swapfile" ); + +sync ();
Can we rather use fsync() and/or fdatasync()? I would like the avoid the system wide sync() here since that may slow down the test unnecessarily.
Signature · 2 lines
Cyril Hrubis [email protected]
PATCH 1/1 swapon: commit swapfile caches to disk
Quoted 5 lines · Cyril Hrubis
Hi!
Quoted 2 lines
Testcase include swapon(8) fails on mainline kernel-v4.17+ as:
swapon01 1 TFAIL : swapon01.c:47: Failed to turn on swapfile:TEST_ERRNO=EINVAL(22): Invalid argument
Quoted 1 line
swapon02 4 TFAIL : swapon02.c:91: swapon(2) failed to produce
Quoted 9 lines
expected error: 16, errno: EBUSY and got 22.
Quoted 2 lines
swapon03 1 TFAIL : swapon03.c:243: Failed to setup swaps swapoff01 1 TBROK : swapoff01.c:64: Failed to turn on the swapfile, skipping test iteration
4.17+ means that this is yet to be release kernel, right? In that situation we wait if the change will survive upon the official release before changing the LTP test.
You are right, I'm OK to wait.
Quoted 8 lines
Review
Quoted 2 lines
(quoted diff) +0
-0
b/testcases/kernel/syscalls/swapoff/swapoff01.cQuoted 5 lines
index a63e661..b587b0d 100644
testcases/kernel/syscalls/swapoff/swapoff01.c +0
-0@@ -105,6 +105,8 @@ static void setup(void)
Quoted 9 lines
testcases/kernel/syscalls/swapoff/swapoff01.c static void setup(void) · line 109
Quoted 4 lines
testcases/kernel/syscalls/swapoff/swapoff01.c +2
-0if (system ("mkswap swapfile01 > tmpfile 2>&1" ) !=0 )tst_brkm (TBROK , cleanup,"Failed to make swapfile" ); + +sync ();Can we rather use
fsync()and/orfdatasync()? I would like the avoid the system widesync()here since that may slow down the test unnecessarily.
Actually I tried to add fsync() and syncfs() in tst_fill_file.c first, but it does NOT works for me. After tracking more, I found that we have to sync up caches in front of the command mkswap swapfile.
Quoted 4 lines
--
Cyril Hrubis
[email protected]
Signature · 5 lines
Regards, Li Wang -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.linux.it/pipermail/ltp/attachments/20180612/4c553103/attachment.html>
PATCH 1/1 swapon: commit swapfile caches to disk
Quoted 40 lines · Li Wang
Quoted 5 lines · Cyril Hrubis
Hi!
Quoted 2 lines
Testcase include swapon(8) fails on mainline kernel-v4.17+ as:
swapon01 1 TFAIL : swapon01.c:47: Failed to turn on swapfile:TEST_ERRNO=EINVAL(22): Invalid argument
Quoted 1 line
swapon02 4 TFAIL : swapon02.c:91: swapon(2) failed to produce
Quoted 9 lines
expected error: 16, errno: EBUSY and got 22.
Quoted 2 lines
swapon03 1 TFAIL : swapon03.c:243: Failed to setup swaps swapoff01 1 TBROK : swapoff01.c:64: Failed to turn on the swapfile, skipping test iteration
4.17+ means that this is yet to be release kernel, right? In that situation we wait if the change will survive upon the official release before changing the LTP test.
You are right, I'm OK to wait.
Quoted 8 lines
Review
Quoted 2 lines
(quoted diff) +0
-0
b/testcases/kernel/syscalls/swapoff/swapoff01.cQuoted 5 lines
index a63e661..b587b0d 100644
testcases/kernel/syscalls/swapoff/swapoff01.c +0
-0@@ -105,6 +105,8 @@ static void setup(void)Quoted 9 lines
testcases/kernel/syscalls/swapoff/swapoff01.c static void setup(void) · line 109
Quoted 4 lines
testcases/kernel/syscalls/swapoff/swapoff01.c +2
-0if (system ("mkswap swapfile01 > tmpfile 2>&1" ) !=0 )tst_brkm (TBROK , cleanup,"Failed to make swapfile" ); + +sync ();Can we rather use
fsync()and/orfdatasync()? I would like the avoid the system widesync()here since that may slow down the test unnecessarily.
Quoted 4 lines
Actually I tried to add
fsync()andsyncfs()intst_fill_file.cfirst, but it does NOT works for me. After tracking more, I found that we have to sync up caches in front of the commandmkswap swapfile.
Oops, this is wrong description, not in front of that.
Should be done sync again after perform command mkswap swapfile.
Quoted 13 lines
Quoted 6 lines
--
Cyril Hrubis
[email protected]--
Regards,
Li Wang
Signature · 5 lines
Regards, Li Wang -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.linux.it/pipermail/ltp/attachments/20180612/7a3ba129/attachment-0001.html>
PATCH 1/1 swapon: commit swapfile caches to disk
In reply to Li Wang----- Original Message -----
Quoted 36 lines
Testcase include swapon(8) fails on mainline kernel-v4.17+ as:
swapon01 1 TFAIL : swapon01.c:47: Failed to turn on swapfile:
TEST_ERRNO=EINVAL(22): Invalid argument
swapon02 4 TFAIL : swapon02.c:91: swapon(2) failed to produce
expected error: 16, errno: EBUSY and got 22.swapon03 1 TFAIL : swapon03.c:243: Failed to setup swaps swapoff01 1 TBROK : swapoff01.c:64: Failed to turn on the swap file,skipping test iteration
And dmesg shows that:
[ 128.698981] swapon: file is not committedThe error located in
IOMAP_F_DIRTY(linux/fs/iomap.c)checking, which means this swapfile have uncommitted metadata in caches. After addingsync()to the end of all mkswap opertaion, these errors were gone.
linux/fs/iomap.c: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; ... /* No uncommitted metadata or shared blocks. */ if (iomap->flags & IOMAP_F_DIRTY) { pr_err("swapon: file is not committed\n"); return -EINVAL; } ... } }
Quoted 7 lines
Signed-off-by Li Wang <[email protected]>---
Notes:
The new changes about swapfile activation function were merged in kernel-4.17
recently, so we didn't hit this failures before.
Comment/code in mkswap, that has been present for years:
/*
* A subsequent swapon() will fail if the signature
* is not actually on disk. (This is a kernel bug.)
* The fsync() in close_fd() will take care of writing. */
I'm assuming your mkswap is recent enough. Any idea why fsync() in mkswap is no longer enough?
Regards,
Jan
Quoted 54 lines
testcases/kernel/syscalls/swapoff/swapoff01.c | 2 ++ testcases/kernel/syscalls/swapoff/swapoff02.c | 2 ++ testcases/kernel/syscalls/swapon/libswapon.c | 2 ++ 3 files changed, 6 insertions(+)testcases/kernel/syscalls/swapoff/swapoff01.c +2
-0@@ -105,6 +105,8 @@ static void setup(void)if (system ("mkswap swapfile01 > tmpfile 2>&1" ) !=0 )tst_brkm (TBROK , cleanup,"Failed to make swapfile" ); + +sync (); }static void cleanup (void )testcases/kernel/syscalls/swapoff/swapoff02.c +2
-0@@ -154,6 +154,8 @@ static void setup(void)if (tst_fill_file ("./swapfile01" ,0x00 ,1024 ,1 ))tst_brkm (TBROK , cleanup,"Failed to create swapfile" ); + +sync (); }static void cleanup (void )testcases/kernel/syscalls/swapon/libswapon.c +2
-0@@ -46,4 +46,6 @@ void make_swapfile(void (cleanup)(void), const char *swapfile) argv[2 ] =NULL ;tst_run_cmd (cleanup, argv,"/dev/null" ,"/dev/null" ,0 ); + +sync ();}
--
1.9.3--
Mailing list info: https://lists.linux.it/listinfo/ltp
PATCH 1/1 swapon: commit swapfile caches to disk
Quoted 55 lines · Jan Stancek
----- Original Message -----
Quoted 7 lines
Testcase include swapon(8) fails on mainline kernel-v4.17+ as:
swapon01 1 TFAIL : swapon01.c:47: Failed to turn on swapfile:
TEST_ERRNO=EINVAL(22): Invalid argument
swapon02 4 TFAIL : swapon02.c:91: swapon(2) failed to produce
expected error: 16, errno: EBUSY and got 22.swapon03 1 TFAIL : swapon03.c:243: Failed to setup swaps swapoff01 1 TBROK : swapoff01.c:64: Failed to turn on the swapfile,
Quoted 6 lines
skipping test iteration
And dmesg shows that:
[ 128.698981] swapon: file is not committedThe error located in
IOMAP_F_DIRTY(linux/fs/iomap.c)checking, whichmeans
Quoted 23 lines
this swapfile have uncommitted metadata in caches. After adding
sync()to the end of all mkswap opertaion, these errors were gone.
linux/fs/iomap.c: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; ... /* No uncommitted metadata or shared blocks. */ if (iomap->flags & IOMAP_F_DIRTY) { pr_err("swapon: file is not committed\n"); return -EINVAL; } ... } }Quoted 7 lines
Signed-off-by Li Wang <[email protected]>---
Notes:
The new changes about swapfile activation function were merged in kernel-4.17
recently, so we didn't hit this failures before.Comment/code in mkswap, that has been present for years:
/*
* A subsequentswapon()will fail if the signature
* is not actually on disk. (This is a kernel bug.)
* Thefsync()inclose_fd()will take care of writing. */
Quoted 3 lines
I'm assuming your mkswap is recent enough. Any idea why
fsync()in mkswap is no longer enough?
​Good catch, Jan. It sounds like a mkswap/fsync issue.
Both of my mkswap failed to sync that, let me re-check it later.
# rpm -qf /usr/sbin/mkswap
util-linux-2.23.2-43.el7.x86_64
# rpm -qf /usr/sbin/mkswap util-linux-2.23.2-52.el7.x86_64
Quoted 59 lines
Regards,
JanQuoted 54 lines
testcases/kernel/syscalls/swapoff/swapoff01.c | 2 ++ testcases/kernel/syscalls/swapoff/swapoff02.c | 2 ++ testcases/kernel/syscalls/swapon/libswapon.c | 2 ++ 3 files changed, 6 insertions(+)testcases/kernel/syscalls/swapoff/swapoff01.c +2
-0@@ -105,6 +105,8 @@ static void setup(void)if (system ("mkswap swapfile01 > tmpfile 2>&1" ) !=0 )tst_brkm (TBROK , cleanup,"Failed to make swapfile" ); + +sync (); }static void cleanup (void )testcases/kernel/syscalls/swapoff/swapoff02.c +2
-0@@ -154,6 +154,8 @@ static void setup(void)if (tst_fill_file ("./swapfile01" ,0x00 ,1024 ,1 ))tst_brkm (TBROK , cleanup,"Failed to create swapfile" ); + +sync (); }static void cleanup (void )testcases/kernel/syscalls/swapon/libswapon.c +2
-0@@ -46,4 +46,6 @@ void make_swapfile(void (cleanup)(void), const char *swapfile) argv[2 ] =NULL ;tst_run_cmd (cleanup, argv,"/dev/null" ,"/dev/null" ,0 ); + +sync ();}
--
1.9.3--
Mailing list info: https://lists.linux.it/listinfo/ltp
Signature · 5 lines
Regards, Li Wang -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.linux.it/pipermail/ltp/attachments/20180612/2924ebc9/attachment.html>
PATCH 1/1 swapon: commit swapfile caches to disk
Quoted 58 lines · Li Wang
Quoted 54 lines · Jan Stancek
----- Original Message -----
Quoted 36 lines
Testcase include swapon(8) fails on mainline kernel-v4.17+ as:
swapon01 1 TFAIL : swapon01.c:47: Failed to turn on swapfile:
TEST_ERRNO=EINVAL(22): Invalid argument
swapon02 4 TFAIL : swapon02.c:91: swapon(2) failed to produce
expected error: 16, errno: EBUSY and got 22.swapon03 1 TFAIL : swapon03.c:243: Failed to setup swaps swapoff01 1 TBROK : swapoff01.c:64: Failed to turn on the swap file,skipping test iteration
And dmesg shows that:
[ 128.698981] swapon: file is not committedThe error located in
IOMAP_F_DIRTY(linux/fs/iomap.c)checking, which means this swapfile have uncommitted metadata in caches. After addingsync()to the end of all mkswap opertaion, these errors were gone.
linux/fs/iomap.c: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; ... /* No uncommitted metadata or shared blocks. */ if (iomap->flags & IOMAP_F_DIRTY) { pr_err("swapon: file is not committed\n"); return -EINVAL; } ... } }Quoted 7 lines
Signed-off-by Li Wang <[email protected]>---
Notes:
The new changes about swapfile activation function were merged in kernel-4.17
recently, so we didn't hit this failures before.Comment/code in mkswap, that has been present for years:
/*
* A subsequentswapon()will fail if the signature
* is not actually on disk. (This is a kernel bug.)
* Thefsync()inclose_fd()will take care of writing. */
Quoted 6 lines
Quoted 2 lines
I'm assuming your mkswap is recent enough. Any idea why
fsync()in mkswap is no longer enough?Good catch, Jan. It sounds like a mkswap/fsync issue.
I looked into this issue today. It seems like a new kernel bug which has been fixed by commit 117a148ffe0 (iomap: fsync swap files before iterating mappings) in linus tree.
------------------
Simple steps to reproduce that:
# dd if=/dev/zero of=swapfile01 bs=2048 count=1024 1024+0 records in 1024+0 records out 2097152 bytes (2.1 MB) copied, 0.00408662 s, 513 MB/s
# mkswap swapfile01 Setting up swapspace version 1, size = 2044 KiB no label, UUID=17830401-9cf2-4143-88a8-5a89c0b2d01d
# chmod 600 swapfile01
# swapon swapfile01 swapon: swapfile01: swapon failed: Invalid argument
Checking dmesg and get the root cause is swapfile01 have uncommitted metadata in caches:
[ 128.698981] swapon: file is not committed
Then, I tracking the __x64_sys_swapon function calltrace on my bad kernel:
xfs_iomap_swapfile_activate [xfs]() {
iomap_swapfile_activate() {
filemap_write_and_wait() { <===== [1]
mapping_needs_writeback();
__filemap_fdatawrite_range() {
_raw_spin_lock();
wbc_attach_and_unlock_inode();
do_writepages() {
xfs_vm_writepages [xfs]() {
_raw_spin_lock();
write_cache_pages() {
tag_pages_for_writeback() {
_raw_spin_lock_irq();
}
pagevec_lookup_range_tag() {
find_get_pages_range_tag();
}
}
}
}
wbc_detach_inode();
}Noticed [1] was being replaced by "ret = vfs_fsync(swap_file, 1);" in the kernel commit 117a148ff. And this change works fine to me.
But I still have a query as Jan mentioned that, the command mkswap has already done the write-back action via fsync() in close_fd(). Why we have to do this fsync() again in kernel side and then have effect to swapfile?
$ tail util-linux/disk-utils/mkswap.c
#endif
/*
* A subsequent swapon() will fail if the signature
* is not actually on disk. (This is a kernel bug.)
* The fsync() in close_fd() will take care of writing.
*/
if (close_fd(ctl.fd) != 0)
err(EXIT_FAILURE, _("write failed"));
return EXIT_SUCCESS;
}$ tail -12 util-linux/disk-utils/include/closestream.h
static inline int
close_fd(int fd)
{
const int fsync_fail = (fsync(fd) != 0);
const int close_fail = (close(fd) != 0);if (fsync_fail || close_fail)
return EOF;
return 0;
}
#endif /* UTIL_LINUX_CLOSESTREAM_H */
--------------
Anyway, this is not a LTP issue, so plz ignore my previous patch.
Thanks,
Li Wang