| Message ID | 164299611275.26253.11641346650863170349.stgit@noble.brown (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Repair SWAP-over_NFS | expand |
On Mon, Jan 24, 2022 at 02:48:32PM +1100, NeilBrown wrote: > If a filesystem wishes to handle all swap IO itself (via ->direct_IO), Right now it's half ->direct_IO and half ->readpage and about to change with your series, isn't it? > rather than just providing devices addresses for submit_bio(), > SWP_FS_OPS must be set. > Currently the protocol for setting this it to have ->swap_activate > return zero. In that case SWP_FS_OPS is set, and add_swap_extent() > is called for the entire file. > > This is a little clumsy as different return values for ->swap_activate > have quite different meanings, and it makes it hard to search for which > filesystems require SWP_FS_OPS to be set. > > So remove the special meaning of a zero return, and require the > filesystem to set SWP_FS_OPS if it so desires, and to always call > add_swap_extent() as required. > > Currently only NFS and CIFS return zero for add_swap_extent(). > > Signed-off-by: NeilBrown <neilb@suse.de> Otherwise looks good: Reviewed-by: Christoph Hellwig <hch@lst.de>
diff --git a/fs/cifs/file.c b/fs/cifs/file.c index 59334be9ed3b..c795d4a9ec4a 100644 --- a/fs/cifs/file.c +++ b/fs/cifs/file.c @@ -4974,7 +4974,8 @@ static int cifs_swap_activate(struct swap_info_struct *sis, * from reading or writing the file */ - return 0; + sis->flags |= SWP_FS_OPS; + return add_swap_extent(sis, 0, sis->max, 0); } static void cifs_swap_deactivate(struct file *file) diff --git a/fs/nfs/file.c b/fs/nfs/file.c index 76d76acbc594..d5aa55c7edb0 100644 --- a/fs/nfs/file.c +++ b/fs/nfs/file.c @@ -488,6 +488,7 @@ static int nfs_swap_activate(struct swap_info_struct *sis, struct file *file, { unsigned long blocks; long long isize; + int ret; struct rpc_clnt *clnt = NFS_CLIENT(file->f_mapping->host); struct inode *inode = file->f_mapping->host; @@ -500,9 +501,17 @@ static int nfs_swap_activate(struct swap_info_struct *sis, struct file *file, return -EINVAL; } + ret = rpc_clnt_swap_activate(clnt); + if (ret) + return ret; + ret = add_swap_extent(sis, 0, sis->max, 0); + if (ret < 0) { + rpc_clnt_swap_deactivate(clnt); + return ret; + } *span = sis->pages; - - return rpc_clnt_swap_activate(clnt); + sis->flags |= SWP_FS_OPS; + return ret; } static void nfs_swap_deactivate(struct file *file) diff --git a/include/linux/swap.h b/include/linux/swap.h index a43929f7033e..b57cff3c5ac2 100644 --- a/include/linux/swap.h +++ b/include/linux/swap.h @@ -573,6 +573,12 @@ static inline swp_entry_t get_swap_page(struct page *page) return entry; } +static inline int add_swap_extent(struct swap_info_struct *sis, + unsigned long start_page, + unsigned long nr_pages, sector_t start_block) +{ + return -EINVAL; +} #endif /* CONFIG_SWAP */ #ifdef CONFIG_THP_SWAP diff --git a/mm/swapfile.c b/mm/swapfile.c index 71c7a31dd291..ed6028aea8bf 100644 --- a/mm/swapfile.c +++ b/mm/swapfile.c @@ -2347,13 +2347,9 @@ static int setup_swap_extents(struct swap_info_struct *sis, sector_t *span) if (mapping->a_ops->swap_activate) { ret = mapping->a_ops->swap_activate(sis, swap_file, span); - if (ret >= 0) - sis->flags |= SWP_ACTIVATED; - if (!ret) { - sis->flags |= SWP_FS_OPS; - ret = add_swap_extent(sis, 0, sis->max, 0); - *span = sis->pages; - } + if (ret < 0) + return ret; + sis->flags |= SWP_ACTIVATED; return ret; }
If a filesystem wishes to handle all swap IO itself (via ->direct_IO), rather than just providing devices addresses for submit_bio(), SWP_FS_OPS must be set. Currently the protocol for setting this it to have ->swap_activate return zero. In that case SWP_FS_OPS is set, and add_swap_extent() is called for the entire file. This is a little clumsy as different return values for ->swap_activate have quite different meanings, and it makes it hard to search for which filesystems require SWP_FS_OPS to be set. So remove the special meaning of a zero return, and require the filesystem to set SWP_FS_OPS if it so desires, and to always call add_swap_extent() as required. Currently only NFS and CIFS return zero for add_swap_extent(). Signed-off-by: NeilBrown <neilb@suse.de> --- fs/cifs/file.c | 3 ++- fs/nfs/file.c | 13 +++++++++++-- include/linux/swap.h | 6 ++++++ mm/swapfile.c | 10 +++------- 4 files changed, 22 insertions(+), 10 deletions(-)