lkml.org 
[lkml]   [2014]   [Nov]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2 5/5] btrfs: enable swap file support
On Sat, Nov 22, 2014 at 12:03:57PM -0800, Omar Sandoval wrote:
> On Fri, Nov 21, 2014 at 07:00:45PM +0100, David Sterba wrote:
> > > + ret = -EINVAL;
> > > + goto out;
> > > + }
> > > + if (test_bit(EXTENT_FLAG_COMPRESSED, &em->flags)) {
> > > + pr_err("BTRFS: swapfile is compresed");
> > > + ret = -EINVAL;
> > > + goto out;
> > > + }
> >
> > I think the preallocated extents should be refused as well. This means
> > the filesystem has enough space to hold the data but it would still have
> > to go through the allocation and could in turn stress the memory
> > management code that triggered the swapping activity in the first place.
> >
> > Though it's probably still possible to reach such corner case even with
> > fully allocated nodatacow file, this should be reviewed anyway.
> >
> I'll definitely take a closer look at this. In particular,
> btrfs_get_blocks_direct and btrfs_get_extent do allocations in some cases which
> I'll look into.
>
Alright, I took a look at this. My understanding is that a PREALLOC extent
represents a region on disk that has already been allocated but isn't in use
yet, but please correct me if I'm wrong. Judging by this comment in
btrfs_get_blocks_direct, we don't have to worry about PREALLOC extents in
general:

/*
* We don't allocate a new extent in the following cases
*
* 1) The inode is marked as NODATACOW. In this case we'll just use the
* existing extent.
* 2) The extent is marked as PREALLOC. We're good to go here and can
* just use the extent.
*
*/

A couple of other considerations that cropped up:

- btrfs_get_extent does a bunch of extra work if the extent is not cached in
the extent map tree that would be nice to avoid when swapping
- We might still have to do a COW if the swap file is in a snapshot

We can avoid the btrfs_get_extent by pinning the extents in memory one way or
another in btrfs_swap_activate.

The snapshot issue is a little tricker to resolve. I see a few options:

1. Just do the COW and hope for the best
2. As part of btrfs_swap_activate, COW any shared extents. If a snapshot
happens while a swap file is active, we'll fall back to 1.
3. Clobber any swap file extents which are in a snapshot, i.e., always use the
existing extent.

I'm partial to 3, as it's the simplest approach, and I don't think it makes
much sense for a swap file to be in a snapshot anyways. I'd appreciate any
comments that anyone might have.

--
Omar


\
 
 \ /
  Last update: 2014-11-24 23:41    [W:0.093 / U:1.888 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site