btrfs: implement swap file support
Subsystems (8)
v3 cover btrfs: implement swap file support
Hi, everyone,
This patch series, based on v3.18, implements support for swap files on BTRFS. Patches 1, 3, and 4 are for the VFS folks, patch 2 is for NFS, and the rest is all BTRFS.
The standard swap file implementation uses bmap() to get a list of physical blocks to do I/O on. This doesn't work for BTRFS, which moves disk blocks around as part of normal operation (COW, defragmentation, etc.).
Swap-over-NFS introduced an interface through which a filesystem can arbitrate swap I/O through address space operations:
- swap_activate() is called by swapon() and informs the address space that the given file is going to be used for swap, so it should take adequate measures like reserving space on disk and pinning block lookup information in memory
- swap_deactivate() is used to clean up on swapoff()
- direct_IO() is used to page in and out (this no longer uses readpage as part of this patch series)
Patches 1-4 clean up the necessary infrastructure. There's more that can make this better (like resurrecting kernel AIO), but that can be done as a follow-up to the work here.
Patches 5 and 6 lay the groundwork needed for using a swap file on BTRFS, and patch 7 implements the actual aops.
Version 3 incorporates a bunch of David Sterba's feedback, both style and design issues. We now audit various ioctls to prevent them from interfering with swap file operation and handle extents which can't be nocow'd.
After some discussion on the mailing list, I decided that for simplicity and reliability, it's best to simply disallow COW files and files with shared extents (like files with extents shared with a snapshot). From a user's perspective, this means that a snapshotted subvolume cannot be used for a swap file, but keeping the swap file in a separate subvolume that is never snapshotted seems entirely reasonable to me. An alternative suggestion was to allow swap files to be snapshotted and to do an implied COW on swap file activation, which I was ready to implement until I realized that we can't permit snapshotting a subvolume with an active swap file, so this creates a surprising inconsistency for users (in my opinion).
As with before, this functionality is tenuously tested in a virtual machine with some artificial workloads, but it "works for me". I'm pretty happy with the results on my end, so please comment away.
Thanks!
Omar Sandoval (7):
direct-io: don't dirty ITER_BVEC pages on read
nfs: don't dirty ITER_BVEC pages read through direct I/O
swap: use direct I/O for SWP_FILE swap_readpage
vfs: update swap_{,de}activate documentation
btrfs: prevent ioctls from interfering with a swap file
btrfs: add EXTENT_FLAG_SWAPFILE
btrfs: enable swap file support
Documentation/filesystems/Locking | 7 +- Documentation/filesystems/vfs.txt | 7 +- fs/btrfs/ctree.h | 3 + fs/btrfs/disk-io.c | 1 + fs/btrfs/extent_io.c | 1 + fs/btrfs/extent_map.h | 1 + fs/btrfs/inode.c | 132 ++++++++++++++++++++++++++++++++++++++ fs/btrfs/ioctl.c | 35 ++++++++-- fs/direct-io.c | 8 ++- fs/nfs/direct.c | 5 +- include/trace/events/btrfs.h | 3 +- mm/page_io.c | 32 +++++++-- 12 files changed, 216 insertions(+), 19 deletions(-)
Signature · 7 lines
2.1.3 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
RFC PATCH v3 1/7 direct-io: don't dirty ITER_BVEC pages on read
Reads through the iov_iter infrastructure for kernel pages shouldn't be dirtied by the direct I/O code.
This is based on Dave Kleikamp's and Ming Lei's previously posted patches.
--- fs/direct-io.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
fs/direct-io.c +5 -3
@@ -120,6 +120,7 @@ struct dio {spinlock_t bio_lock;/* protects BIO fields below */ int page_errors;/* errno from get_user_pages() */ int is_async;/* is IO async ? */ +int should_dirty;/* should we mark read pages dirty? */ bool defer_completion;/* defer AIO completion to workqueue? */ int io_error;/* IO error in completion path */ unsigned long refcount;/* direct_io_worker() and bios */
@@ -392,7 +393,7 @@ static inline void dio_bio_submit(struct dio *dio, struct dio_submit *sdio) dio->refcount++;spin_unlock_irqrestore (&dio->bio_lock, flags); -if (dio->is_async && dio->rw ==READ ) +if (dio->is_async && dio->rw ==READ && dio->should_dirty)bio_set_pages_dirty (bio);if (sdio->submit_io)
@@ -463,13 +464,13 @@ static int dio_bio_complete(struct dio *dio, struct bio *bio)if (!uptodate) dio->io_error = -EIO ; -if (dio->is_async && dio->rw ==READ ) { +if (dio->is_async && dio->rw ==READ && dio->should_dirty) {bio_check_pages_dirty (bio);/* transfers ownership */ }else {bio_for_each_segment_all (bvec, bio, i) {struct page *page = bvec->bv_page; -if (dio->rw ==READ && !PageCompound (page)) +if (dio->rw ==READ && !PageCompound (page) && dio->should_dirty)set_page_dirty_lock (page);page_cache_release (page); }
@@ -1177,6 +1178,7 @@ do_blockdev_direct_IO(int rw, struct kiocb *iocb, struct inode *inode, dio->inode = inode; dio->rw = rw; + dio->should_dirty = !(iter->type &ITER_BVEC );/* * For AIO O_(D)SYNC writes we need to defer completions to a workqueue
Signature · 7 lines
2.1.3 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
RFC PATCH v3 2/7 nfs: don't dirty ITER_BVEC pages read through direct I/O
In reply to Omar SandovalAs with the generic blockdev code, kernel pages shouldn't be dirtied by the direct I/O path.
--- fs/nfs/direct.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
fs/nfs/direct.c +4 -1
@@ -88,6 +88,7 @@ struct nfs_direct_req {struct pnfs_ds_commit_info ds_cinfo;/* Storage for cinfo */ struct work_struct work;int flags; +int should_dirty;/* should we mark read pages dirty? */ #define NFS_ODIRECT_DO_COMMIT (1)/* an unstable reply was received */ #define NFS_ODIRECT_RESCHED_WRITES (2)/* write verification failed */ struct nfs_writeverf verf;/* unstable write verifier */
@@ -370,7 +371,8 @@ static void nfs_direct_read_completion(struct nfs_pgio_header *hdr)struct nfs_page *req =nfs_list_entry (hdr->pages.next);struct page *page = req->wb_page; -if (!PageCompound (page) && bytes < hdr->good_bytes) +if (!PageCompound (page) && bytes < hdr->good_bytes && + dreq->should_dirty)set_page_dirty (page); bytes += req->wb_bytes;nfs_list_remove_request (req);
@@ -542,6 +544,7 @@ ssize_t nfs_file_direct_read(struct kiocb *iocb, struct iov_iter *iter, dreq->inode = inode; dreq->bytes_left = count; dreq->ctx =get_nfs_open_context (nfs_file_open_context (iocb->ki_filp)); + dreq->should_dirty = !(iter->type &ITER_BVEC ); l_ctx =nfs_get_lock_context (dreq->ctx);if (IS_ERR (l_ctx)) { result =PTR_ERR (l_ctx);
Signature · 7 lines
2.1.3 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
RFC PATCH v3 3/7 swap: use direct I/O for SWP_FILE swap_readpage
In reply to Omar SandovalQuoted 3 lines · Christoph Hellwig
With the new
iov_iterinfrastructure that supprots direct I/O to kernel pages please get rid of the ->readpage hack first. I'm still utterly disapoined that this crap ever got merged.
--- mm/page_io.c | 32 ++++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-)
mm/page_io.c +28 -4
@@ -283,8 +283,7 @@ int __swap_writepage(struct page *page, struct writeback_control *wbc,set_page_writeback (page);unlock_page (page); - ret = mapping->a_ops->direct_IO (ITER_BVEC |WRITE , - &kiocb, &from, + ret = mapping->a_ops->direct_IO (WRITE , &kiocb, &from, kiocb.ki_pos);if (ret ==PAGE_SIZE ) {count_vm_event (PSWPOUT );
@@ -348,12 +347,37 @@ int swap_readpage(struct page *page) }if (sis->flags &SWP_FILE ) { +struct kiocb kiocb;struct file *swap_file = sis->swap_file;struct address_space *mapping = swap_file->f_mapping; +struct bio_vec bv = { + .bv_page = page, + .bv_len =PAGE_SIZE , + .bv_offset =0 , + }; +struct iov_iter to = { + .type =ITER_BVEC |READ , + .count =PAGE_SIZE , + .iov_offset =0 , + .nr_segs =1 , + }; + to.bvec = &bv;/* older gcc versions are broken */ + +init_sync_kiocb (&kiocb, swap_file); + kiocb.ki_pos =page_file_offset (page); + kiocb.ki_nbytes =PAGE_SIZE ; - ret = mapping->a_ops->readpage (swap_file, page); -if (!ret) + ret = mapping->a_ops->direct_IO (READ , &kiocb, &to, + kiocb.ki_pos); +if (ret ==PAGE_SIZE ) { +SetPageUptodate (page);count_vm_event (PSWPIN ); + ret =0 ; + }else { +ClearPageUptodate (page); +SetPageError (page); + } +unlock_page (page);return ret; }
Signature · 7 lines
2.1.3 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
RFC PATCH v3 4/7 vfs: update swap_{,de}activate documentation
In reply to Omar SandovalParameters were added to swap_activate in the same patch series that introduced it without updating the documentation. Additionally, the documentation claims that non-existent address space operations swap_{in,out} are used for swap I/O, but it's (now) direct_IO.
--- Documentation/filesystems/Locking | 7 ++++--- Documentation/filesystems/vfs.txt | 7 ++++--- 2 files changed, 8 insertions(+), 6 deletions(-)
Documentation/filesystems/Locking +4 -3
@@ -205,7 +205,8 @@ prototypes: int (*launder_page)(struct page *); int (*is_partially_uptodate)(struct page *, unsigned long, unsigned long); int (*error_remove_page)(struct address_space *, struct page *); - int (*swap_activate)(struct file *); + int (*swap_activate)(struct swap_info_struct *, struct file *, + sector_t *); int (*swap_deactivate)(struct file *); locking rules:
@@ -230,8 +231,8 @@ migratepage: yes (both) launder_page: yes is_partially_uptodate: yes error_remove_page: yes -swap_activate: no -swap_deactivate: no +swap_activate: yes +swap_deactivate: no ->write_begin(), ->write_end(), ->sync_page() and ->readpage() may be called from the request handler (/dev/loop).
Documentation/filesystems/vfs.txt +4 -3
@@ -600,8 +600,9 @@ struct address_space_operations { unsigned long); void (*is_dirty_writeback) (struct page *, bool *, bool *); int (*error_remove_page) (struct mapping *mapping, struct page *page); - int (*swap_activate)(struct file *); - int (*swap_deactivate)(struct file *); + int (*swap_activate)(struct swap_info_struct *, struct file *, + sector_t *); + void (*swap_deactivate)(struct file *); }; writepage: called by the VM to write a dirty page to backing store.
@@ -788,7 +789,7 @@ struct address_space_operations { memory. A return value of zero indicates success, in which case this file can be used to back swapspace. The swapspace operations will be proxied to this address space's - ->swap_{out,in} methods. + ->direct_IO method for both reads and writes. swap_deactivate: Called during swapoff on files where swap_activate was successful.
Signature · 7 lines
2.1.3 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
RFC PATCH v3 5/7 btrfs: prevent ioctls from interfering with a swap file
In reply to Omar SandovalThere are several ioctls which can work around constraints enforced by btrfs_swap_activate and lead to an unsafe situation. We cannot do any of the following on an active swap file in order to avoid creating compressed or shared extents:
- chattr -C or +c
- snapshot create
- defrag
- clone
- dedup
--- fs/btrfs/ctree.h | 3 +++ fs/btrfs/disk-io.c | 1 + fs/btrfs/ioctl.c | 35 +++++++++++++++++++++++++++++++---- 3 files changed, 35 insertions(+), 4 deletions(-)
fs/btrfs/ctree.h +3 -0
@@ -1891,6 +1891,9 @@ struct btrfs_root {int send_in_progress;struct btrfs_subvolume_writers *subv_writers;atomic_t will_be_snapshoted; + +/* Number of active swapfiles */ +atomic_t nr_swapfiles; };struct btrfs_ioctl_defrag_range_args {
fs/btrfs/disk-io.c +1 -0
@@ -1265,6 +1265,7 @@ static void __setup_root(u32 nodesize, u32 sectorsize, u32 stripesize,atomic_set (&root->orphan_inodes,0 );atomic_set (&root->refs,1 );atomic_set (&root->will_be_snapshoted,0 ); +atomic_set (&root->nr_swapfiles,0 ); root->log_transid =0 ; root->log_transid_committed =-1 ; root->last_log_commit =0 ;
fs/btrfs/ioctl.c +31 -4
@@ -291,9 +291,11 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg) }else { ip->flags |=BTRFS_INODE_NODATACOW ; } - }else { + }else if (!IS_SWAPFILE (inode)) {/* -* Revert back under same assuptions as above +* Revert back under same assumptions as above. swap_activate +* checks that we don't swapon a copy-on-write file, but we also +* make sure that it doesn't become copy-on-write here. */ if (S_ISREG (mode)) {if (inode->i_size ==0 )
@@ -316,7 +318,12 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg) ret =btrfs_set_prop (inode,"btrfs.compression" ,NULL ,0 ,0 );if (ret && ret != -ENODATA ) goto out_drop; - }else if (flags &FS_COMPR_FL ) { + }else if (flags &FS_COMPR_FL && !IS_SWAPFILE (inode)) { +/* +* Like nodatacow, swap_activate checks that we don't swapon a +* compressed file, so we shouldn't let it become compressed. +*/ +const char *comp; ip->flags |=BTRFS_INODE_COMPRESS ;
@@ -330,7 +337,6 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg) comp, strlen(comp), 0);if (ret) goto out_drop; - }else { ret =btrfs_set_prop (inode,"btrfs.compression" ,NULL ,0 ,0 );if (ret && ret != -ENODATA )
@@ -647,6 +653,12 @@ static int create_snapshot(struct btrfs_root *root, struct inode *dir,if (!test_bit (BTRFS_ROOT_REF_COWS , &root->state))return -EINVAL ; +if (atomic_read (&root->nr_swapfiles)) { +btrfs_err (root->fs_info, +"cannot create snapshot with active swapfile" ); +return -ETXTBSY ; + } +atomic_inc (&root->will_be_snapshoted);smp_mb__after_atomic ();btrfs_wait_nocow_write (root);
@@ -1292,6 +1304,12 @@ int btrfs_defrag_file(struct inode *inode, struct file *file, compress_type = range->compress_type; } +mutex_lock (&inode->i_mutex); + ret =IS_SWAPFILE (inode) ? -ETXTBSY :0 ; +mutex_unlock (&inode->i_mutex); +if (ret) +return ret; +if (extent_thresh ==0 ) extent_thresh =256 *1024 ;
@@ -2927,6 +2945,11 @@ static int btrfs_extent_same(struct inode *src, u64 loff, u64 len,btrfs_double_lock (src, loff, dst, dst_loff, len); +if (IS_SWAPFILE (src) ||IS_SWAPFILE (dst)) { + ret = -ETXTBSY ; + goto out_unlock; + } + ret =extent_same_check_offsets (src, loff, len);if (ret) goto out_unlock;
@@ -3644,6 +3667,10 @@ static noinline long btrfs_ioctl_clone(struct file *file, unsigned long srcfd,mutex_lock (&src->i_mutex); } + ret = -ETXTBSY ; +if (IS_SWAPFILE (src) ||IS_SWAPFILE (inode)) + goto out_unlock; +/* determine range to clone */ ret = -EINVAL ;if (off + len > src->i_size || off + len < off)
Signature · 7 lines
2.1.3 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
RFC PATCH v3 6/7 btrfs: add EXTENT_FLAG_SWAPFILE
In reply to Omar SandovalExtents mapping a swap file should remain pinned in memory in order to avoid doing allocations to look up an extent when we're already low on memory. Rather than overloading EXTENT_FLAG_PINNED, add a new flag specifically for this purpose.
--- fs/btrfs/extent_io.c | 1 + fs/btrfs/extent_map.h | 1 + fs/btrfs/inode.c | 1 + include/trace/events/btrfs.h | 3 ++- 4 files changed, 5 insertions(+), 1 deletion(-)
fs/btrfs/extent_io.c +1 -0
@@ -4244,6 +4244,7 @@ int try_release_extent_mapping(struct extent_map_tree *map,break ; }if (test_bit (EXTENT_FLAG_PINNED , &em->flags) || +test_bit (EXTENT_FLAG_SWAPFILE , &em->flags) || em->start != start) {write_unlock (&map->lock);free_extent_map (em);
fs/btrfs/extent_map.h +1 -0
@@ -16,6 +16,7 @@#define EXTENT_FLAG_LOGGING 4/* Logging this extent */ #define EXTENT_FLAG_FILLING 5/* Filling in a preallocated extent */ #define EXTENT_FLAG_FS_MAPPING 6/* filesystem extent mapping type */ +#define EXTENT_FLAG_SWAPFILE 7/* this extent maps a swap file */ struct extent_map {struct rb_node rb_node;
fs/btrfs/inode.c +1 -0
@@ -6353,6 +6353,7 @@ again:else goto out; } +WARN_ON_ONCE (IS_SWAPFILE (inode)); em =alloc_extent_map ();if (!em) { err = -ENOMEM ;
include/trace/events/btrfs.h +2 -1
@@ -164,7 +164,8 @@ DEFINE_EVENT(btrfs__inode, btrfs_inode_evict, { (1 << EXTENT_FLAG_PREALLOC), "PREALLOC" },\ { (1 << EXTENT_FLAG_LOGGING), "LOGGING" },\ { (1 << EXTENT_FLAG_FILLING), "FILLING" },\ - { (1 << EXTENT_FLAG_FS_MAPPING), "FS_MAPPING" }) + { (1 << EXTENT_FLAG_FS_MAPPING), "FS_MAPPING" },\ + { (1 << EXTENT_FLAG_SWAPFILE), "SWAPFILE" }) TRACE_EVENT_CONDITION(btrfs_get_extent,
Signature · 7 lines
2.1.3 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [RFC PATCH v3 6/7] btrfs: add EXTENT_FLAG_SWAPFILE
Quoted 6 lines · Omar Sandoval
Extents mapping a swap file should remain pinned in memory in order to avoid doing allocations to look up an extent when we're already low on memory. Rather than overloading
EXTENT_FLAG_PINNED, add a new flag specifically for this purpose.Signed-off-by Omar Sandoval <[email protected]>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
RFC PATCH v3 7/7 btrfs: enable swap file support
In reply to Omar SandovalImplement the swap file a_ops on btrfs. Activation does two things:
1. Checks for a usable swap file: it must be fully allocated (no holes), support direct I/O (so no compressed or inline extents) and must be eligible for nocow in its entirety in order to avoid doing a bunch of allocations for a COW when we're already low on memory
2. Pins the extent maps in memory with EXTENT_FLAG_SWAPFILE
Deactivation unpins all of the extent maps.
--- fs/btrfs/inode.c | 131 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 131 insertions(+)
fs/btrfs/inode.c +131 -0
@@ -7191,6 +7191,7 @@ static int btrfs_get_blocks_direct(struct inode *inode, sector_t iblock,* this will cow the extent, reset the len in case we changed * it above */ +WARN_ON_ONCE (IS_SWAPFILE (inode)); len = bh_result->b_size;free_extent_map (em); em =btrfs_new_extent_direct (inode, start, len);
@@ -9443,6 +9444,134 @@ out_inode: } +static void __clear_swapfile_extents (struct inode *inode) +{ +u64 isize = inode->i_size; +struct extent_map *em; +u64 start, len; + + start =0 ; +while (start < isize) { + len = isize - start; + em =btrfs_get_extent (inode,NULL ,0 , start, len,0 ); +if (IS_ERR (em)) +return ; + +clear_bit (EXTENT_FLAG_SWAPFILE , &em->flags); + + start =extent_map_end (em); +free_extent_map (em); + } +} + +static int btrfs_swap_activate (struct swap_info_struct *sis,struct file *file, +sector_t *span) +{ +struct inode *inode =file_inode (file); +struct btrfs_fs_info *fs_info =BTRFS_I (inode)->root->fs_info; +struct extent_io_tree *io_tree = &BTRFS_I (inode)->io_tree; +int ret =0 ; +u64 isize = inode->i_size; +struct extent_state *cached_state =NULL ; +struct extent_map *em; +u64 start, len; + +if (BTRFS_I (inode)->flags &BTRFS_INODE_COMPRESS ) { +/* Can't do direct I/O on a compressed file. */ +btrfs_err (fs_info,"swapfile is compressed" ); +return -EINVAL ; + } +if (!(BTRFS_I (inode)->flags &BTRFS_INODE_NODATACOW )) { +/* +* Going through the copy-on-write path while swapping pages +* in/out and doing a bunch of allocations could stress the +* memory management code that got us there in the first place, +* and that's sure to be a bad time. +*/ +btrfs_err (fs_info,"swapfile is copy-on-write" ); +return -EINVAL ; + } + +lock_extent_bits (io_tree,0 , isize -1 ,0 , &cached_state); + +/* +* All of the extents must be allocated and support direct I/O. Inline +* extents and compressed extents fall back to buffered I/O, so those +* are no good. Additionally, all of the extents must be safe for nocow. +*/ +atomic_inc (&BTRFS_I (inode)->root->nr_swapfiles); + start =0 ; +while (start < isize) { + len = isize - start; + em =btrfs_get_extent (inode,NULL ,0 , start, len,0 ); +if (IS_ERR (em)) { + ret =PTR_ERR (em); + goto out; + } + +if (test_bit (EXTENT_FLAG_VACANCY , &em->flags) || + em->block_start ==EXTENT_MAP_HOLE ) { +btrfs_err (fs_info,"swapfile has holes" ); + ret = -EINVAL ; + goto out; + } +if (em->block_start ==EXTENT_MAP_INLINE ) { +/* +* It's unlikely we'll ever actually find ourselves +* here, as a file small enough to fit inline won't be +* big enough to store more than the swap header, but in +* case something changes in the future, let's catch it +* here rather than later. +*/ +btrfs_err (fs_info,"swapfile is inline" ); + ret = -EINVAL ; + goto out; + } +if (test_bit (EXTENT_FLAG_COMPRESSED , &em->flags)) { +btrfs_err (fs_info,"swapfile is compresed" ); + ret = -EINVAL ; + goto out; + } + ret =can_nocow_extent (inode, start, &len,NULL ,NULL ,NULL ); +if (ret <0 ) { + goto out; + }else if (ret ==1 ) { + ret =0 ; + }else { +btrfs_err (fs_info,"swapfile has extent requiring COW (%llu-%llu)" , + start, start + len -1 ); + ret = -EINVAL ; + goto out; + } + +set_bit (EXTENT_FLAG_SWAPFILE , &em->flags); + + start =extent_map_end (em); +free_extent_map (em); + } + +out: +if (ret) { +__clear_swapfile_extents (inode); +atomic_dec (&BTRFS_I (inode)->root->nr_swapfiles); + } +unlock_extent_cached (io_tree,0 , isize -1 , &cached_state,GFP_NOFS ); +return ret; +} + +static void btrfs_swap_deactivate (struct file *file) +{ +struct inode *inode =file_inode (file); +struct extent_io_tree *io_tree = &BTRFS_I (inode)->io_tree; +struct extent_state *cached_state =NULL ; +u64 isize = inode->i_size; + +lock_extent_bits (io_tree,0 , isize -1 ,0 , &cached_state); +__clear_swapfile_extents (inode); +unlock_extent_cached (io_tree,0 , isize -1 , &cached_state,GFP_NOFS ); +atomic_dec (&BTRFS_I (inode)->root->nr_swapfiles); +} +static const struct inode_operations btrfs_dir_inode_operations = { .getattr = btrfs_getattr, .lookup = btrfs_lookup,
@@ -9520,6 +9649,8 @@ static const struct address_space_operations btrfs_aops = { .releasepage = btrfs_releasepage, .set_page_dirty = btrfs_set_page_dirty, .error_remove_page = generic_error_remove_page, + .swap_activate = btrfs_swap_activate, + .swap_deactivate = btrfs_swap_deactivate, };static const struct address_space_operations btrfs_symlink_aops = {
Signature · 7 lines
2.1.3 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [RFC PATCH v3 7/7] btrfs: enable swap file support
fs/btrfs/inode.c out_inode: · line 9458
Quoted 12 lines · Omar Sandoval
fs/btrfs/inode.c +12
-0+static void __clear_swapfile_extents (struct inode *inode) +{ +u64 isize = inode->i_size; +struct extent_map *em; +u64 start, len; + + start =0 ; +while (start < isize) { + len = isize - start; + em =btrfs_get_extent (inode,NULL ,0 , start, len,0 ); +if (IS_ERR (em)) +return ;
This could transiently fail if there's no memory to allocate the em, and would leak the following extents.
Quoted 39 lines
fs/btrfs/inode.c +39
-0+ +clear_bit (EXTENT_FLAG_SWAPFILE , &em->flags); + + start =extent_map_end (em); +free_extent_map (em); + } +} + +static int btrfs_swap_activate (struct swap_info_struct *sis,struct file *file, +sector_t *span) +{ +struct inode *inode =file_inode (file); +struct btrfs_fs_info *fs_info =BTRFS_I (inode)->root->fs_info; +struct extent_io_tree *io_tree = &BTRFS_I (inode)->io_tree; +int ret =0 ; +u64 isize = inode->i_size; +struct extent_state *cached_state =NULL ; +struct extent_map *em; +u64 start, len; + +if (BTRFS_I (inode)->flags &BTRFS_INODE_COMPRESS ) { +/* Can't do direct I/O on a compressed file. */ +btrfs_err (fs_info,"swapfile is compressed" ); +return -EINVAL ; + } +if (!(BTRFS_I (inode)->flags &BTRFS_INODE_NODATACOW )) { +/* +* Going through the copy-on-write path while swapping pages +* in/out and doing a bunch of allocations could stress the +* memory management code that got us there in the first place, +* and that's sure to be a bad time. +*/ +btrfs_err (fs_info,"swapfile is copy-on-write" ); +return -EINVAL ; + } + +lock_extent_bits (io_tree,0 , isize -1 ,0 , &cached_state); + +/*
fs/btrfs/inode.c out_inode: · line 9507
Quoted 10 lines
fs/btrfs/inode.c +10
-0+* All of the extents must be allocated and support direct I/O. Inline +* extents and compressed extents fall back to buffered I/O, so those +* are no good. Additionally, all of the extents must be safe for nocow. +*/ +atomic_inc (&BTRFS_I (inode)->root->nr_swapfiles); + start =0 ; +while (start < isize) { + len = isize - start; + em =btrfs_get_extent (inode,NULL ,0 , start, len,0 ); +if (IS_ERR (em)) {
IS_ERR_OR_NULL(em)
fs/btrfs/inode.c out_inode: · line 9515
Quoted 10 lines
From now on the em is valid and has to be free_extent_map()ed ...
fs/btrfs/inode.c +8
-0+ ret =PTR_ERR (em); + goto out; + } + +if (test_bit (EXTENT_FLAG_VACANCY , &em->flags) || + em->block_start ==EXTENT_MAP_HOLE ) { +btrfs_err (fs_info,"swapfile has holes" ); + ret = -EINVAL ;
... and all the error branches would miss it.
fs/btrfs/inode.c out_inode: · line 9527
Quoted 12 lines
fs/btrfs/inode.c +12
-0+ goto out; + } +if (em->block_start ==EXTENT_MAP_INLINE ) { +/* +* It's unlikely we'll ever actually find ourselves +* here, as a file small enough to fit inline won't be +* big enough to store more than the swap header, but in +* case something changes in the future, let's catch it +* here rather than later. +*/ +btrfs_err (fs_info,"swapfile is inline" ); + ret = -EINVAL ;
here
fs/btrfs/inode.c out_inode: · line 9532
Quoted 5 lines
fs/btrfs/inode.c +5
-0+ goto out; + } +if (test_bit (EXTENT_FLAG_COMPRESSED , &em->flags)) { +btrfs_err (fs_info,"swapfile is compresed" ); + ret = -EINVAL ;
here
fs/btrfs/inode.c out_inode: · line 9536
Quoted 4 lines
fs/btrfs/inode.c +4
-0+ goto out; + } + ret =can_nocow_extent (inode, start, &len,NULL ,NULL ,NULL ); +if (ret <0 ) {
here
fs/btrfs/inode.c out_inode: · line 9543
Quoted 7 lines
fs/btrfs/inode.c +7
-0+ goto out; + }else if (ret ==1 ) { + ret =0 ; + }else { +btrfs_err (fs_info,"swapfile has extent requiring COW (%llu-%llu)" , + start, start + len -1 ); + ret = -EINVAL ;
here
fs/btrfs/inode.c out_inode: · line 9554
Quoted 11 lines
fs/btrfs/inode.c +11
-0+ goto out; + } + +set_bit (EXTENT_FLAG_SWAPFILE , &em->flags); + + start =extent_map_end (em); +free_extent_map (em); + } + +out: +if (ret) {
should be fixed by:
if (!IS_ERR_OR_NULL(em)) free_extent_map(em);
fs/btrfs/inode.c out_inode: · line 9560
Quoted 6 lines
fs/btrfs/inode.c +6
-0+__clear_swapfile_extents (inode); +atomic_dec (&BTRFS_I (inode)->root->nr_swapfiles); + } +unlock_extent_cached (io_tree,0 , isize -1 , &cached_state,GFP_NOFS ); +return ret; +}
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [RFC PATCH v3 7/7] btrfs: enable swap file support
Quoted 17 lines · David Sterba
fs/btrfs/inode.c out_inode: · line 9458
Quoted 12 lines · Omar Sandoval
fs/btrfs/inode.c +12
-0+static void __clear_swapfile_extents (struct inode *inode) +{ +u64 isize = inode->i_size; +struct extent_map *em; +u64 start, len; + + start =0 ; +while (start < isize) { + len = isize - start; + em =btrfs_get_extent (inode,NULL ,0 , start, len,0 ); +if (IS_ERR (em)) +return ;This could transiently fail if there's no memory to allocate the em, and would leak the following extents.
This leak I was aware of, and at the time I didn't see a good way to get around it. After all, if we can't get the current extent, there's no way to iterate through the rest of them. Now I see that instead of doing this at the btrfs_get_extent level, I can just go through all of the extent_maps in the extent_map_tree.
Quoted 122 lines
Quoted 39 lines
fs/btrfs/inode.c +39
-0+ +clear_bit (EXTENT_FLAG_SWAPFILE , &em->flags); + + start =extent_map_end (em); +free_extent_map (em); + } +} + +static int btrfs_swap_activate (struct swap_info_struct *sis,struct file *file, +sector_t *span) +{ +struct inode *inode =file_inode (file); +struct btrfs_fs_info *fs_info =BTRFS_I (inode)->root->fs_info; +struct extent_io_tree *io_tree = &BTRFS_I (inode)->io_tree; +int ret =0 ; +u64 isize = inode->i_size; +struct extent_state *cached_state =NULL ; +struct extent_map *em; +u64 start, len; + +if (BTRFS_I (inode)->flags &BTRFS_INODE_COMPRESS ) { +/* Can't do direct I/O on a compressed file. */ +btrfs_err (fs_info,"swapfile is compressed" ); +return -EINVAL ; + } +if (!(BTRFS_I (inode)->flags &BTRFS_INODE_NODATACOW )) { +/* +* Going through the copy-on-write path while swapping pages +* in/out and doing a bunch of allocations could stress the +* memory management code that got us there in the first place, +* and that's sure to be a bad time. +*/ +btrfs_err (fs_info,"swapfile is copy-on-write" ); +return -EINVAL ; + } + +lock_extent_bits (io_tree,0 , isize -1 ,0 , &cached_state); + +/* fs/btrfs/inode.c out_inode: · line 9507
Quoted 10 lines
fs/btrfs/inode.c +10
-0+* All of the extents must be allocated and support direct I/O. Inline +* extents and compressed extents fall back to buffered I/O, so those +* are no good. Additionally, all of the extents must be safe for nocow. +*/ +atomic_inc (&BTRFS_I (inode)->root->nr_swapfiles); + start =0 ; +while (start < isize) { + len = isize - start; + em =btrfs_get_extent (inode,NULL ,0 , start, len,0 ); +if (IS_ERR (em)) {IS_ERR_OR_NULL(em)From now on the em is valid and has to be free_extent_map()ed ...
fs/btrfs/inode.c out_inode: · line 9515
Quoted 8 lines
fs/btrfs/inode.c +8
-0+ ret =PTR_ERR (em); + goto out; + } + +if (test_bit (EXTENT_FLAG_VACANCY , &em->flags) || + em->block_start ==EXTENT_MAP_HOLE ) { +btrfs_err (fs_info,"swapfile has holes" ); + ret = -EINVAL ;... and all the error branches would miss it.
fs/btrfs/inode.c out_inode: · line 9527
Quoted 12 lines
fs/btrfs/inode.c +12
-0+ goto out; + } +if (em->block_start ==EXTENT_MAP_INLINE ) { +/* +* It's unlikely we'll ever actually find ourselves +* here, as a file small enough to fit inline won't be +* big enough to store more than the swap header, but in +* case something changes in the future, let's catch it +* here rather than later. +*/ +btrfs_err (fs_info,"swapfile is inline" ); + ret = -EINVAL ;here
fs/btrfs/inode.c out_inode: · line 9532
Quoted 5 lines
fs/btrfs/inode.c +5
-0+ goto out; + } +if (test_bit (EXTENT_FLAG_COMPRESSED , &em->flags)) { +btrfs_err (fs_info,"swapfile is compresed" ); + ret = -EINVAL ;here
fs/btrfs/inode.c out_inode: · line 9536
Quoted 4 lines
fs/btrfs/inode.c +4
-0+ goto out; + } + ret =can_nocow_extent (inode, start, &len,NULL ,NULL ,NULL ); +if (ret <0 ) {here
fs/btrfs/inode.c out_inode: · line 9543
Quoted 7 lines
fs/btrfs/inode.c +7
-0+ goto out; + }else if (ret ==1 ) { + ret =0 ; + }else { +btrfs_err (fs_info,"swapfile has extent requiring COW (%llu-%llu)" , + start, start + len -1 ); + ret = -EINVAL ;here
fs/btrfs/inode.c out_inode: · line 9554
Quoted 11 lines
fs/btrfs/inode.c +11
-0+ goto out; + } + +set_bit (EXTENT_FLAG_SWAPFILE , &em->flags); + + start =extent_map_end (em); +free_extent_map (em); + } + +out: +if (ret) {should be fixed by:
if (!IS_ERR_OR_NULL(em)) free_extent_map(em);
Quoted 6 lines
fs/btrfs/inode.c out_inode: · line 9560
Quoted 6 lines
fs/btrfs/inode.c +6
-0+__clear_swapfile_extents (inode); +atomic_dec (&BTRFS_I (inode)->root->nr_swapfiles); + } +unlock_extent_cached (io_tree,0 , isize -1 , &cached_state,GFP_NOFS ); +return ret; +}
This leak I completely missed. Thanks.
Signature · 7 lines
Omar -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 6 lines · Omar Sandoval
After some discussion on the mailing list, I decided that for simplicity and reliability, it's best to simply disallow COW files and files with shared extents (like files with extents shared with a snapshot). From a user's perspective, this means that a snapshotted subvolume cannot be used for a swap file, but keeping the swap file in a separate subvolume that is never snapshotted seems entirely reasonable to me.
Well, there are enough special cases how to do things on btrfs and I'd like to avoid introducing another one.
Quoted 5 lines
An alternative suggestion was to
allow swap files to be snapshotted and to do an implied COW on swap file activation, which I was ready to implement until I realized that we can't permit snapshotting a subvolume with an active swap file, so this creates a surprising inconsistency for users (in my opinion).
I still don't see why it's not possible to do the snapshot with an active swapfile.
Quoted 3 lines
As with before, this functionality is tenuously tested in a virtual machine with some artificial workloads, but it "works for me". I'm pretty happy with the results on my end, so please comment away.
The non-btrfs changes can go independently and do not have to wait until we resolve the swap vs snapshot problem.
I did a simple test and it crashed instantly, lockep complains:
memory: 2G
swap file: 1G
kernel: 3.17 + v3
[ 739.790731] Adding 1054716k swap on /mnt/test-swap/mnt/swapfile. Priority:-1 extents:1 across:1054716k
[ 751.848607]
[ 751.851852] =====================================
[ 751.852161] [ BUG: bad unlock balance detected! ]
[ 751.852161] 3.17.0-default+ #199 Not tainted
[ 751.852161] -------------------------------------
[ 751.852161] heavy_swap/4119 is trying to release lock (&sb->s_type->i_mutex_key) at:
[ 751.852161] [<ffffffff81a4f0ce>] mutex_unlock+0xe/0x10
[ 751.852161] but there are no more locks to release!
[ 751.852161]
[ 751.852161] other info that might help us debug this:
[ 751.852161] 1 lock held by heavy_swap/4119:
[ 751.852161] #0: (&mm->mmap_sem){++++++}, at: [<ffffffff81043ba9>] __do_page_fault+0x149/0x560
[ 751.852161]
[ 751.852161] stack backtrace:
[ 751.852161] CPU: 1 PID: 4119 Comm: heavy_swap Not tainted 3.17.0-default+ #199
[ 751.852161] Hardware name: Intel Corporation Santa Rosa platform/Matanzas, BIOS TSRSCRB1.86C.0047.B00.0610170821 10/17/06
[ 751.852161] ffffffff81a4f0ce ffff880075dbb3d8 ffffffff81a4b268 0000000000000001
[ 751.852161] ffff8800775e0000 ffff880075dbb408 ffffffff810b51a9 0000000000000000
[ 751.852161] ffff8800775e0000 00000000ffffffff ffff8800763c9d00 ffff880075dbb4a8
[ 751.852161] Call Trace:
[ 751.852161] [<ffffffff81a4f0ce>] ? mutex_unlock+0xe/0x10
[ 751.852161] [<ffffffff81a4b268>] dump_stack+0x51/0x71
[ 751.852161] [<ffffffff810b51a9>] print_unlock_imbalance_bug+0xf9/0x100
[ 751.852161] [<ffffffff810b8bcf>] lock_release_non_nested+0x2cf/0x3e0
[ 751.852161] [<ffffffff81a541ed>] ? ftrace_call+0x5/0x2f
[ 751.852161] [<ffffffff81a4f0ce>] ? mutex_unlock+0xe/0x10
[ 751.852161] [<ffffffff81a4f0ce>] ? mutex_unlock+0xe/0x10
[ 751.852161] [<ffffffff810b8da9>] lock_release+0xc9/0x240
[ 751.852161] [<ffffffff81a4eef0>] __mutex_unlock_slowpath+0x80/0x190
[ 751.852161] [<ffffffff81a4f0c9>] ? mutex_unlock+0x9/0x10
[ 751.852161] [<ffffffff81a4f0ce>] mutex_unlock+0xe/0x10
[ 751.852161] [<ffffffffa0037c88>] btrfs_direct_IO+0x2b8/0x310 [btrfs]
[ 751.852161] [<ffffffff810acf4d>] ? __wake_up_bit+0xd/0x50
[ 751.852161] [<ffffffff8117e03b>] __swap_writepage+0x10b/0x270
[ 751.852161] [<ffffffff81180723>] ? page_swapcount+0x53/0x70
[ 751.852161] [<ffffffff8117e1d7>] swap_writepage+0x37/0x60
[ 751.852161] [<ffffffff8115a072>] shmem_writepage+0x2a2/0x2e0
[ 751.852161] [<ffffffff811554ae>] shrink_page_list+0x44e/0x9d0
[ 751.852161] [<ffffffff81a51b50>] ? _raw_spin_unlock_irq+0x30/0x40
[ 751.852161] [<ffffffff811560cd>] shrink_inactive_list+0x26d/0x4f0
[ 751.852161] [<ffffffff813adcc9>] ? blk_start_plug+0x9/0x50
[ 751.852161] [<ffffffff81156918>] shrink_lruvec+0x5c8/0x6c0
[ 751.852161] [<ffffffff81165f09>] ? compaction_suitable+0x19/0xc0
[ 751.852161] [<ffffffff81165f09>] ? compaction_suitable+0x19/0xc0
[ 751.852161] [<ffffffff81156a5d>] shrink_zone+0x4d/0x120
[ 751.852161] [<ffffffff811577ea>] do_try_to_free_pages+0x19a/0x3a0
[ 751.852161] [<ffffffff81152a7d>] ? pfmemalloc_watermark_ok+0xd/0xc0
[ 751.852161] [<ffffffff81157b42>] try_to_free_pages+0xb2/0x160
[ 751.852161] [<ffffffff81a4bb79>] ? _cond_resched+0x9/0x30
[ 751.852161] [<ffffffff8114adfb>] __alloc_pages_nodemask+0x5eb/0xa90
[ 751.852161] [<ffffffff81a541ed>] ? ftrace_call+0x5/0x2f
[ 751.852161] [<ffffffff811784f1>] ? anon_vma_prepare+0x21/0x190
[ 751.852161] [<ffffffff811916a8>] do_huge_pmd_anonymous_page+0xe8/0x330
[ 751.852161] [<ffffffff811783c9>] ? is_vma_temporary_stack+0x9/0x30
[ 751.852161] [<ffffffff8116edd5>] handle_mm_fault+0x135/0xb60
[ 751.852161] [<ffffffff81172015>] ? find_vma+0x15/0x80
[ 751.852161] [<ffffffff81166c6d>] ? vmacache_find+0xd/0xd0
[ 751.852161] [<ffffffff81097c2e>] ? __might_sleep+0xe/0x110
[ 751.852161] [<ffffffff81043c0d>] __do_page_fault+0x1ad/0x560
[ 751.852161] [<ffffffff81073000>] ? do_fork+0xe0/0x420
[ 751.852161] [<ffffffff81a53f43>] ? error_sti+0x5/0x6
[ 751.852161] [<ffffffff813e660d>] ? trace_hardirqs_off_thunk+0x3a/0x3c
[ 751.852161] [<ffffffff810440cc>] do_page_fault+0xc/0x10
[ 751.852161] [<ffffffff81a53d42>] page_fault+0x22/0x30--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 11 lines · David Sterba
Quoted 6 lines · Omar Sandoval
After some discussion on the mailing list, I decided that for simplicity and reliability, it's best to simply disallow COW files and files with shared extents (like files with extents shared with a snapshot). From a user's perspective, this means that a snapshotted subvolume cannot be used for a swap file, but keeping the swap file in a separate subvolume that is never snapshotted seems entirely reasonable to me.
Well, there are enough special cases how to do things on btrfs and I'd like to avoid introducing another one.
Quoted 9 lines
Quoted 5 lines
An alternative suggestion was to
allow swap files to be snapshotted and to do an implied COW on swap file activation, which I was ready to implement until I realized that we can't permit snapshotting a subvolume with an active swap file, so this creates a surprising inconsistency for users (in my opinion).I still don't see why it's not possible to do the snapshot with an active swapfile.
Creating a snapshot of an active swapfile would create shared extents, so the next time we have to swap out a page, we'd have to do a COW, which we're already trying pretty hard to avoid. We could allow it, but it might lead to some unreliable behavior and unhappy emails to the mailing list. However, I do see your point about wanting to avoid special cases, so I'd like to get some more input from others on this as well.
Quoted 4 lines
Quoted 3 lines
As with before, this functionality is tenuously tested in a virtual machine with some artificial workloads, but it "works for me". I'm pretty happy with the results on my end, so please comment away.
Quoted 9 lines
The non-btrfs changes can go independently and do not have to wait until we resolve the swap vs snapshot problem.
I did a simple test and it crashed instantly, lockep complains:
memory: 2G
swap file: 1G
kernel: 3.17 + v3
[snip]
That's my fault for not running with lockdep enabled. The problem here is that swap-over-NFS is the only caller of nfs_direct_IO, so nfs_direct_IO doesn't observe the normal direct_IO locking conventions and neither does swap_writepage. I'll have to shuffle around some code on the NFS side to fix that.
It looks like the non-btrfs parts of this might get a bit bigger, so I'll look into getting that in separately.
Thanks!
Signature · 7 lines
Omar -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>