2020-06-23 00:47:14

by Sven Van Asbroeck

[permalink] [raw]
Subject: [PATCH v1 0/2] address romfs performance regression

Tree: next-20200613

Cc: Al Viro <[email protected]>
Cc: Deepa Dinamani <[email protected]>
Cc: David Howells <[email protected]>
Cc: "Darrick J. Wong" <[email protected]>
Cc: Janos Farkas <[email protected]>
Cc: Jeff Layton <[email protected]>
To: [email protected]

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


2020-06-23 00:47:17

by Sven Van Asbroeck

[permalink] [raw]
Subject: [PATCH v1 1/2] romfs: use s_blocksize(_bits) if CONFIG_BLOCK

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 <[email protected]>
---
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

2020-06-23 02:04:25

by Sven Van Asbroeck

[permalink] [raw]
Subject: [PATCH v1 2/2] romfs: address performance regression since v3.10

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 <[email protected]>
---
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