[PATCH v3 0/3] Print when ENOSPC due to lack of inodes.

[PATCH v3 0/3] Print when ENOSPC due to lack of inodes.

[raghu.prabhu13]

From: Raghavendra D Prabhu <rprabhu@wnohang.net>

Currently, when there are no free inodes left / free space to allocate them (usually
without inode64), there is no indication anywhere of this case, making it harder
to diagnose this case.

Hence, this series prints the causes/reasons to kernel log in a ratelimited
manner, when such a situation arises.

Regarding why it is printed at callee location instead at caller, it gives
greater granularity in expressing the precise reason and give more details, and
also some along the path are not ENOSPC (such in xfs_ialloc where ialloc_context is not NULL but
ino is) along with the fact that it (xfs_ialloc) is called at multiple sites, so to avoid duplication.

Version 1: Initial series.
Version 2: Added ratelimited printing to xfs_message and used that.
Version 3: Kept the logic intact in few places, fixed the column requirement. 

Raghavendra D Prabhu (3):
  xfs: Add ratelimited printk for different alert levels
  xfs: Print error when xfs_ialloc_ag_select fails to find continuous
    free space.
  xfs: Print error when unable to allocate inodes or out of free
    inodes.

 fs/xfs/xfs_ialloc.c  | 31 ++++++++++++++++++++++++++++---
 fs/xfs/xfs_linux.h   |  1 +
 fs/xfs/xfs_message.h | 26 ++++++++++++++++++++++++++
 3 files changed, 55 insertions(+), 3 deletions(-)

-- 
1.7.12.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH v3 1/3] xfs: Add ratelimited printk for different alert levels

[raghu.prabhu13]

From: Raghavendra D Prabhu <rprabhu@wnohang.net>

Ratelimited printk will be useful in printing xfs messages which are otherwise
not required to be printed always due to their high rate (to prevent kernel ring
buffer from overflowing), while at the same time required to be printed.

Signed-off-by: Raghavendra D Prabhu <rprabhu@wnohang.net>
---
 fs/xfs/xfs_linux.h   |  1 +
 fs/xfs/xfs_message.h | 26 ++++++++++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/fs/xfs/xfs_linux.h b/fs/xfs/xfs_linux.h
index 828662f..49f8505 100644
--- a/fs/xfs/xfs_linux.h
+++ b/fs/xfs/xfs_linux.h
@@ -71,6 +71,7 @@
 #include <linux/kthread.h>
 #include <linux/freezer.h>
 #include <linux/list_sort.h>
+#include <linux/ratelimit.h>
 
 #include <asm/page.h>
 #include <asm/div64.h>
diff --git a/fs/xfs/xfs_message.h b/fs/xfs/xfs_message.h
index 56dc0c1..76c8198 100644
--- a/fs/xfs/xfs_message.h
+++ b/fs/xfs/xfs_message.h
@@ -30,6 +30,32 @@ void xfs_debug(const struct xfs_mount *mp, const char *fmt, ...)
 }
 #endif
 
+#define xfs_printk_ratelimited(func, dev, fmt, ...)		\
+do {									\
+	static DEFINE_RATELIMIT_STATE(_rs,				\
+				      DEFAULT_RATELIMIT_INTERVAL,	\
+				      DEFAULT_RATELIMIT_BURST);		\
+	if (__ratelimit(&_rs))						\
+		func(dev, fmt, ##__VA_ARGS__);			\
+} while (0)
+
+#define xfs_emerg_ratelimited(dev, fmt, ...)				\
+	xfs_printk_ratelimited(xfs_emerg, dev, fmt, ##__VA_ARGS__)
+#define xfs_alert_ratelimited(dev, fmt, ...)				\
+	xfs_printk_ratelimited(xfs_alert, dev, fmt, ##__VA_ARGS__)
+#define xfs_crit_ratelimited(dev, fmt, ...)				\
+	xfs_printk_ratelimited(xfs_crit, dev, fmt, ##__VA_ARGS__)
+#define xfs_err_ratelimited(dev, fmt, ...)				\
+	xfs_printk_ratelimited(xfs_err, dev, fmt, ##__VA_ARGS__)
+#define xfs_warn_ratelimited(dev, fmt, ...)				\
+	xfs_printk_ratelimited(xfs_warn, dev, fmt, ##__VA_ARGS__)
+#define xfs_notice_ratelimited(dev, fmt, ...)				\
+	xfs_printk_ratelimited(xfs_notice, dev, fmt, ##__VA_ARGS__)
+#define xfs_info_ratelimited(dev, fmt, ...)				\
+	xfs_printk_ratelimited(xfs_info, dev, fmt, ##__VA_ARGS__)
+#define xfs_debug_ratelimited(dev, fmt, ...)				\
+	xfs_printk_ratelimited(xfs_debug, dev, fmt, ##__VA_ARGS__)
+
 extern void assfail(char *expr, char *f, int l);
 
 extern void xfs_hex_dump(void *p, int length);
-- 
1.7.12.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[v3 Repost] xfs: Add ratelimited printk for different alert levels

[Raghavendra D Prabhu]

From: Raghavendra D Prabhu <rprabhu@wnohang.net>

Ratelimited printk will be useful in printing xfs messages which are otherwise
not required to be printed always due to their high rate (to prevent kernel ring
buffer from overflowing), while at the same time required to be printed.

Signed-off-by: Raghavendra D Prabhu <rprabhu@wnohang.net>
Reviewed-by: Rich Johnston <rjohnston@sgi.com>
Reviewed-by: Dave Chinner <dchinner@redhat.com>

---

Sorry this has not been committed and it's been a few months so I am
reposting this patch.  If I don't hear any objections by 10 am CST on
29 March 2013 I will commit this patch.
 

fs/xfs/xfs_linux.h   |  1 +
 fs/xfs/xfs_message.h | 26 ++++++++++++++++++++++++++
 2 files changed, 27 insertions(+)

Index: linux/fs/xfs/xfs_linux.h
===================================================================
--- linux.orig/fs/xfs/xfs_linux.h	2013-03-22 09:19:54.000000000 -0500
+++ linux/fs/xfs/xfs_linux.h	2013-03-27 12:55:33.000000000 -0500
@@ -72,6 +72,7 @@
 #include <linux/kthread.h>
 #include <linux/freezer.h>
 #include <linux/list_sort.h>
+#include <linux/ratelimit.h>
 
 #include <asm/page.h>
 #include <asm/div64.h>
Index: linux/fs/xfs/xfs_message.h
===================================================================
--- linux.orig/fs/xfs/xfs_message.h	2013-03-22 09:19:54.000000000 -0500
+++ linux/fs/xfs/xfs_message.h	2013-03-27 12:55:33.000000000 -0500
@@ -30,6 +30,32 @@
 }
 #endif
 
+#define xfs_printk_ratelimited(func, dev, fmt, ...)		\
+do {									\
+	static DEFINE_RATELIMIT_STATE(_rs,				\
+				      DEFAULT_RATELIMIT_INTERVAL,	\
+				      DEFAULT_RATELIMIT_BURST);		\
+	if (__ratelimit(&_rs))						\
+		func(dev, fmt, ##__VA_ARGS__);			\
+} while (0)
+
+#define xfs_emerg_ratelimited(dev, fmt, ...)				\
+	xfs_printk_ratelimited(xfs_emerg, dev, fmt, ##__VA_ARGS__)
+#define xfs_alert_ratelimited(dev, fmt, ...)				\
+	xfs_printk_ratelimited(xfs_alert, dev, fmt, ##__VA_ARGS__)
+#define xfs_crit_ratelimited(dev, fmt, ...)				\
+	xfs_printk_ratelimited(xfs_crit, dev, fmt, ##__VA_ARGS__)
+#define xfs_err_ratelimited(dev, fmt, ...)				\
+	xfs_printk_ratelimited(xfs_err, dev, fmt, ##__VA_ARGS__)
+#define xfs_warn_ratelimited(dev, fmt, ...)				\
+	xfs_printk_ratelimited(xfs_warn, dev, fmt, ##__VA_ARGS__)
+#define xfs_notice_ratelimited(dev, fmt, ...)				\
+	xfs_printk_ratelimited(xfs_notice, dev, fmt, ##__VA_ARGS__)
+#define xfs_info_ratelimited(dev, fmt, ...)				\
+	xfs_printk_ratelimited(xfs_info, dev, fmt, ##__VA_ARGS__)
+#define xfs_debug_ratelimited(dev, fmt, ...)				\
+	xfs_printk_ratelimited(xfs_debug, dev, fmt, ##__VA_ARGS__)
+
 extern void assfail(char *expr, char *f, int l);
 
 extern void xfs_hex_dump(void *p, int length);

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH v3 2/3] xfs: Print error when xfs_ialloc_ag_select fails to find continuous free space.

[raghu.prabhu13]

From: Raghavendra D Prabhu <rprabhu@wnohang.net>

When xfs_ialloc_ag_select fails to find any AG with continuous free blocks
required for inode allocation, printk the error in ratelimited manner.

Signed-off-by: Raghavendra D Prabhu <rprabhu@wnohang.net>
---
 fs/xfs/xfs_ialloc.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_ialloc.c b/fs/xfs/xfs_ialloc.c
index 5aceb3f..e75a39d 100644
--- a/fs/xfs/xfs_ialloc.c
+++ b/fs/xfs/xfs_ialloc.c
@@ -539,8 +539,11 @@ nextag:
 		if (agno >= agcount)
 			agno = 0;
 		if (agno == pagno) {
-			if (flags == 0)
+			if (flags == 0) {
+				xfs_err_ratelimited(mp,
+					"Out of continuous free blocks for inode allocation");
 				return NULLAGNUMBER;
+			}
 			flags = 0;
 		}
 	}
-- 
1.7.12.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH v3 3/3] xfs: Print error when unable to allocate inodes or out of free inodes.

[raghu.prabhu13]

From: Raghavendra D Prabhu <rprabhu@wnohang.net>

When xfs_dialloc is unable to allocate required number of inodes due to global
ceiling, or AGs are out of free inodes but not allowed to allocate inodes or
unable to allocate without continguous free space, printk the error in
ratelimited manner.

Signed-off-by: Raghavendra D Prabhu <rprabhu@wnohang.net>
---
 fs/xfs/xfs_ialloc.c | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/xfs_ialloc.c b/fs/xfs/xfs_ialloc.c
index e75a39d..e9f911b2 100644
--- a/fs/xfs/xfs_ialloc.c
+++ b/fs/xfs/xfs_ialloc.c
@@ -991,7 +991,7 @@ xfs_dialloc(
 
 			xfs_perag_put(pag);
 			*inop = NULLFSINO;
-			return 0;
+			goto out_spc;
 		}
 
 		if (ialloced) {
@@ -1017,7 +1017,7 @@ nextag:
 			agno = 0;
 		if (agno == start_agno) {
 			*inop = NULLFSINO;
-			return noroom ? ENOSPC : 0;
+			goto out_spc;
 		}
 	}
 
@@ -1027,6 +1027,28 @@ out_alloc:
 out_error:
 	xfs_perag_put(pag);
 	return XFS_ERROR(error);
+out_spc:
+	if (noroom) {
+		xfs_err_ratelimited(mp, "Hit global inode ceiling:");
+		error = ENOSPC;
+	} else if (!okalloc) {
+		/*
+		 * implies noroom=0 && (!pag->pagi_freecount && !okalloc) for
+		 * all pag
+		 */
+		xfs_err_ratelimited(mp,
+			"No AGs with free inodes and allocation not allowed:");
+		error = 0;
+	} else {
+		xfs_err_ratelimited(mp,
+			"Unable to allocate continguous space for inodes:");
+		error = 0;
+	}
+
+	xfs_err_ratelimited(mp, "Required %d, Current %llu, Maximum %llu",
+		XFS_IALLOC_INODES(mp), mp->m_sb.sb_icount, mp->m_maxicount);
+	return error;
+
 }
 
 /*
-- 
1.7.12.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH v3 0/3] Print when ENOSPC due to lack of inodes.

[Raghavendra Prabhu]

Hi,

Checking back on this, whether it has been reviewed and if so, any comments.

On Wed, Sep 26, 2012 at 12:26 PM,  <raghu.prabhu13@gmail.com> wrote:
> From: Raghavendra D Prabhu <rprabhu@wnohang.net>
>
> Currently, when there are no free inodes left / free space to allocate them (usually
> without inode64), there is no indication anywhere of this case, making it harder
> to diagnose this case.
>
> Hence, this series prints the causes/reasons to kernel log in a ratelimited
> manner, when such a situation arises.
>
> Regarding why it is printed at callee location instead at caller, it gives
> greater granularity in expressing the precise reason and give more details, and
> also some along the path are not ENOSPC (such in xfs_ialloc where ialloc_context is not NULL but
> ino is) along with the fact that it (xfs_ialloc) is called at multiple sites, so to avoid duplication.
>
> Version 1: Initial series.
> Version 2: Added ratelimited printing to xfs_message and used that.
> Version 3: Kept the logic intact in few places, fixed the column requirement.
>
> Raghavendra D Prabhu (3):
>   xfs: Add ratelimited printk for different alert levels
>   xfs: Print error when xfs_ialloc_ag_select fails to find continuous
>     free space.
>   xfs: Print error when unable to allocate inodes or out of free
>     inodes.
>
>  fs/xfs/xfs_ialloc.c  | 31 ++++++++++++++++++++++++++++---
>  fs/xfs/xfs_linux.h   |  1 +
>  fs/xfs/xfs_message.h | 26 ++++++++++++++++++++++++++
>  3 files changed, 55 insertions(+), 3 deletions(-)
>
> --
> 1.7.12.1
>

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH v3 0/3] Print when ENOSPC due to lack of inodes.

[Rich Johnston]

Hey Raghu,

Thanks for contributing, I am reviewing and testing this now.

Regards
--Rich

On 10/24/2012 03:56 PM, Raghavendra Prabhu wrote:
> Hi,
>
> Checking back on this, whether it has been reviewed and if so, any comments.
>
> On Wed, Sep 26, 2012 at 12:26 PM,  <raghu.prabhu13@gmail.com> wrote:
>> From: Raghavendra D Prabhu <rprabhu@wnohang.net>
>>
>> Currently, when there are no free inodes left / free space to allocate them (usually
>> without inode64), there is no indication anywhere of this case, making it harder
>> to diagnose this case.
>>
>> Hence, this series prints the causes/reasons to kernel log in a ratelimited
>> manner, when such a situation arises.
>>
>> Regarding why it is printed at callee location instead at caller, it gives
>> greater granularity in expressing the precise reason and give more details, and
>> also some along the path are not ENOSPC (such in xfs_ialloc where ialloc_context is not NULL but
>> ino is) along with the fact that it (xfs_ialloc) is called at multiple sites, so to avoid duplication.
>>
>> Version 1: Initial series.
>> Version 2: Added ratelimited printing to xfs_message and used that.
>> Version 3: Kept the logic intact in few places, fixed the column requirement.
>>
>> Raghavendra D Prabhu (3):
>>    xfs: Add ratelimited printk for different alert levels
>>    xfs: Print error when xfs_ialloc_ag_select fails to find continuous
>>      free space.
>>    xfs: Print error when unable to allocate inodes or out of free
>>      inodes.
>>
>>   fs/xfs/xfs_ialloc.c  | 31 ++++++++++++++++++++++++++++---
>>   fs/xfs/xfs_linux.h   |  1 +
>>   fs/xfs/xfs_message.h | 26 ++++++++++++++++++++++++++
>>   3 files changed, 55 insertions(+), 3 deletions(-)
>>
>> --
>> 1.7.12.1
>>
>
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
>


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [v3,1/3] xfs: Add ratelimited printk for different alert levels

[Rich Johnston]

On 09/26/2012 01:56 AM, Raghavendra D Prabhu wrote:
> From: Raghavendra D Prabhu <rprabhu@wnohang.net>
>
> Ratelimited printk will be useful in printing xfs messages which are otherwise
> not required to be printed always due to their high rate (to prevent kernel ring
> buffer from overflowing), while at the same time required to be printed.
>
> Signed-off-by: Raghavendra D Prabhu <rprabhu@wnohang.net>
>
Thanks Raghavendra,

Looks good.

Reviewed-by: Rich Johnston <rjohnston@sgi.com>

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [v3, 2/3] xfs: Print error when xfs_ialloc_ag_select fails to find continuous free space.

[Rich Johnston]

On 09/26/2012 01:56 AM, Raghavendra D Prabhu wrote:
> From: Raghavendra D Prabhu <rprabhu@wnohang.net>
>
> When xfs_ialloc_ag_select fails to find any AG with continuous free blocks
> required for inode allocation, printk the error in ratelimited manner.
>
> Signed-off-by: Raghavendra D Prabhu <rprabhu@wnohang.net>
>


Thanks Raghavendra,

Looks good.

Reviewed-by: Rich Johnston <rjohnston@sgi.com>

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [v3, 3/3] xfs: Print error when unable to allocate inodes or out of free inodes.

[Rich Johnston]

On 09/26/2012 01:56 AM, Raghavendra D Prabhu wrote:
> From: Raghavendra D Prabhu <rprabhu@wnohang.net>
>
> When xfs_dialloc is unable to allocate required number of inodes due to global
> ceiling, or AGs are out of free inodes but not allowed to allocate inodes or
> unable to allocate without continguous free space, printk the error in
> ratelimited manner.
>
> Signed-off-by: Raghavendra D Prabhu <rprabhu@wnohang.net>
>

Thanks Raghavendra,

Looks good.

Reviewed-by: Rich Johnston <rjohnston@sgi.com>

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH v3 0/3] Print when ENOSPC due to lack of inodes.

[Rich Johnston]

On 10/24/2012 03:56 PM, Raghavendra Prabhu wrote:
> Hi,
>
> Checking back on this, whether it has been reviewed and if so, any comments.
>
> On Wed, Sep 26, 2012 at 12:26 PM,  <raghu.prabhu13@gmail.com> wrote:
>> From: Raghavendra D Prabhu <rprabhu@wnohang.net>
>>
>> Currently, when there are no free inodes left / free space to allocate them (usually
>> without inode64), there is no indication anywhere of this case, making it harder
>> to diagnose this case.
>>
>> Hence, this series prints the causes/reasons to kernel log in a ratelimited
>> manner, when such a situation arises.
>>
>> Regarding why it is printed at callee location instead at caller, it gives
>> greater granularity in expressing the precise reason and give more details, and
>> also some along the path are not ENOSPC (such in xfs_ialloc where ialloc_context is not NULL but
>> ino is) along with the fact that it (xfs_ialloc) is called at multiple sites, so to avoid duplication.
>>
>> Version 1: Initial series.
>> Version 2: Added ratelimited printing to xfs_message and used that.
>> Version 3: Kept the logic intact in few places, fixed the column requirement.
>>
>> Raghavendra D Prabhu (3):
>>    xfs: Add ratelimited printk for different alert levels
>>    xfs: Print error when xfs_ialloc_ag_select fails to find continuous
>>      free space.
>>    xfs: Print error when unable to allocate inodes or out of free
>>      inodes.
>>
>>   fs/xfs/xfs_ialloc.c  | 31 ++++++++++++++++++++++++++++---
>>   fs/xfs/xfs_linux.h   |  1 +
>>   fs/xfs/xfs_message.h | 26 ++++++++++++++++++++++++++
>>   3 files changed, 55 insertions(+), 3 deletions(-)
>>
>> --
>> 1.7.12.1
>>
>
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
>

Ben,

When you have a chance can you pull this series in.  I have tested it 
and it looks good.

Thanks
--Rich

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH v3 0/3] Print when ENOSPC due to lack of inodes.

[Dave Chinner]

On Fri, Oct 26, 2012 at 08:18:59AM -0500, Rich Johnston wrote:
> On 10/24/2012 03:56 PM, Raghavendra Prabhu wrote:
> >Hi,
> >
> >Checking back on this, whether it has been reviewed and if so, any comments.
> >
> >On Wed, Sep 26, 2012 at 12:26 PM,  <raghu.prabhu13@gmail.com> wrote:
> >>From: Raghavendra D Prabhu <rprabhu@wnohang.net>
> >>
> >>Currently, when there are no free inodes left / free space to allocate them (usually
> >>without inode64), there is no indication anywhere of this case, making it harder
> >>to diagnose this case.
> >>
> >>Hence, this series prints the causes/reasons to kernel log in a ratelimited
> >>manner, when such a situation arises.
> >>
> >>Regarding why it is printed at callee location instead at caller, it gives
> >>greater granularity in expressing the precise reason and give more details, and
> >>also some along the path are not ENOSPC (such in xfs_ialloc where ialloc_context is not NULL but
> >>ino is) along with the fact that it (xfs_ialloc) is called at multiple sites, so to avoid duplication.
> >>
> >>Version 1: Initial series.
> >>Version 2: Added ratelimited printing to xfs_message and used that.
> >>Version 3: Kept the logic intact in few places, fixed the column requirement.
> >>
> >>Raghavendra D Prabhu (3):
> >>   xfs: Add ratelimited printk for different alert levels
> >>   xfs: Print error when xfs_ialloc_ag_select fails to find continuous
> >>     free space.
> >>   xfs: Print error when unable to allocate inodes or out of free
> >>     inodes.
> >>
> >>  fs/xfs/xfs_ialloc.c  | 31 ++++++++++++++++++++++++++++---
> >>  fs/xfs/xfs_linux.h   |  1 +
> >>  fs/xfs/xfs_message.h | 26 ++++++++++++++++++++++++++
> >>  3 files changed, 55 insertions(+), 3 deletions(-)
> >>
> >>--
> >>1.7.12.1
> >>
> >
> >_______________________________________________
> >xfs mailing list
> >xfs@oss.sgi.com
> >http://oss.sgi.com/mailman/listinfo/xfs
> >
> 
> Ben,
> 
> When you have a chance can you pull this series in.  I have tested
> it and it looks good.

No, not yet. It doesn't address the comments I made in a previous
iteration.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH v3 1/3] xfs: Add ratelimited printk for different alert levels

[Dave Chinner]

On Wed, Sep 26, 2012 at 12:26:47PM +0530, raghu.prabhu13@gmail.com wrote:
> From: Raghavendra D Prabhu <rprabhu@wnohang.net>
> 
> Ratelimited printk will be useful in printing xfs messages which are otherwise
> not required to be printed always due to their high rate (to prevent kernel ring
> buffer from overflowing), while at the same time required to be printed.
> 
> Signed-off-by: Raghavendra D Prabhu <rprabhu@wnohang.net>

This is fine.

Reviewed-by: Dave Chinner <dchinner@redhat.com>

-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH v3 2/3] xfs: Print error when xfs_ialloc_ag_select fails to find continuous free space.

[Dave Chinner]

On Wed, Sep 26, 2012 at 12:26:48PM +0530, raghu.prabhu13@gmail.com wrote:
> From: Raghavendra D Prabhu <rprabhu@wnohang.net>
> 
> When xfs_ialloc_ag_select fails to find any AG with continuous free blocks
> required for inode allocation, printk the error in ratelimited manner.
> 
> Signed-off-by: Raghavendra D Prabhu <rprabhu@wnohang.net>
> ---
>  fs/xfs/xfs_ialloc.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_ialloc.c b/fs/xfs/xfs_ialloc.c
> index 5aceb3f..e75a39d 100644
> --- a/fs/xfs/xfs_ialloc.c
> +++ b/fs/xfs/xfs_ialloc.c
> @@ -539,8 +539,11 @@ nextag:
>  		if (agno >= agcount)
>  			agno = 0;
>  		if (agno == pagno) {
> -			if (flags == 0)
> +			if (flags == 0) {
> +				xfs_err_ratelimited(mp,
> +					"Out of continuous free blocks for inode allocation");
>  				return NULLAGNUMBER;
> +			}

>From here:

http://oss.sgi.com/archives/xfs/2012-09/msg00138.html

I quote:

| http://oss.sgi.com/archives/xfs/2012-06/msg00041.html
| 
| <quote>
| Couple of things for all 3 patches. Firstly - 80 columns. We tend
| to keep the pformat string on a single line so it is easy to grep
| for like so:
| 
|				pr_err_once(mp,
|                 "Insufficient contiguous free space for inode allocation");
| </quote>
| 
| So, you need to change the error message to the one suggested, and
| follow 80-character width limits like the rest of the code.
| 
| Also, I think the error message is better at the caller site, not in
| the function itself. i.e. if we get a NULLAGNUMBER returned, the
| caller decided whether to emit an error message or not."

So, the message here needs to change to what is suggested above, and
the location of the message needs to change i.e. to the caller, not
within the function itself.

Review comments need to be addressed before you repost patches....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH v3 3/3] xfs: Print error when unable to allocate inodes or out of free inodes.

[Dave Chinner]

On Wed, Sep 26, 2012 at 12:26:49PM +0530, raghu.prabhu13@gmail.com wrote:
> From: Raghavendra D Prabhu <rprabhu@wnohang.net>
> 
> When xfs_dialloc is unable to allocate required number of inodes due to global
> ceiling, or AGs are out of free inodes but not allowed to allocate inodes or
> unable to allocate without continguous free space, printk the error in
> ratelimited manner.
> 
> Signed-off-by: Raghavendra D Prabhu <rprabhu@wnohang.net>
> ---
>  fs/xfs/xfs_ialloc.c | 26 ++++++++++++++++++++++++--
>  1 file changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/xfs/xfs_ialloc.c b/fs/xfs/xfs_ialloc.c
> index e75a39d..e9f911b2 100644
> --- a/fs/xfs/xfs_ialloc.c
> +++ b/fs/xfs/xfs_ialloc.c
> @@ -991,7 +991,7 @@ xfs_dialloc(
>  
>  			xfs_perag_put(pag);
>  			*inop = NULLFSINO;
> -			return 0;
> +			goto out_spc;
>  		}

So this is the case where noroom = 0, okalloc = 1, error = ENOSPC.

This means we selected an AG that we could allocate new inodes in,
but xfs_ialloc_ag_alloc() failed because we are now over the
mp->m_maxicount. That's the only way it can return ENOSPC. More
below....

>  
>  		if (ialloced) {
> @@ -1017,7 +1017,7 @@ nextag:
>  			agno = 0;
>  		if (agno == start_agno) {
>  			*inop = NULLFSINO;
> -			return noroom ? ENOSPC : 0;
> +			goto out_spc;
>  		}
>  	}
>  
> @@ -1027,6 +1027,28 @@ out_alloc:
>  out_error:
>  	xfs_perag_put(pag);
>  	return XFS_ERROR(error);
> +out_spc:

Irrelevant given my next comments, but shouldn't that have been
called "out_nospace"?

> +	if (noroom) {
> +		xfs_err_ratelimited(mp, "Hit global inode ceiling:");
> +		error = ENOSPC;
> +	} else if (!okalloc) {
> +		/*
> +		 * implies noroom=0 && (!pag->pagi_freecount && !okalloc) for
> +		 * all pag
> +		 */
> +		xfs_err_ratelimited(mp,
> +			"No AGs with free inodes and allocation not allowed:");
> +		error = 0;
> +	} else {
> +		xfs_err_ratelimited(mp,
> +			"Unable to allocate continguous space for inodes:");
> +		error = 0;
> +	}

So for the first case we always get "Unable to allocate
continguous space for inodes:". That's wrong (see above) because
we've got the same error as if we were in the noroom case -
allocation is not allowed as we are above the max inode count. i.e:

"Unable to locate free inodes. Allocation failure reason:"
"Over global inode limit"

I think it is much better to issue the error message immediately and
returning instead of jumping to this code. That way we'll know
*exactly* what error occurred and it is simple to verify the message
is, in fact, correct.

In the second case, for both noroom states the second message (the
!okalloc message) is appropriate. We can be above the global inode
ceiling and still have free inodes available for allocation, and we
only fail if we are unable to locate free inodes for allocation.
i.e:

"Unable to locate free inodes. Allocation failure reason:"
if (noroom) {
	"Over global inode limit"
	error = ENOSPC;
else if (!okalloc)
	"Not allowed by caller"
else
	"Insufficient contiguous free space is available"

This is where separating the error messages from the location that
they are generated at is a bad idea - it's taken me 20 minutes of
code reading to get to this point, and for a patch that adds a
couple of error messages that's just, well, wrong.

If you are going to separate them write a small wrapper function
that has the above code snippet in it and takes a couple of boolean
flags to select the allocation failure reason....

> +
> +	xfs_err_ratelimited(mp, "Required %d, Current %llu, Maximum %llu",
> +		XFS_IALLOC_INODES(mp), mp->m_sb.sb_icount, mp->m_maxicount);

That's not completely correct, as we only require a single inode to
be allocated during this call. We only try to allocate an inode
chunk when there are no existing free inodes, but we still only
require 1 inode to be allocated to the caller. Hence I don't think
this is necessary information - we can get if from the superblock if
we really need it....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH v3 0/3] Print when ENOSPC due to lack of inodes.

[Ben Myers]

Hi Dave,

On Mon, Oct 29, 2012 at 09:05:16AM +1100, Dave Chinner wrote:
> On Fri, Oct 26, 2012 at 08:18:59AM -0500, Rich Johnston wrote:
> > On 10/24/2012 03:56 PM, Raghavendra Prabhu wrote:
> > >Hi,
> > >
> > >Checking back on this, whether it has been reviewed and if so, any comments.
> > >
> > >On Wed, Sep 26, 2012 at 12:26 PM,  <raghu.prabhu13@gmail.com> wrote:
> > >>From: Raghavendra D Prabhu <rprabhu@wnohang.net>
> > >>
> > >>Currently, when there are no free inodes left / free space to allocate them (usually
> > >>without inode64), there is no indication anywhere of this case, making it harder
> > >>to diagnose this case.
> > >>
> > >>Hence, this series prints the causes/reasons to kernel log in a ratelimited
> > >>manner, when such a situation arises.
> > >>
> > >>Regarding why it is printed at callee location instead at caller, it gives
> > >>greater granularity in expressing the precise reason and give more details, and
> > >>also some along the path are not ENOSPC (such in xfs_ialloc where ialloc_context is not NULL but
> > >>ino is) along with the fact that it (xfs_ialloc) is called at multiple sites, so to avoid duplication.
> > >>
> > >>Version 1: Initial series.
> > >>Version 2: Added ratelimited printing to xfs_message and used that.
> > >>Version 3: Kept the logic intact in few places, fixed the column requirement.
> > >>
> > >>Raghavendra D Prabhu (3):
> > >>   xfs: Add ratelimited printk for different alert levels
> > >>   xfs: Print error when xfs_ialloc_ag_select fails to find continuous
> > >>     free space.
> > >>   xfs: Print error when unable to allocate inodes or out of free
> > >>     inodes.
> > >>
> > >>  fs/xfs/xfs_ialloc.c  | 31 ++++++++++++++++++++++++++++---
> > >>  fs/xfs/xfs_linux.h   |  1 +
> > >>  fs/xfs/xfs_message.h | 26 ++++++++++++++++++++++++++
> > >>  3 files changed, 55 insertions(+), 3 deletions(-)
> > >>
> > >>--
> > >>1.7.12.1
> > >>
> > >
> > >_______________________________________________
> > >xfs mailing list
> > >xfs@oss.sgi.com
> > >http://oss.sgi.com/mailman/listinfo/xfs
> > >
> > 
> > Ben,
> > 
> > When you have a chance can you pull this series in.  I have tested
> > it and it looks good.
> 
> No, not yet. It doesn't address the comments I made in a previous
> iteration.

Ah, ok.  We'll wait for Ragavendra to address your comments.

Regards,
Ben

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH v3 0/3] Print when ENOSPC due to lack of inodes.

[Eric Sandeen]

On 10/24/12 3:56 PM, Raghavendra Prabhu wrote:
> Hi,
> 
> Checking back on this, whether it has been reviewed and if so, any comments.

Please see Dave's comments down-thread, there are apparently still
some unaddressed concerns.  If you'd like to do a V4 I think this
would be nice to get in.

-Eric

> On Wed, Sep 26, 2012 at 12:26 PM,  <raghu.prabhu13@gmail.com> wrote:
>> From: Raghavendra D Prabhu <rprabhu@wnohang.net>
>>
>> Currently, when there are no free inodes left / free space to allocate them (usually
>> without inode64), there is no indication anywhere of this case, making it harder
>> to diagnose this case.
>>
>> Hence, this series prints the causes/reasons to kernel log in a ratelimited
>> manner, when such a situation arises.
>>
>> Regarding why it is printed at callee location instead at caller, it gives
>> greater granularity in expressing the precise reason and give more details, and
>> also some along the path are not ENOSPC (such in xfs_ialloc where ialloc_context is not NULL but
>> ino is) along with the fact that it (xfs_ialloc) is called at multiple sites, so to avoid duplication.
>>
>> Version 1: Initial series.
>> Version 2: Added ratelimited printing to xfs_message and used that.
>> Version 3: Kept the logic intact in few places, fixed the column requirement.
>>
>> Raghavendra D Prabhu (3):
>>   xfs: Add ratelimited printk for different alert levels
>>   xfs: Print error when xfs_ialloc_ag_select fails to find continuous
>>     free space.
>>   xfs: Print error when unable to allocate inodes or out of free
>>     inodes.
>>
>>  fs/xfs/xfs_ialloc.c  | 31 ++++++++++++++++++++++++++++---
>>  fs/xfs/xfs_linux.h   |  1 +
>>  fs/xfs/xfs_message.h | 26 ++++++++++++++++++++++++++
>>  3 files changed, 55 insertions(+), 3 deletions(-)
>>
>> --
>> 1.7.12.1
>>
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
> 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[v3 Repost] xfs: Add ratelimited printk for different alert levels

[Rich Johnston]

Ratelimited printk will be useful in printing xfs messages which are otherwise
not required to be printed always due to their high rate (to prevent kernel ring
buffer from overflowing), while at the same time required to be printed.

Signed-off-by: Raghavendra D Prabhu <rprabhu@wnohang.net>
Reviewed-by: Rich Johnston <rjohnston@sgi.com>
Reviewed-by: Dave Chinner <dchinner@redhat.com>

---

Sorry this has not been committed and it's been a few months so I am
reposting this patch.  If I don't hear any objections by 10 am CST on
29 March 2013 I will commit this patch.
 

fs/xfs/xfs_linux.h   |  1 +
 fs/xfs/xfs_message.h | 26 ++++++++++++++++++++++++++
 2 files changed, 27 insertions(+)

Index: linux/fs/xfs/xfs_linux.h
===================================================================
--- linux.orig/fs/xfs/xfs_linux.h	2013-03-22 09:19:54.000000000 -0500
+++ linux/fs/xfs/xfs_linux.h	2013-03-27 12:55:33.000000000 -0500
@@ -72,6 +72,7 @@
 #include <linux/kthread.h>
 #include <linux/freezer.h>
 #include <linux/list_sort.h>
+#include <linux/ratelimit.h>
 
 #include <asm/page.h>
 #include <asm/div64.h>
Index: linux/fs/xfs/xfs_message.h
===================================================================
--- linux.orig/fs/xfs/xfs_message.h	2013-03-22 09:19:54.000000000 -0500
+++ linux/fs/xfs/xfs_message.h	2013-03-27 12:55:33.000000000 -0500
@@ -30,6 +30,32 @@
 }
 #endif
 
+#define xfs_printk_ratelimited(func, dev, fmt, ...)		\
+do {									\
+	static DEFINE_RATELIMIT_STATE(_rs,				\
+				      DEFAULT_RATELIMIT_INTERVAL,	\
+				      DEFAULT_RATELIMIT_BURST);		\
+	if (__ratelimit(&_rs))						\
+		func(dev, fmt, ##__VA_ARGS__);			\
+} while (0)
+
+#define xfs_emerg_ratelimited(dev, fmt, ...)				\
+	xfs_printk_ratelimited(xfs_emerg, dev, fmt, ##__VA_ARGS__)
+#define xfs_alert_ratelimited(dev, fmt, ...)				\
+	xfs_printk_ratelimited(xfs_alert, dev, fmt, ##__VA_ARGS__)
+#define xfs_crit_ratelimited(dev, fmt, ...)				\
+	xfs_printk_ratelimited(xfs_crit, dev, fmt, ##__VA_ARGS__)
+#define xfs_err_ratelimited(dev, fmt, ...)				\
+	xfs_printk_ratelimited(xfs_err, dev, fmt, ##__VA_ARGS__)
+#define xfs_warn_ratelimited(dev, fmt, ...)				\
+	xfs_printk_ratelimited(xfs_warn, dev, fmt, ##__VA_ARGS__)
+#define xfs_notice_ratelimited(dev, fmt, ...)				\
+	xfs_printk_ratelimited(xfs_notice, dev, fmt, ##__VA_ARGS__)
+#define xfs_info_ratelimited(dev, fmt, ...)				\
+	xfs_printk_ratelimited(xfs_info, dev, fmt, ##__VA_ARGS__)
+#define xfs_debug_ratelimited(dev, fmt, ...)				\
+	xfs_printk_ratelimited(xfs_debug, dev, fmt, ##__VA_ARGS__)
+
 extern void assfail(char *expr, char *f, int l);
 
 extern void xfs_hex_dump(void *p, int length);

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [v3 Repost] xfs: Add ratelimited printk for different alert levels

[Ben Myers]

On Wed, Mar 27, 2013 at 09:26:49AM -0500, Rich Johnston wrote:
> Ratelimited printk will be useful in printing xfs messages which are otherwise
> not required to be printed always due to their high rate (to prevent kernel ring
> buffer from overflowing), while at the same time required to be printed.
> 
> Signed-off-by: Raghavendra D Prabhu <rprabhu@wnohang.net>
> Reviewed-by: Rich Johnston <rjohnston@sgi.com>
> Reviewed-by: Dave Chinner <dchinner@redhat.com>

Applied.

Thanks Raghavendra!

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs