btrfs: ENOMEM bugfixes

Consider the following interleaving of overlapping calls to alloc_extent_buffer:

Call 1:

- Successfully allocates a few pages with find_or_create_page
- find_or_create_page fails, goto free_eb
- Unlocks the allocated pages

Call 2:
- Calls find_or_create_page and gets a page in call 1's extent_buffer
- Finds that the page is already associated with an extent_buffer
- Grabs a reference to the half-written extent_buffer and calls mark_extent_buffer_accessed on it

mark_extent_buffer_accessed will then try to call mark_page_accessed on a null page and panic.

The fix is to clear page->private of the half-written extent_buffer's pages all at once while holding mapping->private_lock.

Signed-off-by Omar Sandoval <[email protected]>

---
 fs/btrfs/extent_io.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

If io_ctl_prepare_pages fails, the pages in io_ctl.pages are not valid. When we try to access them later, things will blow up in various ways.

Signed-off-by Omar Sandoval <[email protected]>

---
 fs/btrfs/free-space-cache.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

fs/btrfs/free-space-cache.c +6 -4

index d6c03f7..0460632 100644
@@ -1114,7 +1114,7 @@ cleanup_write_cache_enospc(struct inode *inode,
  *
  * This function writes out a free space cache struct to disk for quick recovery
  * on mount.  This will return 0 if it was successfull in writing the cache out,
- * and -1 if it was not.
+ * or an errno if it was not.
  */
 static int __btrfs_write_out_cache(struct btrfs_root *root, struct inode *inode,
 				   struct btrfs_free_space_ctl *ctl,

@@ -1130,11 +1130,11 @@ static int __btrfs_write_out_cache(struct btrfs_root *root, struct inode *inode,
 	int ret;
 
 	if (!i_size_read(inode))
-		return -1;
+		return -EIO;
 
 	ret = io_ctl_init(&io_ctl, inode, root, 1);
 	if (ret)
-		return -1;
+		return ret;
 
 	if (block_group && (block_group->flags & BTRFS_BLOCK_GROUP_DATA)) {
 		down_write(&block_group->data_rwsem);

@@ -1151,7 +1151,9 @@ static int __btrfs_write_out_cache(struct btrfs_root *root, struct inode *inode,
 	}
 
 	/* Lock all pages first so we can lock the extent safely. */
-	io_ctl_prepare_pages(&io_ctl, inode, 0);
+	ret = io_ctl_prepare_pages(&io_ctl, inode, 0);
+	if (ret)
+		goto out;
 
 	lock_extent_bits(&BTRFS_I(inode)->io_tree, 0, i_size_read(inode) - 1,
 			 0, &cached_state);

Signature · 6 lines

2.3.0

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Hi,

As it turns out, running with low memory is a really easy way to shake out undesirable behavior in Btrfs. This can be especially bad when considering that a memory limit is really easy to hit in a container (e.g., by using cgroup memory.limit_in_bytes). Here's a simple script that can hit several problems:

----
#!/bin/sh

cgcreate -g memory:enomem
MEM=$((64 * 1024 * 1024))
echo $MEM > /sys/fs/cgroup/memory/enomem/memory.limit_in_bytes

cgexec -g memory:enomem ~/xfstests/ltp/fsstress -p128 -n999999999 -d /mnt/test & trap "killall fsstress; exit 0" SIGINT SIGTERM

while true; do
cgexec -g memory:enomem python -c '
l = []
while True:

	l.append(0)'

done
----

Ignoring for now the cases that drop the filesystem into read-only mode with relatively little fuss, here are a few patches that fix some of the low-hanging fruit. They apply to Linus' tree as of today.

Thanks!

Omar Sandoval (3):
btrfs: handle ENOMEM in btrfs_alloc_tree_block
btrfs: handle race on ENOMEM in alloc_extent_buffer
btrfs: check io_ctl_prepare_pages return in __btrfs_write_out_cache

 fs/btrfs/extent-tree.c      | 41 ++++++++++++++++++++++++++++-------------
 fs/btrfs/extent_io.c        | 20 ++++++++++++++++----
 fs/btrfs/free-space-cache.c | 10 ++++++----
 3 files changed, 50 insertions(+), 21 deletions(-)

Signature · 1 line

2.3.0

Quoted 22 lines · Omar Sandoval

Hi,

As it turns out, running with low memory is a really easy way to shake out undesirable behavior in Btrfs. This can be especially bad when considering that a memory limit is really easy to hit in a container (e.g., by using cgroup memory.limit_in_bytes). Here's a simple script that can hit several problems:

----
#!/bin/sh

cgcreate -g memory:enomem
MEM=$((64 * 1024 * 1024))
echo $MEM > /sys/fs/cgroup/memory/enomem/memory.limit_in_bytes

cgexec -g memory:enomem ~/xfstests/ltp/fsstress -p128 -n999999999 -d /mnt/test & trap "killall fsstress; exit 0" SIGINT SIGTERM

while true; do
cgexec -g memory:enomem python -c '
l = []
while True:

Quoted 8 lines

	l.append(0)'

done
----

Ignoring for now the cases that drop the filesystem into read-only mode with relatively little fuss, here are a few patches that fix some of the low-hanging fruit. They apply to Linus' tree as of today.

So I didn't realize this until I saw Tetsuo Handa's email to the ext4
list (https://urldefense.proofpoint.com/v1/url?u=http://thread.gmane.org/gmane.comp.file-systems.ext4/47855&k=ZVNjlDMF0FElm4dQtryO4A%3D%3D%0A&r=cKCbChRKsMpTX8ybrSkonQ%3D%3D%0A&m=nzG8bjaiVMyWylHxOvTeXimzfSNyukj4%2BAxs0AZ%2FxOI%3D%0A&s=cbd7d48f1866e79f75b88b7f94a394c53d34adfcc1a30a842382f653c978e180), but it looks like this behavior was exposed by a change to the kernel memory
allocator related to the too-small-to-fail allocation fiasco. To
summarize, Commit 9879de7373fc (mm: page_alloc: embed OOM killing
naturally into allocation slowpath), merged for v3.19-rc7, changed the
behavior of GFP_NOFS allocations which makes it much easier to trigger
allocation failures in filesystems.

This means that Btrfs falls over under memory pressure pretty easily
now, so it might be a good idea to follow the conversation over at
linux-mm (https://urldefense.proofpoint.com/v1/url?u=http://thread.gmane.org/gmane.linux.kernel.mm/126398&k=ZVNjlDMF0FElm4dQtryO4A%3D%3D%0A&r=cKCbChRKsMpTX8ybrSkonQ%3D%3D%0A&m=nzG8bjaiVMyWylHxOvTeXimzfSNyukj4%2BAxs0AZ%2FxOI%3D%0A&s=5177c5ceb03f82d8abb0beeeb4dc5e0c45cc77e9687881590e3ef1701f069a85).

These are bugs regardless of the outcome there, however, so I'd like to see this patch series merged.