All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matthew Wilcox <matthew@wil.cx>
To: Theodore Ts'o <tytso@mit.edu>
Cc: Dave Chinner <david@fromorbit.com>,
	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 08:20:49 -0700	[thread overview]
Message-ID: <20131219152049.GB19166@parisc-linux.org> (raw)
In-Reply-To: <20131219054303.GA4391@thunk.org>

On Thu, Dec 19, 2013 at 12:43:03AM -0500, Theodore Ts'o wrote:
> On Wed, Dec 18, 2013 at 09:12:41PM -0700, Matthew Wilcox wrote:
> > 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).
> 
> If the block was originally allocated via fallocate(2), it will be
> marked as uninitialized.  When you call ext4_get_block(), if the block
> has been allocated, it will be returned --- and ext4_map_block() as
> called by ext4_get_block() does ****not*** clear the uninitialized
> flag.  It can't do so because it would be racy; you can only clear the
> flag once the data blocks has been written.
> 
> As far as patch 3/3, it clears the pages in the page cache, but it
> doesn't zap them in the XIP storage device.  But it only does this on
> the code path when it allocated a block.  But if the block has already
> been previously mapped via fallocate(2), you never hit this section of
> code.

Umm.  That sounds like the real bug then.  Any page returned from
get_xip_mem must be initialised, because we may be about to map it
into userspace.

We could have ext4_get_xip_mem() check buffer_unwritten(); if it's set,
zero the blocks and call ext4_convert_unwritten_extents().  Would that
work?

-- 
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-19 15:20 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
2013-12-19  5:43             ` Theodore Ts'o
2013-12-19 15:20               ` Matthew Wilcox [this message]
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=20131219152049.GB19166@parisc-linux.org \
    --to=matthew@wil.cx \
    --cc=david@fromorbit.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=matthew.r.wilcox@intel.com \
    --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 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.