Linux filesystem development
 help / color / mirror / Atom feed
From: Namjae Jeon <namjae.jeon@samsung.com>
To: Jan Kara <jack@suse.cz>
Cc: linux-fsdevel@vger.kernel.org
Subject: [PATCH] udf: fix lookup performance issue when SLUB_DEBUG is enable
Date: Wed, 12 Nov 2014 20:18:02 +0900	[thread overview]
Message-ID: <009b01cffe6a$52e52090$f8af61b0$@samsung.com> (raw)

Convert frequent small dynamic memory allocation to static allocation
for filename.

It has been observed that there is huge performance drop in udf lookup
performance when CONFIG_SLUB_DEBUG is enable. As per analysis with perf
tool, most of time is spend in kmalloc and kfree in this scenario.
Performance drop seems very severe as compared to other file system in
similar scenario.

Consider below lookup test results on large udf mounted directory which has
4300 files

[Original Kernel]:
$ mount -t udf -o ro,relatime,uid=0,gid=1010,mode=775,dmode=775,utf8 /dev/cdrom /mnt/
$ cd /mnt/dir1
$ time ls
...................
00222610.png  00340607.png  00442904.png  00571618.png  01105104.png
real    4m 31.20s
user    0m 0.05s
sys     4m 9.48s

[After Applying Patch]
$ mount -t udf -o ro,relatime,uid=0,gid=1010,mode=775,dmode=775,utf8 /dev/cdrom /mnt/
$ cd /mnt/dir1
$ time ls
................
00222610.png  00340607.png  00442904.png  00571618.png  01105104.png
real    0m 21.11s
user    0m 0.04s
sys     0m 4.65s

Signed-off-by: Namjae Jeon <namjae.jeon@samsung.com>
Signed-off-by: Vivek Trivedi <t.vivek@samsung.com>
---
 fs/udf/dir.c     |  9 +--------
 fs/udf/namei.c   | 23 +++--------------------
 fs/udf/unicode.c | 37 +++++++++++++------------------------
 3 files changed, 17 insertions(+), 52 deletions(-)

diff --git a/fs/udf/dir.c b/fs/udf/dir.c
index a012c51..490b9dd 100644
--- a/fs/udf/dir.c
+++ b/fs/udf/dir.c
@@ -46,7 +46,7 @@ static int udf_readdir(struct file *file, struct dir_context *ctx)
 	int block, iblock;
 	loff_t nf_pos;
 	int flen;
-	unsigned char *fname = NULL;
+	unsigned char fname[UDF_NAME_LEN];
 	unsigned char *nameptr;
 	uint16_t liu;
 	uint8_t lfi;
@@ -67,12 +67,6 @@ static int udf_readdir(struct file *file, struct dir_context *ctx)
 	if (nf_pos >= size)
 		goto out;
 
-	fname = kmalloc(UDF_NAME_LEN, GFP_NOFS);
-	if (!fname) {
-		ret = -ENOMEM;
-		goto out;
-	}
-
 	if (nf_pos == 0)
 		nf_pos = udf_ext0_offset(dir);
 
@@ -184,7 +178,6 @@ out:
 		brelse(fibh.ebh);
 	brelse(fibh.sbh);
 	brelse(epos.bh);
-	kfree(fname);
 
 	return ret;
 }
diff --git a/fs/udf/namei.c b/fs/udf/namei.c
index c12e260..45cbbb9 100644
--- a/fs/udf/namei.c
+++ b/fs/udf/namei.c
@@ -147,7 +147,7 @@ static struct fileIdentDesc *udf_find_entry(struct inode *dir,
 	struct fileIdentDesc *fi = NULL;
 	loff_t f_pos;
 	int block, flen;
-	unsigned char *fname = NULL;
+	unsigned char fname[UDF_NAME_LEN];
 	unsigned char *nameptr;
 	uint8_t lfi;
 	uint16_t liu;
@@ -183,10 +183,6 @@ static struct fileIdentDesc *udf_find_entry(struct inode *dir,
 			goto out_err;
 	}
 
-	fname = kmalloc(UDF_NAME_LEN, GFP_NOFS);
-	if (!fname)
-		goto out_err;
-
 	while (f_pos < size) {
 		fi = udf_fileident_read(dir, &f_pos, fibh, cfi, &epos, &eloc,
 					&elen, &offset);
@@ -245,7 +241,6 @@ out_err:
 	brelse(fibh->sbh);
 out_ok:
 	brelse(epos.bh);
-	kfree(fname);
 
 	return fi;
 }
@@ -298,7 +293,7 @@ static struct fileIdentDesc *udf_add_entry(struct inode *dir,
 {
 	struct super_block *sb = dir->i_sb;
 	struct fileIdentDesc *fi = NULL;
-	unsigned char *name = NULL;
+	unsigned char name[UDF_NAME_LEN];
 	int namelen;
 	loff_t f_pos;
 	loff_t size = udf_ext0_offset(dir) + dir->i_size;
@@ -313,11 +308,6 @@ static struct fileIdentDesc *udf_add_entry(struct inode *dir,
 	struct udf_inode_info *dinfo;
 
 	fibh->sbh = fibh->ebh = NULL;
-	name = kmalloc(UDF_NAME_LEN, GFP_NOFS);
-	if (!name) {
-		*err = -ENOMEM;
-		goto out_err;
-	}
 
 	if (dentry) {
 		if (!dentry->d_name.len) {
@@ -532,7 +522,6 @@ out_err:
 	brelse(fibh->sbh);
 out_ok:
 	brelse(epos.bh);
-	kfree(name);
 	return fi;
 }
 
@@ -858,7 +847,7 @@ static int udf_symlink(struct inode *dir, struct dentry *dentry,
 	uint8_t *ea;
 	int err;
 	int block;
-	unsigned char *name = NULL;
+	unsigned char name[UDF_NAME_LEN];
 	int namelen;
 	struct udf_inode_info *iinfo;
 	struct super_block *sb = dir->i_sb;
@@ -868,11 +857,6 @@ static int udf_symlink(struct inode *dir, struct dentry *dentry,
 
 	iinfo = UDF_I(inode);
 	down_write(&iinfo->i_data_sem);
-	name = kmalloc(UDF_NAME_LEN, GFP_NOFS);
-	if (!name) {
-		err = -ENOMEM;
-		goto out_no_entry;
-	}
 
 	inode->i_data.a_ops = &udf_symlink_aops;
 	inode->i_op = &udf_symlink_inode_operations;
@@ -984,7 +968,6 @@ static int udf_symlink(struct inode *dir, struct dentry *dentry,
 
 	err = udf_add_nondir(dentry, inode);
 out:
-	kfree(name);
 	return err;
 
 out_no_entry:
diff --git a/fs/udf/unicode.c b/fs/udf/unicode.c
index afd470e..3fbd900 100644
--- a/fs/udf/unicode.c
+++ b/fs/udf/unicode.c
@@ -336,42 +336,31 @@ try_again:
 int udf_get_filename(struct super_block *sb, uint8_t *sname, uint8_t *dname,
 		     int flen)
 {
-	struct ustr *filename, *unifilename;
+	struct ustr filename, unifilename;
 	int len = 0;
 
-	filename = kmalloc(sizeof(struct ustr), GFP_NOFS);
-	if (!filename)
-		return 0;
-
-	unifilename = kmalloc(sizeof(struct ustr), GFP_NOFS);
-	if (!unifilename)
-		goto out1;
-
-	if (udf_build_ustr_exact(unifilename, sname, flen))
-		goto out2;
+	if (udf_build_ustr_exact(&unifilename, sname, flen))
+		goto out;
 
 	if (UDF_QUERY_FLAG(sb, UDF_FLAG_UTF8)) {
-		if (!udf_CS0toUTF8(filename, unifilename)) {
+		if (!udf_CS0toUTF8(&filename, &unifilename)) {
 			udf_debug("Failed in udf_get_filename: sname = %s\n",
 				  sname);
-			goto out2;
+			goto out;
 		}
 	} else if (UDF_QUERY_FLAG(sb, UDF_FLAG_NLS_MAP)) {
-		if (!udf_CS0toNLS(UDF_SB(sb)->s_nls_map, filename,
-				  unifilename)) {
+		if (!udf_CS0toNLS(UDF_SB(sb)->s_nls_map, &filename,
+				  &unifilename)) {
 			udf_debug("Failed in udf_get_filename: sname = %s\n",
 				  sname);
-			goto out2;
+			goto out;
 		}
 	} else
-		goto out2;
-
-	len = udf_translate_to_linux(dname, filename->u_name, filename->u_len,
-				     unifilename->u_name, unifilename->u_len);
-out2:
-	kfree(unifilename);
-out1:
-	kfree(filename);
+		goto out;
+
+	len = udf_translate_to_linux(dname, filename.u_name, filename.u_len,
+				     unifilename.u_name, unifilename.u_len);
+out:
 	return len;
 }
 
-- 
1.8.5.5


             reply	other threads:[~2014-11-12 11:18 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-12 11:18 Namjae Jeon [this message]
2014-11-12 12:41 ` [PATCH] udf: fix lookup performance issue when SLUB_DEBUG is enable Jan Kara

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='009b01cffe6a$52e52090$f8af61b0$@samsung.com' \
    --to=namjae.jeon@samsung.com \
    --cc=jack@suse.cz \
    --cc=linux-fsdevel@vger.kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox