| Message ID | 20200301144925.48343-1-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | iomap: don't override sis->bdev in xfs_iomap_swapfile_activate | expand |
On Sun, Mar 01, 2020 at 07:49:25AM -0700, Christoph Hellwig wrote: > The swapon code itself sets sis->bdev up early, and performs various check > on the block devices. Changing it later in the fact thus will cause a > mismatch of capabilities and must be avoided. What kind of mismatch? Are you talking about the bdi_cap_* and blk_queue_nonrot() logic in swapon()? I wonder how much of that could be moved to after the ->swapfile_activate call. > The practical implication > of this change is that it forbids swapping to the RT subvolume, which might > have had all kinds of issues anyway. <shrug> I didn't find any the one time I tried it on a pair of homogeneous devices. :) > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > fs/xfs/xfs_aops.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c > index 58e937be24ce..f9929a952ef1 100644 > --- a/fs/xfs/xfs_aops.c > +++ b/fs/xfs/xfs_aops.c > @@ -637,7 +637,6 @@ xfs_iomap_swapfile_activate( > struct file *swap_file, > sector_t *span) > { > - sis->bdev = xfs_inode_buftarg(XFS_I(file_inode(swap_file)))->bt_bdev; That said, btrfs copypasta'd this when they ported to iomap swapfile, so that needs fixing too. --D > return iomap_swapfile_activate(sis, swap_file, span, > &xfs_read_iomap_ops); > } > -- > 2.24.1 >
On Tue, Mar 03, 2020 at 08:51:57AM -0800, Darrick J. Wong wrote: > On Sun, Mar 01, 2020 at 07:49:25AM -0700, Christoph Hellwig wrote: > > The swapon code itself sets sis->bdev up early, and performs various check > > on the block devices. Changing it later in the fact thus will cause a > > mismatch of capabilities and must be avoided. > > What kind of mismatch? Are you talking about the bdi_cap_* and > blk_queue_nonrot() logic in swapon()? I wonder how much of that could > be moved to after the ->swapfile_activate call. The thing I ran into is the zone check with my zoned XFS prototype code. But when you look at the nonrot checks that will cause resource leaks due to the override, and thus is the main is the main issue for now. I suspect much of this could be cleaned up one way or another, but the layering of this code is horrible, so it would be a bigger job. btrfs hasn't picked up the iomap changes yet, but the next resend should drop the bdev assignment as well.
diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c index 58e937be24ce..f9929a952ef1 100644 --- a/fs/xfs/xfs_aops.c +++ b/fs/xfs/xfs_aops.c @@ -637,7 +637,6 @@ xfs_iomap_swapfile_activate( struct file *swap_file, sector_t *span) { - sis->bdev = xfs_inode_buftarg(XFS_I(file_inode(swap_file)))->bt_bdev; return iomap_swapfile_activate(sis, swap_file, span, &xfs_read_iomap_ops); }
The swapon code itself sets sis->bdev up early, and performs various check on the block devices. Changing it later in the fact thus will cause a mismatch of capabilities and must be avoided. The practical implication of this change is that it forbids swapping to the RT subvolume, which might have had all kinds of issues anyway. Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/xfs/xfs_aops.c | 1 - 1 file changed, 1 deletion(-)