[PATCH] Btrfs: igrab inode in writepage

[PATCH] Btrfs: igrab inode in writepage

[Josef Bacik]

We hit this panic on a few of our boxes this week where we have an
ordered_extent with an NULL inode.  We do an igrab() of the inode in writepages,
but weren't doing it in writepage which can be called directly from the VM on
dirty pages.  If the inode has been unlinked then we could have I_FREEING set
which means igrab() would return NULL and we get this panic.  Fix this by trying
to igrab in btrfs_writepage, and if it returns NULL then just redirty the page
and return AOP_WRITEPAGE_ACTIVATE; so the VM knows it wasn't successful.  Thanks,

Signed-off-by: Josef Bacik <jbacik@fb.com>
---
 fs/btrfs/inode.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index a0fa725..4d1fdc2 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -8438,15 +8438,28 @@ int btrfs_readpage(struct file *file, struct page *page)
 static int btrfs_writepage(struct page *page, struct writeback_control *wbc)
 {
 	struct extent_io_tree *tree;
-
+	struct inode *inode = page->mapping->host;
+	int ret;
 
 	if (current->flags & PF_MEMALLOC) {
 		redirty_page_for_writepage(wbc, page);
 		unlock_page(page);
 		return 0;
 	}
+
+	/*
+	 * If we are under memory pressure we will call this directly from the
+	 * VM, we need to make sure we have the inode referenced for the ordered
+	 * extent.  If not just return like we didn't do anything.
+	 */
+	if (!igrab(inode)) {
+		redirty_page_for_writepage(wbc, page);
+		return AOP_WRITEPAGE_ACTIVATE;
+	}
 	tree = &BTRFS_I(page->mapping->host)->io_tree;
-	return extent_write_full_page(tree, page, btrfs_get_extent, wbc);
+	ret = extent_write_full_page(tree, page, btrfs_get_extent, wbc);
+	btrfs_add_delayed_iput(inode);
+	return ret;
 }
 
 static int btrfs_writepages(struct address_space *mapping,
-- 
2.1.0

Re: [PATCH] Btrfs: igrab inode in writepage

[Liu Bo]

On 10/23/2015 03:05 AM, Josef Bacik wrote:
> We hit this panic on a few of our boxes this week where we have an
> ordered_extent with an NULL inode.  We do an igrab() of the inode in writepages,
> but weren't doing it in writepage which can be called directly from the VM on
> dirty pages.  If the inode has been unlinked then we could have I_FREEING set
> which means igrab() would return NULL and we get this panic.  Fix this by trying
> to igrab in btrfs_writepage, and if it returns NULL then just redirty the page
> and return AOP_WRITEPAGE_ACTIVATE; so the VM knows it wasn't successful.  Thanks,
>

Reviewed-by: Liu Bo <bo.li.liu@oracle.com>

thanks,

-Liubo

> Signed-off-by: Josef Bacik <jbacik@fb.com>
> ---
>   fs/btrfs/inode.c | 17 +++++++++++++++--
>   1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index a0fa725..4d1fdc2 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -8438,15 +8438,28 @@ int btrfs_readpage(struct file *file, struct page *page)
>   static int btrfs_writepage(struct page *page, struct writeback_control *wbc)
>   {
>   	struct extent_io_tree *tree;
> -
> +	struct inode *inode = page->mapping->host;
> +	int ret;
>
>   	if (current->flags & PF_MEMALLOC) {
>   		redirty_page_for_writepage(wbc, page);
>   		unlock_page(page);
>   		return 0;
>   	}
> +
> +	/*
> +	 * If we are under memory pressure we will call this directly from the
> +	 * VM, we need to make sure we have the inode referenced for the ordered
> +	 * extent.  If not just return like we didn't do anything.
> +	 */
> +	if (!igrab(inode)) {
> +		redirty_page_for_writepage(wbc, page);
> +		return AOP_WRITEPAGE_ACTIVATE;
> +	}
>   	tree = &BTRFS_I(page->mapping->host)->io_tree;
> -	return extent_write_full_page(tree, page, btrfs_get_extent, wbc);
> +	ret = extent_write_full_page(tree, page, btrfs_get_extent, wbc);
> +	btrfs_add_delayed_iput(inode);
> +	return ret;
>   }
>
>   static int btrfs_writepages(struct address_space *mapping,
>