Linux-ext4 Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] ext4: disable mount with both dioread_nolock and nodelalloc
@ 2019-07-31 13:06 Xiaoguang Wang
  2019-08-01  1:57 ` Joseph Qi
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Xiaoguang Wang @ 2019-07-31 13:06 UTC (permalink / raw)
  To: linux-ext4; +Cc: 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");
-- 
2.17.2


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

* Re: [PATCH] ext4: disable mount with both dioread_nolock and nodelalloc
  2019-07-31 13:06 [PATCH] ext4: disable mount with both dioread_nolock and nodelalloc Xiaoguang Wang
@ 2019-08-01  1:57 ` Joseph Qi
  2019-09-02  1:31 ` Xiaoguang Wang
  2019-09-07 16:00 ` Theodore Y. Ts'o
  2 siblings, 0 replies; 5+ messages in thread
From: Joseph Qi @ 2019-08-01  1:57 UTC (permalink / raw)
  To: Xiaoguang Wang, linux-ext4



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");
> 

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

* Re: [PATCH] ext4: disable mount with both dioread_nolock and nodelalloc
  2019-07-31 13:06 [PATCH] ext4: disable mount with both dioread_nolock and nodelalloc Xiaoguang Wang
  2019-08-01  1:57 ` Joseph Qi
@ 2019-09-02  1:31 ` Xiaoguang Wang
  2019-09-07 16:00 ` Theodore Y. Ts'o
  2 siblings, 0 replies; 5+ messages in thread
From: Xiaoguang Wang @ 2019-09-02  1:31 UTC (permalink / raw)
  To: linux-ext4

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");
> 

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

* Re: [PATCH] ext4: disable mount with both dioread_nolock and nodelalloc
  2019-07-31 13:06 [PATCH] ext4: disable mount with both dioread_nolock and nodelalloc Xiaoguang Wang
  2019-08-01  1:57 ` Joseph Qi
  2019-09-02  1:31 ` Xiaoguang Wang
@ 2019-09-07 16:00 ` Theodore Y. Ts'o
  2019-09-09  3:40   ` Xiaoguang Wang
  2 siblings, 1 reply; 5+ messages in thread
From: Theodore Y. Ts'o @ 2019-09-07 16:00 UTC (permalink / raw)
  To: Xiaoguang Wang; +Cc: linux-ext4

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

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

* Re: [PATCH] ext4: disable mount with both dioread_nolock and nodelalloc
  2019-09-07 16:00 ` Theodore Y. Ts'o
@ 2019-09-09  3:40   ` Xiaoguang Wang
  0 siblings, 0 replies; 5+ messages in thread
From: Xiaoguang Wang @ 2019-09-09  3:40 UTC (permalink / raw)
  To: Theodore Y. Ts'o; +Cc: linux-ext4

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
> 

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

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-31 13:06 [PATCH] ext4: disable mount with both dioread_nolock and nodelalloc Xiaoguang Wang
2019-08-01  1:57 ` Joseph Qi
2019-09-02  1:31 ` Xiaoguang Wang
2019-09-07 16:00 ` Theodore Y. Ts'o
2019-09-09  3:40   ` Xiaoguang Wang

Linux-ext4 Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-ext4/0 linux-ext4/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-ext4 linux-ext4/ https://lore.kernel.org/linux-ext4 \
		linux-ext4@vger.kernel.org linux-ext4@archiver.kernel.org
	public-inbox-index linux-ext4

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-ext4


AGPL code for this site: git clone https://public-inbox.org/ public-inbox