diff mbox series

iomap: improve the warnings from iomap_swapfile_activate

Message ID 20210325201753.1292361-1-hch@lst.de (mailing list archive)
State New
Headers show
Series iomap: improve the warnings from iomap_swapfile_activate | expand

Commit Message

Christoph Hellwig March 25, 2021, 8:17 p.m. UTC
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(-)

Comments

Darrick J. Wong March 25, 2021, 10:59 p.m. UTC | #1
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
>
diff mbox series

Patch

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;