diff mbox

[1/2] ext4: Delete an error message for a failed memory allocation in ext4_multi_mount_protect()

Message ID 5bf432ae-398d-718c-8105-3092a240a8ef@users.sourceforge.net
State Rejected, archived
Headers show

Commit Message

SF Markus Elfring Aug. 19, 2017, 11:47 a.m. UTC
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 19 Aug 2017 13:04:50 +0200

Omit an extra message for a memory allocation failure in this function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 fs/ext4/mmp.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Comments

Eric Sandeen Aug. 19, 2017, 5:08 p.m. UTC | #1
On 8/19/17 6:47 AM, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sat, 19 Aug 2017 13:04:50 +0200
> 
> Omit an extra message for a memory allocation failure in this function.

I might be dense, but what makes this message "extra?"

(I suppose kmalloc squawks too if it fails, but is Coccinelle
now warning about explicit memory allocation failure warnings?)

-Eric
 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  fs/ext4/mmp.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/ext4/mmp.c b/fs/ext4/mmp.c
> index eb9835638680..1ce00453f612 100644
> --- a/fs/ext4/mmp.c
> +++ b/fs/ext4/mmp.c
> @@ -371,7 +371,6 @@ int ext4_multi_mount_protect(struct super_block *sb,
> -	if (!mmpd_data) {
> -		ext4_warning(sb, "not enough memory for mmpd_data");
> +	if (!mmpd_data)
>  		goto failed;
> -	}
> +
>  	mmpd_data->sb = sb;
>  	mmpd_data->bh = bh;
>  
>
SF Markus Elfring Aug. 19, 2017, 6 p.m. UTC | #2
>> Omit an extra message for a memory allocation failure in this function.
> 
> I might be dense, but what makes this message "extra?"
> 
> (I suppose kmalloc squawks too if it fails,

Do you find the default allocation failure report sufficient?


> but is Coccinelle now warning about explicit memory allocation failure warnings?)

This software tool can help to find source code places for further
development considerations. Would you like to clarify a corresponding
search pattern a bit more?

Regards,
Markus
Theodore Ts'o Aug. 24, 2017, 5:44 p.m. UTC | #3
On Sat, Aug 19, 2017 at 08:00:31PM +0200, SF Markus Elfring wrote:
> >> Omit an extra message for a memory allocation failure in this function.
> > 
> > I might be dense, but what makes this message "extra?"
> > 
> > (I suppose kmalloc squawks too if it fails,
> 
> Do you find the default allocation failure report sufficient?

From a helpdesk reporting situation, having a more specific message
when there is a MMP failure causing the mount to fail is definitely
useful.

So, NACK.

					- Ted
Dan Carpenter Aug. 24, 2017, 7:16 p.m. UTC | #4
On Sat, Aug 19, 2017 at 12:08:29PM -0500, Eric Sandeen wrote:
> On 8/19/17 6:47 AM, SF Markus Elfring wrote:
> > From: Markus Elfring <elfring@users.sourceforge.net>
> > Date: Sat, 19 Aug 2017 13:04:50 +0200
> > 
> > Omit an extra message for a memory allocation failure in this function.
> 
> I might be dense, but what makes this message "extra?"
> 
> (I suppose kmalloc squawks too if it fails, but is Coccinelle
> now warning about explicit memory allocation failure warnings?)
> 

Yeah...  Checkpatch complains that the kmalloc squawks is enough.
"WARNING: Possible unnecessary 'out of memory' message".  This
allocation is small so it's guaranteed to succeed in current kernels.

regards,
dan carpenter
diff mbox

Patch

diff --git a/fs/ext4/mmp.c b/fs/ext4/mmp.c
index eb9835638680..1ce00453f612 100644
--- a/fs/ext4/mmp.c
+++ b/fs/ext4/mmp.c
@@ -371,7 +371,6 @@  int ext4_multi_mount_protect(struct super_block *sb,
-	if (!mmpd_data) {
-		ext4_warning(sb, "not enough memory for mmpd_data");
+	if (!mmpd_data)
 		goto failed;
-	}
+
 	mmpd_data->sb = sb;
 	mmpd_data->bh = bh;