* [PATCH V2 0/3] Print when ENOSPC due to lack of inodes in non-inode64 mount.
@ 2012-09-11 22:13 raghu.prabhu13
2012-09-11 22:13 ` [PATCH 1/3] Add ratelimited printk for different alert levels raghu.prabhu13
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: raghu.prabhu13 @ 2012-09-11 22:13 UTC (permalink / raw)
To: xfs; +Cc: Raghavendra D Prabhu
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.
-----
Version 1: Initially sent with pr_err_once to print the error only once.
Version 2: Added ratelimited functions to xfs_message.h instead of printing once
with pr_err_once on suggestion of Dave Chinner.
Raghavendra D Prabhu (3):
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 | 24 +++++++++++++++++++-----
fs/xfs/xfs_message.h | 28 ++++++++++++++++++++++++++++
2 files changed, 47 insertions(+), 5 deletions(-)
--
1.7.12
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 1/3] Add ratelimited printk for different alert levels 2012-09-11 22:13 [PATCH V2 0/3] Print when ENOSPC due to lack of inodes in non-inode64 mount raghu.prabhu13 @ 2012-09-11 22:13 ` raghu.prabhu13 2012-09-11 22:43 ` Dave Chinner 2012-09-12 3:22 ` Joe Perches 2012-09-11 22:13 ` [PATCH 2/3] XFS: Print error when xfs_ialloc_ag_select fails to find continuous free space raghu.prabhu13 2012-09-11 22:13 ` [PATCH 3/3] XFS: Print error when unable to allocate inodes or out of free inodes raghu.prabhu13 2 siblings, 2 replies; 11+ messages in thread From: raghu.prabhu13 @ 2012-09-11 22:13 UTC (permalink / raw) To: xfs; +Cc: Raghavendra D Prabhu, Alex Elder, Ben Myers, open list 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_message.h | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/fs/xfs/xfs_message.h b/fs/xfs/xfs_message.h index 56dc0c1..87999a5 100644 --- a/fs/xfs/xfs_message.h +++ b/fs/xfs/xfs_message.h @@ -1,6 +1,8 @@ #ifndef __XFS_MESSAGE_H #define __XFS_MESSAGE_H 1 +#include <linux/ratelimit.h> + struct xfs_mount; extern __printf(2, 3) @@ -30,6 +32,32 @@ void xfs_debug(const struct xfs_mount *mp, const char *fmt, ...) } #endif +#define xfs_printk_ratelimited(xfs_printk, dev, fmt, ...) \ +do { \ + static DEFINE_RATELIMIT_STATE(_rs, \ + DEFAULT_RATELIMIT_INTERVAL, \ + DEFAULT_RATELIMIT_BURST); \ + if (__ratelimit(&_rs)) \ + xfs_printk(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_dbg_ratelimited(dev, fmt, ...) \ + xfs_printk_ratelimited(xfs_dbg, 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 _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] Add ratelimited printk for different alert levels 2012-09-11 22:13 ` [PATCH 1/3] Add ratelimited printk for different alert levels raghu.prabhu13 @ 2012-09-11 22:43 ` Dave Chinner 2012-09-12 3:22 ` Joe Perches 1 sibling, 0 replies; 11+ messages in thread From: Dave Chinner @ 2012-09-11 22:43 UTC (permalink / raw) To: raghu.prabhu13 Cc: open list, Raghavendra D Prabhu, Alex Elder, Ben Myers, xfs On Wed, Sep 12, 2012 at 03:43:22AM +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> > --- > fs/xfs/xfs_message.h | 28 ++++++++++++++++++++++++++++ > 1 file changed, 28 insertions(+) > > diff --git a/fs/xfs/xfs_message.h b/fs/xfs/xfs_message.h > index 56dc0c1..87999a5 100644 > --- a/fs/xfs/xfs_message.h > +++ b/fs/xfs/xfs_message.h > @@ -1,6 +1,8 @@ > #ifndef __XFS_MESSAGE_H > #define __XFS_MESSAGE_H 1 > > +#include <linux/ratelimit.h> > + Include this in xfs_linux.h rather than here. > struct xfs_mount; > > extern __printf(2, 3) > @@ -30,6 +32,32 @@ void xfs_debug(const struct xfs_mount *mp, const char *fmt, ...) > } > #endif > > +#define xfs_printk_ratelimited(xfs_printk, dev, fmt, ...) \ > +do { \ > + static DEFINE_RATELIMIT_STATE(_rs, \ > + DEFAULT_RATELIMIT_INTERVAL, \ > + DEFAULT_RATELIMIT_BURST); \ > + if (__ratelimit(&_rs)) \ > + xfs_printk(dev, fmt, ##__VA_ARGS__); \ > +} while (0) Use "func" not xfs_printk here. xfs_printk looks too much like a real function name (indeed, we already have __xfs_printk) rather than a macro parameter. > +#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_dbg_ratelimited(dev, fmt, ...) \ > + xfs_printk_ratelimited(xfs_dbg, dev, fmt, ##__VA_ARGS__) Here's the problem with adding macros that aren't used. xfs_dbg does not exist - the function is xfs_debug(). The compiler won't catch that until the macro is used, so only add the macros which are needed for this patch series. Cheers, Dave. -- Dave Chinner david@fromorbit.com _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] Add ratelimited printk for different alert levels 2012-09-11 22:13 ` [PATCH 1/3] Add ratelimited printk for different alert levels raghu.prabhu13 2012-09-11 22:43 ` Dave Chinner @ 2012-09-12 3:22 ` Joe Perches 2012-09-13 0:51 ` Dave Chinner 1 sibling, 1 reply; 11+ messages in thread From: Joe Perches @ 2012-09-12 3:22 UTC (permalink / raw) To: raghu.prabhu13 Cc: Alex Elder, Ben Myers, open list, xfs, Raghavendra D Prabhu On Wed, 2012-09-12 at 03:43 +0530, raghu.prabhu13@gmail.com 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. [] > diff --git a/fs/xfs/xfs_message.h b/fs/xfs/xfs_message.h [] > @@ -30,6 +32,32 @@ void xfs_debug(const struct xfs_mount *mp, const char *fmt, ...) > } > #endif > > +#define xfs_printk_ratelimited(xfs_printk, dev, fmt, ...) \ > +do { \ > + static DEFINE_RATELIMIT_STATE(_rs, \ > + DEFAULT_RATELIMIT_INTERVAL, \ > + DEFAULT_RATELIMIT_BURST); \ > + if (__ratelimit(&_rs)) \ > + xfs_printk(dev, fmt, ##__VA_ARGS__); \ > +} while (0) It might be better to use an xfs singleton RATELIMIT_STATE DEFINE_RATELIMIT_STATE(xfs_rs); ... #define xfs_printk_ratelimited(xfs_printk, dev, fmt, ...) \ do { \ if (__ratelimit(&xfs_rs)) \ xfs_printk(dev, fmt, ##__VA_ARGS__); \ } while (0) _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] Add ratelimited printk for different alert levels 2012-09-12 3:22 ` Joe Perches @ 2012-09-13 0:51 ` Dave Chinner 0 siblings, 0 replies; 11+ messages in thread From: Dave Chinner @ 2012-09-13 0:51 UTC (permalink / raw) To: Joe Perches Cc: Alex Elder, Ben Myers, open list, xfs, Raghavendra D Prabhu, raghu.prabhu13 On Tue, Sep 11, 2012 at 08:22:39PM -0700, Joe Perches wrote: > On Wed, 2012-09-12 at 03:43 +0530, raghu.prabhu13@gmail.com 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. > [] > > diff --git a/fs/xfs/xfs_message.h b/fs/xfs/xfs_message.h > [] > > @@ -30,6 +32,32 @@ void xfs_debug(const struct xfs_mount *mp, const char *fmt, ...) > > } > > #endif > > > > +#define xfs_printk_ratelimited(xfs_printk, dev, fmt, ...) \ > > +do { \ > > + static DEFINE_RATELIMIT_STATE(_rs, \ > > + DEFAULT_RATELIMIT_INTERVAL, \ > > + DEFAULT_RATELIMIT_BURST); \ > > + if (__ratelimit(&_rs)) \ > > + xfs_printk(dev, fmt, ##__VA_ARGS__); \ > > +} while (0) > > It might be better to use an xfs singleton RATELIMIT_STATE > > DEFINE_RATELIMIT_STATE(xfs_rs); > ... > #define xfs_printk_ratelimited(xfs_printk, dev, fmt, ...) \ > do { \ > if (__ratelimit(&xfs_rs)) \ > xfs_printk(dev, fmt, ##__VA_ARGS__); \ > } while (0) Which would then result in ratelimiting dropping potentially important, unique messages. I think it's much better to guarantee ratelimited messages get emitted at least once, especially as there is the potential for multiple filesystems to emit messages simultaneously. I think per-location rate limiting is fine for the current usage - ratelimiting is not widespread so there isn't a massive increase in size as a result of this. If we do start to use ratelimiting in lots of places in XFS, then we might have to revisit this, but it's OK for now. Cheers, Dave. -- Dave Chinner david@fromorbit.com _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/3] XFS: Print error when xfs_ialloc_ag_select fails to find continuous free space. 2012-09-11 22:13 [PATCH V2 0/3] Print when ENOSPC due to lack of inodes in non-inode64 mount raghu.prabhu13 2012-09-11 22:13 ` [PATCH 1/3] Add ratelimited printk for different alert levels raghu.prabhu13 @ 2012-09-11 22:13 ` raghu.prabhu13 2012-09-11 22:48 ` Dave Chinner 2012-09-11 22:13 ` [PATCH 3/3] XFS: Print error when unable to allocate inodes or out of free inodes raghu.prabhu13 2 siblings, 1 reply; 11+ messages in thread From: raghu.prabhu13 @ 2012-09-11 22:13 UTC (permalink / raw) To: xfs; +Cc: Raghavendra D Prabhu, Alex Elder, Ben Myers, open list 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 _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] XFS: Print error when xfs_ialloc_ag_select fails to find continuous free space. 2012-09-11 22:13 ` [PATCH 2/3] XFS: Print error when xfs_ialloc_ag_select fails to find continuous free space raghu.prabhu13 @ 2012-09-11 22:48 ` Dave Chinner 0 siblings, 0 replies; 11+ messages in thread From: Dave Chinner @ 2012-09-11 22:48 UTC (permalink / raw) To: raghu.prabhu13 Cc: open list, Raghavendra D Prabhu, Alex Elder, Ben Myers, xfs On Wed, Sep 12, 2012 at 03:43:23AM +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"); 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. Cheers, Dave. -- Dave Chinner david@fromorbit.com _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/3] XFS: Print error when unable to allocate inodes or out of free inodes. 2012-09-11 22:13 [PATCH V2 0/3] Print when ENOSPC due to lack of inodes in non-inode64 mount raghu.prabhu13 2012-09-11 22:13 ` [PATCH 1/3] Add ratelimited printk for different alert levels raghu.prabhu13 2012-09-11 22:13 ` [PATCH 2/3] XFS: Print error when xfs_ialloc_ag_select fails to find continuous free space raghu.prabhu13 @ 2012-09-11 22:13 ` raghu.prabhu13 2012-09-11 23:21 ` Dave Chinner 2 siblings, 1 reply; 11+ messages in thread From: raghu.prabhu13 @ 2012-09-11 22:13 UTC (permalink / raw) To: xfs; +Cc: Raghavendra D Prabhu, Alex Elder, Ben Myers, open list From: Raghavendra D Prabhu <rprabhu@wnohang.net> When xfs_dialloc is unable to allocate required number of inodes or there are no AGs with free inodes, printk the error in ratelimited manner. Signed-off-by: Raghavendra D Prabhu <rprabhu@wnohang.net> --- fs/xfs/xfs_ialloc.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/fs/xfs/xfs_ialloc.c b/fs/xfs/xfs_ialloc.c index e75a39d..034131b 100644 --- a/fs/xfs/xfs_ialloc.c +++ b/fs/xfs/xfs_ialloc.c @@ -990,8 +990,11 @@ xfs_dialloc( goto out_error; xfs_perag_put(pag); - *inop = NULLFSINO; - return 0; + + xfs_err_ratelimited(mp, + "Unable to allocate inodes in AG %d: Required %d, Current %llu, Maximum %llu", + agno, XFS_IALLOC_INODES(mp), mp->m_sb.sb_icount, mp->m_maxicount); + goto out_spc; } if (ialloced) { @@ -1016,11 +1019,19 @@ nextag: if (++agno == mp->m_sb.sb_agcount) agno = 0; if (agno == start_agno) { - *inop = NULLFSINO; - return noroom ? ENOSPC : 0; + if (noroom) { + xfs_err_ratelimited(mp, + "Out of AGs with free inodes: Required %d, Current %llu, Maximum %llu", + XFS_IALLOC_INODES(mp), mp->m_sb.sb_icount, mp->m_maxicount); + goto out_spc; + } + return 0; } } +out_spc: + *inop = NULLFSINO; + return ENOSPC; out_alloc: *IO_agbp = NULL; return xfs_dialloc_ag(tp, agbp, parent, inop); -- 1.7.12 _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] XFS: Print error when unable to allocate inodes or out of free inodes. 2012-09-11 22:13 ` [PATCH 3/3] XFS: Print error when unable to allocate inodes or out of free inodes raghu.prabhu13 @ 2012-09-11 23:21 ` Dave Chinner 2012-09-21 7:16 ` Raghavendra D Prabhu 0 siblings, 1 reply; 11+ messages in thread From: Dave Chinner @ 2012-09-11 23:21 UTC (permalink / raw) To: raghu.prabhu13 Cc: open list, Raghavendra D Prabhu, Alex Elder, Ben Myers, xfs On Wed, Sep 12, 2012 at 03:43:24AM +0530, raghu.prabhu13@gmail.com wrote: > From: Raghavendra D Prabhu <rprabhu@wnohang.net> > > When xfs_dialloc is unable to allocate required number of inodes or there are no > AGs with free inodes, printk the error in ratelimited manner. > > Signed-off-by: Raghavendra D Prabhu <rprabhu@wnohang.net> > --- > fs/xfs/xfs_ialloc.c | 19 +++++++++++++++---- > 1 file changed, 15 insertions(+), 4 deletions(-) > > diff --git a/fs/xfs/xfs_ialloc.c b/fs/xfs/xfs_ialloc.c > index e75a39d..034131b 100644 > --- a/fs/xfs/xfs_ialloc.c > +++ b/fs/xfs/xfs_ialloc.c > @@ -990,8 +990,11 @@ xfs_dialloc( > goto out_error; > > xfs_perag_put(pag); > - *inop = NULLFSINO; > - return 0; > + > + xfs_err_ratelimited(mp, > + "Unable to allocate inodes in AG %d: Required %d, Current %llu, Maximum %llu", > + agno, XFS_IALLOC_INODES(mp), mp->m_sb.sb_icount, mp->m_maxicount); > + goto out_spc; This changes the error to be returned from 0 to ENOSPC. Adding error messages shouldn't change the logic of the code. Also, you might want tolook at how ENOSPC is returned from xfs_ialloc_ag_alloc(). it only occurs when: if (mp->m_maxicount && mp->m_sb.sb_icount + newlen > mp->m_maxicount) { i.e. it is exactly the same error case as the "noroom" error below. It has nothing to do with being unable to allocate inodes in the specific AG - the global inode count is too high. IOWs, the error message is not correct. Also, 80 columns. > } > > if (ialloced) { > @@ -1016,11 +1019,19 @@ nextag: > if (++agno == mp->m_sb.sb_agcount) > agno = 0; > if (agno == start_agno) { > - *inop = NULLFSINO; > - return noroom ? ENOSPC : 0; > + if (noroom) { > + xfs_err_ratelimited(mp, > + "Out of AGs with free inodes: Required %d, Current %llu, Maximum %llu", > + XFS_IALLOC_INODES(mp), mp->m_sb.sb_icount, mp->m_maxicount); The error message here is misleading - the error is that we've exceeded the maximum inode count for the filesystem (same as the above error message case), so no further allocations are allowed. What about the !noroom case? Isn't that a real ENOSPC condition? i.e. we've tried to allocate inodes in every AG and yet we've failed in all of them because there is no aligned, contiguous free space in any of the AGs. Shouldn't that emit an appropriate warning? > + goto out_spc; > + } > + return 0; > } > } > > +out_spc: > + *inop = NULLFSINO; > + return ENOSPC; > out_alloc: > *IO_agbp = NULL; > return xfs_dialloc_ag(tp, agbp, parent, inop); Default behaviour on a loop break is to allocate inodes, not return ENOSPC. BTW, there's no need to cc LKML for XFS specific patches. LKML is noisy enough as it is without unnecessary cross-posts.... Cheers, Dave. -- Dave Chinner david@fromorbit.com _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Re: [PATCH 3/3] XFS: Print error when unable to allocate inodes or out of free inodes. 2012-09-11 23:21 ` Dave Chinner @ 2012-09-21 7:16 ` Raghavendra D Prabhu 2012-09-26 6:14 ` Raghavendra Prabhu 0 siblings, 1 reply; 11+ messages in thread From: Raghavendra D Prabhu @ 2012-09-21 7:16 UTC (permalink / raw) To: Dave Chinner; +Cc: Ben Myers, Alex Elder, xfs [-- Attachment #1.1: Type: text/plain, Size: 4477 bytes --] Hi, * On Wed, Sep 12, 2012 at 09:21:44AM +1000, Dave Chinner <david@fromorbit.com> wrote: >On Wed, Sep 12, 2012 at 03:43:24AM +0530, raghu.prabhu13@gmail.com wrote: >> From: Raghavendra D Prabhu <rprabhu@wnohang.net> >> >> When xfs_dialloc is unable to allocate required number of inodes or there are no >> AGs with free inodes, printk the error in ratelimited manner. >> >> Signed-off-by: Raghavendra D Prabhu <rprabhu@wnohang.net> >> --- >> fs/xfs/xfs_ialloc.c | 19 +++++++++++++++---- >> 1 file changed, 15 insertions(+), 4 deletions(-) >> >> diff --git a/fs/xfs/xfs_ialloc.c b/fs/xfs/xfs_ialloc.c >> index e75a39d..034131b 100644 >> --- a/fs/xfs/xfs_ialloc.c >> +++ b/fs/xfs/xfs_ialloc.c >> @@ -990,8 +990,11 @@ xfs_dialloc( >> goto out_error; >> >> xfs_perag_put(pag); >> - *inop = NULLFSINO; >> - return 0; >> + >> + xfs_err_ratelimited(mp, >> + "Unable to allocate inodes in AG %d: Required %d, Current %llu, Maximum %llu", >> + agno, XFS_IALLOC_INODES(mp), mp->m_sb.sb_icount, mp->m_maxicount); >> + goto out_spc; > >This changes the error to be returned from 0 to ENOSPC. Adding error >messages shouldn't change the logic of the code. 1) Yes, there is a confusion regarding that. > >Also, you might want tolook at how ENOSPC is returned from >xfs_ialloc_ag_alloc(). it only occurs when: > > if (mp->m_maxicount && > mp->m_sb.sb_icount + newlen > mp->m_maxicount) { > >i.e. it is exactly the same error case as the "noroom" error below. >It has nothing to do with being unable to allocate inodes in the >specific AG - the global inode count is too high. IOWs, the error >message is not correct. Now, in xfs_dialloc if (mp->m_maxicount && mp->m_sb.sb_icount + XFS_IALLOC_INODES(mp) > mp->m_maxicount) { noroom = 1; okalloc = 0; } why does it not make sense to bail out with ENOSPC then itself? I mean, what is the point of the loop when there is no room (noroom=1) and no allocations are allowed (okalloc = 0), also since the xfs_ialloc_ag_alloc in the loop uses same global logic to return. > >Also, 80 columns. > >> } >> >> if (ialloced) { >> @@ -1016,11 +1019,19 @@ nextag: >> if (++agno == mp->m_sb.sb_agcount) >> agno = 0; >> if (agno == start_agno) { >> - *inop = NULLFSINO; >> - return noroom ? ENOSPC : 0; >> + if (noroom) { >> + xfs_err_ratelimited(mp, >> + "Out of AGs with free inodes: Required %d, Current %llu, Maximum %llu", >> + XFS_IALLOC_INODES(mp), mp->m_sb.sb_icount, mp->m_maxicount); > >The error message here is misleading - the error is that we've >exceeded the maximum inode count for the filesystem (same as the >above error message case), so no further allocations are allowed. > >What about the !noroom case? Isn't that a real ENOSPC condition? >i.e. we've tried to allocate inodes in every AG and yet we've failed >in all of them because there is no aligned, contiguous free space in >any of the AGs. Shouldn't that emit an appropriate warning? The warning is already emitted in call to xfs_ialloc_ag_select. Now, what does inop = NULLFSINO, noroom = 0 and return value of 0 mean, from the call chain of xfs_dir_ialloc -> xfs_ialloc -> xfs_dialloc I see that, it is a true ENOSPC only if both the buffer pointed by ialloc_context and the inode are NULL or there is an error returned, in former case (noroom=0) xfs_dir_ialloc retries the allocation (ie. when AGI buffer is non-NULL). Now, in case of global inode exhaustion, it is hard error which can be fixed only by remounting with inode64 and nothing else will do, hence, I think ENOSPC must be returned as error instead of 0. (also in case of point #1 above) So, are the assumptions made above correct? > >> + goto out_spc; >> + } >> + return 0; >> } >> } >> >> +out_spc: >> + *inop = NULLFSINO; >> + return ENOSPC; >> out_alloc: >> *IO_agbp = NULL; >> return xfs_dialloc_ag(tp, agbp, parent, inop); > >Default behaviour on a loop break is to allocate inodes, not return >ENOSPC. > >BTW, there's no need to cc LKML for XFS specific patches. LKML is >noisy enough as it is without unnecessary cross-posts.... > >Cheers, > >Dave. >-- >Dave Chinner >david@fromorbit.com > Regards, -- Raghavendra Prabhu GPG Id : 0xD72BE977 Fingerprint: B93F EBCB 8E05 7039 CD3C A4B8 A616 DCA1 D72B E977 www: wnohang.net [-- Attachment #1.2: Type: application/pgp-signature, Size: 490 bytes --] [-- Attachment #2: Type: text/plain, Size: 121 bytes --] _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Re: [PATCH 3/3] XFS: Print error when unable to allocate inodes or out of free inodes. 2012-09-21 7:16 ` Raghavendra D Prabhu @ 2012-09-26 6:14 ` Raghavendra Prabhu 0 siblings, 0 replies; 11+ messages in thread From: Raghavendra Prabhu @ 2012-09-26 6:14 UTC (permalink / raw) To: Dave Chinner; +Cc: Ben Myers, Alex Elder, xfs Hi, On Fri, Sep 21, 2012 at 12:46 PM, Raghavendra D Prabhu <raghu.prabhu13@gmail.com> wrote: > Hi, > > >> >>> + goto out_spc; >>> + } >>> + return 0; >>> } >>> } >>> >>> +out_spc: >>> + *inop = NULLFSINO; >>> + return ENOSPC; >>> out_alloc: >>> *IO_agbp = NULL; >>> return xfs_dialloc_ag(tp, agbp, parent, inop); >> >> >> Default behaviour on a loop break is to allocate inodes, not return >> ENOSPC. >> >> BTW, there's no need to cc LKML for XFS specific patches. LKML is >> noisy enough as it is without unnecessary cross-posts.... >> >> Cheers, >> >> Dave. >> -- >> Dave Chinner >> david@fromorbit.com >> > Ignore the previous. Resending the patch with fixes mentioned. _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2012-09-26 6:12 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-09-11 22:13 [PATCH V2 0/3] Print when ENOSPC due to lack of inodes in non-inode64 mount raghu.prabhu13 2012-09-11 22:13 ` [PATCH 1/3] Add ratelimited printk for different alert levels raghu.prabhu13 2012-09-11 22:43 ` Dave Chinner 2012-09-12 3:22 ` Joe Perches 2012-09-13 0:51 ` Dave Chinner 2012-09-11 22:13 ` [PATCH 2/3] XFS: Print error when xfs_ialloc_ag_select fails to find continuous free space raghu.prabhu13 2012-09-11 22:48 ` Dave Chinner 2012-09-11 22:13 ` [PATCH 3/3] XFS: Print error when unable to allocate inodes or out of free inodes raghu.prabhu13 2012-09-11 23:21 ` Dave Chinner 2012-09-21 7:16 ` Raghavendra D Prabhu 2012-09-26 6:14 ` Raghavendra Prabhu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox