| Message ID | 20210325201753.1292361-1-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | iomap: improve the warnings from iomap_swapfile_activate | expand |
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 --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;
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(-)