Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1342434 > unrolled thread

Re: [PATCH] ext2, ext4: Fix issue with missing journal entry

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH] ext2, ext4: Fix issue with missing journal entry Ross Zwisler <ross.zwisler@linux.intel.com> - 2016-02-24 21:50 +0100
    Re: [PATCH] ext2, ext4: Fix issue with missing journal entry Jan Kara <jack@suse.cz> - 2016-02-25 09:40 +0100
      Re: [PATCH] ext2, ext4: Fix issue with missing journal entry Theodore Ts'o <tytso@mit.edu> - 2016-02-27 20:30 +0100

#1342434 — Re: [PATCH] ext2, ext4: Fix issue with missing journal entry

On Thu, Jan 28, 2016 at 09:32:11AM -0700, Ross Zwisler wrote:
> On Thu, Jan 28, 2016 at 02:16:30PM +0100, Jan Kara wrote:
> > On Wed 27-01-16 12:01:48, Ross Zwisler wrote:
> > > As it is currently written ext4_dax_mkwrite() assumes that the call into
> > > __dax_mkwrite() will not have to do a block allocation so it doesn't create
> > > a journal entry.  For a read that creates a zero page to cover a hole
> > > followed by a write that actually allocates storage this is incorrect.  The
> > > ext4_dax_mkwrite() -> __dax_mkwrite() -> __dax_fault() path calls
> > > get_blocks() to allocate storage.
> > > 
> > > Fix this by having the ->page_mkwrite fault handler call ext4_dax_fault()
> > > as this function already has all the logic needed to allocate a journal
> > > entry and call __dax_fault().
> > > 
> > > Also update the ext2 fault handlers in this same way to remove duplicate
> > > code and keep the logic between ext2 and ext4 the same.
> > > 
> > > Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> > 
> > Ah, ok, you are right. The patch looks good but Matthew is reworking the
> > area more (so ext4_da_mkwrite() is likely to return) so it this worth it?
> > Or do you expect Matthew's patches to land much later?
> 
> Yep, Matthew is in the process of reworking all of the DAX fault handling.
> 
> I was thinking that we might want to take this patch for v4.5, since it fixes
> a bug that I'm guessing could lead to some sort of corruption (lack of a
> journal entry entry for an allocating write), and then Matthew's reworks would
> land in v4.6?

Hey Jan,

Looks like this patch didn't ever get merged for v4.5?  Is it still queued for
v4.6?

Thanks,
- Ross

[toc] | [next] | [standalone]

#1342976

On Wed 24-02-16 13:44:46, Ross Zwisler wrote:
> On Thu, Jan 28, 2016 at 09:32:11AM -0700, Ross Zwisler wrote:
> > On Thu, Jan 28, 2016 at 02:16:30PM +0100, Jan Kara wrote:
> > > On Wed 27-01-16 12:01:48, Ross Zwisler wrote:
> > > > As it is currently written ext4_dax_mkwrite() assumes that the call into
> > > > __dax_mkwrite() will not have to do a block allocation so it doesn't create
> > > > a journal entry.  For a read that creates a zero page to cover a hole
> > > > followed by a write that actually allocates storage this is incorrect.  The
> > > > ext4_dax_mkwrite() -> __dax_mkwrite() -> __dax_fault() path calls
> > > > get_blocks() to allocate storage.
> > > > 
> > > > Fix this by having the ->page_mkwrite fault handler call ext4_dax_fault()
> > > > as this function already has all the logic needed to allocate a journal
> > > > entry and call __dax_fault().
> > > > 
> > > > Also update the ext2 fault handlers in this same way to remove duplicate
> > > > code and keep the logic between ext2 and ext4 the same.
> > > > 
> > > > Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> > > 
> > > Ah, ok, you are right. The patch looks good but Matthew is reworking the
> > > area more (so ext4_da_mkwrite() is likely to return) so it this worth it?
> > > Or do you expect Matthew's patches to land much later?
> > 
> > Yep, Matthew is in the process of reworking all of the DAX fault handling.
> > 
> > I was thinking that we might want to take this patch for v4.5, since it fixes
> > a bug that I'm guessing could lead to some sort of corruption (lack of a
> > journal entry entry for an allocating write), and then Matthew's reworks would
> > land in v4.6?
> 
> Hey Jan,
> 
> Looks like this patch didn't ever get merged for v4.5?  Is it still queued for
> v4.6?

Ted has been pretty busy lately and we probably didn't make it sufficiently
clear that he should pick up this patch. Ted, can you please pick up this
patch and push it to 4.5? Thanks. Feel free to add my:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[toc] | [prev] | [next] | [standalone]

#1345070

On Thu, Feb 25, 2016 at 09:37:28AM +0100, Jan Kara wrote:
> > Looks like this patch didn't ever get merged for v4.5?  Is it still queued for
> > v4.6?
> 
> Ted has been pretty busy lately and we probably didn't make it sufficiently
> clear that he should pick up this patch. Ted, can you please pick up this
> patch and push it to 4.5? Thanks. Feel free to add my:
> 
> Reviewed-by: Jan Kara <jack@suse.cz>

Sorry, yes, I missed this one since most of the DAX patches have been
going through other trees.   I've just sent a pull request to Linus.

      	      	    	     	       - Ted

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web