All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: raghu.prabhu13@gmail.com
Cc: open list <linux-kernel@vger.kernel.org>,
	Raghavendra D Prabhu <rprabhu@wnohang.net>,
	Alex Elder <elder@kernel.org>, Ben Myers <bpm@sgi.com>,
	xfs@oss.sgi.com
Subject: Re: [PATCH 2/3] XFS: Print error when xfs_ialloc_ag_select fails to find continuous free space.
Date: Wed, 12 Sep 2012 08:48:05 +1000	[thread overview]
Message-ID: <20120911224805.GG11511@dastard> (raw)
In-Reply-To: <b4227a4a67fb7b50bcf83fa369ba8999d3409d64.1347396641.git.rprabhu@wnohang.net>

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

WARNING: multiple messages have this Message-ID (diff)
From: Dave Chinner <david@fromorbit.com>
To: raghu.prabhu13@gmail.com
Cc: xfs@oss.sgi.com, Raghavendra D Prabhu <rprabhu@wnohang.net>,
	Ben Myers <bpm@sgi.com>, Alex Elder <elder@kernel.org>,
	open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 2/3] XFS: Print error when xfs_ialloc_ag_select fails to find continuous free space.
Date: Wed, 12 Sep 2012 08:48:05 +1000	[thread overview]
Message-ID: <20120911224805.GG11511@dastard> (raw)
In-Reply-To: <b4227a4a67fb7b50bcf83fa369ba8999d3409d64.1347396641.git.rprabhu@wnohang.net>

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

  reply	other threads:[~2012-09-11 22:47 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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:43   ` Dave Chinner
2012-09-11 22:43     ` Dave Chinner
2012-09-12  3:22   ` Joe Perches
2012-09-12  3:22     ` Joe Perches
2012-09-13  0:51     ` Dave Chinner
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:13   ` raghu.prabhu13
2012-09-11 22:48   ` Dave Chinner [this message]
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 22:13   ` raghu.prabhu13
2012-09-11 23:21   ` Dave Chinner
2012-09-11 23:21     ` Dave Chinner
2012-09-21  7:16     ` Raghavendra D Prabhu
2012-09-26  6:14       ` Raghavendra Prabhu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120911224805.GG11511@dastard \
    --to=david@fromorbit.com \
    --cc=bpm@sgi.com \
    --cc=elder@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=raghu.prabhu13@gmail.com \
    --cc=rprabhu@wnohang.net \
    --cc=xfs@oss.sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
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.