linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Matthew Wilcox <matthew@wil.cx>
To: Dave Chinner <david@fromorbit.com>
Cc: Ross Zwisler <ross.zwisler@linux.intel.com>,
	linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	carsteno@de.ibm.com, matthew.r.wilcox@intel.com,
	andreas.dilger@intel.com
Subject: Re: [PATCH v2 2/4] ext4: Add XIP functionality
Date: Thu, 5 Dec 2013 21:07:22 -0700	[thread overview]
Message-ID: <20131206040722.GA15325@parisc-linux.org> (raw)
In-Reply-To: <20131206031354.GS10988@dastard>

On Fri, Dec 06, 2013 at 02:13:54PM +1100, Dave Chinner wrote:
> I think I see a significant problem here with XIP write support:
> unwritten extents.
> 
> xip_file_write() has no concept of post IO completion processing -
> it assumes that all that is necessary is to memcpy() the data into
> the backing memory obtained by ->get_xip_mem(), and that's all it
> needs to do.
> 
> For ext4 (and other filesystems that use unwritten extents) they
> need a callback - normally done from bio completion - to run
> transactions to convert extent status from unwritten to written, or
> run other post-IO completion operations.
> 
> I don't see any hooks into ext4 to turn off preallocation (e.g.
> fallocate is explicitly hooked up for XIP) when XIP is in use, so I
> can't see how XIP can work with such filesystem requirements without
> further infrastructure being added. i.e. bypassing the need for the
> page cache does not remove the need to post-IO completion
> notification to the filesystem....

The two are mutually exclusive:

        if (ext4_use_xip(inode->i_sb))
                inode->i_mapping->a_ops = &ext4_xip_aops;
        else if (test_opt(inode->i_sb, DELALLOC))
                inode->i_mapping->a_ops = &ext4_da_aops;
        else
                inode->i_mapping->a_ops = &ext4_aops;

Is it worth implementing delayed allocation support on top of XIP?  Indeed,
what would that *mean*?  Assuming that the backing store is close to DRAM
speeds, we don't want to cache in DRAM first, then copy to the backing
store, we just want to write to the backing store.

> Indeed, for making filesystems like XFS be able to use XIP, we're
> going to need such facilities to be provided by the XIP
> infrastructure....

I have a patch in my development tree right now which changes the
create argument to get_xip_mem into a flags argument, with 'GXM_CREATE'
and 'GXM_HINT' as the first two flags.  Adding a GXM_ALLOC flag would
presumably be enough of a hint to the filesystem that it's time to commit
this range to disk.  Admitedly, it's pre-write and not post-write,
but does that matter when the write is a memcpy?  I must admit to not
quite understanding all 100k+ lines of XFS, so maybe you really do need
to know when the memcpy has finished.

I also don't see a problem with the filesystem either having a wrapper
around xip_file_write or providing its own entire implementation of
->write.  Equally, I'm sure we could add some other callback in, say,
address_space_operations that the XIP code could call after the memcpy
if that's what XFS needs.

-- 
Matthew Wilcox				Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

  reply	other threads:[~2013-12-06  4:07 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-05 20:02 [PATCH v2 0/4] ext4: Add XIP functionality Ross Zwisler
2013-12-05 20:02 ` [PATCH v2 1/4] Fix XIP fault vs truncate race Ross Zwisler
2013-12-05 20:02 ` [PATCH v2 2/4] ext4: Add XIP functionality Ross Zwisler
2013-12-06  3:13   ` Dave Chinner
2013-12-06  4:07     ` Matthew Wilcox [this message]
2013-12-06  5:28       ` Dave Chinner
2013-12-06 20:58     ` Dilger, Andreas
2013-12-09  3:16     ` Ross Zwisler
2013-12-09  8:19       ` Dave Chinner
2013-12-10 16:22         ` Matthew Wilcox
2013-12-10 23:09           ` Dave Chinner
2013-12-05 20:02 ` [PATCH v2 3/4] xip: Add xip_zero_page_range Ross Zwisler
2013-12-05 20:02 ` [PATCH v2 4/4] ext4: Add xip hole punching Ross Zwisler

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=20131206040722.GA15325@parisc-linux.org \
    --to=matthew@wil.cx \
    --cc=andreas.dilger@intel.com \
    --cc=carsteno@de.ibm.com \
    --cc=david@fromorbit.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=matthew.r.wilcox@intel.com \
    --cc=ross.zwisler@linux.intel.com \
    /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).