All of lore.kernel.org
 help / color / mirror / Atom feed
From: Theodore Ts'o <tytso@mit.edu>
To: Lukas Czerner <lczerner@redhat.com>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	linux-fsdevel@vger.kernel.org, linux-ext4@vger.kernel.org,
	akpm@linux-foundation.org, hughd@google.com
Subject: Re: [PATCH v4 15/20] ext4: use ext4_zero_partial_blocks in punch_hole
Date: Thu, 13 Jun 2013 23:01:54 -0400	[thread overview]
Message-ID: <20130614030154.GA18731@thunk.org> (raw)
In-Reply-To: <1368549454-8930-16-git-send-email-lczerner@redhat.com>

On Tue, May 14, 2013 at 06:37:29PM +0200, Lukas Czerner wrote:
> We're doing to get rid of ext4_discard_partial_page_buffers() since it is
> duplicating some code and also partially duplicating work of
> truncate_pagecache_range(), moreover the old implementation was much
> clearer.
> 
> Now when the truncate_inode_pages_range() can handle truncating non page
> aligned regions we can use this to invalidate and zero out block aligned
> region of the punched out range and then use ext4_block_truncate_page()
> to zero the unaligned blocks on the start and end of the range. This
> will greatly simplify the punch hole code. Moreover after this commit we
> can get rid of the ext4_discard_partial_page_buffers() completely.
> 
> We also introduce function ext4_prepare_punch_hole() to do come common
> operations before we attempt to do the actual punch hole on
> indirect or extent file which saves us some code duplication.
> 
> This has been tested on ppc64 with 1k block size with fsx and xfstests
> without any problems.
> 
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>

Hi Lukas,

I've been seeing xfstests failures on test generic/300 in nojournal
mode.

BEGIN TEST: Ext4 4k block w/ no journal Thu Jun 13 22:38:47 EDT 2013
Device: /dev/vdb
mk2fs options: -q -O ^has_journal
mount options: -o block_validity,noload
FSTYP         -- ext4
PLATFORM      -- Linux/i686 candygram 3.10.0-rc2-00477-g1e1cad7
MKFS_OPTIONS  -- -q -O ^has_journal /dev/vdc
MOUNT_OPTIONS -- -o acl,user_xattr -o block_validity,noload /dev/vdc /vdc

generic/300		[20:42:18][  116.877278] fio (3320) used greatest stack depth: 5580 bytes left
[  116.967122] fio (3321) used greatest stack depth: 5560 bytes left
[  117.573861] fio (3325) used greatest stack depth: 5504 bytes left
 [20:44:01] [failed, exit status 1] - output mismatch (see /root/xfstests/results/generic/300.out.bad)
    --- tests/generic/300.out	 2013-06-04 22:42:55.000000000 -0400
    +++ /root/xfstests/results/generic/300.out.bad	       2013-06-13 20:44:01.306666665 -0400
    @@ -2,3 +2,4 @@
     
     Run fio with random aio-dio pattern
     
    +_check_generic_filesystem: filesystem on /dev/vdc is inconsistent (see /root/xfstests/results/generic/300.full)
     ...
     (Run 'diff -u tests/generic/300.out /root/xfstests/results/generic/300.out.bad' to see the entire diff)

It bisects down to this patch, and if I take the dev branch, and
revert patches #15 through #19 in this series, the problem goes away.

Can you investigate and recommend a better fix?

Thanks,

					- Ted

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

WARNING: multiple messages have this Message-ID (diff)
From: "Theodore Ts'o" <tytso@mit.edu>
To: Lukas Czerner <lczerner@redhat.com>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	linux-fsdevel@vger.kernel.org, linux-ext4@vger.kernel.org,
	akpm@linux-foundation.org, hughd@google.com
Subject: Re: [PATCH v4 15/20] ext4: use ext4_zero_partial_blocks in punch_hole
Date: Thu, 13 Jun 2013 23:01:54 -0400	[thread overview]
Message-ID: <20130614030154.GA18731@thunk.org> (raw)
In-Reply-To: <1368549454-8930-16-git-send-email-lczerner@redhat.com>

On Tue, May 14, 2013 at 06:37:29PM +0200, Lukas Czerner wrote:
> We're doing to get rid of ext4_discard_partial_page_buffers() since it is
> duplicating some code and also partially duplicating work of
> truncate_pagecache_range(), moreover the old implementation was much
> clearer.
> 
> Now when the truncate_inode_pages_range() can handle truncating non page
> aligned regions we can use this to invalidate and zero out block aligned
> region of the punched out range and then use ext4_block_truncate_page()
> to zero the unaligned blocks on the start and end of the range. This
> will greatly simplify the punch hole code. Moreover after this commit we
> can get rid of the ext4_discard_partial_page_buffers() completely.
> 
> We also introduce function ext4_prepare_punch_hole() to do come common
> operations before we attempt to do the actual punch hole on
> indirect or extent file which saves us some code duplication.
> 
> This has been tested on ppc64 with 1k block size with fsx and xfstests
> without any problems.
> 
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>

Hi Lukas,

I've been seeing xfstests failures on test generic/300 in nojournal
mode.

BEGIN TEST: Ext4 4k block w/ no journal Thu Jun 13 22:38:47 EDT 2013
Device: /dev/vdb
mk2fs options: -q -O ^has_journal
mount options: -o block_validity,noload
FSTYP         -- ext4
PLATFORM      -- Linux/i686 candygram 3.10.0-rc2-00477-g1e1cad7
MKFS_OPTIONS  -- -q -O ^has_journal /dev/vdc
MOUNT_OPTIONS -- -o acl,user_xattr -o block_validity,noload /dev/vdc /vdc

generic/300		[20:42:18][  116.877278] fio (3320) used greatest stack depth: 5580 bytes left
[  116.967122] fio (3321) used greatest stack depth: 5560 bytes left
[  117.573861] fio (3325) used greatest stack depth: 5504 bytes left
 [20:44:01] [failed, exit status 1] - output mismatch (see /root/xfstests/results/generic/300.out.bad)
    --- tests/generic/300.out	 2013-06-04 22:42:55.000000000 -0400
    +++ /root/xfstests/results/generic/300.out.bad	       2013-06-13 20:44:01.306666665 -0400
    @@ -2,3 +2,4 @@
     
     Run fio with random aio-dio pattern
     
    +_check_generic_filesystem: filesystem on /dev/vdc is inconsistent (see /root/xfstests/results/generic/300.full)
     ...
     (Run 'diff -u tests/generic/300.out /root/xfstests/results/generic/300.out.bad' to see the entire diff)

It bisects down to this patch, and if I take the dev branch, and
revert patches #15 through #19 in this series, the problem goes away.

Can you investigate and recommend a better fix?

Thanks,

					- Ted

  reply	other threads:[~2013-06-14  3:01 UTC|newest]

Thread overview: 73+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-14 16:37 [PATCH v4 00/20] change invalidatepage prototype to accept length Lukas Czerner
2013-05-14 16:37 ` Lukas Czerner
2013-05-14 16:37 ` [PATCH v4 01/20] mm: " Lukas Czerner
2013-05-14 16:37   ` Lukas Czerner
2013-05-14 16:37 ` [PATCH v4 02/20] jbd2: change jbd2_journal_invalidatepage " Lukas Czerner
2013-05-14 16:37   ` Lukas Czerner
2013-05-14 16:37 ` [PATCH v4 03/20] ext4: use ->invalidatepage() length argument Lukas Czerner
2013-05-14 16:37   ` Lukas Czerner
2013-05-14 16:37 ` [PATCH v4 04/20] jbd: change journal_invalidatepage() to accept length Lukas Czerner
2013-05-14 16:37   ` Lukas Czerner
2013-05-14 16:37 ` [PATCH v4 05/20] xfs: use ->invalidatepage() length argument Lukas Czerner
2013-05-14 16:37   ` Lukas Czerner
2013-05-14 16:37   ` Lukas Czerner
2013-05-15  7:30   ` Dave Chinner
2013-05-15  7:30     ` Dave Chinner
2013-05-15  7:30     ` Dave Chinner
2013-05-14 16:37 ` [PATCH v4 06/20] ocfs2: " Lukas Czerner
2013-05-14 16:37   ` Lukas Czerner
2013-05-14 16:37 ` [PATCH v4 07/20] ceph: " Lukas Czerner
2013-05-14 16:37   ` Lukas Czerner
2013-05-14 16:37 ` [Cluster-devel] [PATCH v4 08/20] gfs2: " Lukas Czerner
2013-05-14 16:37   ` Lukas Czerner
2013-05-14 16:37   ` Lukas Czerner
2013-05-14 16:37 ` [PATCH v4 09/20] reiserfs: " Lukas Czerner
2013-05-14 16:37   ` Lukas Czerner
2013-05-14 16:37 ` [PATCH v4 10/20] mm: teach truncate_inode_pages_range() to handle non page aligned ranges Lukas Czerner
2013-05-14 16:37   ` Lukas Czerner
2013-05-14 16:37 ` [PATCH v4 11/20] Revert "ext4: remove no longer used functions in inode.c" Lukas Czerner
2013-05-14 16:37   ` Lukas Czerner
2013-05-14 16:37 ` [PATCH v4 12/20] ext4: Call ext4_jbd2_file_inode() after zeroing block Lukas Czerner
2013-05-14 16:37   ` Lukas Czerner
2013-05-14 16:37 ` [PATCH v4 13/20] Revert "ext4: fix fsx truncate failure" Lukas Czerner
2013-05-14 16:37   ` Lukas Czerner
2013-05-14 16:37 ` [PATCH v4 14/20] ext4: truncate_inode_pages() in orphan cleanup path Lukas Czerner
2013-05-14 16:37   ` Lukas Czerner
2013-05-14 16:37 ` [PATCH v4 15/20] ext4: use ext4_zero_partial_blocks in punch_hole Lukas Czerner
2013-05-14 16:37   ` Lukas Czerner
2013-06-14  3:01   ` Theodore Ts'o [this message]
2013-06-14  3:01     ` Theodore Ts'o
2013-06-14 10:16     ` Lukáš Czerner
2013-06-14 10:16       ` Lukáš Czerner
2013-06-14 13:39       ` Ted Ts'o
2013-06-17  9:08         ` Lukáš Czerner
2013-06-17 12:25           ` Theodore Ts'o
2013-06-17 12:46             ` Lukáš Czerner
2013-06-17 12:30         ` Lukáš Czerner
2013-06-19 16:37     ` Lukáš Czerner
2013-06-19 16:37       ` Lukáš Czerner
2013-06-19 23:42       ` Theodore Ts'o
2013-06-20  9:12         ` Lukáš Czerner
2013-06-20  9:14         ` [PATCH] ext4: Only zero partial blocks in ext4_zero_partial_blocks() Lukas Czerner
2013-06-20 15:01           ` Theodore Ts'o
2013-05-14 16:37 ` [PATCH v4 16/20] ext4: remove unused discard_partial_page_buffers Lukas Czerner
2013-05-14 16:37   ` Lukas Czerner
2013-05-14 16:37 ` [PATCH v4 17/20] ext4: remove unused code from ext4_remove_blocks() Lukas Czerner
2013-05-14 16:37   ` Lukas Czerner
2013-05-14 16:37 ` [PATCH v4 18/20] ext4: update ext4_ext_remove_space trace point Lukas Czerner
2013-05-14 16:37   ` Lukas Czerner
2013-05-14 16:37 ` [PATCH v4 19/20] ext4: make punch hole code path work with bigalloc Lukas Czerner
2013-05-14 16:37   ` Lukas Czerner
2013-05-14 16:37 ` [PATCH v4 20/20] ext4: Allow punch hole with bigalloc enabled Lukas Czerner
2013-05-14 16:37   ` Lukas Czerner
2013-05-31 15:14   ` Theodore Ts'o
2013-05-31 15:14     ` Theodore Ts'o
2013-06-05 10:04     ` Lukáš Czerner
2013-06-05 10:04       ` Lukáš Czerner
2013-06-11 12:54     ` Lukáš Czerner
2013-06-18 12:34       ` Tomas Racek
2013-05-21 14:34 ` [PATCH v4 00/20] change invalidatepage prototype to accept length Lukáš Czerner
2013-05-21 14:34   ` Lukáš Czerner
2013-05-28 11:21   ` Theodore Ts'o
2013-05-28 11:21     ` Theodore Ts'o
2013-05-28 11:21     ` Theodore Ts'o

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=20130614030154.GA18731@thunk.org \
    --to=tytso@mit.edu \
    --cc=akpm@linux-foundation.org \
    --cc=hughd@google.com \
    --cc=lczerner@redhat.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.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 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.