linux-next: manual merge of the f2fs tree with the ext4 tree

linux-next: manual merge of the f2fs tree with the ext4 tree

[Stephen Rothwell]

Hi Jaegeuk,

Today's linux-next merge of the f2fs tree got a conflict in:

  fs/ext4/ext4.h

between commit:

  c8585c6fcaf2 ("ext4: fix races between changing inode journal mode and ext4_writepages")

from the ext4 tree and commit:

  a618a2a1dda4 ("ext4 crypto: migrate into vfs's crypto engine")

from the f2fs tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

P.S. I would expect to see a Reviewed-by or Acked-by from the ext4
maintainer on that f2fs tree commit ...

-- 
Cheers,
Stephen Rothwell

diff --cc fs/ext4/ext4.h
index ba5aecc07fbc,91b62e54ef51..000000000000
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@@ -32,8 -32,8 +32,9 @@@
  #include <linux/percpu_counter.h>
  #include <linux/ratelimit.h>
  #include <crypto/hash.h>
+ #include <linux/fscrypto.h>
  #include <linux/falloc.h>
 +#include <linux/percpu-rwsem.h>
  #ifdef __KERNEL__
  #include <linux/compat.h>
  #endif
@@@ -1509,9 -1498,10 +1502,13 @@@ struct ext4_sb_info 
  	struct ratelimit_state s_err_ratelimit_state;
  	struct ratelimit_state s_warning_ratelimit_state;
  	struct ratelimit_state s_msg_ratelimit_state;
 +
 +	/* Barrier between changing inodes' journal flags and writepages ops. */
 +	struct percpu_rw_semaphore s_journal_flag_rwsem;
+ #ifdef CONFIG_EXT4_FS_ENCRYPTION
+ 	u8 key_prefix[EXT4_KEY_DESC_PREFIX_SIZE];
+ 	u8 key_prefix_size;
+ #endif
  };
  
  static inline struct ext4_sb_info *EXT4_SB(struct super_block *sb)

Re: linux-next: manual merge of the f2fs tree with the ext4 tree

[Jaegeuk Kim]

Hi Stephen,

Thank you for the notice.
I've been waiting for a comment about the below patch targeted to v4.7 from Ted.
Meanwhile, I intended to prepare -next for that patch in advance.
Surely, once I get a sense that I need to consider v4.8, I'll drop this patch
for -next right away.

Thanks,

On Mon, May 09, 2016 at 09:36:46AM +1000, Stephen Rothwell wrote:
> Hi Jaegeuk,
> 
> Today's linux-next merge of the f2fs tree got a conflict in:
> 
>   fs/ext4/ext4.h
> 
> between commit:
> 
>   c8585c6fcaf2 ("ext4: fix races between changing inode journal mode and ext4_writepages")
> 
> from the ext4 tree and commit:
> 
>   a618a2a1dda4 ("ext4 crypto: migrate into vfs's crypto engine")
> 
> from the f2fs tree.
> 
> I fixed it up (see below) and can carry the fix as necessary. This
> is now fixed as far as linux-next is concerned, but any non trivial
> conflicts should be mentioned to your upstream maintainer when your tree
> is submitted for merging.  You may also want to consider cooperating
> with the maintainer of the conflicting tree to minimise any particularly
> complex conflicts.
> 
> P.S. I would expect to see a Reviewed-by or Acked-by from the ext4
> maintainer on that f2fs tree commit ...
> 
> -- 
> Cheers,
> Stephen Rothwell
> 
> diff --cc fs/ext4/ext4.h
> index ba5aecc07fbc,91b62e54ef51..000000000000
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@@ -32,8 -32,8 +32,9 @@@
>   #include <linux/percpu_counter.h>
>   #include <linux/ratelimit.h>
>   #include <crypto/hash.h>
> + #include <linux/fscrypto.h>
>   #include <linux/falloc.h>
>  +#include <linux/percpu-rwsem.h>
>   #ifdef __KERNEL__
>   #include <linux/compat.h>
>   #endif
> @@@ -1509,9 -1498,10 +1502,13 @@@ struct ext4_sb_info 
>   	struct ratelimit_state s_err_ratelimit_state;
>   	struct ratelimit_state s_warning_ratelimit_state;
>   	struct ratelimit_state s_msg_ratelimit_state;
>  +
>  +	/* Barrier between changing inodes' journal flags and writepages ops. */
>  +	struct percpu_rw_semaphore s_journal_flag_rwsem;
> + #ifdef CONFIG_EXT4_FS_ENCRYPTION
> + 	u8 key_prefix[EXT4_KEY_DESC_PREFIX_SIZE];
> + 	u8 key_prefix_size;
> + #endif
>   };
>   
>   static inline struct ext4_sb_info *EXT4_SB(struct super_block *sb)

Re: linux-next: manual merge of the f2fs tree with the ext4 tree

[Theodore Ts'o]

On Mon, May 09, 2016 at 10:15:03AM -0700, Jaegeuk Kim wrote:
> Hi Stephen,
> 
> Thank you for the notice.
> I've been waiting for a comment about the below patch targeted to v4.7 from Ted.
> Meanwhile, I intended to prepare -next for that patch in advance.
> Surely, once I get a sense that I need to consider v4.8, I'll drop this patch
> for -next right away.

Yeah, I think it would be better for me to take the ext4 "migrate into
vfs's crypto engine" patch, and at this point, for 4.8.

Sorry, I just ran out of time to try to verify that the patch wouldn't
break anything, and given that we're going to need to wait for
"fscrypto/f2fs: allow fs-specific key prefix for fs encryption" to go
upstream.

Do you have any other planned changes for the fscrypto tree planned
for 4.8.  If not, then perhaps it will be easier if I take the pen for
any changes needed for fs/crypto, and moving forward, we probably need
to find ways to make changes where commits specific for fs/crypto
should be isolated from ext4 or f2fs changes, and as much as possible
to be backwards compatible so that as we add new features to
fs/crypto, we don't need to synchronize changes across multiple file
systems.

Cheers,

						- Ted

Re: linux-next: manual merge of the f2fs tree with the ext4 tree

[Jaegeuk Kim]

On Mon, May 16, 2016 at 05:30:28PM -0400, Theodore Ts'o wrote:
> On Mon, May 09, 2016 at 10:15:03AM -0700, Jaegeuk Kim wrote:
> > Hi Stephen,
> > 
> > Thank you for the notice.
> > I've been waiting for a comment about the below patch targeted to v4.7 from Ted.
> > Meanwhile, I intended to prepare -next for that patch in advance.
> > Surely, once I get a sense that I need to consider v4.8, I'll drop this patch
> > for -next right away.
> 
> Yeah, I think it would be better for me to take the ext4 "migrate into
> vfs's crypto engine" patch, and at this point, for 4.8.

Fair enough.

> Sorry, I just ran out of time to try to verify that the patch wouldn't
> break anything, and given that we're going to need to wait for
> "fscrypto/f2fs: allow fs-specific key prefix for fs encryption" to go
> upstream.

Agreed. IIUC, let me push the fscrypto/f2fs patch to v4.7 first?

> Do you have any other planned changes for the fscrypto tree planned
> for 4.8.  If not, then perhaps it will be easier if I take the pen for
> any changes needed for fs/crypto, and moving forward, we probably need
> to find ways to make changes where commits specific for fs/crypto
> should be isolated from ext4 or f2fs changes, and as much as possible
> to be backwards compatible so that as we add new features to
> fs/crypto, we don't need to synchronize changes across multiple file
> systems.

I have no planned patch right now, and of course, it must have no problem for
you to treat with further patches.
Also, let me take a look at any missing part again, regarding to your concerns.

Thanks,

Re: linux-next: manual merge of the f2fs tree with the ext4 tree

[Theodore Ts'o]

On Mon, May 16, 2016 at 03:22:41PM -0700, Jaegeuk Kim wrote:
> > Sorry, I just ran out of time to try to verify that the patch wouldn't
> > break anything, and given that we're going to need to wait for
> > "fscrypto/f2fs: allow fs-specific key prefix for fs encryption" to go
> > upstream.
> 
> Agreed. IIUC, let me push the fscrypto/f2fs patch to v4.7 first?

Right --- that's in linux-next already, right?  And currently it's a
combined fscrypto/f2fs patch, which is why I suspect letting it go
into v4.7 first makes sense.  I'll make sure the ext4 move to
fs/crypto will be one of the first development patches for 4.8 (modulo
any urgent bug fixes that need to go into 4.7 final first).

> I have no planned patch right now, and of course, it must have no problem for
> you to treat with further patches.
> Also, let me take a look at any missing part again, regarding to your concerns.

I'm sure there may be some missing pieces around using file system
level crypto for the desktop / server use case.  Some of them are in
how we handle removable thumb drives, for example.

There are definitely some missing pieces about how to handle removable
SD cards for Android, as well, including some kernel-side patches that
are currently living in the unstable portion of the ext4 patch queue.
We never got the design, implementation and kernel<->userspace API's
fully baked, so that's not going upstream any time soon, but all of
this means that we will need to figure out what's the best way to
develop, test, and push fs/crypto changes in the long term.  This may
mean a new git tree with shared maintenance, as one way that we could
do things.

       		       	       	       	     - Ted

linux-next: manual merge of the f2fs tree with the ext4 tree

[Stephen Rothwell]

Hi Jaegeuk,

Today's linux-next merge of the f2fs tree got a conflict in:

  fs/f2fs/dir.c

between commit:

  59aa5a3aeead ("fscrypto: make filename crypto functions return 0 on success")

from the ext4 tree and commit:

  e06f86e61d7a ("f2fs crypto: avoid unneeded memory allocation in ->readdir")

from the f2fs tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc fs/f2fs/dir.c
index 8716943335b1,2fb20fc58346..000000000000
--- a/fs/f2fs/dir.c
+++ b/fs/f2fs/dir.c
@@@ -786,19 -813,12 +813,10 @@@ bool f2fs_fill_dentries(struct dir_cont
  
  		if (f2fs_encrypted_inode(d->inode)) {
  			int save_len = fstr->len;
- 			int err;
 -			int ret;
  
- 			de_name.name = f2fs_kmalloc(de_name.len, GFP_NOFS);
- 			if (!de_name.name)
- 				return false;
- 
- 			memcpy(de_name.name, d->filename[bit_pos], de_name.len);
- 
- 			err = fscrypt_fname_disk_to_usr(d->inode,
 -			ret = fscrypt_fname_disk_to_usr(d->inode,
++			if (fscrypt_fname_disk_to_usr(d->inode,
  						(u32)de->hash_code, 0,
--						&de_name, fstr);
- 			kfree(de_name.name);
- 			if (err)
 -			if (ret < 0)
++						&de_name, fstr))
  				return true;
  
  			de_name = *fstr;