fs: Remove unnecessary OOM messages

PATCH Applied Joe Perches 5 participants 13 messages Jan 31, 2012 – Feb 3, 2012

Subsystems (4)

Memory technology devices (MTD) Journalling flash file system V2 (JFFS2) UDF filesystem Coda file system

lore

Share & tools

lore

Joe Perches

author

2012-01-31 22:42 UTC #0 lore raw

cover fs: Remove unnecessary OOM messages

Joe Perches (4):
coda: Remove unnecessary OOM messages
jffs2: Remove unnecessary OOM messages
reiserfs: Remove unnecessary OOM messages
udf: Remove unnecessary OOM messages

 fs/coda/coda_linux.h  |   13 ++++++-------
 fs/jffs2/compr_lzo.c  |    1 -
 fs/jffs2/compr_zlib.c |    6 ++----
 fs/reiserfs/journal.c |    8 ++------
 fs/reiserfs/resize.c  |    1 -
 fs/udf/super.c        |    5 +----
 6 files changed, 11 insertions(+), 23 deletions(-)

Signature · 1 line

1.7.8.111.gad25c.dirty

Joe Perches

author

2012-01-31 22:42 UTC #1 lore raw

PATCH 1/4 coda: Remove unnecessary OOM messages

Per call site OOM messages are unnecessary.
k.alloc and v.alloc failures use dump_stack().

Signed-off-by Joe Perches <[email protected]>

---
 fs/coda/coda_linux.h |   13 ++++++-------
 1 files changed, 6 insertions(+), 7 deletions(-)

fs/coda/coda_linux.h +6 -7

index cc0ea9f..7b2c3a1 100644
@@ -57,13 +57,12 @@ unsigned short coda_flags_to_cflags(unsigned short);
 void coda_sysctl_init(void);
 void coda_sysctl_clean(void);
 
-#define CODA_ALLOC(ptr, cast, size) do { \
-    if (size < PAGE_SIZE) \
-        ptr = kzalloc((unsigned long) size, GFP_KERNEL); \
-    else \
-        ptr = (cast)vzalloc((unsigned long) size); \
-    if (!ptr) \
-        printk("kernel malloc returns 0 at %s:%d\n", __FILE__, __LINE__); \
+#define CODA_ALLOC(ptr, cast, size)			\
+do {							\
+	if (size < PAGE_SIZE)				\
+		ptr = kzalloc(size, GFP_KERNEL);	\
+	else						\
+		ptr = vzalloc(size);			\
 } while (0)

Signature · 1 line

1.7.8.111.gad25c.dirty

Since CODA_ALLOC no longer uses __FILE__ and __LINE__ and doesn't use the cast argument any more, it can be replaced with a static inline function. Something like this (untested, applies on top of your patch):
----

Since CODA_ALLOC no longer needs to be defined as a macro, replace it with a static inline function and make it return the allocated pointer. Rename it to coda_alloc to be more consistent with other function naming. Similarly, replace CODA_FREE with a static inline function.

Also fix a bug in coda_psdev_write where the return value of CODA_ALLOC was not being checked.

Signed-off-by Ryan Mallon <[email protected]>

---

fs/coda/coda_linux.h +13 ~~-10~~

index 7b2c3a1..f5da0a7 100644
@@ -57,17 +57,20 @@ unsigned short coda_flags_to_cflags(unsigned short);
 void coda_sysctl_init(void);
 void coda_sysctl_clean(void);
 
-#define CODA_ALLOC(ptr, cast, size)			\
-do {							\
-	if (size < PAGE_SIZE)				\
-		ptr = kzalloc(size, GFP_KERNEL);	\
-	else						\
-		ptr = vzalloc(size);			\
-} while (0)
-
+static inline void *coda_alloc(size_t size)
+{
+	if (size < PAGE_SIZE)
+		return kzalloc(size, GFP_KERNEL);
+	return vzalloc(size);
+}
 
-#define CODA_FREE(ptr,size) \
-    do { if (size < PAGE_SIZE) kfree((ptr)); else vfree((ptr)); } while (0)
+static inline void coda_free(const void *ptr, size_t size)
+{
+	if (size < PAGE_SIZE)
+		kfree(ptr);
+	else
+		vfree(ptr);
+}
 
 /* inode to cnode access functions */

fs/coda/psdev.c +9 -5

index 8f616e0..4afcb81 100644
@@ -124,9 +124,13 @@ static ssize_t coda_psdev_write(struct file *file, const char __user *buf,
 			       hdr.opcode, hdr.unique);
 		        nbytes = size;
 		}
-		CODA_ALLOC(dcbuf, union outputArgs *, nbytes);
+		dcbuf = coda_alloc(nbytes);
+		if (!dcbuf) {
+			retval = -ENOMEM;
+			goto out;
+		}
 		if (copy_from_user(dcbuf, buf, nbytes)) {
-			CODA_FREE(dcbuf, nbytes);
+			coda_free(dcbuf, nbytes);
 			retval = -EFAULT;
 			goto out;
 		}

@@ -134,7 +138,7 @@ static ssize_t coda_psdev_write(struct file *file, const char __user *buf,
 		/* what downcall errors does Venus handle ? */
 		error = coda_downcall(vcp, hdr.opcode, dcbuf);
 
-		CODA_FREE(dcbuf, nbytes);
+		coda_free(dcbuf, nbytes);
 		if (error) {
 		        printk("psdev_write: coda_downcall error: %d\n", error);
 			retval = error;

@@ -255,7 +259,7 @@ static ssize_t coda_psdev_read(struct file * file, char __user * buf,
 		goto out;
 	}
 
-	CODA_FREE(req->uc_data, sizeof(struct coda_in_hdr));
+	coda_free(req->uc_data, sizeof(struct coda_in_hdr));
 	kfree(req);
 out:
 	mutex_unlock(&vcp->vc_mutex);

@@ -311,7 +315,7 @@ static int coda_psdev_release(struct inode * inode, struct file * file)
 
 		/* Async requests need to be freed here */
 		if (req->uc_flags & CODA_REQ_ASYNC) {
-			CODA_FREE(req->uc_data, sizeof(struct coda_in_hdr));
+			coda_free(req->uc_data, sizeof(struct coda_in_hdr));
 			kfree(req);
 			continue;
 		}

fs/coda/upcall.c +20 ~~-20~~

index 9727e0c..5c43991 100644
@@ -46,7 +46,7 @@ static void *alloc_upcall(int opcode, int size)
 {
 	union inputArgs *inp;
 
-	CODA_ALLOC(inp, union inputArgs *, size);
+	inp = coda_alloc(size);
         if (!inp)
 		return ERR_PTR(-ENOMEM);

@@ -85,7 +85,7 @@ int venus_rootfid(struct super_block *sb, struct CodaFid *fidp)
 	if (!error)
 		*fidp = outp->coda_root.VFid;
 
-	CODA_FREE(inp, insize);
+	coda_free(inp, insize);
 	return error;
 }

@@ -104,7 +104,7 @@ int venus_getattr(struct super_block *sb, struct CodaFid *fid,
 	if (!error)
 		*attr = outp->coda_getattr.attr;
 
-	CODA_FREE(inp, insize);
+	coda_free(inp, insize);
         return error;
 }

@@ -123,7 +123,7 @@ int venus_setattr(struct super_block *sb, struct CodaFid *fid,
 
 	error = coda_upcall(coda_vcp(sb), insize, &outsize, inp);
 
-        CODA_FREE(inp, insize);
+        coda_free(inp, insize);
         return error;
 }

@@ -153,7 +153,7 @@ int venus_lookup(struct super_block *sb, struct CodaFid *fid,
 		*type = outp->coda_lookup.vtype;
 	}
 
-	CODA_FREE(inp, insize);
+	coda_free(inp, insize);
 	return error;
 }

@@ -173,7 +173,7 @@ int venus_close(struct super_block *sb, struct CodaFid *fid, int flags,
 
 	error = coda_upcall(coda_vcp(sb), insize, &outsize, inp);
 
-	CODA_FREE(inp, insize);
+	coda_free(inp, insize);
         return error;
 }

@@ -194,7 +194,7 @@ int venus_open(struct super_block *sb, struct CodaFid *fid,
 	if (!error)
 		*fh = outp->coda_open_by_fd.fh;
 
-	CODA_FREE(inp, insize);
+	coda_free(inp, insize);
 	return error;
 }

@@ -224,7 +224,7 @@ int venus_mkdir(struct super_block *sb, struct CodaFid *dirfid,
 		*newfid = outp->coda_mkdir.VFid;
 	}
 
-	CODA_FREE(inp, insize);
+	coda_free(inp, insize);
 	return error;        
 }

@@ -262,7 +262,7 @@ int venus_rename(struct super_block *sb, struct CodaFid *old_fid,
 
 	error = coda_upcall(coda_vcp(sb), insize, &outsize, inp);
 
-	CODA_FREE(inp, insize);
+	coda_free(inp, insize);
 	return error;
 }

@@ -295,7 +295,7 @@ int venus_create(struct super_block *sb, struct CodaFid *dirfid,
 		*newfid = outp->coda_create.VFid;
 	}
 
-	CODA_FREE(inp, insize);
+	coda_free(inp, insize);
 	return error;        
 }

@@ -318,7 +318,7 @@ int venus_rmdir(struct super_block *sb, struct CodaFid *dirfid,
 
 	error = coda_upcall(coda_vcp(sb), insize, &outsize, inp);
 
-	CODA_FREE(inp, insize);
+	coda_free(inp, insize);
 	return error;
 }

@@ -340,7 +340,7 @@ int venus_remove(struct super_block *sb, struct CodaFid *dirfid,
 
 	error = coda_upcall(coda_vcp(sb), insize, &outsize, inp);
 
-	CODA_FREE(inp, insize);
+	coda_free(inp, insize);
 	return error;
 }

@@ -370,7 +370,7 @@ int venus_readlink(struct super_block *sb, struct CodaFid *fid,
 		*(buffer + retlen) = '\0';
 	}
 
-        CODA_FREE(inp, insize);
+        coda_free(inp, insize);
         return error;
 }

@@ -398,7 +398,7 @@ int venus_link(struct super_block *sb, struct CodaFid *fid,
 
 	error = coda_upcall(coda_vcp(sb), insize, &outsize, inp);
 
-	CODA_FREE(inp, insize);
+	coda_free(inp, insize);
         return error;
 }

@@ -433,7 +433,7 @@ int venus_symlink(struct super_block *sb, struct CodaFid *fid,
 
 	error = coda_upcall(coda_vcp(sb), insize, &outsize, inp);
 
-	CODA_FREE(inp, insize);
+	coda_free(inp, insize);
         return error;
 }

@@ -450,7 +450,7 @@ int venus_fsync(struct super_block *sb, struct CodaFid *fid)
 	error = coda_upcall(coda_vcp(sb), sizeof(union inputArgs),
 			    &outsize, inp);
 
-	CODA_FREE(inp, insize);
+	coda_free(inp, insize);
 	return error;
 }

@@ -468,7 +468,7 @@ int venus_access(struct super_block *sb, struct CodaFid *fid, int mask)
 
 	error = coda_upcall(coda_vcp(sb), insize, &outsize, inp);
 
-	CODA_FREE(inp, insize);
+	coda_free(inp, insize);
 	return error;
 }

@@ -544,7 +544,7 @@ int venus_pioctl(struct super_block *sb, struct CodaFid *fid,
 	}
 
  exit:
-	CODA_FREE(inp, insize);
+	coda_free(inp, insize);
 	return error;
 }

@@ -566,7 +566,7 @@ int venus_statfs(struct dentry *dentry, struct kstatfs *sfs)
 		sfs->f_ffree  = outp->coda_statfs.stat.f_ffree;
 	}
 
-        CODA_FREE(inp, insize);
+        coda_free(inp, insize);
         return error;
 }

@@ -744,7 +744,7 @@ static int coda_upcall(struct venus_comm *vcp,
 	sig_req = kmalloc(sizeof(struct upc_req), GFP_KERNEL);
 	if (!sig_req) goto exit;
 
-	CODA_ALLOC((sig_req->uc_data), char *, sizeof(struct coda_in_hdr));
+	sig_req->uc_data = coda_alloc(sizeof(struct coda_in_hdr));
 	if (!sig_req->uc_data) {
 		kfree(sig_req);
 		goto exit;

Joe Perches

author

2012-02-01 00:21 UTC #3 lore raw

Re: [PATCH 1/4] coda: Remove unnecessary OOM messages

Quoted 3 lines

Since CODA_ALLOC no longer uses __FILE__ and __LINE__ and doesn't use the cast argument any more, it can be replaced with a static inline function. Something like this (untested, applies on top of your patch):

Hi Ryan.

I didn't want to be quite so invasive but this looks like a nice cleanup to me.

Maybe because alloc/free functions are used relatively infrequently, maybe it'd be better to not declare the functions inline but add them to coda_linux.c

fs/coda/coda_linux.h unsigned short coda_flags_to_cflags(unsigned short); · line 65

Quoted 6 lines

fs/coda/coda_linux.h +6 -0

+static inline void *coda_alloc(size_t size)
+{
+	if (size < PAGE_SIZE)
+		return kzalloc(size, GFP_KERNEL);
+	return vzalloc(size);
+}

[]

Quoted 7 lines

fs/coda/coda_linux.h +7 -0

+static inline void coda_free(const void *ptr, size_t size)
+{
+	if (size < PAGE_SIZE)
+		kfree(ptr);
+	else
+		vfree(ptr);
+}

Ryan Mallon

2012-02-01 00:23 UTC #4 lore raw

Re: [PATCH 1/4] coda: Remove unnecessary OOM messages

Quoted 8 lines · Joe Perches

Quoted 3 lines
Since CODA_ALLOC no longer uses __FILE__ and __LINE__ and doesn't use the cast argument any more, it can be replaced with a static inline function. Something like this (untested, applies on top of your patch):
Hi Ryan.
I didn't want to be quite so invasive but this looks like a nice cleanup to me.

Yeah, it needed to be done as a separate patch. Just a good time to do it now that it no longer needs to be a macro.

Quoted 4 lines

Maybe because alloc/free functions are used relatively infrequently, maybe it'd be better to not declare the functions inline but add them to coda_linux.c

Sure, either approach is fine. Do you want to add the patch to your series?

~Ryan

Quoted 17 lines

fs/coda/coda_linux.h unsigned short coda_flags_to_cflags(unsigned short); · line 65
Quoted 7 lines
fs/coda/coda_linux.h +6 -0
+static inline void *coda_alloc(size_t size)
+{
+	if (size < PAGE_SIZE)
+		return kzalloc(size, GFP_KERNEL);
+	return vzalloc(size);
+}
[]
Quoted 7 lines
fs/coda/coda_linux.h +7 -0
+static inline void coda_free(const void *ptr, size_t size)
+{
+	if (size < PAGE_SIZE)
+		kfree(ptr);
+	else
+		vfree(ptr);
+}

Joe Perches

author

2012-02-01 00:26 UTC #5 lore raw

Re: [PATCH 1/4] coda: Remove unnecessary OOM messages

Quoted 6 lines · Ryan Mallon

Quoted 4 lines · Joe Perches
Maybe because alloc/free functions are used relatively infrequently, maybe it'd be better to not declare the functions inline but add them to coda_linux.c
Sure, either approach is fine. Do you want to add the patch to your series?

I think Jan can work it out properly if he wants it.

Jan Harkes

maintainer

2012-02-01 03:40 UTC #6 lore raw

Re: [PATCH 1/4] coda: Remove unnecessary OOM messages

Quoted 10 lines · Joe Perches

Quoted 6 lines · Ryan Mallon
Quoted 4 lines · Joe Perches
Maybe because alloc/free functions are used relatively infrequently, maybe it'd be better to not declare the functions inline but add them to coda_linux.c
Sure, either approach is fine. Do you want to add the patch to your
series?
I think Jan can work it out properly if he wants it.

It looks good to me, I have no problem if you want to add it to your series.

Jan

Joe Perches

author

2012-02-01 03:42 UTC #7 lore raw

Re: [PATCH 1/4] coda: Remove unnecessary OOM messages

Quoted 11 lines · Jan Harkes

Quoted 9 lines · Joe Perches
Quoted 6 lines · Ryan Mallon
Quoted 4 lines · Joe Perches
Maybe because alloc/free functions are used relatively infrequently, maybe it'd be better to not declare the functions inline but add them to coda_linux.c
Sure, either approach is fine. Do you want to add the patch to your
series?
I think Jan can work it out properly if he wants it.
It looks good to me, I have no problem if you want to add it to your series.

(adding Al Viro and Andrew Morton to cc:)

Hi Jan.

I think Ryan's patches are fine as well.

Quoted 1 line

From my perspective as a contributor, you should ack the

patches you apply to your own tree with your own "Signed-off-by:" tag and later send a pull request to Linus or another upstream maintainer like Al Viro or you should ack the patches and forward them to another upstream maintainer to be applied in their tree with your "Acked-by:" tag.

cheers, Joe

Joe Perches

author

2012-01-31 22:42 UTC #8 lore raw

PATCH 2/4 jffs2: Remove unnecessary OOM messages

In reply to Joe Perches

Per call site OOM messages are unnecessary.
k.alloc and v.alloc failures use dump_stack().

Signed-off-by Joe Perches <[email protected]>

---
 fs/jffs2/compr_lzo.c  |    1 -
 fs/jffs2/compr_zlib.c |    6 ++----
 2 files changed, 2 insertions(+), 5 deletions(-)

fs/jffs2/compr_lzo.c +0 -1

index af186ee..c553bd6 100644
@@ -33,7 +33,6 @@ static int __init alloc_workspace(void)
 	lzo_compress_buf = vmalloc(lzo1x_worst_compress(PAGE_SIZE));
 
 	if (!lzo_mem || !lzo_compress_buf) {
-		printk(KERN_WARNING "Failed to allocate lzo deflate workspace\n");
 		free_workspace();
 		return -ENOMEM;
 	}

fs/jffs2/compr_zlib.c +2 -4

index 5a00102..4e7a138 100644
@@ -42,14 +42,12 @@ static int __init alloc_workspaces(void)
 {
 	def_strm.workspace = vmalloc(zlib_deflate_workspacesize(MAX_WBITS,
 							MAX_MEM_LEVEL));
-	if (!def_strm.workspace) {
-		printk(KERN_WARNING "Failed to allocate %d bytes for deflate workspace\n", zlib_deflate_workspacesize(MAX_WBITS, MAX_MEM_LEVEL));
+	if (!def_strm.workspace)
 		return -ENOMEM;
-	}
+
 	D1(printk(KERN_DEBUG "Allocated %d bytes for deflate workspace\n", zlib_deflate_workspacesize(MAX_WBITS, MAX_MEM_LEVEL)));
 	inf_strm.workspace = vmalloc(zlib_inflate_workspacesize());
 	if (!inf_strm.workspace) {
-		printk(KERN_WARNING "Failed to allocate %d bytes for inflate workspace\n", zlib_inflate_workspacesize());
 		vfree(def_strm.workspace);
 		return -ENOMEM;
 	}

Signature · 1 line

1.7.8.111.gad25c.dirty

Artem Bityutskiy

Applied

2012-02-03 06:58 UTC #9 lore raw

Re: [PATCH 2/4] jffs2: Remove unnecessary OOM messages

Quoted 4 lines · Joe Perches

Per call site OOM messages are unnecessary.
k.alloc and v.alloc failures use dump_stack().
Signed-off-by Joe Perches <[email protected]>

Pushed to l2-mtd.git, thanks!

Signature · 2 lines

Best Regards,
Artem Bityutskiy

Joe Perches

author

2012-01-31 22:42 UTC #10 lore raw

PATCH 3/4 reiserfs: Remove unnecessary OOM messages

In reply to Joe Perches

Per call site OOM messages are unnecessary.
k.alloc and v.alloc failures use dump_stack().

Signed-off-by Joe Perches <[email protected]>

---
 fs/reiserfs/journal.c |    8 ++------
 fs/reiserfs/resize.c  |    1 -
 2 files changed, 2 insertions(+), 7 deletions(-)

fs/reiserfs/journal.c +2 -6

index c3cf54f..66bdea8 100644
@@ -293,8 +293,6 @@ int reiserfs_allocate_list_bitmaps(struct super_block *sb,
 		jb->journal_list = NULL;
 		jb->bitmaps = vzalloc(mem);
 		if (!jb->bitmaps) {
-			reiserfs_warning(sb, "clm-2000", "unable to "
-					 "allocate bitmaps for journal lists");
 			failed = 1;
 			break;
 		}

@@ -2679,11 +2677,9 @@ int journal_init(struct super_block *sb, const char *j_dev_name,
 	int ret;
 
 	journal = SB_JOURNAL(sb) = vzalloc(sizeof(struct reiserfs_journal));
-	if (!journal) {
-		reiserfs_warning(sb, "journal-1256",
-				 "unable to get memory for journal structure");
+	if (!journal)
 		return 1;
-	}
+
 	INIT_LIST_HEAD(&journal->j_bitmap_nodes);
 	INIT_LIST_HEAD(&journal->j_prealloc_list);
 	INIT_LIST_HEAD(&journal->j_working_list);

fs/reiserfs/resize.c +0 -1

index 7483279..d688aea 100644
@@ -115,7 +115,6 @@ int reiserfs_resize(struct super_block *s, unsigned long block_count_new)
 		if (!bitmap) {
 			/* Journal bitmaps are still supersized, but the memory isn't
 			 * leaked, so I guess it's ok */
-			printk("reiserfs_resize: unable to allocate memory.\n");
 			return -ENOMEM;
 		}
 		for (i = 0; i < bmap_nr; i++)

Signature · 1 line

1.7.8.111.gad25c.dirty

Joe Perches

author

2012-01-31 22:42 UTC #11 lore raw

PATCH 4/4 udf: Remove unnecessary OOM messages

In reply to Joe Perches

Per call site OOM messages are unnecessary.
k.alloc and v.alloc failures use dump_stack().

Signed-off-by Joe Perches <[email protected]>

---
 fs/udf/super.c |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

fs/udf/super.c +1 -4

index c09a84d..19111f9 100644
@@ -948,11 +948,8 @@ static struct udf_bitmap *udf_sb_alloc_bitmap(struct super_block *sb, u32 index)
 	else
 		bitmap = vzalloc(size); /* TODO: get rid of vzalloc */
 
-	if (bitmap == NULL) {
-		udf_err(sb, "Unable to allocate space for bitmap and %d buffer_head pointers\n",
-			nr_groups);
+	if (bitmap == NULL)
 		return NULL;
-	}
 
 	bitmap->s_block_bitmap = (struct buffer_head **)(bitmap + 1);
 	bitmap->s_nr_groups = nr_groups;

Signature · 1 line

1.7.8.111.gad25c.dirty

Jan Kara

2012-02-01 11:29 UTC #12 lore raw

Re: [PATCH 4/4] udf: Remove unnecessary OOM messages

Quoted 2 lines · Joe Perches

Per call site OOM messages are unnecessary.
k.alloc and v.alloc failures use dump_stack().

Thanks. Applied.

Honza

Quoted 26 lines

Signed-off-by Joe Perches <[email protected]>

---
 fs/udf/super.c |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

fs/udf/super.c +1 -4

index c09a84d..19111f9 100644
@@ -948,11 +948,8 @@ static struct udf_bitmap *udf_sb_alloc_bitmap(struct super_block *sb, u32 index)
 	else
 		bitmap = vzalloc(size); /* TODO: get rid of vzalloc */
 
-	if (bitmap == NULL) {
-		udf_err(sb, "Unable to allocate space for bitmap and %d buffer_head pointers\n",
-			nr_groups);
+	if (bitmap == NULL)
 		return NULL;
-	}
 
 	bitmap->s_block_bitmap = (struct buffer_head **)(bitmap + 1);
 	bitmap->s_nr_groups = nr_groups;

Signature · 1 line

1.7.8.111.gad25c.dirty

Signature · 2 lines

Jan Kara <[email protected]>
SUSE Labs, CR