mbox series

[v9,0/6] Btrfs: implement swap file support

Message ID cover.1538072009.git.osandov@fb.com (mailing list archive)
Headers show
Series Btrfs: implement swap file support | expand

Message

Omar Sandoval Sept. 27, 2018, 6:17 p.m. UTC
From: Omar Sandoval <osandov@fb.com>

Hi,

This series implements swap file support for Btrfs.

Changes from v8 [1]:

- Fixed a bug in btrfs_swap_activate() which would cause us to miss some
  file extents if they were merged into one extent map entry.
- Fixed build for !CONFIG_SWAP.
- Changed all error messages to KERN_WARN.
- Unindented long error messages.

I've Cc'd Jon and Al on patch 3 this time, so hopefully we can get an
ack for that one, too.

Thanks!

1: https://www.spinics.net/lists/linux-btrfs/msg82267.html

Omar Sandoval (6):
  mm: split SWP_FILE into SWP_ACTIVATED and SWP_FS
  mm: export add_swap_extent()
  vfs: update swap_{,de}activate documentation
  Btrfs: prevent ioctls from interfering with a swap file
  Btrfs: rename get_chunk_map() and make it non-static
  Btrfs: support swap files

 Documentation/filesystems/Locking |  17 +-
 Documentation/filesystems/vfs.txt |  12 +-
 fs/btrfs/ctree.h                  |  29 +++
 fs/btrfs/dev-replace.c            |   8 +
 fs/btrfs/disk-io.c                |   4 +
 fs/btrfs/inode.c                  | 338 ++++++++++++++++++++++++++++++
 fs/btrfs/ioctl.c                  |  31 ++-
 fs/btrfs/relocation.c             |  18 +-
 fs/btrfs/volumes.c                |  82 ++++++--
 fs/btrfs/volumes.h                |   2 +
 include/linux/swap.h              |  13 +-
 mm/page_io.c                      |   6 +-
 mm/swapfile.c                     |  14 +-
 13 files changed, 523 insertions(+), 51 deletions(-)

Comments

David Sterba Oct. 19, 2018, 3:43 p.m. UTC | #1
On Thu, Sep 27, 2018 at 11:17:32AM -0700, Omar Sandoval wrote:
> From: Omar Sandoval <osandov@fb.com>
> This series implements swap file support for Btrfs.
> 
> Changes from v8 [1]:
> 
> - Fixed a bug in btrfs_swap_activate() which would cause us to miss some
>   file extents if they were merged into one extent map entry.
> - Fixed build for !CONFIG_SWAP.
> - Changed all error messages to KERN_WARN.
> - Unindented long error messages.
> 
> I've Cc'd Jon and Al on patch 3 this time, so hopefully we can get an
> ack for that one, too.
> 
> Thanks!
> 
> 1: https://www.spinics.net/lists/linux-btrfs/msg82267.html
> 
> Omar Sandoval (6):
>   mm: split SWP_FILE into SWP_ACTIVATED and SWP_FS
>   mm: export add_swap_extent()
>   vfs: update swap_{,de}activate documentation
>   Btrfs: prevent ioctls from interfering with a swap file
>   Btrfs: rename get_chunk_map() and make it non-static
>   Btrfs: support swap files

Patches 1 and 2 now going through Andrew's tree, the btrfs part will be
delayed and not merged to 4.20. This is a bit unfortuante, I was busy
with the non-feature patches and other things, sorry.
Omar Sandoval Oct. 22, 2018, 9:13 p.m. UTC | #2
On Fri, Oct 19, 2018 at 05:43:18PM +0200, David Sterba wrote:
> On Thu, Sep 27, 2018 at 11:17:32AM -0700, Omar Sandoval wrote:
> > From: Omar Sandoval <osandov@fb.com>
> > This series implements swap file support for Btrfs.
> > 
> > Changes from v8 [1]:
> > 
> > - Fixed a bug in btrfs_swap_activate() which would cause us to miss some
> >   file extents if they were merged into one extent map entry.
> > - Fixed build for !CONFIG_SWAP.
> > - Changed all error messages to KERN_WARN.
> > - Unindented long error messages.
> > 
> > I've Cc'd Jon and Al on patch 3 this time, so hopefully we can get an
> > ack for that one, too.
> > 
> > Thanks!
> > 
> > 1: https://www.spinics.net/lists/linux-btrfs/msg82267.html
> > 
> > Omar Sandoval (6):
> >   mm: split SWP_FILE into SWP_ACTIVATED and SWP_FS
> >   mm: export add_swap_extent()
> >   vfs: update swap_{,de}activate documentation
> >   Btrfs: prevent ioctls from interfering with a swap file
> >   Btrfs: rename get_chunk_map() and make it non-static
> >   Btrfs: support swap files
> 
> Patches 1 and 2 now going through Andrew's tree, the btrfs part will be
> delayed and not merged to 4.20. This is a bit unfortuante, I was busy
> with the non-feature patches and other things, sorry.

That's perfectly fine with me, thanks, Dave!
David Sterba Nov. 5, 2018, 6:07 p.m. UTC | #3
On Mon, Oct 22, 2018 at 02:13:27PM -0700, Omar Sandoval wrote:
> > > Omar Sandoval (6):
> > >   mm: split SWP_FILE into SWP_ACTIVATED and SWP_FS
> > >   mm: export add_swap_extent()
> > >   vfs: update swap_{,de}activate documentation

> > >   Btrfs: prevent ioctls from interfering with a swap file
> > >   Btrfs: rename get_chunk_map() and make it non-static
> > >   Btrfs: support swap files
> > 
> > Patches 1 and 2 now going through Andrew's tree, the btrfs part will be
> > delayed and not merged to 4.20. This is a bit unfortuante, I was busy
> > with the non-feature patches and other things, sorry.
> 
> That's perfectly fine with me, than

The 3 btrfs patches are now in misc-next.

Setting up the swap file needs the extra steps to make sure it's a NOCOW
file and preallocated, that's just 2 more commands using common tools.
The surprise may come on an a multi-device filesystem when the chunks
get spread over more devices and the user has no control over that.
Reducing the file size until it fits is a workaround, tough not totally
reliable.

I've explored the error cases (balance, dev delete, adding more
swapfiles). Also a stress test (make -j on kernel). The OOM killer was
able to get the system back after each round after which I added one
more swapfile, until the system was effectively dead.

So the stability seems to be ok, we will need to document the usecase,
constraints and how to properly set up the swapfile, and that's about
it. Thanks.
David Sterba Nov. 6, 2018, 9:54 a.m. UTC | #4
On Thu, Sep 27, 2018 at 11:17:32AM -0700, Omar Sandoval wrote:
> From: Omar Sandoval <osandov@fb.com>
> This series implements swap file support for Btrfs.
> 
> Changes from v8 [1]:
> 
> - Fixed a bug in btrfs_swap_activate() which would cause us to miss some
>   file extents if they were merged into one extent map entry.
> - Fixed build for !CONFIG_SWAP.
> - Changed all error messages to KERN_WARN.
> - Unindented long error messages.
> 
> I've Cc'd Jon and Al on patch 3 this time, so hopefully we can get an
> ack for that one, too.
> 
> Thanks!
> 
> 1: https://www.spinics.net/lists/linux-btrfs/msg82267.html
> 
> Omar Sandoval (6):
>   mm: split SWP_FILE into SWP_ACTIVATED and SWP_FS
>   mm: export add_swap_extent()
>   vfs: update swap_{,de}activate documentation
>   Btrfs: prevent ioctls from interfering with a swap file
>   Btrfs: rename get_chunk_map() and make it non-static
>   Btrfs: support swap files

fstest generic/472 reports an assertion failure. This is on the updated fstests
git (70c4067285b0bc076), though it should not matter:

[16597.002190] assertion failed: IS_ALIGNED(start, fs_info->sectorsize) && IS_ALIGNED(end + 1, fs_info->sectorsize), file: fs/btrfs/file-item.c, line: 319
[16597.016154] ------------[ cut here ]------------
[16597.020911] kernel BUG at fs/btrfs/ctree.h:3448!
[16597.025692] invalid opcode: 0000 [#1] PREEMPT SMP
[16597.030540] CPU: 1 PID: 2216 Comm: swapon Tainted: G        W         4.19.0-1.ge195904-vanilla+ #343
[16597.030542] Hardware name: empty empty/S3993, BIOS PAQEX0-3 02/24/2008
[16597.030617] RIP: 0010:btrfs_lookup_csums_range+0x3ee/0x6e0 [btrfs]
[16597.030623] RSP: 0018:ffff959a8210fc00 EFLAGS: 00010286
[16597.030626] RAX: 000000000000008b RBX: ffff959a8210fc40 RCX: 0000000000000000
[16597.030628] RDX: 0000000000000000 RSI: ffff89c766fd60a8 RDI: ffff89c766fd60a8
[16597.030630] RBP: 0000000000f10000 R08: 0000000000000001 R09: 0000000000000000
[16597.030631] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
[16597.030633] R13: ffff89c74f47c000 R14: 0000000000000000 R15: 0000000000003e4f
[16597.030635] FS:  00007f3d80a18700(0000) GS:ffff89c766e00000(0000) knlGS:0000000000000000
[16597.030637] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[16597.030639] CR2: 00007f3d7fa733b0 CR3: 000000021e0b5000 CR4: 00000000000006e0
[16597.030640] Call Trace:
[16597.030657]  ? __mutex_unlock_slowpath+0x4b/0x2b0
[16597.142625]  ? btrfs_cross_ref_exist+0x6d/0xa0 [btrfs]
[16597.142674]  csum_exist_in_range+0x43/0xc2 [btrfs]
[16597.152938]  can_nocow_extent+0x442/0x4a0 [btrfs]
[16597.152993]  btrfs_swap_activate+0x19f/0x680 [btrfs]
[16597.162947]  __do_sys_swapon+0xdf8/0x1550
[16597.162965]  do_syscall_64+0x5c/0x1b0
[16597.162971]  entry_SYSCALL_64_after_hwframe+0x49/0xbe
[16597.162974] RIP: 0033:0x7f3d7fb1f067
[16597.198875] RSP: 002b:00007ffcf9c49808 EFLAGS: 00000202 ORIG_RAX: 00000000000000a7
[16597.198877] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f3d7fb1f067
[16597.198879] RDX: 00007ffcf9c49910 RSI: 0000000000000000 RDI: 00007ffcf9c4a8f0
[16597.198884] RBP: 0000000000000000 R08: 00007f3d7fdd8c40 R09: 00007f3d8081dfc0
[16597.228520] R10: 000000001cc0c4fd R11: 0000000000000202 R12: 00000000004006a0
[16597.228522] R13: 00007ffcf9c498f0 R14: 0000000000000000 R15: 0000000000000000
[16597.311530] ---[ end trace f4dc8cabefc3e36d ]---
[16597.316318] RIP: 0010:btrfs_lookup_csums_range+0x3ee/0x6e0 [btrfs]
[16597.341828] RSP: 0018:ffff959a8210fc00 EFLAGS: 00010286
[16597.341834] RAX: 000000000000008b RBX: ffff959a8210fc40 RCX: 0000000000000000
[16597.354588] RDX: 0000000000000000 RSI: ffff89c766fd60a8 RDI: ffff89c766fd60a8
[16597.354590] RBP: 0000000000f10000 R08: 0000000000000001 R09: 0000000000000000
[16597.354592] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
[16597.354593] R13: ffff89c74f47c000 R14: 0000000000000000 R15: 0000000000003e4f
[16597.354595] FS:  00007f3d80a18700(0000) GS:ffff89c766e00000(0000) knlGS:0000000000000000
[16597.354596] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[16597.354598] CR2: 00007f3d7fa733b0 CR3: 000000021e0b5000 CR4: 00000000000006e0
David Sterba Nov. 7, 2018, 2:49 p.m. UTC | #5
On Tue, Nov 06, 2018 at 10:54:51AM +0100, David Sterba wrote:
> On Thu, Sep 27, 2018 at 11:17:32AM -0700, Omar Sandoval wrote:
> > From: Omar Sandoval <osandov@fb.com>
> > This series implements swap file support for Btrfs.
> > 
> > Changes from v8 [1]:
> > 
> > - Fixed a bug in btrfs_swap_activate() which would cause us to miss some
> >   file extents if they were merged into one extent map entry.
> > - Fixed build for !CONFIG_SWAP.
> > - Changed all error messages to KERN_WARN.
> > - Unindented long error messages.
> > 
> > I've Cc'd Jon and Al on patch 3 this time, so hopefully we can get an
> > ack for that one, too.
> > 
> > Thanks!
> > 
> > 1: https://www.spinics.net/lists/linux-btrfs/msg82267.html
> > 
> > Omar Sandoval (6):
> >   mm: split SWP_FILE into SWP_ACTIVATED and SWP_FS
> >   mm: export add_swap_extent()
> >   vfs: update swap_{,de}activate documentation
> >   Btrfs: prevent ioctls from interfering with a swap file
> >   Btrfs: rename get_chunk_map() and make it non-static
> >   Btrfs: support swap files
> 
> fstest generic/472 reports an assertion failure. This is on the updated fstests
> git (70c4067285b0bc076), though it should not matter:
> 
> [16597.002190] assertion failed: IS_ALIGNED(start, fs_info->sectorsize) && IS_ALIGNED(end + 1, fs_info->sectorsize), file: fs/btrfs/file-item.c, line: 319

I have to revert the patch for now as it kills the testing machines.
Nikolay Borisov Nov. 7, 2018, 3:07 p.m. UTC | #6
On 7.11.18 г. 16:49 ч., David Sterba wrote:
> On Tue, Nov 06, 2018 at 10:54:51AM +0100, David Sterba wrote:
>> On Thu, Sep 27, 2018 at 11:17:32AM -0700, Omar Sandoval wrote:
>>> From: Omar Sandoval <osandov@fb.com>
>>> This series implements swap file support for Btrfs.
>>>
>>> Changes from v8 [1]:
>>>
>>> - Fixed a bug in btrfs_swap_activate() which would cause us to miss some
>>>   file extents if they were merged into one extent map entry.
>>> - Fixed build for !CONFIG_SWAP.
>>> - Changed all error messages to KERN_WARN.
>>> - Unindented long error messages.
>>>
>>> I've Cc'd Jon and Al on patch 3 this time, so hopefully we can get an
>>> ack for that one, too.
>>>
>>> Thanks!
>>>
>>> 1: https://www.spinics.net/lists/linux-btrfs/msg82267.html
>>>
>>> Omar Sandoval (6):
>>>   mm: split SWP_FILE into SWP_ACTIVATED and SWP_FS
>>>   mm: export add_swap_extent()
>>>   vfs: update swap_{,de}activate documentation
>>>   Btrfs: prevent ioctls from interfering with a swap file
>>>   Btrfs: rename get_chunk_map() and make it non-static
>>>   Btrfs: support swap files
>>
>> fstest generic/472 reports an assertion failure. This is on the updated fstests
>> git (70c4067285b0bc076), though it should not matter:
>>
>> [16597.002190] assertion failed: IS_ALIGNED(start, fs_info->sectorsize) && IS_ALIGNED(end + 1, fs_info->sectorsize), file: fs/btrfs/file-item.c, line: 319
> 
> I have to revert the patch for now as it kills the testing machines.

The reason is that the isize is not aligned to a sectorsize. Ie it
should be:

+       u64 isize = ALIGN_DOWN(i_size_read(inode), fs_info->sectorsize);

With this fixlet generic/472 succeeds.

>
David Sterba Nov. 7, 2018, 3:28 p.m. UTC | #7
On Wed, Nov 07, 2018 at 05:07:00PM +0200, Nikolay Borisov wrote:
> 
> 
> On 7.11.18 г. 16:49 ч., David Sterba wrote:
> > On Tue, Nov 06, 2018 at 10:54:51AM +0100, David Sterba wrote:
> >> On Thu, Sep 27, 2018 at 11:17:32AM -0700, Omar Sandoval wrote:
> >>> From: Omar Sandoval <osandov@fb.com>
> >>> This series implements swap file support for Btrfs.
> >>>
> >>> Changes from v8 [1]:
> >>>
> >>> - Fixed a bug in btrfs_swap_activate() which would cause us to miss some
> >>>   file extents if they were merged into one extent map entry.
> >>> - Fixed build for !CONFIG_SWAP.
> >>> - Changed all error messages to KERN_WARN.
> >>> - Unindented long error messages.
> >>>
> >>> I've Cc'd Jon and Al on patch 3 this time, so hopefully we can get an
> >>> ack for that one, too.
> >>>
> >>> Thanks!
> >>>
> >>> 1: https://www.spinics.net/lists/linux-btrfs/msg82267.html
> >>>
> >>> Omar Sandoval (6):
> >>>   mm: split SWP_FILE into SWP_ACTIVATED and SWP_FS
> >>>   mm: export add_swap_extent()
> >>>   vfs: update swap_{,de}activate documentation
> >>>   Btrfs: prevent ioctls from interfering with a swap file
> >>>   Btrfs: rename get_chunk_map() and make it non-static
> >>>   Btrfs: support swap files
> >>
> >> fstest generic/472 reports an assertion failure. This is on the updated fstests
> >> git (70c4067285b0bc076), though it should not matter:
> >>
> >> [16597.002190] assertion failed: IS_ALIGNED(start, fs_info->sectorsize) && IS_ALIGNED(end + 1, fs_info->sectorsize), file: fs/btrfs/file-item.c, line: 319
> > 
> > I have to revert the patch for now as it kills the testing machines.
> 
> The reason is that the isize is not aligned to a sectorsize. Ie it
> should be:
> 
> +       u64 isize = ALIGN_DOWN(i_size_read(inode), fs_info->sectorsize);
> 
> With this fixlet generic/472 succeeds.

Thanks for the fix, I'll fold it in.
Omar Sandoval Nov. 10, 2018, 4:12 a.m. UTC | #8
On Wed, Nov 07, 2018 at 04:28:10PM +0100, David Sterba wrote:
> On Wed, Nov 07, 2018 at 05:07:00PM +0200, Nikolay Borisov wrote:
> > 
> > 
> > On 7.11.18 г. 16:49 ч., David Sterba wrote:
> > > On Tue, Nov 06, 2018 at 10:54:51AM +0100, David Sterba wrote:
> > >> On Thu, Sep 27, 2018 at 11:17:32AM -0700, Omar Sandoval wrote:
> > >>> From: Omar Sandoval <osandov@fb.com>
> > >>> This series implements swap file support for Btrfs.
> > >>>
> > >>> Changes from v8 [1]:
> > >>>
> > >>> - Fixed a bug in btrfs_swap_activate() which would cause us to miss some
> > >>>   file extents if they were merged into one extent map entry.
> > >>> - Fixed build for !CONFIG_SWAP.
> > >>> - Changed all error messages to KERN_WARN.
> > >>> - Unindented long error messages.
> > >>>
> > >>> I've Cc'd Jon and Al on patch 3 this time, so hopefully we can get an
> > >>> ack for that one, too.
> > >>>
> > >>> Thanks!
> > >>>
> > >>> 1: https://www.spinics.net/lists/linux-btrfs/msg82267.html
> > >>>
> > >>> Omar Sandoval (6):
> > >>>   mm: split SWP_FILE into SWP_ACTIVATED and SWP_FS
> > >>>   mm: export add_swap_extent()
> > >>>   vfs: update swap_{,de}activate documentation
> > >>>   Btrfs: prevent ioctls from interfering with a swap file
> > >>>   Btrfs: rename get_chunk_map() and make it non-static
> > >>>   Btrfs: support swap files
> > >>
> > >> fstest generic/472 reports an assertion failure. This is on the updated fstests
> > >> git (70c4067285b0bc076), though it should not matter:
> > >>
> > >> [16597.002190] assertion failed: IS_ALIGNED(start, fs_info->sectorsize) && IS_ALIGNED(end + 1, fs_info->sectorsize), file: fs/btrfs/file-item.c, line: 319
> > > 
> > > I have to revert the patch for now as it kills the testing machines.
> > 
> > The reason is that the isize is not aligned to a sectorsize. Ie it
> > should be:
> > 
> > +       u64 isize = ALIGN_DOWN(i_size_read(inode), fs_info->sectorsize);
> > 
> > With this fixlet generic/472 succeeds.
> 
> Thanks for the fix, I'll fold it in.

Thanks, Nikolay, I missed that. I don't think i_size_read() is
necessary, though, since the inode is locked.
David Sterba Nov. 12, 2018, 10:01 p.m. UTC | #9
On Fri, Nov 09, 2018 at 08:12:39PM -0800, Omar Sandoval wrote:
> On Wed, Nov 07, 2018 at 04:28:10PM +0100, David Sterba wrote:
> > On Wed, Nov 07, 2018 at 05:07:00PM +0200, Nikolay Borisov wrote:
> > > 
> > > 
> > > On 7.11.18 г. 16:49 ч., David Sterba wrote:
> > > > On Tue, Nov 06, 2018 at 10:54:51AM +0100, David Sterba wrote:
> > > >> On Thu, Sep 27, 2018 at 11:17:32AM -0700, Omar Sandoval wrote:
> > > >>> From: Omar Sandoval <osandov@fb.com>
> > > >>> This series implements swap file support for Btrfs.
> > > >>>
> > > >>> Changes from v8 [1]:
> > > >>>
> > > >>> - Fixed a bug in btrfs_swap_activate() which would cause us to miss some
> > > >>>   file extents if they were merged into one extent map entry.
> > > >>> - Fixed build for !CONFIG_SWAP.
> > > >>> - Changed all error messages to KERN_WARN.
> > > >>> - Unindented long error messages.
> > > >>>
> > > >>> I've Cc'd Jon and Al on patch 3 this time, so hopefully we can get an
> > > >>> ack for that one, too.
> > > >>>
> > > >>> Thanks!
> > > >>>
> > > >>> 1: https://www.spinics.net/lists/linux-btrfs/msg82267.html
> > > >>>
> > > >>> Omar Sandoval (6):
> > > >>>   mm: split SWP_FILE into SWP_ACTIVATED and SWP_FS
> > > >>>   mm: export add_swap_extent()
> > > >>>   vfs: update swap_{,de}activate documentation
> > > >>>   Btrfs: prevent ioctls from interfering with a swap file
> > > >>>   Btrfs: rename get_chunk_map() and make it non-static
> > > >>>   Btrfs: support swap files
> > > >>
> > > >> fstest generic/472 reports an assertion failure. This is on the updated fstests
> > > >> git (70c4067285b0bc076), though it should not matter:
> > > >>
> > > >> [16597.002190] assertion failed: IS_ALIGNED(start, fs_info->sectorsize) && IS_ALIGNED(end + 1, fs_info->sectorsize), file: fs/btrfs/file-item.c, line: 319
> > > > 
> > > > I have to revert the patch for now as it kills the testing machines.
> > > 
> > > The reason is that the isize is not aligned to a sectorsize. Ie it
> > > should be:
> > > 
> > > +       u64 isize = ALIGN_DOWN(i_size_read(inode), fs_info->sectorsize);
> > > 
> > > With this fixlet generic/472 succeeds.
> > 
> > Thanks for the fix, I'll fold it in.
> 
> Thanks, Nikolay, I missed that. I don't think i_size_read() is
> necessary, though, since the inode is locked.

Indeed, i_size used and patch pushed to misc-next.