thr3ads.net - Btrfs devel - [PATCH] Btrfs: deal with enomem in the rewind path V3 [Aug 2013]

If this information is useful, please help other people find it:
Share via:

Josef Bacik

2013-Aug-07 21:03 UTC

[PATCH] Btrfs: deal with enomem in the rewind path V3

We can get ENOMEM trying to allocate dummy bufs for the rewind operation of the
tree mod log.  Instead of BUG_ON()''ing in this case pass up ENOMEM.  I
looked
back through the callers and I''m pretty sure I got everybody who did
BUG_ON(ret)
in this path.  Thanks,

Signed-off-by: Josef Bacik <jbacik@fusionio.com>
---
V2->V3:
-unlock and free the original buffer on error
-return NULL instead of ERR_PTR(-ENOMEM)
V1->V2: missed a BUG_ON() for alloc_dummy_extent_buffer.

 fs/btrfs/ctree.c     |   16 +++++-
 fs/btrfs/extent_io.c |  145 +++++++++++++++++++++++++------------------------
 2 files changed, 88 insertions(+), 73 deletions(-)

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 0d5c686..1dd8a71 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -1211,7 +1211,11 @@ tree_mod_log_rewind(struct btrfs_fs_info *fs_info, struct
extent_buffer *eb,
 		BUG_ON(tm->slot != 0);
 		eb_rewin = alloc_dummy_extent_buffer(eb->start,
 						fs_info->tree_root->nodesize);
-		BUG_ON(!eb_rewin);
+		if (!eb_rewin) {
+			btrfs_tree_read_unlock(eb);
+			free_extent_buffer(eb);
+			return NULL;
+		}
 		btrfs_set_header_bytenr(eb_rewin, eb->start);
 		btrfs_set_header_backref_rev(eb_rewin,
 					     btrfs_header_backref_rev(eb));
@@ -1219,7 +1223,11 @@ tree_mod_log_rewind(struct btrfs_fs_info *fs_info, struct
extent_buffer *eb,
 		btrfs_set_header_level(eb_rewin, btrfs_header_level(eb));
 	} else {
 		eb_rewin = btrfs_clone_extent_buffer(eb);
-		BUG_ON(!eb_rewin);
+		if (!eb_rewin) {
+			btrfs_tree_read_unlock(eb);
+			free_extent_buffer(eb);
+			return NULL;
+		}
 	}
 
 	btrfs_tree_read_unlock(eb);
@@ -2772,6 +2780,10 @@ again:
 							  BTRFS_READ_LOCK);
 			}
 			b = tree_mod_log_rewind(root->fs_info, b, time_seq);
+			if (!b) {
+				ret = -ENOMEM;
+				goto done;
+			}
 			p->locks[level] = BTRFS_READ_LOCK;
 			p->nodes[level] = b;
 		} else {
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index deaea9c..b422cba 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -4222,6 +4222,76 @@ static void __free_extent_buffer(struct extent_buffer
*eb)
 	kmem_cache_free(extent_buffer_cache, eb);
 }
 
+static int extent_buffer_under_io(struct extent_buffer *eb)
+{
+	return (atomic_read(&eb->io_pages) ||
+		test_bit(EXTENT_BUFFER_WRITEBACK, &eb->bflags) ||
+		test_bit(EXTENT_BUFFER_DIRTY, &eb->bflags));
+}
+
+/*
+ * Helper for releasing extent buffer page.
+ */
+static void btrfs_release_extent_buffer_page(struct extent_buffer *eb,
+						unsigned long start_idx)
+{
+	unsigned long index;
+	unsigned long num_pages;
+	struct page *page;
+	int mapped = !test_bit(EXTENT_BUFFER_DUMMY, &eb->bflags);
+
+	BUG_ON(extent_buffer_under_io(eb));
+
+	num_pages = num_extent_pages(eb->start, eb->len);
+	index = start_idx + num_pages;
+	if (start_idx >= index)
+		return;
+
+	do {
+		index--;
+		page = extent_buffer_page(eb, index);
+		if (page && mapped) {
+			spin_lock(&page->mapping->private_lock);
+			/*
+			 * We do this since we''ll remove the pages after we''ve
+			 * removed the eb from the radix tree, so we could race
+			 * and have this page now attached to the new eb.  So
+			 * only clear page_private if it''s still connected to
+			 * this eb.
+			 */
+			if (PagePrivate(page) &&
+			    page->private == (unsigned long)eb) {
+				BUG_ON(test_bit(EXTENT_BUFFER_DIRTY, &eb->bflags));
+				BUG_ON(PageDirty(page));
+				BUG_ON(PageWriteback(page));
+				/*
+				 * We need to make sure we haven''t be attached
+				 * to a new eb.
+				 */
+				ClearPagePrivate(page);
+				set_page_private(page, 0);
+				/* One for the page private */
+				page_cache_release(page);
+			}
+			spin_unlock(&page->mapping->private_lock);
+
+		}
+		if (page) {
+			/* One for when we alloced the page */
+			page_cache_release(page);
+		}
+	} while (index != start_idx);
+}
+
+/*
+ * Helper for releasing the extent buffer.
+ */
+static inline void btrfs_release_extent_buffer(struct extent_buffer *eb)
+{
+	btrfs_release_extent_buffer_page(eb, 0);
+	__free_extent_buffer(eb);
+}
+
 static struct extent_buffer *__alloc_extent_buffer(struct extent_io_tree *tree,
 						   u64 start,
 						   unsigned long len,
@@ -4276,7 +4346,10 @@ struct extent_buffer *btrfs_clone_extent_buffer(struct
extent_buffer *src)
 
 	for (i = 0; i < num_pages; i++) {
 		p = alloc_page(GFP_ATOMIC);
-		BUG_ON(!p);
+		if (!p) {
+			btrfs_release_extent_buffer(new);
+			return NULL;
+		}
 		attach_extent_buffer_page(new, p);
 		WARN_ON(PageDirty(p));
 		SetPageUptodate(p);
@@ -4317,76 +4390,6 @@ err:
 	return NULL;
 }
 
-static int extent_buffer_under_io(struct extent_buffer *eb)
-{
-	return (atomic_read(&eb->io_pages) ||
-		test_bit(EXTENT_BUFFER_WRITEBACK, &eb->bflags) ||
-		test_bit(EXTENT_BUFFER_DIRTY, &eb->bflags));
-}
-
-/*
- * Helper for releasing extent buffer page.
- */
-static void btrfs_release_extent_buffer_page(struct extent_buffer *eb,
-						unsigned long start_idx)
-{
-	unsigned long index;
-	unsigned long num_pages;
-	struct page *page;
-	int mapped = !test_bit(EXTENT_BUFFER_DUMMY, &eb->bflags);
-
-	BUG_ON(extent_buffer_under_io(eb));
-
-	num_pages = num_extent_pages(eb->start, eb->len);
-	index = start_idx + num_pages;
-	if (start_idx >= index)
-		return;
-
-	do {
-		index--;
-		page = extent_buffer_page(eb, index);
-		if (page && mapped) {
-			spin_lock(&page->mapping->private_lock);
-			/*
-			 * We do this since we''ll remove the pages after we''ve
-			 * removed the eb from the radix tree, so we could race
-			 * and have this page now attached to the new eb.  So
-			 * only clear page_private if it''s still connected to
-			 * this eb.
-			 */
-			if (PagePrivate(page) &&
-			    page->private == (unsigned long)eb) {
-				BUG_ON(test_bit(EXTENT_BUFFER_DIRTY, &eb->bflags));
-				BUG_ON(PageDirty(page));
-				BUG_ON(PageWriteback(page));
-				/*
-				 * We need to make sure we haven''t be attached
-				 * to a new eb.
-				 */
-				ClearPagePrivate(page);
-				set_page_private(page, 0);
-				/* One for the page private */
-				page_cache_release(page);
-			}
-			spin_unlock(&page->mapping->private_lock);
-
-		}
-		if (page) {
-			/* One for when we alloced the page */
-			page_cache_release(page);
-		}
-	} while (index != start_idx);
-}
-
-/*
- * Helper for releasing the extent buffer.
- */
-static inline void btrfs_release_extent_buffer(struct extent_buffer *eb)
-{
-	btrfs_release_extent_buffer_page(eb, 0);
-	__free_extent_buffer(eb);
-}
-
 static void check_buffer_tree_ref(struct extent_buffer *eb)
 {
 	int refs;
-- 
1.7.7.6

--
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

Jan Schmidt

2013-Aug-08 07:36 UTC

head link

Re: [PATCH] Btrfs: deal with enomem in the rewind path V3

On Wed, August 07, 2013 at 23:03 (+0200), Josef Bacik
wrote:> We can get ENOMEM trying to allocate dummy bufs for the rewind operation of
the
> tree mod log.  Instead of BUG_ON()''ing in this case pass up
ENOMEM.  I looked
> back through the callers and I''m pretty sure I got everybody who
did BUG_ON(ret)
> in this path.  Thanks,
> 
> Signed-off-by: Josef Bacik <jbacik@fusionio.com>
> ---
> V2->V3:
> -unlock and free the original buffer on error
> -return NULL instead of ERR_PTR(-ENOMEM)
> V1->V2: missed a BUG_ON() for alloc_dummy_extent_buffer.
> 
>  fs/btrfs/ctree.c     |   16 +++++-
>  fs/btrfs/extent_io.c |  145
+++++++++++++++++++++++++------------------------
>  2 files changed, 88 insertions(+), 73 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index 0d5c686..1dd8a71 100644
> --- a/fs/btrfs/ctree.c
> +++ b/fs/btrfs/ctree.c
> @@ -1211,7 +1211,11 @@ tree_mod_log_rewind(struct btrfs_fs_info *fs_info,
struct extent_buffer *eb,
>  		BUG_ON(tm->slot != 0);
>  		eb_rewin = alloc_dummy_extent_buffer(eb->start,
>  						fs_info->tree_root->nodesize);
> -		BUG_ON(!eb_rewin);
> +		if (!eb_rewin) {
> +			btrfs_tree_read_unlock(eb);
> +			free_extent_buffer(eb);
> +			return NULL;
> +		}
>  		btrfs_set_header_bytenr(eb_rewin, eb->start);
>  		btrfs_set_header_backref_rev(eb_rewin,
>  					     btrfs_header_backref_rev(eb));
> @@ -1219,7 +1223,11 @@ tree_mod_log_rewind(struct btrfs_fs_info *fs_info,
struct extent_buffer *eb,
>  		btrfs_set_header_level(eb_rewin, btrfs_header_level(eb));
>  	} else {
>  		eb_rewin = btrfs_clone_extent_buffer(eb);
> -		BUG_ON(!eb_rewin);
> +		if (!eb_rewin) {
> +			btrfs_tree_read_unlock(eb);
> +			free_extent_buffer(eb);
> +			return NULL;
> +		}
>  	}
>  
>  	btrfs_tree_read_unlock(eb);
> @@ -2772,6 +2780,10 @@ again:
>  							  BTRFS_READ_LOCK);
>  			}
>  			b = tree_mod_log_rewind(root->fs_info, b, time_seq);
> +			if (!b) {
> +				ret = -ENOMEM;
> +				goto done;
> +			}
>  			p->locks[level] = BTRFS_READ_LOCK;
>  			p->nodes[level] = b;
>  		} else {
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index deaea9c..b422cba 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -4222,6 +4222,76 @@ static void __free_extent_buffer(struct
extent_buffer *eb)
>  	kmem_cache_free(extent_buffer_cache, eb);
>  }
>  
> +static int extent_buffer_under_io(struct extent_buffer *eb)
> +{
> +	return (atomic_read(&eb->io_pages) ||
> +		test_bit(EXTENT_BUFFER_WRITEBACK, &eb->bflags) ||
> +		test_bit(EXTENT_BUFFER_DIRTY, &eb->bflags));
> +}
> +
> +/*
> + * Helper for releasing extent buffer page.
> + */
> +static void btrfs_release_extent_buffer_page(struct extent_buffer *eb,
> +						unsigned long start_idx)
> +{
> +	unsigned long index;
> +	unsigned long num_pages;
> +	struct page *page;
> +	int mapped = !test_bit(EXTENT_BUFFER_DUMMY, &eb->bflags);
> +
> +	BUG_ON(extent_buffer_under_io(eb));
> +
> +	num_pages = num_extent_pages(eb->start, eb->len);
> +	index = start_idx + num_pages;
> +	if (start_idx >= index)
> +		return;
> +
> +	do {
> +		index--;
> +		page = extent_buffer_page(eb, index);
> +		if (page && mapped) {
> +			spin_lock(&page->mapping->private_lock);
> +			/*
> +			 * We do this since we''ll remove the pages after
we''ve
> +			 * removed the eb from the radix tree, so we could race
> +			 * and have this page now attached to the new eb.  So
> +			 * only clear page_private if it''s still connected to
> +			 * this eb.
> +			 */
> +			if (PagePrivate(page) &&
> +			    page->private == (unsigned long)eb) {
> +				BUG_ON(test_bit(EXTENT_BUFFER_DIRTY, &eb->bflags));
> +				BUG_ON(PageDirty(page));
> +				BUG_ON(PageWriteback(page));
> +				/*
> +				 * We need to make sure we haven''t be attached
> +				 * to a new eb.
> +				 */
> +				ClearPagePrivate(page);
> +				set_page_private(page, 0);
> +				/* One for the page private */
> +				page_cache_release(page);
> +			}
> +			spin_unlock(&page->mapping->private_lock);
> +
> +		}
> +		if (page) {
> +			/* One for when we alloced the page */
> +			page_cache_release(page);
> +		}
> +	} while (index != start_idx);
> +}
> +
> +/*
> + * Helper for releasing the extent buffer.
> + */
> +static inline void btrfs_release_extent_buffer(struct extent_buffer *eb)
> +{
> +	btrfs_release_extent_buffer_page(eb, 0);
> +	__free_extent_buffer(eb);
> +}
> +
>  static struct extent_buffer *__alloc_extent_buffer(struct extent_io_tree
*tree,
>  						   u64 start,
>  						   unsigned long len,
> @@ -4276,7 +4346,10 @@ struct extent_buffer
*btrfs_clone_extent_buffer(struct extent_buffer *src)
>  
>  	for (i = 0; i < num_pages; i++) {
>  		p = alloc_page(GFP_ATOMIC);
> -		BUG_ON(!p);
> +		if (!p) {
> +			btrfs_release_extent_buffer(new);
> +			return NULL;
> +		}
>  		attach_extent_buffer_page(new, p);
>  		WARN_ON(PageDirty(p));
>  		SetPageUptodate(p);
> @@ -4317,76 +4390,6 @@ err:
>  	return NULL;
>  }
>  
> -static int extent_buffer_under_io(struct extent_buffer *eb)
> -{
> -	return (atomic_read(&eb->io_pages) ||
> -		test_bit(EXTENT_BUFFER_WRITEBACK, &eb->bflags) ||
> -		test_bit(EXTENT_BUFFER_DIRTY, &eb->bflags));
> -}
> -
> -/*
> - * Helper for releasing extent buffer page.
> - */
> -static void btrfs_release_extent_buffer_page(struct extent_buffer *eb,
> -						unsigned long start_idx)
> -{
> -	unsigned long index;
> -	unsigned long num_pages;
> -	struct page *page;
> -	int mapped = !test_bit(EXTENT_BUFFER_DUMMY, &eb->bflags);
> -
> -	BUG_ON(extent_buffer_under_io(eb));
> -
> -	num_pages = num_extent_pages(eb->start, eb->len);
> -	index = start_idx + num_pages;
> -	if (start_idx >= index)
> -		return;
> -
> -	do {
> -		index--;
> -		page = extent_buffer_page(eb, index);
> -		if (page && mapped) {
> -			spin_lock(&page->mapping->private_lock);
> -			/*
> -			 * We do this since we''ll remove the pages after
we''ve
> -			 * removed the eb from the radix tree, so we could race
> -			 * and have this page now attached to the new eb.  So
> -			 * only clear page_private if it''s still connected to
> -			 * this eb.
> -			 */
> -			if (PagePrivate(page) &&
> -			    page->private == (unsigned long)eb) {
> -				BUG_ON(test_bit(EXTENT_BUFFER_DIRTY, &eb->bflags));
> -				BUG_ON(PageDirty(page));
> -				BUG_ON(PageWriteback(page));
> -				/*
> -				 * We need to make sure we haven''t be attached
> -				 * to a new eb.
> -				 */
> -				ClearPagePrivate(page);
> -				set_page_private(page, 0);
> -				/* One for the page private */
> -				page_cache_release(page);
> -			}
> -			spin_unlock(&page->mapping->private_lock);
> -
> -		}
> -		if (page) {
> -			/* One for when we alloced the page */
> -			page_cache_release(page);
> -		}
> -	} while (index != start_idx);
> -}
> -
> -/*
> - * Helper for releasing the extent buffer.
> - */
> -static inline void btrfs_release_extent_buffer(struct extent_buffer *eb)
> -{
> -	btrfs_release_extent_buffer_page(eb, 0);
> -	__free_extent_buffer(eb);
> -}
> -
>  static void check_buffer_tree_ref(struct extent_buffer *eb)
>  {
>  	int refs;
> 
Weird patch formatting concerning extent_io.c, I assume there are no changes in
extent_buffer_under_io and btrfs_release_extent_buffer_page, you just moved
btrfs_clone_extent_buffer, right? Perhaps --patience or --minimal could do
better? Otherwise,

Reviewed-by: Jan Schmidt <list.xfs@jan-o-sch.net>

Thanks,
-Jan
--
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

Josef Bacik

2013-Aug-08 13:11 UTC

head link

Re: [PATCH] Btrfs: deal with enomem in the rewind path V3

On Thu, Aug 08, 2013 at 09:36:52AM +0200, Jan Schmidt
wrote:> On Wed, August 07, 2013 at 23:03 (+0200), Josef Bacik wrote:
> > We can get ENOMEM trying to allocate dummy bufs for the rewind
operation of the
> > tree mod log.  Instead of BUG_ON()''ing in this case pass up
ENOMEM.  I looked
> > back through the callers and I''m pretty sure I got everybody
who did BUG_ON(ret)
> > in this path.  Thanks,
> > 
> > Signed-off-by: Josef Bacik <jbacik@fusionio.com>
> > ---
> > V2->V3:
> > -unlock and free the original buffer on error
> > -return NULL instead of ERR_PTR(-ENOMEM)
> > V1->V2: missed a BUG_ON() for alloc_dummy_extent_buffer.
> > 
> >  fs/btrfs/ctree.c     |   16 +++++-
> >  fs/btrfs/extent_io.c |  145
+++++++++++++++++++++++++------------------------
> >  2 files changed, 88 insertions(+), 73 deletions(-)
> > 
> > diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> > index 0d5c686..1dd8a71 100644
> > --- a/fs/btrfs/ctree.c
> > +++ b/fs/btrfs/ctree.c
> > @@ -1211,7 +1211,11 @@ tree_mod_log_rewind(struct btrfs_fs_info
*fs_info, struct extent_buffer *eb,
> >  		BUG_ON(tm->slot != 0);
> >  		eb_rewin = alloc_dummy_extent_buffer(eb->start,
> >  						fs_info->tree_root->nodesize);
> > -		BUG_ON(!eb_rewin);
> > +		if (!eb_rewin) {
> > +			btrfs_tree_read_unlock(eb);
> > +			free_extent_buffer(eb);
> > +			return NULL;
> > +		}
> >  		btrfs_set_header_bytenr(eb_rewin, eb->start);
> >  		btrfs_set_header_backref_rev(eb_rewin,
> >  					     btrfs_header_backref_rev(eb));
> > @@ -1219,7 +1223,11 @@ tree_mod_log_rewind(struct btrfs_fs_info
*fs_info, struct extent_buffer *eb,
> >  		btrfs_set_header_level(eb_rewin, btrfs_header_level(eb));
> >  	} else {
> >  		eb_rewin = btrfs_clone_extent_buffer(eb);
> > -		BUG_ON(!eb_rewin);
> > +		if (!eb_rewin) {
> > +			btrfs_tree_read_unlock(eb);
> > +			free_extent_buffer(eb);
> > +			return NULL;
> > +		}
> >  	}
> >  
> >  	btrfs_tree_read_unlock(eb);
> > @@ -2772,6 +2780,10 @@ again:
> >  							  BTRFS_READ_LOCK);
> >  			}
> >  			b = tree_mod_log_rewind(root->fs_info, b, time_seq);
> > +			if (!b) {
> > +				ret = -ENOMEM;
> > +				goto done;
> > +			}
> >  			p->locks[level] = BTRFS_READ_LOCK;
> >  			p->nodes[level] = b;
> >  		} else {
> > diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> > index deaea9c..b422cba 100644
> > --- a/fs/btrfs/extent_io.c
> > +++ b/fs/btrfs/extent_io.c
> > @@ -4222,6 +4222,76 @@ static void __free_extent_buffer(struct
extent_buffer *eb)
> >  	kmem_cache_free(extent_buffer_cache, eb);
> >  }
> >  
> > +static int extent_buffer_under_io(struct extent_buffer *eb)
> > +{
> > +	return (atomic_read(&eb->io_pages) ||
> > +		test_bit(EXTENT_BUFFER_WRITEBACK, &eb->bflags) ||
> > +		test_bit(EXTENT_BUFFER_DIRTY, &eb->bflags));
> > +}
> > +
> > +/*
> > + * Helper for releasing extent buffer page.
> > + */
> > +static void btrfs_release_extent_buffer_page(struct extent_buffer
*eb,
> > +						unsigned long start_idx)
> > +{
> > +	unsigned long index;
> > +	unsigned long num_pages;
> > +	struct page *page;
> > +	int mapped = !test_bit(EXTENT_BUFFER_DUMMY, &eb->bflags);
> > +
> > +	BUG_ON(extent_buffer_under_io(eb));
> > +
> > +	num_pages = num_extent_pages(eb->start, eb->len);
> > +	index = start_idx + num_pages;
> > +	if (start_idx >= index)
> > +		return;
> > +
> > +	do {
> > +		index--;
> > +		page = extent_buffer_page(eb, index);
> > +		if (page && mapped) {
> > +			spin_lock(&page->mapping->private_lock);
> > +			/*
> > +			 * We do this since we''ll remove the pages after
we''ve
> > +			 * removed the eb from the radix tree, so we could race
> > +			 * and have this page now attached to the new eb.  So
> > +			 * only clear page_private if it''s still connected to
> > +			 * this eb.
> > +			 */
> > +			if (PagePrivate(page) &&
> > +			    page->private == (unsigned long)eb) {
> > +				BUG_ON(test_bit(EXTENT_BUFFER_DIRTY, &eb->bflags));
> > +				BUG_ON(PageDirty(page));
> > +				BUG_ON(PageWriteback(page));
> > +				/*
> > +				 * We need to make sure we haven''t be attached
> > +				 * to a new eb.
> > +				 */
> > +				ClearPagePrivate(page);
> > +				set_page_private(page, 0);
> > +				/* One for the page private */
> > +				page_cache_release(page);
> > +			}
> > +			spin_unlock(&page->mapping->private_lock);
> > +
> > +		}
> > +		if (page) {
> > +			/* One for when we alloced the page */
> > +			page_cache_release(page);
> > +		}
> > +	} while (index != start_idx);
> > +}
> > +
> > +/*
> > + * Helper for releasing the extent buffer.
> > + */
> > +static inline void btrfs_release_extent_buffer(struct extent_buffer
*eb)
> > +{
> > +	btrfs_release_extent_buffer_page(eb, 0);
> > +	__free_extent_buffer(eb);
> > +}
> > +
> >  static struct extent_buffer *__alloc_extent_buffer(struct
extent_io_tree *tree,
> >  						   u64 start,
> >  						   unsigned long len,
> > @@ -4276,7 +4346,10 @@ struct extent_buffer
*btrfs_clone_extent_buffer(struct extent_buffer *src)
> >  
> >  	for (i = 0; i < num_pages; i++) {
> >  		p = alloc_page(GFP_ATOMIC);
> > -		BUG_ON(!p);
> > +		if (!p) {
> > +			btrfs_release_extent_buffer(new);
> > +			return NULL;
> > +		}
> >  		attach_extent_buffer_page(new, p);
> >  		WARN_ON(PageDirty(p));
> >  		SetPageUptodate(p);
> > @@ -4317,76 +4390,6 @@ err:
> >  	return NULL;
> >  }
> >  
> > -static int extent_buffer_under_io(struct extent_buffer *eb)
> > -{
> > -	return (atomic_read(&eb->io_pages) ||
> > -		test_bit(EXTENT_BUFFER_WRITEBACK, &eb->bflags) ||
> > -		test_bit(EXTENT_BUFFER_DIRTY, &eb->bflags));
> > -}
> > -
> > -/*
> > - * Helper for releasing extent buffer page.
> > - */
> > -static void btrfs_release_extent_buffer_page(struct extent_buffer
*eb,
> > -						unsigned long start_idx)
> > -{
> > -	unsigned long index;
> > -	unsigned long num_pages;
> > -	struct page *page;
> > -	int mapped = !test_bit(EXTENT_BUFFER_DUMMY, &eb->bflags);
> > -
> > -	BUG_ON(extent_buffer_under_io(eb));
> > -
> > -	num_pages = num_extent_pages(eb->start, eb->len);
> > -	index = start_idx + num_pages;
> > -	if (start_idx >= index)
> > -		return;
> > -
> > -	do {
> > -		index--;
> > -		page = extent_buffer_page(eb, index);
> > -		if (page && mapped) {
> > -			spin_lock(&page->mapping->private_lock);
> > -			/*
> > -			 * We do this since we''ll remove the pages after
we''ve
> > -			 * removed the eb from the radix tree, so we could race
> > -			 * and have this page now attached to the new eb.  So
> > -			 * only clear page_private if it''s still connected to
> > -			 * this eb.
> > -			 */
> > -			if (PagePrivate(page) &&
> > -			    page->private == (unsigned long)eb) {
> > -				BUG_ON(test_bit(EXTENT_BUFFER_DIRTY, &eb->bflags));
> > -				BUG_ON(PageDirty(page));
> > -				BUG_ON(PageWriteback(page));
> > -				/*
> > -				 * We need to make sure we haven''t be attached
> > -				 * to a new eb.
> > -				 */
> > -				ClearPagePrivate(page);
> > -				set_page_private(page, 0);
> > -				/* One for the page private */
> > -				page_cache_release(page);
> > -			}
> > -			spin_unlock(&page->mapping->private_lock);
> > -
> > -		}
> > -		if (page) {
> > -			/* One for when we alloced the page */
> > -			page_cache_release(page);
> > -		}
> > -	} while (index != start_idx);
> > -}
> > -
> > -/*
> > - * Helper for releasing the extent buffer.
> > - */
> > -static inline void btrfs_release_extent_buffer(struct extent_buffer
*eb)
> > -{
> > -	btrfs_release_extent_buffer_page(eb, 0);
> > -	__free_extent_buffer(eb);
> > -}
> > -
> >  static void check_buffer_tree_ref(struct extent_buffer *eb)
> >  {
> >  	int refs;
> > 
> 
> Weird patch formatting concerning extent_io.c, I assume there are no
changes in
> extent_buffer_under_io and btrfs_release_extent_buffer_page, you just moved
> btrfs_clone_extent_buffer, right? Perhaps --patience or --minimal could do
> better? Otherwise,
> 
Yeah I just moved them up so I could call them, I do hate how it makes the diff
look massive when there''s basically no change.  Thanks,

Josef
--
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

David Sterba

2013-Aug-08 14:28 UTC

head link

Re: [PATCH] Btrfs: deal with enomem in the rewind path V3

On Thu, Aug 08, 2013 at 09:36:52AM +0200, Jan Schmidt
wrote:> Weird patch formatting concerning extent_io.c, I assume there are no
changes in
> extent_buffer_under_io and btrfs_release_extent_buffer_page, you just moved
> btrfs_clone_extent_buffer, right? Perhaps --patience or --minimal could do
> better? Otherwise,
git diff --patience produces identical result for me (1.8.3.1).
> Reviewed-by: Jan Schmidt <list.xfs@jan-o-sch.net>                                 ^^^
xfs? :)
--
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

Jan Schmidt

2013-Aug-08 15:01 UTC

head link

Re: [PATCH] Btrfs: deal with enomem in the rewind path V3

On Thu, August 08, 2013 at 16:28 (+0200), David Sterba
wrote:> On Thu, Aug 08, 2013 at 09:36:52AM +0200, Jan Schmidt wrote:
>> Weird patch formatting concerning extent_io.c, I assume there are no
changes in
>> extent_buffer_under_io and btrfs_release_extent_buffer_page, you just
moved
>> btrfs_clone_extent_buffer, right? Perhaps --patience or --minimal could
do
>> better? Otherwise,
> 
> git diff --patience produces identical result for me (1.8.3.1).
Yeah, I expected that after Josef said that he actually moved the other two
functions, so the structure really changed in a way git cannot diff any better.
>> Reviewed-by: Jan Schmidt <list.xfs@jan-o-sch.net>
>                                  ^^^
> xfs? :)
Whoops :-) Replace that by btrfs if you wish.

-Jan
--
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

Btrfs devel - Aug 2013 - [PATCH] Btrfs: deal with enomem in the rewind path V3

[PATCH] Btrfs: deal with enomem in the rewind path V3

Re: [PATCH] Btrfs: deal with enomem in the rewind path V3

Re: [PATCH] Btrfs: deal with enomem in the rewind path V3

Re: [PATCH] Btrfs: deal with enomem in the rewind path V3

Re: [PATCH] Btrfs: deal with enomem in the rewind path V3