| Message ID | 20190731130600.7867-1-xiaoguang.wang@linux.alibaba.com |
|---|---|
| State | New |
| Headers | show |
| Series | ext4: disable mount with both dioread_nolock and nodelalloc | expand |
On 19/7/31 21:06, Xiaoguang Wang wrote: > Mount with both dioread_nolock and nodelalloc will result in huge > performance drop, which indeed is an known issue, so before we fix > this issue, currently we disable this behaviour. Below test reproducer > can reveal this performance drop. > > mount -o remount,dioread_nolock,delalloc /dev/vdb1 > rm -f testfile > start_time=$(date +%s) > dd if=/dev/zero of=testfile bs=4096 count=$((1024*256)) > sync > end_time=$(date +%s) > echo $((end_time - start_time)) > > mount -o remount,dioread_nolock,nodelalloc /dev/vdb1 > rm -f testfile > start_time=$(date +%s) > dd if=/dev/zero of=testfile bs=4096 count=$((1024*256)) > sync > end_time=$(date +%s) > echo $((end_time - start_time)) > > Signed-off-by: Xiaoguang Wang <xiaoguang.wang@linux.alibaba.com> > --- > fs/ext4/super.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/fs/ext4/super.c b/fs/ext4/super.c > index 4079605d437a..1a2b2c0cd1b8 100644 > --- a/fs/ext4/super.c > +++ b/fs/ext4/super.c > @@ -2098,6 +2098,12 @@ static int parse_options(char *options, struct super_block *sb, > int blocksize = > BLOCK_SIZE << le32_to_cpu(sbi->s_es->s_log_block_size); > > + if (!test_opt(sb, DELALLOC)) { > + ext4_msg(sb, KERN_ERR, "can't mount with " > + "both dioread_nolock and nodelalloc"); > + return 0; > + } > + I suggest move it down to keep blocksize check logic together. Other than that, looks good to me. Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com> > if (blocksize < PAGE_SIZE) { > ext4_msg(sb, KERN_ERR, "can't mount with " > "dioread_nolock if block size != PAGE_SIZE"); >
hi, Ping. For this patch, I'm afraid someone else still takes effort to look into this issue. Regards, Xiaoguang Wang > Mount with both dioread_nolock and nodelalloc will result in huge > performance drop, which indeed is an known issue, so before we fix > this issue, currently we disable this behaviour. Below test reproducer > can reveal this performance drop. > > mount -o remount,dioread_nolock,delalloc /dev/vdb1 > rm -f testfile > start_time=$(date +%s) > dd if=/dev/zero of=testfile bs=4096 count=$((1024*256)) > sync > end_time=$(date +%s) > echo $((end_time - start_time)) > > mount -o remount,dioread_nolock,nodelalloc /dev/vdb1 > rm -f testfile > start_time=$(date +%s) > dd if=/dev/zero of=testfile bs=4096 count=$((1024*256)) > sync > end_time=$(date +%s) > echo $((end_time - start_time)) > > Signed-off-by: Xiaoguang Wang <xiaoguang.wang@linux.alibaba.com> > --- > fs/ext4/super.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/fs/ext4/super.c b/fs/ext4/super.c > index 4079605d437a..1a2b2c0cd1b8 100644 > --- a/fs/ext4/super.c > +++ b/fs/ext4/super.c > @@ -2098,6 +2098,12 @@ static int parse_options(char *options, struct super_block *sb, > int blocksize = > BLOCK_SIZE << le32_to_cpu(sbi->s_es->s_log_block_size); > > + if (!test_opt(sb, DELALLOC)) { > + ext4_msg(sb, KERN_ERR, "can't mount with " > + "both dioread_nolock and nodelalloc"); > + return 0; > + } > + > if (blocksize < PAGE_SIZE) { > ext4_msg(sb, KERN_ERR, "can't mount with " > "dioread_nolock if block size != PAGE_SIZE"); >
On Wed, Jul 31, 2019 at 09:06:00PM +0800, Xiaoguang Wang wrote: > Mount with both dioread_nolock and nodelalloc will result in huge > performance drop, which indeed is an known issue, so before we fix > this issue, currently we disable this behaviour. Below test reproducer > can reveal this performance drop. Is it really worth it to disable this combination? Nothing goes *wrong* per se; it's just slower than we would like. - Ted
hi, > On Wed, Jul 31, 2019 at 09:06:00PM +0800, Xiaoguang Wang wrote: >> Mount with both dioread_nolock and nodelalloc will result in huge >> performance drop, which indeed is an known issue, so before we fix >> this issue, currently we disable this behaviour. Below test reproducer >> can reveal this performance drop. > > Is it really worth it to disable this combination? Nothing goes > *wrong* per se; it's just slower than we would like. Yes, agree, then should we have some places to record this issue? In case somebody looks into it again. Regards, Xiaoguang Wang > > - Ted >
diff --git a/fs/ext4/super.c b/fs/ext4/super.c index 4079605d437a..1a2b2c0cd1b8 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -2098,6 +2098,12 @@ static int parse_options(char *options, struct super_block *sb, int blocksize = BLOCK_SIZE << le32_to_cpu(sbi->s_es->s_log_block_size); + if (!test_opt(sb, DELALLOC)) { + ext4_msg(sb, KERN_ERR, "can't mount with " + "both dioread_nolock and nodelalloc"); + return 0; + } + if (blocksize < PAGE_SIZE) { ext4_msg(sb, KERN_ERR, "can't mount with " "dioread_nolock if block size != PAGE_SIZE");
Mount with both dioread_nolock and nodelalloc will result in huge performance drop, which indeed is an known issue, so before we fix this issue, currently we disable this behaviour. Below test reproducer can reveal this performance drop. mount -o remount,dioread_nolock,delalloc /dev/vdb1 rm -f testfile start_time=$(date +%s) dd if=/dev/zero of=testfile bs=4096 count=$((1024*256)) sync end_time=$(date +%s) echo $((end_time - start_time)) mount -o remount,dioread_nolock,nodelalloc /dev/vdb1 rm -f testfile start_time=$(date +%s) dd if=/dev/zero of=testfile bs=4096 count=$((1024*256)) sync end_time=$(date +%s) echo $((end_time - start_time)) Signed-off-by: Xiaoguang Wang <xiaoguang.wang@linux.alibaba.com> --- fs/ext4/super.c | 6 ++++++ 1 file changed, 6 insertions(+)