[v2,6/6] Btrfs: use less memory for delalloc sanity tests
diff mbox

Message ID c75f5b086bbf8ee43b2bb18e74aebcaeb81fa77a.1474580472.git.osandov@fb.com
State Superseded
Headers show

Commit Message

Omar Sandoval Sept. 23, 2016, 12:24 a.m. UTC
From: Omar Sandoval <osandov@fb.com>

test_find_delalloc() allocates 256 MB worth of pages. That's all of the
RAM that my MIPS emulator has, so it ends up panicking after it OOM
kills everything. We don't actually need to use that much for this test.

Signed-off-by: Omar Sandoval <osandov@fb.com>
---
 fs/btrfs/tests/extent-io-tests.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

David Sterba Sept. 23, 2016, 9:27 a.m. UTC | #1
On Thu, Sep 22, 2016 at 05:24:25PM -0700, Omar Sandoval wrote:
> From: Omar Sandoval <osandov@fb.com>
> 
> test_find_delalloc() allocates 256 MB worth of pages. That's all of the
> RAM that my MIPS emulator has, so it ends up panicking after it OOM
> kills everything. We don't actually need to use that much for this test.

I'm not sure we should limit it that way as more bytes can give it more
stress. Can we do it somehow dynamically ? Like start with 256M and fall
back to the numbers you've used.

Or maybe start from the low bound and allocate until it fails with first
ENOMEM.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Omar Sandoval Sept. 23, 2016, 4:52 p.m. UTC | #2
On Fri, Sep 23, 2016 at 11:27:27AM +0200, David Sterba wrote:
> On Thu, Sep 22, 2016 at 05:24:25PM -0700, Omar Sandoval wrote:
> > From: Omar Sandoval <osandov@fb.com>
> > 
> > test_find_delalloc() allocates 256 MB worth of pages. That's all of the
> > RAM that my MIPS emulator has, so it ends up panicking after it OOM
> > kills everything. We don't actually need to use that much for this test.
> 
> I'm not sure we should limit it that way as more bytes can give it more
> stress. Can we do it somehow dynamically ? Like start with 256M and fall
> back to the numbers you've used.
> 
> Or maybe start from the low bound and allocate until it fails with first
> ENOMEM.

I figured out how to get qemu to do highmem on MIPS, so we can drop this
patch. It's reasonable to assume that the sanity tests are running on a
machine with a decent amount of memory.
Omar Sandoval Sept. 23, 2016, 9:22 p.m. UTC | #3
On Fri, Sep 23, 2016 at 09:52:01AM -0700, Omar Sandoval wrote:
> On Fri, Sep 23, 2016 at 11:27:27AM +0200, David Sterba wrote:
> > On Thu, Sep 22, 2016 at 05:24:25PM -0700, Omar Sandoval wrote:
> > > From: Omar Sandoval <osandov@fb.com>
> > > 
> > > test_find_delalloc() allocates 256 MB worth of pages. That's all of the
> > > RAM that my MIPS emulator has, so it ends up panicking after it OOM
> > > kills everything. We don't actually need to use that much for this test.
> > 
> > I'm not sure we should limit it that way as more bytes can give it more
> > stress. Can we do it somehow dynamically ? Like start with 256M and fall
> > back to the numbers you've used.
> > 
> > Or maybe start from the low bound and allocate until it fails with first
> > ENOMEM.
> 
> I figured out how to get qemu to do highmem on MIPS, so we can drop this
> patch. It's reasonable to assume that the sanity tests are running on a
> machine with a decent amount of memory.

I lied, that didn't fix the problem for me, since these page allocations
can't use highmem. Anyways, the tests seem to be testing more for logic
errors manipulating the extent io tree, so just scaling it down
shouldn't be a huge issue, right?

The problem with relying on ENOMEM for anything is that the OOM killer
will kick in before we actually get the ENOMEM. I tried adding
__GFP_NORETRY, which claims that it doesn't invoke the OOM killer, but
it still did :(

Anyways, Cc Josef to see what he thinks since he wrote the test.
David Sterba Sept. 26, 2016, 3:58 p.m. UTC | #4
On Fri, Sep 23, 2016 at 02:22:53PM -0700, Omar Sandoval wrote:
> On Fri, Sep 23, 2016 at 09:52:01AM -0700, Omar Sandoval wrote:
> > On Fri, Sep 23, 2016 at 11:27:27AM +0200, David Sterba wrote:
> > > On Thu, Sep 22, 2016 at 05:24:25PM -0700, Omar Sandoval wrote:
> > > > From: Omar Sandoval <osandov@fb.com>
> > > > 
> > > > test_find_delalloc() allocates 256 MB worth of pages. That's all of the
> > > > RAM that my MIPS emulator has, so it ends up panicking after it OOM
> > > > kills everything. We don't actually need to use that much for this test.
> > > 
> > > I'm not sure we should limit it that way as more bytes can give it more
> > > stress. Can we do it somehow dynamically ? Like start with 256M and fall
> > > back to the numbers you've used.
> > > 
> > > Or maybe start from the low bound and allocate until it fails with first
> > > ENOMEM.
> > 
> > I figured out how to get qemu to do highmem on MIPS, so we can drop this
> > patch. It's reasonable to assume that the sanity tests are running on a
> > machine with a decent amount of memory.
> 
> I lied, that didn't fix the problem for me, since these page allocations
> can't use highmem. Anyways, the tests seem to be testing more for logic
> errors manipulating the extent io tree, so just scaling it down
> shouldn't be a huge issue, right?
> 
> The problem with relying on ENOMEM for anything is that the OOM killer
> will kick in before we actually get the ENOMEM. I tried adding
> __GFP_NORETRY, which claims that it doesn't invoke the OOM killer, but
> it still did :(

I think we cannot rely on NORETRY behaviour wrt OOM killer anyway. For
now I'd rather leave out the patch and let people test on machines with
enough memory. Any furhter ideas to make it work on low memory are
welcome but I don't want to block this patchset.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Omar Sandoval Sept. 26, 2016, 5:33 p.m. UTC | #5
On Mon, Sep 26, 2016 at 05:58:02PM +0200, David Sterba wrote:
> On Fri, Sep 23, 2016 at 02:22:53PM -0700, Omar Sandoval wrote:
> > On Fri, Sep 23, 2016 at 09:52:01AM -0700, Omar Sandoval wrote:
> > > On Fri, Sep 23, 2016 at 11:27:27AM +0200, David Sterba wrote:
> > > > On Thu, Sep 22, 2016 at 05:24:25PM -0700, Omar Sandoval wrote:
> > > > > From: Omar Sandoval <osandov@fb.com>
> > > > > 
> > > > > test_find_delalloc() allocates 256 MB worth of pages. That's all of the
> > > > > RAM that my MIPS emulator has, so it ends up panicking after it OOM
> > > > > kills everything. We don't actually need to use that much for this test.
> > > > 
> > > > I'm not sure we should limit it that way as more bytes can give it more
> > > > stress. Can we do it somehow dynamically ? Like start with 256M and fall
> > > > back to the numbers you've used.
> > > > 
> > > > Or maybe start from the low bound and allocate until it fails with first
> > > > ENOMEM.
> > > 
> > > I figured out how to get qemu to do highmem on MIPS, so we can drop this
> > > patch. It's reasonable to assume that the sanity tests are running on a
> > > machine with a decent amount of memory.
> > 
> > I lied, that didn't fix the problem for me, since these page allocations
> > can't use highmem. Anyways, the tests seem to be testing more for logic
> > errors manipulating the extent io tree, so just scaling it down
> > shouldn't be a huge issue, right?
> > 
> > The problem with relying on ENOMEM for anything is that the OOM killer
> > will kick in before we actually get the ENOMEM. I tried adding
> > __GFP_NORETRY, which claims that it doesn't invoke the OOM killer, but
> > it still did :(
> 
> I think we cannot rely on NORETRY behaviour wrt OOM killer anyway. For
> now I'd rather leave out the patch and let people test on machines with
> enough memory. Any furhter ideas to make it work on low memory are
> welcome but I don't want to block this patchset.

Yeah that's fine, we can come back to this another time.

Patch
diff mbox

diff --git a/fs/btrfs/tests/extent-io-tests.c b/fs/btrfs/tests/extent-io-tests.c
index caad80b..ea9c91b 100644
--- a/fs/btrfs/tests/extent-io-tests.c
+++ b/fs/btrfs/tests/extent-io-tests.c
@@ -73,8 +73,8 @@  static int test_find_delalloc(u32 sectorsize)
 	struct page *page;
 	struct page *locked_page = NULL;
 	unsigned long index = 0;
-	u64 total_dirty = SZ_256M;
-	u64 max_bytes = SZ_128M;
+	u64 total_dirty = sectorsize * 256;
+	u64 max_bytes = total_dirty >> 1;
 	u64 start, end, test_start;
 	u64 found;
 	int ret = -EINVAL;
@@ -138,7 +138,7 @@  static int test_find_delalloc(u32 sectorsize)
 	 * |--- delalloc ---|
 	 *           |--- search ---|
 	 */
-	test_start = SZ_64M;
+	test_start = max_bytes >> 1;
 	locked_page = find_lock_page(inode->i_mapping,
 				     test_start >> PAGE_SHIFT);
 	if (!locked_page) {
@@ -226,7 +226,7 @@  static int test_find_delalloc(u32 sectorsize)
 	 * range we want to find.
 	 */
 	page = find_get_page(inode->i_mapping,
-			     (max_bytes + SZ_1M) >> PAGE_SHIFT);
+			     (max_bytes + PAGE_SIZE) >> PAGE_SHIFT);
 	if (!page) {
 		test_msg("Couldn't find our page\n");
 		goto out_bits;