All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 0/2] address romfs performance regression
@ 2020-06-23  0:42 Sven Van Asbroeck
  2020-06-23  0:43 ` [PATCH v1 1/2] romfs: use s_blocksize(_bits) if CONFIG_BLOCK Sven Van Asbroeck
  2020-06-23  0:43 ` [PATCH v1 2/2] romfs: address performance regression since v3.10 Sven Van Asbroeck
  0 siblings, 2 replies; 4+ messages in thread
From: Sven Van Asbroeck @ 2020-06-23  0:42 UTC (permalink / raw)
  To: linux-kernel
  Cc: Al Viro, Deepa Dinamani, David Howells, Darrick J. Wong,
	Janos Farkas, Jeff Layton

Tree: next-20200613

Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Deepa Dinamani <deepa.kernel@gmail.com>
Cc: David Howells <dhowells@redhat.com>
Cc: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: Janos Farkas <chexum+dev@gmail.com>
Cc: Jeff Layton <jlayton@kernel.org>
To: linux-kernel@vger.kernel.org

Sven Van Asbroeck (2):
  romfs: use s_blocksize(_bits) if CONFIG_BLOCK
  romfs: address performance regression since v3.10

 fs/romfs/storage.c | 25 ++++++++--------
 fs/romfs/super.c   | 71 ++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 78 insertions(+), 18 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v1 1/2] romfs: use s_blocksize(_bits) if CONFIG_BLOCK
  2020-06-23  0:42 [PATCH v1 0/2] address romfs performance regression Sven Van Asbroeck
@ 2020-06-23  0:43 ` Sven Van Asbroeck
  2020-06-23  0:43 ` [PATCH v1 2/2] romfs: address performance regression since v3.10 Sven Van Asbroeck
  1 sibling, 0 replies; 4+ messages in thread
From: Sven Van Asbroeck @ 2020-06-23  0:43 UTC (permalink / raw)
  To: linux-kernel

The super_block fields s_blocksize and s_blocksize_bits always
reflect the actual configured blocksize for a filesystem.

Use these in all calculations where blocksize is required.
This allows us to easily change the blocksize in a later patch.

Note that I cannot determine what happens if !CONFIG_BLOCK, as
I have no access to such a system. Out of an abundance of caution,
I have left all !CONFIG_BLOCK codepaths in their original state.

Signed-off-by: Sven Van Asbroeck <TheSven73@gmail.com>
---
 fs/romfs/storage.c | 25 +++++++++++++------------
 fs/romfs/super.c   |  9 ++++++++-
 2 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/fs/romfs/storage.c b/fs/romfs/storage.c
index 6b2b4362089e..5e84efadac3f 100644
--- a/fs/romfs/storage.c
+++ b/fs/romfs/storage.c
@@ -109,9 +109,9 @@ static int romfs_blk_read(struct super_block *sb, unsigned long pos,
 
 	/* copy the string up to blocksize bytes at a time */
 	while (buflen > 0) {
-		offset = pos & (ROMBSIZE - 1);
-		segment = min_t(size_t, buflen, ROMBSIZE - offset);
-		bh = sb_bread(sb, pos >> ROMBSBITS);
+		offset = pos & (sb->s_blocksize - 1);
+		segment = min_t(size_t, buflen, sb->s_blocksize - offset);
+		bh = sb_bread(sb, pos >> sb->s_blocksize_bits);
 		if (!bh)
 			return -EIO;
 		memcpy(buf, bh->b_data + offset, segment);
@@ -138,9 +138,9 @@ static ssize_t romfs_blk_strnlen(struct super_block *sb,
 
 	/* scan the string up to blocksize bytes at a time */
 	while (limit > 0) {
-		offset = pos & (ROMBSIZE - 1);
-		segment = min_t(size_t, limit, ROMBSIZE - offset);
-		bh = sb_bread(sb, pos >> ROMBSBITS);
+		offset = pos & (sb->s_blocksize - 1);
+		segment = min_t(size_t, limit, sb->s_blocksize - offset);
+		bh = sb_bread(sb, pos >> sb->s_blocksize_bits);
 		if (!bh)
 			return -EIO;
 		buf = bh->b_data + offset;
@@ -170,9 +170,9 @@ static int romfs_blk_strcmp(struct super_block *sb, unsigned long pos,
 
 	/* compare string up to a block at a time */
 	while (size > 0) {
-		offset = pos & (ROMBSIZE - 1);
-		segment = min_t(size_t, size, ROMBSIZE - offset);
-		bh = sb_bread(sb, pos >> ROMBSBITS);
+		offset = pos & (sb->s_blocksize - 1);
+		segment = min_t(size_t, size, sb->s_blocksize - offset);
+		bh = sb_bread(sb, pos >> sb->s_blocksize_bits);
 		if (!bh)
 			return -EIO;
 		matched = (memcmp(bh->b_data + offset, str, segment) == 0);
@@ -180,7 +180,8 @@ static int romfs_blk_strcmp(struct super_block *sb, unsigned long pos,
 		size -= segment;
 		pos += segment;
 		str += segment;
-		if (matched && size == 0 && offset + segment < ROMBSIZE) {
+		if (matched && size == 0 &&
+				offset + segment < sb->s_blocksize) {
 			if (!bh->b_data[offset + segment])
 				terminated = true;
 			else
@@ -194,8 +195,8 @@ static int romfs_blk_strcmp(struct super_block *sb, unsigned long pos,
 	if (!terminated) {
 		/* the terminating NUL must be on the first byte of the next
 		 * block */
-		BUG_ON((pos & (ROMBSIZE - 1)) != 0);
-		bh = sb_bread(sb, pos >> ROMBSBITS);
+		BUG_ON((pos & (sb->s_blocksize - 1)) != 0);
+		bh = sb_bread(sb, pos >> sb->s_blocksize_bits);
 		if (!bh)
 			return -EIO;
 		matched = !bh->b_data[0];
diff --git a/fs/romfs/super.c b/fs/romfs/super.c
index e582d001f792..6fecdea791f1 100644
--- a/fs/romfs/super.c
+++ b/fs/romfs/super.c
@@ -411,10 +411,17 @@ static int romfs_statfs(struct dentry *dentry, struct kstatfs *buf)
 
 	buf->f_type = ROMFS_MAGIC;
 	buf->f_namelen = ROMFS_MAXFN;
-	buf->f_bsize = ROMBSIZE;
 	buf->f_bfree = buf->f_bavail = buf->f_ffree;
+#ifdef CONFIG_BLOCK
+	buf->f_bsize = sb->s_blocksize;
+	buf->f_blocks =
+		(romfs_maxsize(dentry->d_sb) + sb->s_blocksize - 1) >>
+		sb->s_blocksize_bits;
+#else
+	buf->f_bsize = ROMBSIZE;
 	buf->f_blocks =
 		(romfs_maxsize(dentry->d_sb) + ROMBSIZE - 1) >> ROMBSBITS;
+#endif
 	buf->f_fsid.val[0] = (u32)id;
 	buf->f_fsid.val[1] = (u32)(id >> 32);
 	return 0;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH v1 2/2] romfs: address performance regression since v3.10
  2020-06-23  0:42 [PATCH v1 0/2] address romfs performance regression Sven Van Asbroeck
  2020-06-23  0:43 ` [PATCH v1 1/2] romfs: use s_blocksize(_bits) if CONFIG_BLOCK Sven Van Asbroeck
@ 2020-06-23  0:43 ` Sven Van Asbroeck
  1 sibling, 0 replies; 4+ messages in thread
From: Sven Van Asbroeck @ 2020-06-23  0:43 UTC (permalink / raw)
  To: linux-kernel

Problem
-------
romfs sequential read performance has regressed very badly since
v3.10. Currently, reading a large file inside a romfs image is
up to 12x slower compared to reading the romfs image directly.

Benchmarks:
- use a romfs image which contains a single 250M file
- calculate the md5sum of the romfs image directly (test 1)
  $ time md5sum image.romfs
- loop-mount the romfs image, and calc the md5sum of the file
  inside it (test 2)
  $ mount -o loop,ro image.romfs /mnt/romfs
  $ time md5sum /mnt/romfs/file
- drop caches in between
  $ echo 3 > /proc/sys/vm/drop_caches

imx6 (arm cortex a9) on emmc, running v5.7.2:
(test 1)  5 seconds
(test 2) 60 seconds (12x slower)

Intel i7-3630QM on Samsung SSD 850 EVO (EMT02B6Q),
    running Ubuntu with v4.15.0-106-generic:
(test 1) 1.3 seconds
(test 2) 3.3 seconds (2.5x slower)

To show that a regression has occurred since v3.10:

imx6 on emmc, running v3.10.17:
(test 1) 16 seconds
(test 2) 18 seconds

Proposed Solution
-----------------
Increase the blocksize from 1K to PAGE_SIZE. This brings the
sequential read performance close to where it was on v3.10:

imx6 on emmc, running v5.7.2:
(test 2 1K blocksize) 60 seconds
(test 2 4K blocksize) 22 seconds

Intel on Ubuntu running v4.15:
(test 2 1K blocksize) 3.3 seconds
(test 2 4K blocksize) 1.9 seconds

There is a risk that this may increase latency on random-
access workloads. But the test below suggests that this
is not a concern:

Benchmark:
- use a 630M romfs image consisting of 9600 files
- loop-mount the romfs image
  $ mount -o loop,ro image.romfs /mnt/romfs
- drop all caches
- list all files in the filesystem (test 3)
  $ time find /mnt/romfs > /dev/null

imx6 on emmc, running v5.7.2:
(test 3 1K blocksize) 9.5 seconds
(test 3 4K blocksize) 9   seconds

Intel on Ubuntu, running v4.15:
(test 3 1K blocksize) 1.4 seconds
(test 3 4K blocksize) 1.2 seconds

Practical Solution
------------------
Introduce a mount-option called 'largeblocks'. If present,
increase the blocksize for much better sequential performance.

Note that the Linux block layer can only support n-K blocks if
the underlying block device length is also aligned to n-K. This
may not always be the case. Therefore, the driver will pick the
largest blocksize which the underlying block device can support.

Signed-off-by: Sven Van Asbroeck <TheSven73@gmail.com>
---
 fs/romfs/super.c | 62 ++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 57 insertions(+), 5 deletions(-)

diff --git a/fs/romfs/super.c b/fs/romfs/super.c
index 6fecdea791f1..93565aeaa43c 100644
--- a/fs/romfs/super.c
+++ b/fs/romfs/super.c
@@ -65,7 +65,7 @@
 #include <linux/slab.h>
 #include <linux/init.h>
 #include <linux/blkdev.h>
-#include <linux/fs_context.h>
+#include <linux/fs_parser.h>
 #include <linux/mount.h>
 #include <linux/namei.h>
 #include <linux/statfs.h>
@@ -460,6 +460,54 @@ static __u32 romfs_checksum(const void *data, int size)
 	return sum;
 }
 
+enum romfs_param {
+	Opt_largeblocks,
+};
+
+static const struct fs_parameter_spec romfs_fs_parameters[] = {
+	fsparam_flag("largeblocks", Opt_largeblocks),
+	{}
+};
+
+/*
+ * Parse a single mount parameter.
+ */
+static int romfs_parse_param(struct fs_context *fc, struct fs_parameter *param)
+{
+	struct fs_parse_result result;
+	int opt;
+
+	opt = fs_parse(fc, romfs_fs_parameters, param, &result);
+	if (opt < 0)
+		return opt;
+
+	switch (opt) {
+	case Opt_largeblocks:
+		fc->fs_private = (void *) 1;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+/*
+ * pick the largest blocksize which the underlying block device
+ * is a multiple of. Or fall back to legacy (ROMBSIZE).
+ */
+static int romfs_largest_blocksize(struct super_block *sb)
+{
+	loff_t device_sz = i_size_read(sb->s_bdev->bd_inode);
+	int blksz;
+
+	for (blksz = PAGE_SIZE; blksz > ROMBSIZE; blksz >>= 1)
+		if ((device_sz % blksz) == 0)
+			break;
+
+	return blksz;
+}
+
 /*
  * fill in the superblock
  */
@@ -467,17 +515,19 @@ static int romfs_fill_super(struct super_block *sb, struct fs_context *fc)
 {
 	struct romfs_super_block *rsb;
 	struct inode *root;
-	unsigned long pos, img_size;
+	unsigned long pos, img_size, dev_blocksize;
 	const char *storage;
 	size_t len;
 	int ret;
 
 #ifdef CONFIG_BLOCK
+	dev_blocksize = fc->fs_private ? romfs_largest_blocksize(sb) :
+					 ROMBSIZE;
 	if (!sb->s_mtd) {
-		sb_set_blocksize(sb, ROMBSIZE);
+		sb_set_blocksize(sb, dev_blocksize);
 	} else {
-		sb->s_blocksize = ROMBSIZE;
-		sb->s_blocksize_bits = blksize_bits(ROMBSIZE);
+		sb->s_blocksize = dev_blocksize;
+		sb->s_blocksize_bits = blksize_bits(dev_blocksize);
 	}
 #endif
 
@@ -573,6 +623,7 @@ static int romfs_get_tree(struct fs_context *fc)
 static const struct fs_context_operations romfs_context_ops = {
 	.get_tree	= romfs_get_tree,
 	.reconfigure	= romfs_reconfigure,
+	.parse_param	= romfs_parse_param,
 };
 
 /*
@@ -607,6 +658,7 @@ static struct file_system_type romfs_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "romfs",
 	.init_fs_context = romfs_init_fs_context,
+	.parameters	= romfs_fs_parameters,
 	.kill_sb	= romfs_kill_sb,
 	.fs_flags	= FS_REQUIRES_DEV,
 };
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH v1 1/2] romfs: use s_blocksize(_bits) if CONFIG_BLOCK
  2020-06-23  0:45 [PATCH v1 0/2] address romfs performance regression Sven Van Asbroeck
@ 2020-06-23  0:45 ` Sven Van Asbroeck
  0 siblings, 0 replies; 4+ messages in thread
From: Sven Van Asbroeck @ 2020-06-23  0:45 UTC (permalink / raw)
  To: linux-kernel
  Cc: Al Viro, Deepa Dinamani, David Howells, Darrick J. Wong,
	Janos Farkas, Jeff Layton

The super_block fields s_blocksize and s_blocksize_bits always
reflect the actual configured blocksize for a filesystem.

Use these in all calculations where blocksize is required.
This allows us to easily change the blocksize in a later patch.

Note that I cannot determine what happens if !CONFIG_BLOCK, as
I have no access to such a system. Out of an abundance of caution,
I have left all !CONFIG_BLOCK codepaths in their original state.

Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Deepa Dinamani <deepa.kernel@gmail.com>
Cc: David Howells <dhowells@redhat.com>
Cc: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: Janos Farkas <chexum+dev@gmail.com>
Cc: Jeff Layton <jlayton@kernel.org>
To: linux-kernel@vger.kernel.org
Signed-off-by: Sven Van Asbroeck <TheSven73@gmail.com>
---
 fs/romfs/storage.c | 25 +++++++++++++------------
 fs/romfs/super.c   |  9 ++++++++-
 2 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/fs/romfs/storage.c b/fs/romfs/storage.c
index 6b2b4362089e..5e84efadac3f 100644
--- a/fs/romfs/storage.c
+++ b/fs/romfs/storage.c
@@ -109,9 +109,9 @@ static int romfs_blk_read(struct super_block *sb, unsigned long pos,
 
 	/* copy the string up to blocksize bytes at a time */
 	while (buflen > 0) {
-		offset = pos & (ROMBSIZE - 1);
-		segment = min_t(size_t, buflen, ROMBSIZE - offset);
-		bh = sb_bread(sb, pos >> ROMBSBITS);
+		offset = pos & (sb->s_blocksize - 1);
+		segment = min_t(size_t, buflen, sb->s_blocksize - offset);
+		bh = sb_bread(sb, pos >> sb->s_blocksize_bits);
 		if (!bh)
 			return -EIO;
 		memcpy(buf, bh->b_data + offset, segment);
@@ -138,9 +138,9 @@ static ssize_t romfs_blk_strnlen(struct super_block *sb,
 
 	/* scan the string up to blocksize bytes at a time */
 	while (limit > 0) {
-		offset = pos & (ROMBSIZE - 1);
-		segment = min_t(size_t, limit, ROMBSIZE - offset);
-		bh = sb_bread(sb, pos >> ROMBSBITS);
+		offset = pos & (sb->s_blocksize - 1);
+		segment = min_t(size_t, limit, sb->s_blocksize - offset);
+		bh = sb_bread(sb, pos >> sb->s_blocksize_bits);
 		if (!bh)
 			return -EIO;
 		buf = bh->b_data + offset;
@@ -170,9 +170,9 @@ static int romfs_blk_strcmp(struct super_block *sb, unsigned long pos,
 
 	/* compare string up to a block at a time */
 	while (size > 0) {
-		offset = pos & (ROMBSIZE - 1);
-		segment = min_t(size_t, size, ROMBSIZE - offset);
-		bh = sb_bread(sb, pos >> ROMBSBITS);
+		offset = pos & (sb->s_blocksize - 1);
+		segment = min_t(size_t, size, sb->s_blocksize - offset);
+		bh = sb_bread(sb, pos >> sb->s_blocksize_bits);
 		if (!bh)
 			return -EIO;
 		matched = (memcmp(bh->b_data + offset, str, segment) == 0);
@@ -180,7 +180,8 @@ static int romfs_blk_strcmp(struct super_block *sb, unsigned long pos,
 		size -= segment;
 		pos += segment;
 		str += segment;
-		if (matched && size == 0 && offset + segment < ROMBSIZE) {
+		if (matched && size == 0 &&
+				offset + segment < sb->s_blocksize) {
 			if (!bh->b_data[offset + segment])
 				terminated = true;
 			else
@@ -194,8 +195,8 @@ static int romfs_blk_strcmp(struct super_block *sb, unsigned long pos,
 	if (!terminated) {
 		/* the terminating NUL must be on the first byte of the next
 		 * block */
-		BUG_ON((pos & (ROMBSIZE - 1)) != 0);
-		bh = sb_bread(sb, pos >> ROMBSBITS);
+		BUG_ON((pos & (sb->s_blocksize - 1)) != 0);
+		bh = sb_bread(sb, pos >> sb->s_blocksize_bits);
 		if (!bh)
 			return -EIO;
 		matched = !bh->b_data[0];
diff --git a/fs/romfs/super.c b/fs/romfs/super.c
index e582d001f792..6fecdea791f1 100644
--- a/fs/romfs/super.c
+++ b/fs/romfs/super.c
@@ -411,10 +411,17 @@ static int romfs_statfs(struct dentry *dentry, struct kstatfs *buf)
 
 	buf->f_type = ROMFS_MAGIC;
 	buf->f_namelen = ROMFS_MAXFN;
-	buf->f_bsize = ROMBSIZE;
 	buf->f_bfree = buf->f_bavail = buf->f_ffree;
+#ifdef CONFIG_BLOCK
+	buf->f_bsize = sb->s_blocksize;
+	buf->f_blocks =
+		(romfs_maxsize(dentry->d_sb) + sb->s_blocksize - 1) >>
+		sb->s_blocksize_bits;
+#else
+	buf->f_bsize = ROMBSIZE;
 	buf->f_blocks =
 		(romfs_maxsize(dentry->d_sb) + ROMBSIZE - 1) >> ROMBSBITS;
+#endif
 	buf->f_fsid.val[0] = (u32)id;
 	buf->f_fsid.val[1] = (u32)(id >> 32);
 	return 0;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-06-23  0:46 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-23  0:42 [PATCH v1 0/2] address romfs performance regression Sven Van Asbroeck
2020-06-23  0:43 ` [PATCH v1 1/2] romfs: use s_blocksize(_bits) if CONFIG_BLOCK Sven Van Asbroeck
2020-06-23  0:43 ` [PATCH v1 2/2] romfs: address performance regression since v3.10 Sven Van Asbroeck
2020-06-23  0:45 [PATCH v1 0/2] address romfs performance regression Sven Van Asbroeck
2020-06-23  0:45 ` [PATCH v1 1/2] romfs: use s_blocksize(_bits) if CONFIG_BLOCK Sven Van Asbroeck

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.