* [PATCH v3 0/2] iomap: swapfile tweaks
@ 2018-05-16 17:54 Omar Sandoval
2018-05-16 17:54 ` [PATCH v3 1/2] iomap: provide more useful errors for invalid swap files Omar Sandoval
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Omar Sandoval @ 2018-05-16 17:54 UTC (permalink / raw)
To: Darrick J . Wong, linux-xfs
Cc: linux-fsdevel, Jan Kara, Christoph Hellwig, Aleksei Besogonov,
kernel-team
From: Omar Sandoval <osandov@fb.com>
Changes from v2:
- Move up inline flag check, which Christoph is going to turn into a
type at some point
- Remove null address check, since it doesn't make sense for
IOMAP_MAPPED or IOMAP_UNWRITTEN
Omar Sandoval (2):
iomap: provide more useful errors for invalid swap files
iomap: don't allow holes in swapfiles
fs/iomap.c | 43 +++++++++++++++++++++++--------------------
1 file changed, 23 insertions(+), 20 deletions(-)
--
2.17.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3 1/2] iomap: provide more useful errors for invalid swap files
2018-05-16 17:54 [PATCH v3 0/2] iomap: swapfile tweaks Omar Sandoval
@ 2018-05-16 17:54 ` Omar Sandoval
2018-05-16 18:10 ` Christoph Hellwig
2018-05-17 8:58 ` Jan Kara
2018-05-16 17:54 ` [PATCH v3 2/2] iomap: don't allow holes in swapfiles Omar Sandoval
2018-05-17 17:23 ` [PATCH v3 0/2] iomap: swapfile tweaks Darrick J. Wong
2 siblings, 2 replies; 8+ messages in thread
From: Omar Sandoval @ 2018-05-16 17:54 UTC (permalink / raw)
To: Darrick J . Wong, linux-xfs
Cc: linux-fsdevel, Jan Kara, Christoph Hellwig, Aleksei Besogonov,
kernel-team
From: Omar Sandoval <osandov@fb.com>
Currently, for an invalid swap file, we print the same error message
regardless of the reason. This isn't very useful for an admin, who will
likely want to know why exactly they can't use their swap file. So,
let's add specific error messages for each reason, and also move the
bdev check after the flags checks, since the latter are more
fundamental.
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Omar Sandoval <osandov@fb.com>
---
fs/iomap.c | 40 ++++++++++++++++++++++++----------------
1 file changed, 24 insertions(+), 16 deletions(-)
diff --git a/fs/iomap.c b/fs/iomap.c
index d193390a1c20..89517442e296 100644
--- a/fs/iomap.c
+++ b/fs/iomap.c
@@ -1214,26 +1214,37 @@ static loff_t iomap_swapfile_activate_actor(struct inode *inode, loff_t pos,
struct iomap_swapfile_info *isi = data;
int error;
+ /* No inline data. */
+ if (iomap->flags & IOMAP_F_DATA_INLINE) {
+ pr_err("swapon: file is inline\n");
+ return -EINVAL;
+ }
+
/* 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;
+ if (iomap->type != IOMAP_MAPPED && iomap->type != IOMAP_UNWRITTEN) {
+ pr_err("swapon: file has unallocated extents\n");
+ return -EINVAL;
+ }
- /* 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 uncommitted metadata or shared blocks. */
+ if (iomap->flags & IOMAP_F_DIRTY) {
+ pr_err("swapon: file is not committed\n");
+ return -EINVAL;
+ }
+ if (iomap->flags & IOMAP_F_SHARED) {
+ pr_err("swapon: file has shared extents\n");
+ return -EINVAL;
+ }
- /* No null physical addresses. */
- if (iomap->addr == IOMAP_NULL_ADDR)
- goto err;
+ /* Only one bdev per swap file. */
+ if (iomap->bdev != isi->sis->bdev) {
+ pr_err("swapon: file is on multiple devices\n");
+ return -EINVAL;
+ }
if (isi->iomap.length == 0) {
/* No accumulated extent, so just store it. */
@@ -1250,9 +1261,6 @@ static loff_t iomap_swapfile_activate_actor(struct inode *inode, loff_t pos,
}
out:
return count;
-err:
- pr_err("swapon: file cannot be used for swap\n");
- return -EINVAL;
}
/*
--
2.17.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v3 2/2] iomap: don't allow holes in swapfiles
2018-05-16 17:54 [PATCH v3 0/2] iomap: swapfile tweaks Omar Sandoval
2018-05-16 17:54 ` [PATCH v3 1/2] iomap: provide more useful errors for invalid swap files Omar Sandoval
@ 2018-05-16 17:54 ` Omar Sandoval
2018-05-16 18:11 ` Christoph Hellwig
2018-05-17 8:59 ` Jan Kara
2018-05-17 17:23 ` [PATCH v3 0/2] iomap: swapfile tweaks Darrick J. Wong
2 siblings, 2 replies; 8+ messages in thread
From: Omar Sandoval @ 2018-05-16 17:54 UTC (permalink / raw)
To: Darrick J . Wong, linux-xfs
Cc: linux-fsdevel, Jan Kara, Christoph Hellwig, Aleksei Besogonov,
kernel-team
From: Omar Sandoval <osandov@fb.com>
generic_swapfile_activate() doesn't allow holes, so we should be
consistent here. This is also a bit safer: if the user creates a
swapfile with, say, truncate -s $SIZE followed by mkswap, they should
really get an error and not much less swap space than they expected.
swapon(8) will error out before calling swapon(2) if the file has holes,
anyways.
Fixes: 9d93388b0afe ("iomap: add a swapfile activation function")
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Omar Sandoval <osandov@fb.com>
---
fs/iomap.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/fs/iomap.c b/fs/iomap.c
index 89517442e296..f2456d0d8ddd 100644
--- a/fs/iomap.c
+++ b/fs/iomap.c
@@ -1220,10 +1220,6 @@ static loff_t iomap_swapfile_activate_actor(struct inode *inode, loff_t pos,
return -EINVAL;
}
- /* Skip holes. */
- if (iomap->type == IOMAP_HOLE)
- goto out;
-
/* Only real or unwritten extents. */
if (iomap->type != IOMAP_MAPPED && iomap->type != IOMAP_UNWRITTEN) {
pr_err("swapon: file has unallocated extents\n");
@@ -1259,7 +1255,6 @@ static loff_t iomap_swapfile_activate_actor(struct inode *inode, loff_t pos,
return error;
memcpy(&isi->iomap, iomap, sizeof(isi->iomap));
}
-out:
return count;
}
--
2.17.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v3 1/2] iomap: provide more useful errors for invalid swap files
2018-05-16 17:54 ` [PATCH v3 1/2] iomap: provide more useful errors for invalid swap files Omar Sandoval
@ 2018-05-16 18:10 ` Christoph Hellwig
2018-05-17 8:58 ` Jan Kara
1 sibling, 0 replies; 8+ messages in thread
From: Christoph Hellwig @ 2018-05-16 18:10 UTC (permalink / raw)
To: Omar Sandoval
Cc: Darrick J . Wong, linux-xfs, linux-fsdevel, Jan Kara,
Christoph Hellwig, Aleksei Besogonov, kernel-team
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 2/2] iomap: don't allow holes in swapfiles
2018-05-16 17:54 ` [PATCH v3 2/2] iomap: don't allow holes in swapfiles Omar Sandoval
@ 2018-05-16 18:11 ` Christoph Hellwig
2018-05-17 8:59 ` Jan Kara
1 sibling, 0 replies; 8+ messages in thread
From: Christoph Hellwig @ 2018-05-16 18:11 UTC (permalink / raw)
To: Omar Sandoval
Cc: Darrick J . Wong, linux-xfs, linux-fsdevel, Jan Kara,
Christoph Hellwig, Aleksei Besogonov, kernel-team
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
And now lets get the series merged quickly so that I can rebase my
iomap series on top of this :)
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 1/2] iomap: provide more useful errors for invalid swap files
2018-05-16 17:54 ` [PATCH v3 1/2] iomap: provide more useful errors for invalid swap files Omar Sandoval
2018-05-16 18:10 ` Christoph Hellwig
@ 2018-05-17 8:58 ` Jan Kara
1 sibling, 0 replies; 8+ messages in thread
From: Jan Kara @ 2018-05-17 8:58 UTC (permalink / raw)
To: Omar Sandoval
Cc: Darrick J . Wong, linux-xfs, linux-fsdevel, Jan Kara,
Christoph Hellwig, Aleksei Besogonov, kernel-team
On Wed 16-05-18 10:54:10, Omar Sandoval wrote:
> From: Omar Sandoval <osandov@fb.com>
>
> Currently, for an invalid swap file, we print the same error message
> regardless of the reason. This isn't very useful for an admin, who will
> likely want to know why exactly they can't use their swap file. So,
> let's add specific error messages for each reason, and also move the
> bdev check after the flags checks, since the latter are more
> fundamental.
>
> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> Signed-off-by: Omar Sandoval <osandov@fb.com>
Yup, looks good to me. You can add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/iomap.c | 40 ++++++++++++++++++++++++----------------
> 1 file changed, 24 insertions(+), 16 deletions(-)
>
> diff --git a/fs/iomap.c b/fs/iomap.c
> index d193390a1c20..89517442e296 100644
> --- a/fs/iomap.c
> +++ b/fs/iomap.c
> @@ -1214,26 +1214,37 @@ static loff_t iomap_swapfile_activate_actor(struct inode *inode, loff_t pos,
> struct iomap_swapfile_info *isi = data;
> int error;
>
> + /* No inline data. */
> + if (iomap->flags & IOMAP_F_DATA_INLINE) {
> + pr_err("swapon: file is inline\n");
> + return -EINVAL;
> + }
> +
> /* 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;
> + if (iomap->type != IOMAP_MAPPED && iomap->type != IOMAP_UNWRITTEN) {
> + pr_err("swapon: file has unallocated extents\n");
> + return -EINVAL;
> + }
>
> - /* 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 uncommitted metadata or shared blocks. */
> + if (iomap->flags & IOMAP_F_DIRTY) {
> + pr_err("swapon: file is not committed\n");
> + return -EINVAL;
> + }
> + if (iomap->flags & IOMAP_F_SHARED) {
> + pr_err("swapon: file has shared extents\n");
> + return -EINVAL;
> + }
>
> - /* No null physical addresses. */
> - if (iomap->addr == IOMAP_NULL_ADDR)
> - goto err;
> + /* Only one bdev per swap file. */
> + if (iomap->bdev != isi->sis->bdev) {
> + pr_err("swapon: file is on multiple devices\n");
> + return -EINVAL;
> + }
>
> if (isi->iomap.length == 0) {
> /* No accumulated extent, so just store it. */
> @@ -1250,9 +1261,6 @@ static loff_t iomap_swapfile_activate_actor(struct inode *inode, loff_t pos,
> }
> out:
> return count;
> -err:
> - pr_err("swapon: file cannot be used for swap\n");
> - return -EINVAL;
> }
>
> /*
> --
> 2.17.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 2/2] iomap: don't allow holes in swapfiles
2018-05-16 17:54 ` [PATCH v3 2/2] iomap: don't allow holes in swapfiles Omar Sandoval
2018-05-16 18:11 ` Christoph Hellwig
@ 2018-05-17 8:59 ` Jan Kara
1 sibling, 0 replies; 8+ messages in thread
From: Jan Kara @ 2018-05-17 8:59 UTC (permalink / raw)
To: Omar Sandoval
Cc: Darrick J . Wong, linux-xfs, linux-fsdevel, Jan Kara,
Christoph Hellwig, Aleksei Besogonov, kernel-team
On Wed 16-05-18 10:54:11, Omar Sandoval wrote:
> From: Omar Sandoval <osandov@fb.com>
>
> generic_swapfile_activate() doesn't allow holes, so we should be
> consistent here. This is also a bit safer: if the user creates a
> swapfile with, say, truncate -s $SIZE followed by mkswap, they should
> really get an error and not much less swap space than they expected.
> swapon(8) will error out before calling swapon(2) if the file has holes,
> anyways.
>
> Fixes: 9d93388b0afe ("iomap: add a swapfile activation function")
> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> Signed-off-by: Omar Sandoval <osandov@fb.com>
Good catch. You can add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/iomap.c | 5 -----
> 1 file changed, 5 deletions(-)
>
> diff --git a/fs/iomap.c b/fs/iomap.c
> index 89517442e296..f2456d0d8ddd 100644
> --- a/fs/iomap.c
> +++ b/fs/iomap.c
> @@ -1220,10 +1220,6 @@ static loff_t iomap_swapfile_activate_actor(struct inode *inode, loff_t pos,
> return -EINVAL;
> }
>
> - /* Skip holes. */
> - if (iomap->type == IOMAP_HOLE)
> - goto out;
> -
> /* Only real or unwritten extents. */
> if (iomap->type != IOMAP_MAPPED && iomap->type != IOMAP_UNWRITTEN) {
> pr_err("swapon: file has unallocated extents\n");
> @@ -1259,7 +1255,6 @@ static loff_t iomap_swapfile_activate_actor(struct inode *inode, loff_t pos,
> return error;
> memcpy(&isi->iomap, iomap, sizeof(isi->iomap));
> }
> -out:
> return count;
> }
>
> --
> 2.17.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 0/2] iomap: swapfile tweaks
2018-05-16 17:54 [PATCH v3 0/2] iomap: swapfile tweaks Omar Sandoval
2018-05-16 17:54 ` [PATCH v3 1/2] iomap: provide more useful errors for invalid swap files Omar Sandoval
2018-05-16 17:54 ` [PATCH v3 2/2] iomap: don't allow holes in swapfiles Omar Sandoval
@ 2018-05-17 17:23 ` Darrick J. Wong
2 siblings, 0 replies; 8+ messages in thread
From: Darrick J. Wong @ 2018-05-17 17:23 UTC (permalink / raw)
To: Omar Sandoval
Cc: linux-xfs, linux-fsdevel, Jan Kara, Christoph Hellwig,
Aleksei Besogonov, kernel-team
On Wed, May 16, 2018 at 10:54:09AM -0700, Omar Sandoval wrote:
> From: Omar Sandoval <osandov@fb.com>
>
> Changes from v2:
>
> - Move up inline flag check, which Christoph is going to turn into a
> type at some point
> - Remove null address check, since it doesn't make sense for
> IOMAP_MAPPED or IOMAP_UNWRITTEN
>
> Omar Sandoval (2):
> iomap: provide more useful errors for invalid swap files
> iomap: don't allow holes in swapfiles
>
> fs/iomap.c | 43 +++++++++++++++++++++++--------------------
Tested ok overnight, so:
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
> 1 file changed, 23 insertions(+), 20 deletions(-)
>
> --
> 2.17.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-05-17 17:24 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-16 17:54 [PATCH v3 0/2] iomap: swapfile tweaks Omar Sandoval
2018-05-16 17:54 ` [PATCH v3 1/2] iomap: provide more useful errors for invalid swap files Omar Sandoval
2018-05-16 18:10 ` Christoph Hellwig
2018-05-17 8:58 ` Jan Kara
2018-05-16 17:54 ` [PATCH v3 2/2] iomap: don't allow holes in swapfiles Omar Sandoval
2018-05-16 18:11 ` Christoph Hellwig
2018-05-17 8:59 ` Jan Kara
2018-05-17 17:23 ` [PATCH v3 0/2] iomap: swapfile tweaks 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).