linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Goldwyn Rodrigues <rgoldwyn@suse.de>
Cc: linux-btrfs@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	Goldwyn Rodrigues <rgoldwyn@suse.com>
Subject: Re: [PATCH 04/15] dax: Introduce IOMAP_F_COW for copy-on-write
Date: Mon, 1 Apr 2019 15:38:52 +1100	[thread overview]
Message-ID: <20190401043851.GO26298@dastard> (raw)
In-Reply-To: <20190326190301.32365-5-rgoldwyn@suse.de>

On Tue, Mar 26, 2019 at 02:02:50PM -0500, Goldwyn Rodrigues wrote:
> From: Goldwyn Rodrigues <rgoldwyn@suse.com>
> 
> The IOMAP_F_COW is a flag to notify dax that it needs to copy
> the data from iomap->cow_addr to iomap->addr, if the start/end
> of I/O are not page aligned.

I see what you are trying to do here, but this is kinda gross.

> This also introduces dax_to_dax_copy() which performs a copy
> from one part of the device to another, to a maximum of one page.
> 
> Question: Using iomap.cow_addr == 0 means the CoW is to be copied
> (or memset) from a hole. Would this be better handled through a flag?

That's what all these checks in the iomap code do:

	if (iomap->type == IOMAP_HOLE || iomap->type == IOMAP_UNWRITTEN)

Oh, wait, you're trying to map two ranges in a single iomap and then
infer state from information that got chucked away.... IOWs, you're
doing it wrong - iomap algorithms are driven by how we manipulate
iomaps to do data operations efficiently, not how we copy data page
by page.

IOWs, what we really should have here is two iomaps - a source
and a destination iomap. The source is a read mapping of the
current address (where we are going to copy the data from), the
destination is the post-cow allocation mapping (where the data
goes).

Now you just copy the data from one map to the other iterating
source mappings until the necessary range of the destination has
been covered.  And you can check if the source map is IOMAP_HOLE or
IOMAP_UNWRITTEN and hence optimise the copy (i.e. zero the new
allocation) before copying in the new data.

Even better, only do the source mapping if the write isn't
page/filesystem block aligned, and hence only do the slow path
copying if a source mapping exists....

> diff --git a/include/linux/iomap.h b/include/linux/iomap.h
> index 0fefb5455bda..391785de1428 100644
> --- a/include/linux/iomap.h
> +++ b/include/linux/iomap.h
> @@ -35,6 +35,7 @@ struct vm_fault;
>  #define IOMAP_F_NEW		0x01	/* blocks have been newly allocated */
>  #define IOMAP_F_DIRTY		0x02	/* uncommitted metadata */
>  #define IOMAP_F_BUFFER_HEAD	0x04	/* file system requires buffer heads */
> +#define IOMAP_F_COW		0x08	/* cow before write */

"Copy on write before write"? :)

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

  parent reply	other threads:[~2019-04-01  4:38 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20190326190301.32365-1-rgoldwyn@suse.de>
2019-03-26 19:02 ` [PATCH 01/15] btrfs: create a mount option for dax Goldwyn Rodrigues
2019-03-26 19:10   ` Matthew Wilcox
2019-03-27 11:00     ` Goldwyn Rodrigues
2019-03-27 12:00       ` Matthew Wilcox
2019-03-27 12:26         ` Goldwyn Rodrigues
2019-03-27 23:31         ` Goldwyn Rodrigues
2019-03-27 17:38     ` Adam Borowski
2019-03-28 14:49   ` David Sterba
2019-03-28 17:28   ` David Sterba
2019-03-28 17:57     ` Darrick J. Wong
2019-04-01 20:43     ` Goldwyn Rodrigues
2019-03-26 19:02 ` [PATCH 02/15] btrfs: Carve out btrfs_get_extent_map_write() out of btrfs_get_blocks_write() Goldwyn Rodrigues
2019-03-26 19:02 ` [PATCH 03/15] btrfs: basic dax read Goldwyn Rodrigues
2019-03-26 19:02 ` [PATCH 04/15] dax: Introduce IOMAP_F_COW for copy-on-write Goldwyn Rodrigues
2019-03-27 17:54   ` Darrick J. Wong
2019-03-27 18:58     ` Goldwyn Rodrigues
2019-03-28 14:45       ` Darrick J. Wong
2019-04-01  4:38   ` Dave Chinner [this message]
2019-04-01 21:41     ` Goldwyn Rodrigues
2019-04-01 23:06       ` Dave Chinner
2019-04-03  1:56         ` Goldwyn Rodrigues
2019-04-03  3:20           ` Dave Chinner
2019-04-07  7:26     ` Christoph Hellwig
2019-03-26 19:02 ` [PATCH 05/15] btrfs: return whether extent is nocow or not Goldwyn Rodrigues
2019-03-31 18:42   ` Nikolay Borisov
2019-03-26 19:02 ` [PATCH 06/15] btrfs: Rename __endio_write_update_ordered() to btrfs_update_ordered_extent() Goldwyn Rodrigues
2019-03-26 19:02 ` [PATCH 07/15] btrfs: add dax write support Goldwyn Rodrigues
2019-03-28 14:53   ` Darrick J. Wong
2019-04-01 20:39     ` Goldwyn Rodrigues
2019-03-26 19:02 ` [PATCH 08/15] dax: add dax_iomap_cow to copy a mmap page before writing Goldwyn Rodrigues
2019-03-28 15:41   ` Darrick J. Wong
2019-03-26 19:02 ` [PATCH 09/15] btrfs: add dax mmap support Goldwyn Rodrigues
2019-03-28 15:45   ` Darrick J. Wong
2019-03-26 19:02 ` [PATCH 10/15] btrfs: Add dax specific address_space_operations Goldwyn Rodrigues
2019-03-26 19:02 ` [PATCH 11/15] fs: dedup file range to use a compare function Goldwyn Rodrigues
2019-03-28 17:04   ` Darrick J. Wong
2019-04-01 20:36     ` Goldwyn Rodrigues
2019-03-26 19:02 ` [PATCH 12/15] btrfs: trace functions for btrfs_iomap_begin/end Goldwyn Rodrigues
2019-03-26 19:02 ` [PATCH 13/15] btrfs: handle dax page zeroing Goldwyn Rodrigues
2019-03-26 19:03 ` [PATCH 14/15] btrfs: Disable dax-based defrag and send Goldwyn Rodrigues
2019-03-26 19:03 ` [PATCH 15/15] btrfs: Writeprotect mmap pages on snapshot Goldwyn Rodrigues
2019-03-28 15:48   ` Darrick J. Wong
2019-03-26 19:09 ` [PATCH v2 00/15] btrfs dax support Goldwyn Rodrigues
2019-03-27 20:14   ` Adam Borowski
2019-03-27 23:26     ` Goldwyn Rodrigues
2019-03-28 10:24       ` [PATCH] btrfs: allow MAP_SYNC mmap Adam Borowski
2019-03-28 10:42         ` Adam Borowski
2019-04-01 20:08         ` Goldwyn Rodrigues

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=20190401043851.GO26298@dastard \
    --to=david@fromorbit.com \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=rgoldwyn@suse.com \
    --cc=rgoldwyn@suse.de \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).