linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Matthew Wilcox <matthew@wil.cx>
Cc: Theodore Ts'o <tytso@mit.edu>,
	Matthew Wilcox <matthew.r.wilcox@intel.com>,
	linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH v3 0/3] Add XIP support to ext4
Date: Thu, 19 Dec 2013 15:37:56 +1100	[thread overview]
Message-ID: <20131219043756.GY31386@dastard> (raw)
In-Reply-To: <20131219041240.GA19166@parisc-linux.org>

On Wed, Dec 18, 2013 at 09:12:41PM -0700, Matthew Wilcox wrote:
> On Wed, Dec 18, 2013 at 09:07:59PM -0500, Theodore Ts'o wrote:
> > On Wed, Dec 18, 2013 at 07:27:49AM -0700, Matthew Wilcox wrote:
> > > 
> > > I think there is a callback in xip_file_write(), and it's get_xip_mem().
> > > From what you're saying, it sounds like it's just not doing enough.
> > 
> > The problem is that git_xip_mem() is called before we write to the
> > memory, right?
> > 
> > We need to convert the uninit extents to be marked as initialized in
> > *after* the write has been sent to the storage medium.
> 
> Now that I've spent the best part of a day looking at the ext4 code, I
> still don't think there's a problem here.  With the way the XIP code is
> currently written (calling ext4_get_block with create=1), we won't get an
> uninitialised extent in the caller.  Instead, we'll get one that's been
> zeroed (the zeroing is part of patch 3/3 and done only for xip files).

You will with XFS. And you will when an application uses fallocate()
to allocate the space before the write(2) call. So it's irrelevant
what the write call does - the generic infrastructure needs to
handle the fact that it can be writing into an unwritten region and
be required to call a filesystem specific IO completion handler to
deal with it.

> As I understand it, when ext4 uses direct I/O, it can pass
> ext4_get_block_write() as the get_block method, which uses the
> magic EXT4_GET_BLOCKS_IO_CREATE_EXT flag to permit the allocation of
> uninitialised extents.  But the regular ext4_get_block cannot create
> uninitialised extents (it can return them in the case of create=0,
> and we handle that correctly as a hole).

Stop thinking that ext4 is the entire world. XFS creates unwritten
extents via the direct IO getblock callout when create=1 is set an
dthe write lands in a hole. It then uses the IO completion callout
ot convert them to written extents once the write IO is complete.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

  reply	other threads:[~2013-12-19  4:38 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-17 19:18 [PATCH v3 0/3] Add XIP support to ext4 Matthew Wilcox
2013-12-17 19:18 ` [PATCH v3 1/3] Fix XIP fault vs truncate race Matthew Wilcox
2013-12-17 19:18 ` [PATCH v3 2/3] xip: Add xip_zero_page_range Matthew Wilcox
2013-12-17 19:18 ` [PATCH v3 3/3] ext4: Add XIP functionality Matthew Wilcox
2013-12-17 22:30 ` [PATCH v3 0/3] Add XIP support to ext4 Dave Chinner
2013-12-18  2:31   ` Matthew Wilcox
2013-12-18  5:01     ` Theodore Ts'o
2013-12-18 14:27       ` Matthew Wilcox
2013-12-19  2:07         ` Theodore Ts'o
2013-12-19  4:12           ` Matthew Wilcox
2013-12-19  4:37             ` Dave Chinner [this message]
2013-12-19  5:43             ` Theodore Ts'o
2013-12-19 15:20               ` Matthew Wilcox
2013-12-19 16:17                 ` Theodore Ts'o
2013-12-19 17:12                   ` Matthew Wilcox
2013-12-19 17:18                     ` Theodore Ts'o
2013-12-20 18:17                       ` Matthew Wilcox
2013-12-20 19:34                         ` Theodore Ts'o
2013-12-20 20:11                           ` Matthew Wilcox
2013-12-23  3:36                             ` Dave Chinner
2013-12-23  3:45                               ` Matthew Wilcox
2013-12-23  4:32                                 ` Dave Chinner
2013-12-23  6:56                                 ` Dave Chinner
2013-12-23 14:51                                   ` Theodore Ts'o
2013-12-23  3:16                         ` Dave Chinner
2013-12-24 16:27                           ` Matthew Wilcox
2013-12-18 12:33     ` Dave Chinner
2013-12-18 15:22       ` Matthew Wilcox
2013-12-19  0:48         ` Dave Chinner
2013-12-19  1:05           ` Matthew Wilcox
2013-12-19  1:58             ` Dave Chinner
2013-12-19 15:32               ` Matthew Wilcox
2013-12-19 23:46                 ` Dave Chinner
2013-12-20 16:45                   ` Matthew Wilcox
2013-12-23  4:14                     ` Dave Chinner
2013-12-18 18:13   ` Eric Sandeen

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=20131219043756.GY31386@dastard \
    --to=david@fromorbit.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=matthew.r.wilcox@intel.com \
    --cc=matthew@wil.cx \
    --cc=tytso@mit.edu \
    /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).