| Message ID | 20171023222948.10648-1-nefelim4ag@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Gentle ping 2017-10-24 1:29 GMT+03:00 Timofey Titovets <nefelim4ag@gmail.com>: > We need to call extent_range_clear_dirty_for_io() > on compression range to prevent application from changing > page content, while pages compressing. > > extent_range_clear_dirty_for_io() run on each loop iteration, > "(end - start)" can be much (up to 1024 times) bigger > then compression range (BTRFS_MAX_UNCOMPRESSED). > > That produce extra calls to page managment code. > > Fix that behaviour by call extent_range_clear_dirty_for_io() > only once. > > v1 -> v2: > - Make that more obviously and more safeprone > > v2 -> v3: > - Rebased on: > Btrfs: compress_file_range() remove dead variable num_bytes > - Update change log > - Add comments > > v3 -> v4: > - Rebased on: kdave for-next > - To avoid dirty bit clear/set behaviour change > call clear_bit once, istead of per compression range > > Signed-off-by: Timofey Titovets <nefelim4ag@gmail.com> > --- > fs/btrfs/inode.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c > index b93fe05a39c7..5816dd3cb6e6 100644 > --- a/fs/btrfs/inode.c > +++ b/fs/btrfs/inode.c > @@ -536,8 +536,10 @@ static noinline void compress_file_range(struct inode *inode, > * If the compression fails for any reason, we set the pages > * dirty again later on. > */ > - extent_range_clear_dirty_for_io(inode, start, end); > - redirty = 1; > + if (!redirty) { > + extent_range_clear_dirty_for_io(inode, start, end); > + redirty = 1; > + } > > /* Compression level is applied here and only here */ > ret = btrfs_compress_pages( > -- > 2.14.2
On Tue, Oct 24, 2017 at 01:29:48AM +0300, Timofey Titovets wrote: > We need to call extent_range_clear_dirty_for_io() > on compression range to prevent application from changing > page content, while pages compressing. > > extent_range_clear_dirty_for_io() run on each loop iteration, > "(end - start)" can be much (up to 1024 times) bigger > then compression range (BTRFS_MAX_UNCOMPRESSED). > > That produce extra calls to page managment code. > > Fix that behaviour by call extent_range_clear_dirty_for_io() > only once. > > v1 -> v2: > - Make that more obviously and more safeprone > > v2 -> v3: > - Rebased on: > Btrfs: compress_file_range() remove dead variable num_bytes > - Update change log > - Add comments > > v3 -> v4: > - Rebased on: kdave for-next > - To avoid dirty bit clear/set behaviour change > call clear_bit once, istead of per compression range This version looks safe to me. There's no functional difference if we redirty the pages repeatedly, but there's probably a noticeable runtime difference if we do it just once. Reviewed-by: David Sterba <dsterba@suse.com> -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index b93fe05a39c7..5816dd3cb6e6 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -536,8 +536,10 @@ static noinline void compress_file_range(struct inode *inode, * If the compression fails for any reason, we set the pages * dirty again later on. */ - extent_range_clear_dirty_for_io(inode, start, end); - redirty = 1; + if (!redirty) { + extent_range_clear_dirty_for_io(inode, start, end); + redirty = 1; + } /* Compression level is applied here and only here */ ret = btrfs_compress_pages(
We need to call extent_range_clear_dirty_for_io() on compression range to prevent application from changing page content, while pages compressing. extent_range_clear_dirty_for_io() run on each loop iteration, "(end - start)" can be much (up to 1024 times) bigger then compression range (BTRFS_MAX_UNCOMPRESSED). That produce extra calls to page managment code. Fix that behaviour by call extent_range_clear_dirty_for_io() only once. v1 -> v2: - Make that more obviously and more safeprone v2 -> v3: - Rebased on: Btrfs: compress_file_range() remove dead variable num_bytes - Update change log - Add comments v3 -> v4: - Rebased on: kdave for-next - To avoid dirty bit clear/set behaviour change call clear_bit once, istead of per compression range Signed-off-by: Timofey Titovets <nefelim4ag@gmail.com> --- fs/btrfs/inode.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) -- 2.14.2 -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html