[Ocfs2-devel] [PATCH 4/4] ocfs2: limit the number of inodes in orphanscan

[Ocfs2-devel] [PATCH 4/4] ocfs2: limit the number of inodes in orphanscan

[Wengang Wang]

During orphanscan, limit the scan number in a single loop to avoid OOM.

Signed-off-by: Wengang Wang <wen.gang.wang@oracle.com>
---
 fs/ocfs2/journal.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/fs/ocfs2/journal.c b/fs/ocfs2/journal.c
index 7d4e1ca..1c0487b 100644
--- a/fs/ocfs2/journal.c
+++ b/fs/ocfs2/journal.c
@@ -56,6 +56,7 @@
 DEFINE_SPINLOCK(trans_inc_lock);
 
 #define ORPHAN_SCAN_SCHEDULE_TIMEOUT 300000
+#define ORPHAN_SCAN_ITEMS_PER_LOOP 1000
 
 static int ocfs2_force_read_journal(struct inode *inode);
 static int ocfs2_recover_node(struct ocfs2_super *osb,
@@ -1926,6 +1927,7 @@ struct ocfs2_orphan_filldir_priv {
 	struct inode		*head;
 	struct ocfs2_super	*osb;
 	struct 			inode *orphan_dir;
+	int			count;
 };
 
 static int ocfs2_orphan_filldir(void *priv, const char *name, int name_len,
@@ -1964,6 +1966,12 @@ static int ocfs2_orphan_filldir(void *priv, const char *name, int name_len,
 	}
 	spin_unlock(&o_orphandir->ip_lock);
 
+	/*
+	 * reading limited number of inodes in a single loop to avoid OOM
+	 */
+	if ( ++p->count == ORPHAN_SCAN_ITEMS_PER_LOOP )
+		return -EAGAIN;
+
 	return 0;
 }
 
@@ -1978,6 +1986,7 @@ static int ocfs2_queue_orphans(struct ocfs2_super *osb,
 
 	priv.osb = osb;
 	priv.head = *head;
+	priv.count = 0;
 
 	orphan_dir_inode = ocfs2_get_system_file_inode(osb,
 						       ORPHAN_DIR_SYSTEM_INODE,
-- 
1.7.5.2

[Ocfs2-devel] [PATCH 4/4] ocfs2: limit the number of inodes in orphanscan

[Joel Becker]

On Fri, Jul 29, 2011 at 06:06:23PM +0800, Wengang Wang wrote:
> During orphanscan, limit the scan number in a single loop to avoid OOM.

How does this OOM?  Please explain this in the patch and in the patch
description.  Or, conversely, free the resources the orphan scan is done
using.

Joel

> 
> Signed-off-by: Wengang Wang <wen.gang.wang@oracle.com>
> ---
>  fs/ocfs2/journal.c |    9 +++++++++
>  1 files changed, 9 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/ocfs2/journal.c b/fs/ocfs2/journal.c
> index 7d4e1ca..1c0487b 100644
> --- a/fs/ocfs2/journal.c
> +++ b/fs/ocfs2/journal.c
> @@ -56,6 +56,7 @@
>  DEFINE_SPINLOCK(trans_inc_lock);
>  
>  #define ORPHAN_SCAN_SCHEDULE_TIMEOUT 300000
> +#define ORPHAN_SCAN_ITEMS_PER_LOOP 1000
>  
>  static int ocfs2_force_read_journal(struct inode *inode);
>  static int ocfs2_recover_node(struct ocfs2_super *osb,
> @@ -1926,6 +1927,7 @@ struct ocfs2_orphan_filldir_priv {
>  	struct inode		*head;
>  	struct ocfs2_super	*osb;
>  	struct 			inode *orphan_dir;
> +	int			count;
>  };
>  
>  static int ocfs2_orphan_filldir(void *priv, const char *name, int name_len,
> @@ -1964,6 +1966,12 @@ static int ocfs2_orphan_filldir(void *priv, const char *name, int name_len,
>  	}
>  	spin_unlock(&o_orphandir->ip_lock);
>  
> +	/*
> +	 * reading limited number of inodes in a single loop to avoid OOM
> +	 */
> +	if ( ++p->count == ORPHAN_SCAN_ITEMS_PER_LOOP )
> +		return -EAGAIN;
> +
>  	return 0;
>  }
>  
> @@ -1978,6 +1986,7 @@ static int ocfs2_queue_orphans(struct ocfs2_super *osb,
>  
>  	priv.osb = osb;
>  	priv.head = *head;
> +	priv.count = 0;
>  
>  	orphan_dir_inode = ocfs2_get_system_file_inode(osb,
>  						       ORPHAN_DIR_SYSTEM_INODE,
> -- 
> 1.7.5.2
> 
> 
> _______________________________________________
> Ocfs2-devel mailing list
> Ocfs2-devel at oss.oracle.com
> http://oss.oracle.com/mailman/listinfo/ocfs2-devel

-- 

"Get right to the heart of matters.
 It's the heart that matters more."

			http://www.jlbec.org/
			jlbec at evilplan.org

[Ocfs2-devel] [PATCH 4/4] ocfs2: limit the number of inodes in orphanscan

[Wengang Wang]

Hi Joel,

On 11-08-21 21:47, Joel Becker wrote:
> On Fri, Jul 29, 2011 at 06:06:23PM +0800, Wengang Wang wrote:
> > During orphanscan, limit the scan number in a single loop to avoid OOM.
> 
> How does this OOM?  Please explain this in the patch and in the patch
> description.  Or, conversely, free the resources the orphan scan is done
> using.

Explaination why it OOM:
In current code, when recoverying orphan dirs, we are doing it like this:

lock orphandir
read out all the inodes under the orphandir to a list
unlock the orphandir
#above see ocfs2_queue_orphans()
and the iput all the inodes in the list to delete the on disk inode
#above see ocfs2_recover_orphans()

For normal case, there are no many files under orphandir, there is no problem.
But for an extreme case, there are a huge number of files under orphandir,
huge size of memory is needed to hold the inodes. It is possible to exhaust
memory, then OOM occurs.

The fix in this patch depends on patch
  "ocfs2: ocfs2_dir_foreach returns the error turned by filldir"
Since NAK for that patch, please ignore this too.

thanks,
wengang.
 
> Joel
> 
> > 

[Ocfs2-devel] [PATCH 4/4] ocfs2: limit the number of inodes in orphanscan

[Joel Becker]

On Mon, Aug 22, 2011 at 01:51:01PM +0800, Wengang Wang wrote:
> Hi Joel,
> 
> On 11-08-21 21:47, Joel Becker wrote:
> > On Fri, Jul 29, 2011 at 06:06:23PM +0800, Wengang Wang wrote:
> > > During orphanscan, limit the scan number in a single loop to avoid OOM.
> > 
> > How does this OOM?  Please explain this in the patch and in the patch
> > description.  Or, conversely, free the resources the orphan scan is done
> > using.
> 
> Explaination why it OOM:
> In current code, when recoverying orphan dirs, we are doing it like this:
> 
> lock orphandir
> read out all the inodes under the orphandir to a list
> unlock the orphandir
> #above see ocfs2_queue_orphans()
> and the iput all the inodes in the list to delete the on disk inode
> #above see ocfs2_recover_orphans()
> 
> For normal case, there are no many files under orphandir, there is no problem.
> But for an extreme case, there are a huge number of files under orphandir,
> huge size of memory is needed to hold the inodes. It is possible to exhaust
> memory, then OOM occurs.

	This makes sense.  But you didn't describe this in the patch or
the patch description ;-)

> The fix in this patch depends on patch
>   "ocfs2: ocfs2_dir_foreach returns the error turned by filldir"
> Since NAK for that patch, please ignore this too.

	Why is that required?

Joel

-- 

"Practice random acts of kindness and senseless acts of beauty."

 Oh, and don't forget where your towel is.

			http://www.jlbec.org/
			jlbec at evilplan.org

[Ocfs2-devel] [PATCH 4/4] ocfs2: limit the number of inodes in orphanscan

[Wengang Wang]

On 11-08-22 01:19, Joel Becker wrote:
> On Mon, Aug 22, 2011 at 01:51:01PM +0800, Wengang Wang wrote:
> > Hi Joel,
> > 
> > On 11-08-21 21:47, Joel Becker wrote:
> > > On Fri, Jul 29, 2011 at 06:06:23PM +0800, Wengang Wang wrote:
> > > > During orphanscan, limit the scan number in a single loop to avoid OOM.
> > > 
> > > How does this OOM?  Please explain this in the patch and in the patch
> > > description.  Or, conversely, free the resources the orphan scan is done
> > > using.
> > 
> > Explaination why it OOM:
> > In current code, when recoverying orphan dirs, we are doing it like this:
> > 
> > lock orphandir
> > read out all the inodes under the orphandir to a list
> > unlock the orphandir
> > #above see ocfs2_queue_orphans()
> > and the iput all the inodes in the list to delete the on disk inode
> > #above see ocfs2_recover_orphans()
> > 
> > For normal case, there are no many files under orphandir, there is no problem.
> > But for an extreme case, there are a huge number of files under orphandir,
> > huge size of memory is needed to hold the inodes. It is possible to exhaust
> > memory, then OOM occurs.
> 
> 	This makes sense.  But you didn't describe this in the patch or
> the patch description ;-)

Yes :) Since the following, I didn't. And sure I will add them if the
patch will get committed.
> 
> > The fix in this patch depends on patch
> >   "ocfs2: ocfs2_dir_foreach returns the error turned by filldir"
> > Since NAK for that patch, please ignore this too.
> 
> 	Why is that required?
> 

ocfs2_dir_foreach deals with all direntries from begining to end if
nothing breaks it. filldir returning an error is a breaker.

For current code, though filldir returning error breaks the loop, it doesn't
tell why. The caller then doesn't know it's break or a finish thus no way
to decide next action should be "continue looping" or "returning to caller"

The code in this patch is not full fix for the problem, it depends on
also patch
	[PATCH 3/4] ocfs2: fix slow deleting
#this patch is [PATCH 4/4].

For the 4 patches 
	[PATCH 1/4] ocfs2: make direntry invalid when deleting it
	[PATCH 2/4] ocfs2: ocfs2_dir_foreach returns the error turned by filldir
	[PATCH 3/4] ocfs2: fix slow deleting
	[PATCH 4/4] ocfs2: limit the number of inodes in orphanscan
The later depends on the earlier.

Looking at all of them, you can know I am using the error code returned
by filldir, -EAGAIN, to indicate the break.

thanks,
wengang.