* [PATCH] iomap: improve the warnings from iomap_swapfile_activate
@ 2021-03-25 20:17 Christoph Hellwig
2021-03-25 22:59 ` Darrick J. Wong
0 siblings, 1 reply; 2+ messages in thread
From: Christoph Hellwig @ 2021-03-25 20:17 UTC (permalink / raw)
To: djwong; +Cc: linux-xfs
Print the path name of the swapfile that failed to active to ease
debugging the problem and to avoid a scare if xfstests hits these
cases. Also reword one warning a bit, as the error is not about
a file being on multiple devices, but one that has at least an
extent outside the main device known to the VFS and swap code.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/iomap/swapfile.c | 38 ++++++++++++++++++++++----------------
1 file changed, 22 insertions(+), 16 deletions(-)
diff --git a/fs/iomap/swapfile.c b/fs/iomap/swapfile.c
index a648dbf6991e4e..1dc63beae0c5b8 100644
--- a/fs/iomap/swapfile.c
+++ b/fs/iomap/swapfile.c
@@ -18,6 +18,7 @@ struct iomap_swapfile_info {
uint64_t highest_ppage; /* highest physical addr seen (pages) */
unsigned long nr_pages; /* number of pages collected */
int nr_extents; /* extent count */
+ struct file *file;
};
/*
@@ -70,6 +71,18 @@ static int iomap_swapfile_add_extent(struct iomap_swapfile_info *isi)
return 0;
}
+static int iomap_swapfile_fail(struct iomap_swapfile_info *isi, const char *str)
+{
+ char *buf, *p = ERR_PTR(-ENOMEM);
+
+ buf = kmalloc(PATH_MAX, GFP_KERNEL);
+ if (buf)
+ p = file_path(isi->file, buf, PATH_MAX);
+ pr_err("swapon: file %s %s\n", IS_ERR(p) ? "<unknown>" : p, str);
+ kfree(buf);
+ return -EINVAL;
+}
+
/*
* Accumulate iomaps for this swap file. We have to accumulate iomaps because
* swap only cares about contiguous page-aligned physical extents and makes no
@@ -89,28 +102,20 @@ static loff_t iomap_swapfile_activate_actor(struct inode *inode, loff_t pos,
break;
case IOMAP_INLINE:
/* No inline data. */
- pr_err("swapon: file is inline\n");
- return -EINVAL;
+ return iomap_swapfile_fail(isi, "is inline");
default:
- pr_err("swapon: file has unallocated extents\n");
- return -EINVAL;
+ return iomap_swapfile_fail(isi, "has unallocated extents");
}
/* 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;
- }
+ if (iomap->flags & IOMAP_F_DIRTY)
+ return iomap_swapfile_fail(isi, "is not committed");
+ if (iomap->flags & IOMAP_F_SHARED)
+ return iomap_swapfile_fail(isi, "has shared extents");
/* Only one bdev per swap file. */
- if (iomap->bdev != isi->sis->bdev) {
- pr_err("swapon: file is on multiple devices\n");
- return -EINVAL;
- }
+ if (iomap->bdev != isi->sis->bdev)
+ return iomap_swapfile_fail(isi, "outside the main device");
if (isi->iomap.length == 0) {
/* No accumulated extent, so just store it. */
@@ -139,6 +144,7 @@ int iomap_swapfile_activate(struct swap_info_struct *sis,
struct iomap_swapfile_info isi = {
.sis = sis,
.lowest_ppage = (sector_t)-1ULL,
+ .file = swap_file,
};
struct address_space *mapping = swap_file->f_mapping;
struct inode *inode = mapping->host;
--
2.30.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] iomap: improve the warnings from iomap_swapfile_activate
2021-03-25 20:17 [PATCH] iomap: improve the warnings from iomap_swapfile_activate Christoph Hellwig
@ 2021-03-25 22:59 ` Darrick J. Wong
0 siblings, 0 replies; 2+ messages in thread
From: Darrick J. Wong @ 2021-03-25 22:59 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Thu, Mar 25, 2021 at 09:17:53PM +0100, Christoph Hellwig wrote:
> Print the path name of the swapfile that failed to active to ease
> debugging the problem and to avoid a scare if xfstests hits these
> cases. Also reword one warning a bit, as the error is not about
> a file being on multiple devices, but one that has at least an
> extent outside the main device known to the VFS and swap code.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Looks good to me.
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
--D
> ---
> fs/iomap/swapfile.c | 38 ++++++++++++++++++++++----------------
> 1 file changed, 22 insertions(+), 16 deletions(-)
>
> diff --git a/fs/iomap/swapfile.c b/fs/iomap/swapfile.c
> index a648dbf6991e4e..1dc63beae0c5b8 100644
> --- a/fs/iomap/swapfile.c
> +++ b/fs/iomap/swapfile.c
> @@ -18,6 +18,7 @@ struct iomap_swapfile_info {
> uint64_t highest_ppage; /* highest physical addr seen (pages) */
> unsigned long nr_pages; /* number of pages collected */
> int nr_extents; /* extent count */
> + struct file *file;
> };
>
> /*
> @@ -70,6 +71,18 @@ static int iomap_swapfile_add_extent(struct iomap_swapfile_info *isi)
> return 0;
> }
>
> +static int iomap_swapfile_fail(struct iomap_swapfile_info *isi, const char *str)
> +{
> + char *buf, *p = ERR_PTR(-ENOMEM);
> +
> + buf = kmalloc(PATH_MAX, GFP_KERNEL);
> + if (buf)
> + p = file_path(isi->file, buf, PATH_MAX);
> + pr_err("swapon: file %s %s\n", IS_ERR(p) ? "<unknown>" : p, str);
> + kfree(buf);
> + return -EINVAL;
> +}
> +
> /*
> * Accumulate iomaps for this swap file. We have to accumulate iomaps because
> * swap only cares about contiguous page-aligned physical extents and makes no
> @@ -89,28 +102,20 @@ static loff_t iomap_swapfile_activate_actor(struct inode *inode, loff_t pos,
> break;
> case IOMAP_INLINE:
> /* No inline data. */
> - pr_err("swapon: file is inline\n");
> - return -EINVAL;
> + return iomap_swapfile_fail(isi, "is inline");
> default:
> - pr_err("swapon: file has unallocated extents\n");
> - return -EINVAL;
> + return iomap_swapfile_fail(isi, "has unallocated extents");
> }
>
> /* 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;
> - }
> + if (iomap->flags & IOMAP_F_DIRTY)
> + return iomap_swapfile_fail(isi, "is not committed");
> + if (iomap->flags & IOMAP_F_SHARED)
> + return iomap_swapfile_fail(isi, "has shared extents");
>
> /* Only one bdev per swap file. */
> - if (iomap->bdev != isi->sis->bdev) {
> - pr_err("swapon: file is on multiple devices\n");
> - return -EINVAL;
> - }
> + if (iomap->bdev != isi->sis->bdev)
> + return iomap_swapfile_fail(isi, "outside the main device");
>
> if (isi->iomap.length == 0) {
> /* No accumulated extent, so just store it. */
> @@ -139,6 +144,7 @@ int iomap_swapfile_activate(struct swap_info_struct *sis,
> struct iomap_swapfile_info isi = {
> .sis = sis,
> .lowest_ppage = (sector_t)-1ULL,
> + .file = swap_file,
> };
> struct address_space *mapping = swap_file->f_mapping;
> struct inode *inode = mapping->host;
> --
> 2.30.1
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-03-25 23:00 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-25 20:17 [PATCH] iomap: improve the warnings from iomap_swapfile_activate Christoph Hellwig
2021-03-25 22:59 ` Darrick J. Wong
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.