All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Stop searching for free slots in an inode chunk when there are none
@ 2017-08-03 15:19 Carlos Maiolino
  2017-08-03 22:35 ` Dave Chinner
  0 siblings, 1 reply; 8+ messages in thread
From: Carlos Maiolino @ 2017-08-03 15:19 UTC (permalink / raw)
  To: linux-xfs

In a filesystem without finobt, the Space manager selects an AG to alloc a new
inode, where xfs_dialloc_ag_inobt() will search the AG for the free slot chunk.

When the new inode is in the samge AG as its parent, the btree will be
searched starting on the parent's record, and then retried from the top
if no slot is available beyond the parent's record.

To exit this loop though, xfs_dialloc_ag_inobt(), relies on the fact that the
btree must have a free slot available, once its callers relied on the
agi->freecount when deciding how/where to allocate this new inode.

In the case when the agi->freecount is corrupted, showing available
inodes in an AG, when in fact there is none, this becomes an infinite
loop.

Add a way to stop the loop when a free slot is not found in the btree,
making the function to fall into the whole AG scan which will then, be
able to detect the corruption and shut the filesystem down.

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---

I have a xfstest to catch this agi->freecount corruption case, I'll send it to
the list soon.

 fs/xfs/libxfs/xfs_ialloc.c | 37 ++++++++++++++++++++++++-------------
 1 file changed, 24 insertions(+), 13 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
index d41ade5d293e..8ebe0d89bdc5 100644
--- a/fs/xfs/libxfs/xfs_ialloc.c
+++ b/fs/xfs/libxfs/xfs_ialloc.c
@@ -1123,6 +1123,7 @@ xfs_dialloc_ag_inobt(
 	int			error;
 	int			offset;
 	int			i, j;
+	int			retry = true; /* Search tree from the top */
 
 	pag = xfs_perag_get(mp, agno);
 
@@ -1205,6 +1206,12 @@ xfs_dialloc_ag_inobt(
 			error = xfs_ialloc_next_rec(cur, &rec, &doneright, 0);
 			if (error)
 				goto error1;
+
+			/*
+			 * We've already scanned the whole btree, no need to
+			 * retry the search.
+			 */
+			retry = false;
 		}
 
 		/*
@@ -1268,19 +1275,23 @@ xfs_dialloc_ag_inobt(
 				goto error1;
 		}
 
-		/*
-		 * We've reached the end of the btree. because
-		 * we are only searching a small chunk of the
-		 * btree each search, there is obviously free
-		 * inodes closer to the parent inode than we
-		 * are now. restart the search again.
-		 */
-		pag->pagl_pagino = NULLAGINO;
-		pag->pagl_leftrec = NULLAGINO;
-		pag->pagl_rightrec = NULLAGINO;
-		xfs_btree_del_cursor(tcur, XFS_BTREE_NOERROR);
-		xfs_btree_del_cursor(cur, XFS_BTREE_NOERROR);
-		goto restart_pagno;
+		if (retry) {
+			/*
+			 * We've reached the end of the btree. because
+			 * we are only searching a small chunk of the
+			 * btree each search, there must be free
+			 * inodes (unless something is corrupted)
+			 * closer to the parent inode than we
+			 * are now. restart the search again.
+			 */
+			pag->pagl_pagino = NULLAGINO;
+			pag->pagl_leftrec = NULLAGINO;
+			pag->pagl_rightrec = NULLAGINO;
+			xfs_btree_del_cursor(tcur, XFS_BTREE_NOERROR);
+			xfs_btree_del_cursor(cur, XFS_BTREE_NOERROR);
+
+			goto restart_pagno;
+		}
 	}
 
 	/*
-- 
2.13.3


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] Stop searching for free slots in an inode chunk when there are none
  2017-08-03 15:19 [PATCH] Stop searching for free slots in an inode chunk when there are none Carlos Maiolino
@ 2017-08-03 22:35 ` Dave Chinner
  2017-08-04  8:55   ` Carlos Eduardo Maiolino
  0 siblings, 1 reply; 8+ messages in thread
From: Dave Chinner @ 2017-08-03 22:35 UTC (permalink / raw)
  To: Carlos Maiolino; +Cc: linux-xfs

On Thu, Aug 03, 2017 at 05:19:15PM +0200, Carlos Maiolino wrote:
> In a filesystem without finobt, the Space manager selects an AG to alloc a new
> inode, where xfs_dialloc_ag_inobt() will search the AG for the free slot chunk.
> 
> When the new inode is in the samge AG as its parent, the btree will be
> searched starting on the parent's record, and then retried from the top
> if no slot is available beyond the parent's record.
> 
> To exit this loop though, xfs_dialloc_ag_inobt(), relies on the fact that the
> btree must have a free slot available, once its callers relied on the
> agi->freecount when deciding how/where to allocate this new inode.
> 
> In the case when the agi->freecount is corrupted, showing available
> inodes in an AG, when in fact there is none, this becomes an infinite
> loop.
> 
> Add a way to stop the loop when a free slot is not found in the btree,
> making the function to fall into the whole AG scan which will then, be
> able to detect the corruption and shut the filesystem down.

That doesn't sound quite right. The initial scan and the restart
loop are both limited to scanning search_distance records - we never
search the entire tree except when it's really small (i..e less than
10-20 records (640-1280 inodes) depending on balance). If the
pagino record to end of btree distance in both directions is shorter
than the search distance for a given loop (i.e. less than 10 records
from pagino to end-of-btree) then that is the only time a corrupted
agi->freecount can cause this problem.

IOWs, on production systems where there's more than a few hundred
inodes (i.e. the vast majority of installations) a corrupted
agi->freecount won't lead to a endless loop because search_distance
will terminate the retry loop and we'll allocate a new inode.

To tell the truth, I'd much rather we just use the search distance
to prevent endless looping than add a second method of limiting
the search loop. i.e. don't reset search_distance when we restart
the search loop at pagino.  That means even for small trees (<
search_distance * 2 records) we'll retry when we get to the end of
tree, but we'll still break out of the loop and allocate new inodes
as soon as we hit the search distance limit.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Stop searching for free slots in an inode chunk when there are none
  2017-08-03 22:35 ` Dave Chinner
@ 2017-08-04  8:55   ` Carlos Eduardo Maiolino
  2017-08-04  9:36     ` Carlos Eduardo Maiolino
  0 siblings, 1 reply; 8+ messages in thread
From: Carlos Eduardo Maiolino @ 2017-08-04  8:55 UTC (permalink / raw)
  To: Dave Chinner; +Cc: linux-xfs

Hi Dave.


> > Add a way to stop the loop when a free slot is not found in the btree,
> > making the function to fall into the whole AG scan which will then, be
> > able to detect the corruption and shut the filesystem down.
> 
> That doesn't sound quite right. The initial scan and the restart
> loop are both limited to scanning search_distance records - we never
> search the entire tree except when it's really small (i..e less than
> 10-20 records (640-1280 inodes) depending on balance). If the
> pagino record to end of btree distance in both directions is shorter
> than the search distance for a given loop (i.e. less than 10 records
> from pagino to end-of-btree) then that is the only time a corrupted
> agi->freecount can cause this problem.
> 

I agree with you, but still, we are feasible to have this corruption happening,
and I've seen reports of users hitting it.


> IOWs, on production systems where there's more than a few hundred
> inodes (i.e. the vast majority of installations) a corrupted
> agi->freecount won't lead to a endless loop because search_distance
> will terminate the retry loop and we'll allocate a new inode.
> 
> To tell the truth, I'd much rather we just use the search distance
> to prevent endless looping than add a second method of limiting
> the search loop. i.e. don't reset search_distance when we restart
> the search loop at pagino.  That means even for small trees (<
> search_distance * 2 records) we'll retry when we get to the end of
> tree, but we'll still break out of the loop and allocate new inodes
> as soon as we hit the search distance limit.
> 

Sounds reasonable, I'll try that and send a V2,

Thank you!!

-- 
--Carlos

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Stop searching for free slots in an inode chunk when there are none
  2017-08-04  8:55   ` Carlos Eduardo Maiolino
@ 2017-08-04  9:36     ` Carlos Eduardo Maiolino
  2017-08-04 23:17       ` Dave Chinner
  0 siblings, 1 reply; 8+ messages in thread
From: Carlos Eduardo Maiolino @ 2017-08-04  9:36 UTC (permalink / raw)
  To: Dave Chinner; +Cc: linux-xfs

One more thing.



----- Original Message -----
> From: "Carlos Eduardo Maiolino" <cmaiolin@redhat.com>
> To: "Dave Chinner" <david@fromorbit.com>
> Cc: linux-xfs@vger.kernel.org
> Sent: Friday, August 4, 2017 10:55:26 AM
> Subject: Re: [PATCH] Stop searching for free slots in an inode chunk when there are none
> 
> Hi Dave.
> 
> 
> > > Add a way to stop the loop when a free slot is not found in the btree,
> > > making the function to fall into the whole AG scan which will then, be
> > > able to detect the corruption and shut the filesystem down.
> > 
> > That doesn't sound quite right. The initial scan and the restart
> > loop are both limited to scanning search_distance records - we never
> > search the entire tree except when it's really small (i..e less than
> > 10-20 records (640-1280 inodes) depending on balance). If the
> > pagino record to end of btree distance in both directions is shorter
> > than the search distance for a given loop (i.e. less than 10 records
> > from pagino to end-of-btree) then that is the only time a corrupted
> > agi->freecount can cause this problem.
> > 
> 
> I agree with you, but still, we are feasible to have this corruption
> happening,
> and I've seen reports of users hitting it.
> 
> 
> > IOWs, on production systems where there's more than a few hundred
> > inodes (i.e. the vast majority of installations) a corrupted
> > agi->freecount won't lead to a endless loop because search_distance
> > will terminate the retry loop and we'll allocate a new inode.
> > 
> > To tell the truth, I'd much rather we just use the search distance
> > to prevent endless looping than add a second method of limiting
> > the search loop. i.e. don't reset search_distance when we restart
> > the search loop at pagino.  That means even for small trees (<
> > search_distance * 2 records) we'll retry when we get to the end of
> > tree, but we'll still break out of the loop and allocate new inodes
> > as soon as we hit the search distance limit.
> > 
> 

Here, you are assuming we enter into the 

while (!doneleft || !doneright) { }

on every interaction, so it will be able to decrease the searchdistance or you
mean by moving the --searchdistance somewhere else?

In very small trees we don't even enter the while loop (both doneleft and doneright are 1),
so searchdistance isn't decremented at all, resetting it or not will not make any difference
in this case.


My first thought about fixing this was to check both doneleft and doneright, with something like:

if ((pagino == NULLAGINO) && doneleft && doneright))
      /* nothing more to search, break the loop */

But talking with Brian on irc yesterday, we agreed it doesn't sound quite right.

Also, if you want to see the xfstests I'm using for it:

https://raw.githubusercontent.com/cmaiolino/xfstests-dev/872e98ce156292ae2ab69ee55812e22c58556efe/tests/xfs/057

I didn't send it to the list yet because I want to add some better comments and '-d' flag to
the xfs_io as Darrick suggested, but it's on a good state to trigger the bug 100% of the times.


> Sounds reasonable, I'll try that and send a V2,
> 
> Thank you!!
> 
> --
> --Carlos
> 

-- 
--Carlos

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Stop searching for free slots in an inode chunk when there are none
  2017-08-04  9:36     ` Carlos Eduardo Maiolino
@ 2017-08-04 23:17       ` Dave Chinner
  0 siblings, 0 replies; 8+ messages in thread
From: Dave Chinner @ 2017-08-04 23:17 UTC (permalink / raw)
  To: Carlos Eduardo Maiolino; +Cc: linux-xfs

On Fri, Aug 04, 2017 at 05:36:01AM -0400, Carlos Eduardo Maiolino wrote:
> One more thing.
> 
> 
> 
> ----- Original Message -----
> > From: "Carlos Eduardo Maiolino" <cmaiolin@redhat.com>
> > To: "Dave Chinner" <david@fromorbit.com>
> > Cc: linux-xfs@vger.kernel.org
> > Sent: Friday, August 4, 2017 10:55:26 AM
> > Subject: Re: [PATCH] Stop searching for free slots in an inode chunk when there are none
> > 
> > Hi Dave.
> > 
> > 
> > > > Add a way to stop the loop when a free slot is not found in the btree,
> > > > making the function to fall into the whole AG scan which will then, be
> > > > able to detect the corruption and shut the filesystem down.
> > > 
> > > That doesn't sound quite right. The initial scan and the restart
> > > loop are both limited to scanning search_distance records - we never
> > > search the entire tree except when it's really small (i..e less than
> > > 10-20 records (640-1280 inodes) depending on balance). If the
> > > pagino record to end of btree distance in both directions is shorter
> > > than the search distance for a given loop (i.e. less than 10 records
> > > from pagino to end-of-btree) then that is the only time a corrupted
> > > agi->freecount can cause this problem.
> > > 
> > 
> > I agree with you, but still, we are feasible to have this corruption
> > happening,
> > and I've seen reports of users hitting it.
> > 
> > 
> > > IOWs, on production systems where there's more than a few hundred
> > > inodes (i.e. the vast majority of installations) a corrupted
> > > agi->freecount won't lead to a endless loop because search_distance
> > > will terminate the retry loop and we'll allocate a new inode.
> > > 
> > > To tell the truth, I'd much rather we just use the search distance
> > > to prevent endless looping than add a second method of limiting
> > > the search loop. i.e. don't reset search_distance when we restart
> > > the search loop at pagino.  That means even for small trees (<
> > > search_distance * 2 records) we'll retry when we get to the end of
> > > tree, but we'll still break out of the loop and allocate new inodes
> > > as soon as we hit the search distance limit.
> > > 
> > 
> 
> Here, you are assuming we enter into the 
> 
> while (!doneleft || !doneright) { }
> 
> on every interaction, so it will be able to decrease the searchdistance or you
> mean by moving the --searchdistance somewhere else?
> 
> In very small trees we don't even enter the while loop (both doneleft and doneright are 1),
> so searchdistance isn't decremented at all, resetting it or not will not make any difference
> in this case.

Seems like a minor issue - the first search step left+right is
outside the while loop, and we don't account for that. So change
where the search distance check to take that into account:

	while (--searchdistance > 0 && (!doneleft || !doneright)) {
		.....
	}

	if (searchdistance <= 0) {
		/* save current chunk indexes */
		....
		goto newino;
	}

	/* restart at pagino */
	.....
	goto restart_pagno;


-Dave.
-- 
Dave Chinner
david@fromorbit.com

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Stop searching for free slots in an inode chunk when there are none
  2017-08-14 10:53 Carlos Maiolino
  2017-08-14 11:36 ` Carlos Maiolino
@ 2017-08-14 22:48 ` Darrick J. Wong
  1 sibling, 0 replies; 8+ messages in thread
From: Darrick J. Wong @ 2017-08-14 22:48 UTC (permalink / raw)
  To: Carlos Maiolino; +Cc: linux-xfs

On Mon, Aug 14, 2017 at 12:53:49PM +0200, Carlos Maiolino wrote:
> In a filesystem without finobt, the Space manager selects an AG to alloc a new
> inode, where xfs_dialloc_ag_inobt() will search the AG for the free slot chunk.
> 
> When the new inode is in the samge AG as its parent, the btree will be searched
> starting on the parent's record, and then retried from the top if no slot is
> available beyond the parent's record.
> 
> To exit this loop though, xfs_dialloc_ag_inobt() relies on the fact that the
> btree must have a free slot available, once its callers relied on the
> agi->freecount when deciding how/where to allocate this new inode.
> 
> In the case when the agi->freecount is corrupted, showing available inodes in an
> AG, when in fact there is none, this becomes an infinite loop.
> 
> Add a way to stop the loop when a free slot is not found in the btree, making
> the function to fall into the whole AG scan which will then, be able to detect
> the corruption and shut the filesystem down.
> 
> As pointed by Brian, this might impact performance, giving the fact we
> don't reset the search distance anymore when we reach the end of the
> tree, giving it fewer tries before falling back to the whole AG search, but
> it will only affect searches that start within 10 records to the end of the tree.
> 
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>

Looks ok, will test... hey, what's the xfstest number?

Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

> ---
> Changelog:
> 
> 	V3: - Move searchdistance state saving out of the while loop
> 	    - Remove newino goto label
> 
> 	V2: - Use searchdistance variable to stop the infinite loop
> 	      instead of adding a new mechanism
> 
>  fs/xfs/libxfs/xfs_ialloc.c | 55 +++++++++++++++++++++++-----------------------
>  1 file changed, 27 insertions(+), 28 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
> index d41ade5d293e..0e1cc51f05a1 100644
> --- a/fs/xfs/libxfs/xfs_ialloc.c
> +++ b/fs/xfs/libxfs/xfs_ialloc.c
> @@ -1123,6 +1123,7 @@ xfs_dialloc_ag_inobt(
>  	int			error;
>  	int			offset;
>  	int			i, j;
> +	int			searchdistance = 10;
>  
>  	pag = xfs_perag_get(mp, agno);
>  
> @@ -1149,7 +1150,6 @@ xfs_dialloc_ag_inobt(
>  	if (pagno == agno) {
>  		int		doneleft;	/* done, to the left */
>  		int		doneright;	/* done, to the right */
> -		int		searchdistance = 10;
>  
>  		error = xfs_inobt_lookup(cur, pagino, XFS_LOOKUP_LE, &i);
>  		if (error)
> @@ -1210,21 +1210,9 @@ xfs_dialloc_ag_inobt(
>  		/*
>  		 * Loop until we find an inode chunk with a free inode.
>  		 */
> -		while (!doneleft || !doneright) {
> +		while (--searchdistance > 0 && (!doneleft || !doneright)) {
>  			int	useleft;  /* using left inode chunk this time */
>  
> -			if (!--searchdistance) {
> -				/*
> -				 * Not in range - save last search
> -				 * location and allocate a new inode
> -				 */
> -				xfs_btree_del_cursor(tcur, XFS_BTREE_NOERROR);
> -				pag->pagl_leftrec = trec.ir_startino;
> -				pag->pagl_rightrec = rec.ir_startino;
> -				pag->pagl_pagino = pagino;
> -				goto newino;
> -			}
> -
>  			/* figure out the closer block if both are valid. */
>  			if (!doneleft && !doneright) {
>  				useleft = pagino -
> @@ -1268,26 +1256,37 @@ xfs_dialloc_ag_inobt(
>  				goto error1;
>  		}
>  
> -		/*
> -		 * We've reached the end of the btree. because
> -		 * we are only searching a small chunk of the
> -		 * btree each search, there is obviously free
> -		 * inodes closer to the parent inode than we
> -		 * are now. restart the search again.
> -		 */
> -		pag->pagl_pagino = NULLAGINO;
> -		pag->pagl_leftrec = NULLAGINO;
> -		pag->pagl_rightrec = NULLAGINO;
> -		xfs_btree_del_cursor(tcur, XFS_BTREE_NOERROR);
> -		xfs_btree_del_cursor(cur, XFS_BTREE_NOERROR);
> -		goto restart_pagno;
> +		if (searchdistance <= 0) {
> +			/*
> +			 * Not in range - save last search
> +			 * location and allocate a new inode
> +			 */
> +			xfs_btree_del_cursor(tcur, XFS_BTREE_NOERROR);
> +			pag->pagl_leftrec = trec.ir_startino;
> +			pag->pagl_rightrec = rec.ir_startino;
> +			pag->pagl_pagino = pagino;
> +
> +		} else {
> +			/*
> +			 * We've reached the end of the btree. because
> +			 * we are only searching a small chunk of the
> +			 * btree each search, there is obviously free
> +			 * inodes closer to the parent inode than we
> +			 * are now. restart the search again.
> +			 */
> +			pag->pagl_pagino = NULLAGINO;
> +			pag->pagl_leftrec = NULLAGINO;
> +			pag->pagl_rightrec = NULLAGINO;
> +			xfs_btree_del_cursor(tcur, XFS_BTREE_NOERROR);
> +			xfs_btree_del_cursor(cur, XFS_BTREE_NOERROR);
> +			goto restart_pagno;
> +		}
>  	}
>  
>  	/*
>  	 * In a different AG from the parent.
>  	 * See if the most recently allocated block has any free.
>  	 */
> -newino:
>  	if (agi->agi_newino != cpu_to_be32(NULLAGINO)) {
>  		error = xfs_inobt_lookup(cur, be32_to_cpu(agi->agi_newino),
>  					 XFS_LOOKUP_EQ, &i);
> -- 
> 2.13.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Stop searching for free slots in an inode chunk when there are none
  2017-08-14 10:53 Carlos Maiolino
@ 2017-08-14 11:36 ` Carlos Maiolino
  2017-08-14 22:48 ` Darrick J. Wong
  1 sibling, 0 replies; 8+ messages in thread
From: Carlos Maiolino @ 2017-08-14 11:36 UTC (permalink / raw)
  To: linux-xfs

I do apologize for forgetting to update the subject before sending it, this is
the 3rd version of this patch

> 
> 	V3: - Move searchdistance state saving out of the while loop
> 	    - Remove newino goto label
> 
> 	V2: - Use searchdistance variable to stop the infinite loop
> 	      instead of adding a new mechanism
> 

-- 
Carlos

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH] Stop searching for free slots in an inode chunk when there are none
@ 2017-08-14 10:53 Carlos Maiolino
  2017-08-14 11:36 ` Carlos Maiolino
  2017-08-14 22:48 ` Darrick J. Wong
  0 siblings, 2 replies; 8+ messages in thread
From: Carlos Maiolino @ 2017-08-14 10:53 UTC (permalink / raw)
  To: linux-xfs

In a filesystem without finobt, the Space manager selects an AG to alloc a new
inode, where xfs_dialloc_ag_inobt() will search the AG for the free slot chunk.

When the new inode is in the samge AG as its parent, the btree will be searched
starting on the parent's record, and then retried from the top if no slot is
available beyond the parent's record.

To exit this loop though, xfs_dialloc_ag_inobt() relies on the fact that the
btree must have a free slot available, once its callers relied on the
agi->freecount when deciding how/where to allocate this new inode.

In the case when the agi->freecount is corrupted, showing available inodes in an
AG, when in fact there is none, this becomes an infinite loop.

Add a way to stop the loop when a free slot is not found in the btree, making
the function to fall into the whole AG scan which will then, be able to detect
the corruption and shut the filesystem down.

As pointed by Brian, this might impact performance, giving the fact we
don't reset the search distance anymore when we reach the end of the
tree, giving it fewer tries before falling back to the whole AG search, but
it will only affect searches that start within 10 records to the end of the tree.

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
Changelog:

	V3: - Move searchdistance state saving out of the while loop
	    - Remove newino goto label

	V2: - Use searchdistance variable to stop the infinite loop
	      instead of adding a new mechanism

 fs/xfs/libxfs/xfs_ialloc.c | 55 +++++++++++++++++++++++-----------------------
 1 file changed, 27 insertions(+), 28 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
index d41ade5d293e..0e1cc51f05a1 100644
--- a/fs/xfs/libxfs/xfs_ialloc.c
+++ b/fs/xfs/libxfs/xfs_ialloc.c
@@ -1123,6 +1123,7 @@ xfs_dialloc_ag_inobt(
 	int			error;
 	int			offset;
 	int			i, j;
+	int			searchdistance = 10;
 
 	pag = xfs_perag_get(mp, agno);
 
@@ -1149,7 +1150,6 @@ xfs_dialloc_ag_inobt(
 	if (pagno == agno) {
 		int		doneleft;	/* done, to the left */
 		int		doneright;	/* done, to the right */
-		int		searchdistance = 10;
 
 		error = xfs_inobt_lookup(cur, pagino, XFS_LOOKUP_LE, &i);
 		if (error)
@@ -1210,21 +1210,9 @@ xfs_dialloc_ag_inobt(
 		/*
 		 * Loop until we find an inode chunk with a free inode.
 		 */
-		while (!doneleft || !doneright) {
+		while (--searchdistance > 0 && (!doneleft || !doneright)) {
 			int	useleft;  /* using left inode chunk this time */
 
-			if (!--searchdistance) {
-				/*
-				 * Not in range - save last search
-				 * location and allocate a new inode
-				 */
-				xfs_btree_del_cursor(tcur, XFS_BTREE_NOERROR);
-				pag->pagl_leftrec = trec.ir_startino;
-				pag->pagl_rightrec = rec.ir_startino;
-				pag->pagl_pagino = pagino;
-				goto newino;
-			}
-
 			/* figure out the closer block if both are valid. */
 			if (!doneleft && !doneright) {
 				useleft = pagino -
@@ -1268,26 +1256,37 @@ xfs_dialloc_ag_inobt(
 				goto error1;
 		}
 
-		/*
-		 * We've reached the end of the btree. because
-		 * we are only searching a small chunk of the
-		 * btree each search, there is obviously free
-		 * inodes closer to the parent inode than we
-		 * are now. restart the search again.
-		 */
-		pag->pagl_pagino = NULLAGINO;
-		pag->pagl_leftrec = NULLAGINO;
-		pag->pagl_rightrec = NULLAGINO;
-		xfs_btree_del_cursor(tcur, XFS_BTREE_NOERROR);
-		xfs_btree_del_cursor(cur, XFS_BTREE_NOERROR);
-		goto restart_pagno;
+		if (searchdistance <= 0) {
+			/*
+			 * Not in range - save last search
+			 * location and allocate a new inode
+			 */
+			xfs_btree_del_cursor(tcur, XFS_BTREE_NOERROR);
+			pag->pagl_leftrec = trec.ir_startino;
+			pag->pagl_rightrec = rec.ir_startino;
+			pag->pagl_pagino = pagino;
+
+		} else {
+			/*
+			 * We've reached the end of the btree. because
+			 * we are only searching a small chunk of the
+			 * btree each search, there is obviously free
+			 * inodes closer to the parent inode than we
+			 * are now. restart the search again.
+			 */
+			pag->pagl_pagino = NULLAGINO;
+			pag->pagl_leftrec = NULLAGINO;
+			pag->pagl_rightrec = NULLAGINO;
+			xfs_btree_del_cursor(tcur, XFS_BTREE_NOERROR);
+			xfs_btree_del_cursor(cur, XFS_BTREE_NOERROR);
+			goto restart_pagno;
+		}
 	}
 
 	/*
 	 * In a different AG from the parent.
 	 * See if the most recently allocated block has any free.
 	 */
-newino:
 	if (agi->agi_newino != cpu_to_be32(NULLAGINO)) {
 		error = xfs_inobt_lookup(cur, be32_to_cpu(agi->agi_newino),
 					 XFS_LOOKUP_EQ, &i);
-- 
2.13.5


^ permalink raw reply related	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2017-08-14 22:48 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-03 15:19 [PATCH] Stop searching for free slots in an inode chunk when there are none Carlos Maiolino
2017-08-03 22:35 ` Dave Chinner
2017-08-04  8:55   ` Carlos Eduardo Maiolino
2017-08-04  9:36     ` Carlos Eduardo Maiolino
2017-08-04 23:17       ` Dave Chinner
2017-08-14 10:53 Carlos Maiolino
2017-08-14 11:36 ` Carlos Maiolino
2017-08-14 22:48 ` Darrick J. Wong

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.