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

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

[Stephen Rothwell]

Hi Al,

Today's linux-next merge of the vfs tree got a conflict in
fs/ext4/inode.c between commit dae1e52cb126 ("ext4: move ext4_ind_*
functions from inode.c to indirect.c") from the ext4 tree and commit
d1769fea6879 ("fs: simplify the blockdev_direct_IO prototype") from the
vfs tree.

A function moved to a new file.  I fixed it up and can carry the fix as
necessary.

I also needed the following merge fix patch:

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Mon, 18 Jul 2011 13:28:44 +1000
Subject: [PATCH] vfs/ext4: merge fixup for moved function

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 fs/ext4/indirect.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/fs/ext4/indirect.c b/fs/ext4/indirect.c
index 6c27111..b8602cd 100644
--- a/fs/ext4/indirect.c
+++ b/fs/ext4/indirect.c
@@ -806,10 +806,8 @@ retry:
 				 offset, nr_segs,
 				 ext4_get_block, NULL, NULL, 0);
 	else {
-		ret = blockdev_direct_IO(rw, iocb, inode,
-				 inode->i_sb->s_bdev, iov,
-				 offset, nr_segs,
-				 ext4_get_block, NULL);
+		ret = blockdev_direct_IO(rw, iocb, inode, iov,
+				 offset, nr_segs, ext4_get_block);
 
 		if (unlikely((rw & WRITE) && ret < 0)) {
 			loff_t isize = i_size_read(inode);
-- 
1.7.5.4

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

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

[Stephen Rothwell]

Hi all,

On Mon, 18 Jul 2011 13:36:25 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Today's linux-next merge of the vfs tree got a conflict in
> fs/ext4/inode.c between commit dae1e52cb126 ("ext4: move ext4_ind_*
> functions from inode.c to indirect.c") from the ext4 tree and commit
> d1769fea6879 ("fs: simplify the blockdev_direct_IO prototype") from the
> vfs tree.
> 
> A function moved to a new file.  I fixed it up and can carry the fix as
> necessary.
> 
> I also needed the following merge fix patch:

This patchg is now needed after merging the ext4 tree with Linius' tree
(repeated here):

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Mon, 18 Jul 2011 13:28:44 +1000
Subject: [PATCH] vfs/ext4: merge fixup for moved function

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 fs/ext4/indirect.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/fs/ext4/indirect.c b/fs/ext4/indirect.c
index 6c27111..b8602cd 100644
--- a/fs/ext4/indirect.c
+++ b/fs/ext4/indirect.c
@@ -806,10 +806,8 @@ retry:
 				 offset, nr_segs,
 				 ext4_get_block, NULL, NULL, 0);
 	else {
-		ret = blockdev_direct_IO(rw, iocb, inode,
-				 inode->i_sb->s_bdev, iov,
-				 offset, nr_segs,
-				 ext4_get_block, NULL);
+		ret = blockdev_direct_IO(rw, iocb, inode, iov,
+				 offset, nr_segs, ext4_get_block);
 
 		if (unlikely((rw & WRITE) && ret < 0)) {
 			loff_t isize = i_size_read(inode);
-- 
1.7.5.4

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1251 bytes --]

Hi all,

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

  fs/jbd2/journal.c

between commit:

  7f6225e446cc ("jbd2: clean __jbd2_journal_abort_hard() and __journal_abort_soft()")

from the ext4 tree and commit:

  79decc2b6ea4 ("fs: Enable bmap() function to properly return errors")

from the vfs 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/jbd2/journal.c
index 60bf8ff78913,01fa5d247e39..000000000000
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@@ -804,8 -806,12 +805,12 @@@ int jbd2_journal_bmap(journal_t *journa
  					"at offset %lu on %s\n",
  			       __func__, blocknr, journal->j_devname);
  			err = -EIO;
 -			__journal_abort_soft(journal, err);
 +			jbd2_journal_abort(journal, err);
+ 
+ 		} else {
+ 			*retp = block;
  		}
+ 
  	} else {
  		*retp = blocknr; /* +journal->j_blk_offset */
  	}

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Stephen Rothwell]

Hi Arnd,

On Wed, 18 May 2016 16:25:39 +0200 Arnd Bergmann <arnd@arndb.de> wrote:
>
> I'm getting a warning here because the 'offset' variable is no longer
> used, I've fixed it up on my test box like this:

Thanks.  I have applied that to linux-next today and fixed up the merge
fix patch from tomorrow.

> commit 21fffc41b151a6146981487a3fee974e33c7005e
> Author: Arnd Bergmann <arnd@arndb.de>
> Date:   Tue May 17 13:23:39 2016 +0200
> 
>     ext4: fix linux-next mismerge
>     
>     fs/ext4/inode.c: In function 'ext4_direct_IO_read':
>     fs/ext4/inode.c:3502:9: error: unused variable 'offset' [-Werror=unused-variable]
>       loff_t offset = iocb->ki_pos;
>     
>     Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index cd72f208c405..f7140ca66e3b 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -3499,7 +3499,6 @@ static ssize_t ext4_direct_IO_read(struct kiocb *iocb, struct iov_iter *iter)
>  {
>  	int unlocked = 0;
>  	struct inode *inode = iocb->ki_filp->f_mapping->host;
> -	loff_t offset = iocb->ki_pos;
>  	ssize_t ret;
>  
>  	if (ext4_should_dioread_nolock(inode)) {

-- 
Cheers,
Stephen Rothwell

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

[Arnd Bergmann]

On Tuesday 17 May 2016 10:23:55 Stephen Rothwell wrote:
> ++static ssize_t ext4_direct_IO_read(struct kiocb *iocb, struct iov_iter *iter)
>  +{
>  +      int unlocked = 0;
>  +      struct inode *inode = iocb->ki_filp->f_mapping->host;
> ++      loff_t offset = iocb->ki_pos;
>  +      ssize_t ret;
>  +
>  +      if (ext4_should_dioread_nolock(inode)) {
>  +              /*
>  +               * Nolock dioread optimization may be dynamically disabled
>  +               * via ext4_inode_block_unlocked_dio(). Check inode's state
>  +               * while holding extra i_dio_count ref.
>  +               */
>  +              inode_dio_begin(inode);
>  +              smp_mb();
>  +              if (unlikely(ext4_test_inode_state(inode,
>  +                                                  EXT4_STATE_DIOREAD_LOCK)))
>  +                      inode_dio_end(inode);
>  +              else
>  +                      unlocked = 1;
>  +      }
>  +      if (IS_DAX(inode)) {
> -               ret = dax_do_io(iocb, inode, iter, offset, ext4_dio_get_block,
> ++              ret = dax_do_io(iocb, inode, iter, ext4_dio_get_block,
>  +                              NULL, unlocked ? 0 : DIO_LOCKING);
>  +      } else {
>  +              ret = __blockdev_direct_IO(iocb, inode, inode->i_sb->s_bdev,
> -                                          iter, offset, ext4_dio_get_block,
> ++                                         iter, ext4_dio_get_block,
>  +                                         NULL, NULL,
>  +                                         unlocked ? 0 : DIO_LOCKING);
>  +      }
>  +      if (unlocked)
>  +              inode_dio_end(inode);
>         return ret;
>   }
> 

I'm getting a warning here because the 'offset' variable is no longer
used, I've fixed it up on my test box like this:

commit 21fffc41b151a6146981487a3fee974e33c7005e
Author: Arnd Bergmann <arnd@arndb.de>
Date:   Tue May 17 13:23:39 2016 +0200

    ext4: fix linux-next mismerge
    
    fs/ext4/inode.c: In function 'ext4_direct_IO_read':
    fs/ext4/inode.c:3502:9: error: unused variable 'offset' [-Werror=unused-variable]
      loff_t offset = iocb->ki_pos;
    
    Signed-off-by: Arnd Bergmann <arnd@arndb.de>

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index cd72f208c405..f7140ca66e3b 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -3499,7 +3499,6 @@ static ssize_t ext4_direct_IO_read(struct kiocb *iocb, struct iov_iter *iter)
 {
 	int unlocked = 0;
 	struct inode *inode = iocb->ki_filp->f_mapping->host;
-	loff_t offset = iocb->ki_pos;
 	ssize_t ret;
 
 	if (ext4_should_dioread_nolock(inode)) {

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

[Theodore Ts'o]

On Tue, May 17, 2016 at 10:23:55AM +1000, Stephen Rothwell wrote:
> Hi all,
> 
> Today's linux-next merge of the vfs tree got conflicts in:
> 
>   fs/ext4/ext4.h
>   fs/ext4/indirect.c
>   fs/ext4/inode.c
> 
> between commit:
> 
>   914f82a32d02 ("ext4: refactor direct IO code")
> 
> from the ext4 tree and commit:
> 
>   c8b8e32d700f ("direct-io: eliminate the offset argument to ->direct_IO")
> 
> from the vfs tree.
> 
> I fixed it up (hopefully - 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.

Thanks for the heads up.  My merge resolution was backwards from yours
(because I merged the ext4 tree into vfs tree while you apparently did
the reverse), and this resolution was complex enough that I'm waiting
for you to publish next-20160517 to make sure you came up with the
same final result of fs/ext4/inode.c (minus the f2fs's ext4 crypto
merge, which I think Jaeguk is going to be dropping from his tree, but
I don't know if that will have happened by next-20160517).

I'm kicking off a set of tests to make sure there aren't problems with
the resulting merge going beyond the purely syntactic merge
resolution.

Cheers,

							- Ted

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

[Stephen Rothwell]

Hi all,

Today's linux-next merge of the vfs tree got conflicts in:

  fs/ext4/ext4.h
  fs/ext4/indirect.c
  fs/ext4/inode.c

between commit:

  914f82a32d02 ("ext4: refactor direct IO code")

from the ext4 tree and commit:

  c8b8e32d700f ("direct-io: eliminate the offset argument to ->direct_IO")

from the vfs tree.

I fixed it up (hopefully - 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/ext4/ext4.h
index b84aa1ca480a,72f4c9e00e97..000000000000
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
diff --cc fs/ext4/indirect.c
index bc15c2c17633,627b7e8f9ef3..000000000000
--- a/fs/ext4/indirect.c
+++ b/fs/ext4/indirect.c
diff --cc fs/ext4/inode.c
index f9ab1e8cc416,79b298d397b4..000000000000
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@@ -3327,13 -3334,12 +3327,13 @@@ static int ext4_end_io_dio(struct kioc
   * if the machine crashes during the write.
   *
   */
- static ssize_t ext4_direct_IO_write(struct kiocb *iocb, struct iov_iter *iter,
- 				    loff_t offset)
 -static ssize_t ext4_ext_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
++static ssize_t ext4_direct_IO_write(struct kiocb *iocb, struct iov_iter *iter)
  {
  	struct file *file = iocb->ki_filp;
  	struct inode *inode = file->f_mapping->host;
 +	struct ext4_inode_info *ei = EXT4_I(inode);
  	ssize_t ret;
+ 	loff_t offset = iocb->ki_pos;
  	size_t count = iov_iter_count(iter);
  	int overwrite = 0;
  	get_block_t *get_block_func = NULL;
@@@ -3423,12 -3399,12 +3423,12 @@@
  #ifdef CONFIG_EXT4_FS_ENCRYPTION
  	BUG_ON(ext4_encrypted_inode(inode) && S_ISREG(inode->i_mode));
  #endif
 -	if (IS_DAX(inode))
 +	if (IS_DAX(inode)) {
- 		ret = dax_do_io(iocb, inode, iter, offset, get_block_func,
+ 		ret = dax_do_io(iocb, inode, iter, get_block_func,
  				ext4_end_io_dio, dio_flags);
 -	else
 +	} else
  		ret = __blockdev_direct_IO(iocb, inode,
- 					   inode->i_sb->s_bdev, iter, offset,
+ 					   inode->i_sb->s_bdev, iter,
  					   get_block_func,
  					   ext4_end_io_dio, NULL, dio_flags);
  
@@@ -3451,82 -3428,6 +3451,82 @@@
  	if (overwrite)
  		inode_lock(inode);
  
 +	if (ret < 0 && final_size > inode->i_size)
 +		ext4_truncate_failed_write(inode);
 +
 +	/* Handle extending of i_size after direct IO write */
 +	if (orphan) {
 +		int err;
 +
 +		/* Credits for sb + inode write */
 +		handle = ext4_journal_start(inode, EXT4_HT_INODE, 2);
 +		if (IS_ERR(handle)) {
 +			/* This is really bad luck. We've written the data
 +			 * but cannot extend i_size. Bail out and pretend
 +			 * the write failed... */
 +			ret = PTR_ERR(handle);
 +			if (inode->i_nlink)
 +				ext4_orphan_del(NULL, inode);
 +
 +			goto out;
 +		}
 +		if (inode->i_nlink)
 +			ext4_orphan_del(handle, inode);
 +		if (ret > 0) {
 +			loff_t end = offset + ret;
 +			if (end > inode->i_size) {
 +				ei->i_disksize = end;
 +				i_size_write(inode, end);
 +				/*
 +				 * We're going to return a positive `ret'
 +				 * here due to non-zero-length I/O, so there's
 +				 * no way of reporting error returns from
 +				 * ext4_mark_inode_dirty() to userspace.  So
 +				 * ignore it.
 +				 */
 +				ext4_mark_inode_dirty(handle, inode);
 +			}
 +		}
 +		err = ext4_journal_stop(handle);
 +		if (ret == 0)
 +			ret = err;
 +	}
 +out:
 +	return ret;
 +}
 +
- static ssize_t ext4_direct_IO_read(struct kiocb *iocb, struct iov_iter *iter,
- 				   loff_t offset)
++static ssize_t ext4_direct_IO_read(struct kiocb *iocb, struct iov_iter *iter)
 +{
 +	int unlocked = 0;
 +	struct inode *inode = iocb->ki_filp->f_mapping->host;
++	loff_t offset = iocb->ki_pos;
 +	ssize_t ret;
 +
 +	if (ext4_should_dioread_nolock(inode)) {
 +		/*
 +		 * Nolock dioread optimization may be dynamically disabled
 +		 * via ext4_inode_block_unlocked_dio(). Check inode's state
 +		 * while holding extra i_dio_count ref.
 +		 */
 +		inode_dio_begin(inode);
 +		smp_mb();
 +		if (unlikely(ext4_test_inode_state(inode,
 +						    EXT4_STATE_DIOREAD_LOCK)))
 +			inode_dio_end(inode);
 +		else
 +			unlocked = 1;
 +	}
 +	if (IS_DAX(inode)) {
- 		ret = dax_do_io(iocb, inode, iter, offset, ext4_dio_get_block,
++		ret = dax_do_io(iocb, inode, iter, ext4_dio_get_block,
 +				NULL, unlocked ? 0 : DIO_LOCKING);
 +	} else {
 +		ret = __blockdev_direct_IO(iocb, inode, inode->i_sb->s_bdev,
- 					   iter, offset, ext4_dio_get_block,
++					   iter, ext4_dio_get_block,
 +					   NULL, NULL,
 +					   unlocked ? 0 : DIO_LOCKING);
 +	}
 +	if (unlocked)
 +		inode_dio_end(inode);
  	return ret;
  }
  
@@@ -3554,10 -3455,10 +3554,10 @@@ static ssize_t ext4_direct_IO(struct ki
  		return 0;
  
  	trace_ext4_direct_IO_enter(inode, offset, count, iov_iter_rw(iter));
 -	if (ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS))
 -		ret = ext4_ext_direct_IO(iocb, iter);
 +	if (iov_iter_rw(iter) == READ)
- 		ret = ext4_direct_IO_read(iocb, iter, offset);
++		ret = ext4_direct_IO_read(iocb, iter);
  	else
- 		ret = ext4_direct_IO_write(iocb, iter, offset);
 -		ret = ext4_ind_direct_IO(iocb, iter);
++		ret = ext4_direct_IO_write(iocb, iter);
  	trace_ext4_direct_IO_exit(inode, offset, count, iov_iter_rw(iter), ret);
  	return ret;
  }

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1846 bytes --]

Hi Al,

Today's linux-next merge of the vfs tree got a conflict in
fs/ext4/symlink.c between commit b7236e21d55f ("ext4 crypto: reorganize
how we store keys in the inode") from the ext4 tree and commit
680baacbca69 ("new ->follow_link() and ->put_link() calling
conventions") from the vfs tree.

I fixed it up (see below) and can carry the fix as necessary (no action
is required).

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

diff --cc fs/ext4/symlink.c
index 68e915aac0fe,ba5bd18a9825..000000000000
--- a/fs/ext4/symlink.c
+++ b/fs/ext4/symlink.c
@@@ -34,20 -35,19 +34,17 @@@ static const char *ext4_follow_link(str
  	int res;
  	u32 plen, max_size = inode->i_sb->s_blocksize;
  
- 	if (!ext4_encrypted_inode(inode))
- 		return page_follow_link_light(dentry, nd);
- 
 -	ctx = ext4_get_fname_crypto_ctx(inode, inode->i_sb->s_blocksize);
 -	if (IS_ERR(ctx))
 -		return ERR_CAST(ctx);
 +	res = ext4_get_encryption_info(inode);
 +	if (res)
 +		return ERR_PTR(res);
  
  	if (ext4_inode_is_fast_symlink(inode)) {
  		caddr = (char *) EXT4_I(inode)->i_data;
  		max_size = sizeof(EXT4_I(inode)->i_data);
  	} else {
  		cpage = read_mapping_page(inode->i_mapping, 0, NULL);
 -		if (IS_ERR(cpage)) {
 -			ext4_put_fname_crypto_ctx(&ctx);
 +		if (IS_ERR(cpage))
- 			return cpage;
+ 			return ERR_CAST(cpage);
 -		}
  		caddr = kmap(cpage);
  		caddr[size] = 0;
  	}
@@@ -78,13 -77,14 +75,12 @@@
  	/* Null-terminate the name */
  	if (res <= plen)
  		paddr[res] = '\0';
- 	nd_set_link(nd, paddr);
 -	ext4_put_fname_crypto_ctx(&ctx);
  	if (cpage) {
  		kunmap(cpage);
  		page_cache_release(cpage);
  	}
- 	return NULL;
+ 	return *cookie = paddr;
  errout:
 -	ext4_put_fname_crypto_ctx(&ctx);
  	if (cpage) {
  		kunmap(cpage);
  		page_cache_release(cpage);

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1943 bytes --]

Hi Al,

Today's linux-next merge of the vfs tree got a conflict in
fs/ext4/symlink.c between commit fd64e6fd4575 ("ext4 crypto: reorganize
how we store keys in the inode") from the ext4 tree and commits
5542f03602af ("ext4: split inode_operations for encrypted symlinks off
the rest") and cf41cea5a829 ("new ->follow_link() and ->put_link()
calling conventions") from the vfs tree.

I fixed it up (I think - see below) and can carry the fix as necessary
(no action is required).

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

diff --cc fs/ext4/symlink.c
index 32870881188e,ba5bd18a9825..000000000000
--- a/fs/ext4/symlink.c
+++ b/fs/ext4/symlink.c
@@@ -34,20 -35,19 +34,17 @@@ static const char *ext4_follow_link(str
  	int res;
  	u32 plen, max_size = inode->i_sb->s_blocksize;
  
- 	if (!ext4_encrypted_inode(inode))
- 		return page_follow_link_light(dentry, nd);
- 
 -	ctx = ext4_get_fname_crypto_ctx(inode, inode->i_sb->s_blocksize);
 -	if (IS_ERR(ctx))
 -		return ERR_CAST(ctx);
 +	res = ext4_setup_fname_crypto(inode);
 +	if (res)
 +		return ERR_PTR(res);
  
  	if (ext4_inode_is_fast_symlink(inode)) {
  		caddr = (char *) EXT4_I(inode)->i_data;
  		max_size = sizeof(EXT4_I(inode)->i_data);
  	} else {
  		cpage = read_mapping_page(inode->i_mapping, 0, NULL);
 -		if (IS_ERR(cpage)) {
 -			ext4_put_fname_crypto_ctx(&ctx);
 +		if (IS_ERR(cpage))
- 			return cpage;
+ 			return ERR_CAST(cpage);
 -		}
  		caddr = kmap(cpage);
  		caddr[size] = 0;
  	}
@@@ -78,13 -77,14 +75,12 @@@
  	/* Null-terminate the name */
  	if (res <= plen)
  		paddr[res] = '\0';
- 	nd_set_link(nd, paddr);
 -	ext4_put_fname_crypto_ctx(&ctx);
  	if (cpage) {
  		kunmap(cpage);
  		page_cache_release(cpage);
  	}
- 	return NULL;
+ 	return *cookie = paddr;
  errout:
 -	ext4_put_fname_crypto_ctx(&ctx);
  	if (cpage) {
  		kunmap(cpage);
  		page_cache_release(cpage);

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 3597 bytes --]

Hi Al,

Today's linux-next merge of the vfs tree got a conflict in
fs/ext4/symlink.c between commit 48e72c7a0127 ("ext4 crypto: Add
symlink encryption") from the ext4 tree and commit 5dd3dc06371a ("VFS:
normal filesystems (and lustre): d_inode() annotations") from the vfs
tree.

[The ext4 tree commit has been modified.]

I fixed it up (see below) and can carry the fix as necessary (no action
is required).

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

diff --cc fs/ext4/symlink.c
index a7b3a646dde0,57f50091b8d1..000000000000
--- a/fs/ext4/symlink.c
+++ b/fs/ext4/symlink.c
@@@ -21,100 -21,11 +21,100 @@@
  #include <linux/namei.h>
  #include "ext4.h"
  #include "xattr.h"
 +#include "ext4_crypto.h"
  
 +#ifdef CONFIG_EXT4_FS_ENCRYPTION
  static void *ext4_follow_link(struct dentry *dentry, struct nameidata *nd)
  {
 +	struct page *cpage = NULL;
 +	char *caddr, *paddr = NULL;
 +	struct ext4_str cstr, pstr;
- 	struct inode *inode = dentry->d_inode;
++	struct inode *inode = d_inode(dentry);
 +	struct ext4_fname_crypto_ctx *ctx = NULL;
 +	struct ext4_encrypted_symlink_data *sd;
 +	loff_t size = min_t(loff_t, i_size_read(inode), PAGE_SIZE - 1);
 +	int res;
 +	u32 plen, max_size = inode->i_sb->s_blocksize;
 +
 +	if (!ext4_encrypted_inode(inode))
 +		return page_follow_link_light(dentry, nd);
 +
 +	ctx = ext4_get_fname_crypto_ctx(inode, inode->i_sb->s_blocksize);
 +	if (IS_ERR(ctx))
 +		return ctx;
 +
 +	if (ext4_inode_is_fast_symlink(inode)) {
- 		caddr = (char *) EXT4_I(dentry->d_inode)->i_data;
- 		max_size = sizeof(EXT4_I(dentry->d_inode)->i_data);
++		caddr = (char *) EXT4_I(d_inode(dentry))->i_data;
++		max_size = sizeof(EXT4_I(d_inode(dentry))->i_data);
 +	} else {
 +		cpage = read_mapping_page(inode->i_mapping, 0, NULL);
 +		if (IS_ERR(cpage)) {
 +			ext4_put_fname_crypto_ctx(&ctx);
 +			return cpage;
 +		}
 +		caddr = kmap(cpage);
 +		caddr[size] = 0;
 +	}
 +
 +	/* Symlink is encrypted */
 +	sd = (struct ext4_encrypted_symlink_data *)caddr;
 +	cstr.name = sd->encrypted_path;
 +	cstr.len  = le32_to_cpu(sd->len);
 +	if ((cstr.len +
 +	     sizeof(struct ext4_encrypted_symlink_data) - 1) >
 +	    max_size) {
 +		/* Symlink data on the disk is corrupted */
 +		res = -EIO;
 +		goto errout;
 +	}
 +	plen = (cstr.len < EXT4_FNAME_CRYPTO_DIGEST_SIZE*2) ?
 +		EXT4_FNAME_CRYPTO_DIGEST_SIZE*2 : cstr.len;
 +	paddr = kmalloc(plen + 1, GFP_NOFS);
 +	if (!paddr) {
 +		res = -ENOMEM;
 +		goto errout;
 +	}
 +	pstr.name = paddr;
 +	res = _ext4_fname_disk_to_usr(ctx, &cstr, &pstr);
 +	if (res < 0)
 +		goto errout;
 +	/* Null-terminate the name */
 +	if (res <= plen)
 +		paddr[res] = '\0';
 +	nd_set_link(nd, paddr);
 +	ext4_put_fname_crypto_ctx(&ctx);
 +	if (cpage) {
 +		kunmap(cpage);
 +		page_cache_release(cpage);
 +	}
 +	return NULL;
 +errout:
 +	ext4_put_fname_crypto_ctx(&ctx);
 +	if (cpage) {
 +		kunmap(cpage);
 +		page_cache_release(cpage);
 +	}
 +	kfree(paddr);
 +	return ERR_PTR(res);
 +}
 +
 +static void ext4_put_link(struct dentry *dentry, struct nameidata *nd,
 +			  void *cookie)
 +{
 +	struct page *page = cookie;
 +
 +	if (!page) {
 +		kfree(nd_get_link(nd));
 +	} else {
 +		kunmap(page);
 +		page_cache_release(page);
 +	}
 +}
 +#endif
 +
 +static void *ext4_follow_fast_link(struct dentry *dentry, struct nameidata *nd)
 +{
- 	struct ext4_inode_info *ei = EXT4_I(dentry->d_inode);
+ 	struct ext4_inode_info *ei = EXT4_I(d_inode(dentry));
  	nd_set_link(nd, (char *) ei->i_data);
  	return NULL;
  }

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

[Al Viro]

On Tue, Apr 14, 2015 at 05:02:15PM -0400, Theodore Ts'o wrote:
> On Tue, Apr 14, 2015 at 06:17:43PM +0100, Al Viro wrote:
> > Except that you do not handle the slow unencrypted case - you end up with
> > kfree() on the freshly kunmaped address.
> 
> Ah, right, we're actually kmalloc'ing the space that case as well ---
> so hanging on the cpage is pointless; which was the point you were
> making.   Sorry, I was being slow.
> 
> So what we have works, but it's not optimal.  Will fix.

BTW, take a look at kfree_put_link(); no need to reinvent that wheel...

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

[Theodore Ts'o]

On Tue, Apr 14, 2015 at 06:17:43PM +0100, Al Viro wrote:
> Except that you do not handle the slow unencrypted case - you end up with
> kfree() on the freshly kunmaped address.

Ah, right, we're actually kmalloc'ing the space that case as well ---
so hanging on the cpage is pointless; which was the point you were
making.   Sorry, I was being slow.

So what we have works, but it's not optimal.  Will fix.

   	   	       	   	    	      	    - Ted

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

[Theodore Ts'o]

On Tue, Apr 14, 2015 at 06:18:41PM +0200, Christoph Hellwig wrote:
> Also for something that while only
> implemented in one filesystem has pretty broad API implications I'd
> really expect a generalist VFS review (I plan to get to it ASAP..).

There really isn't much of an API.  We have an ioctl to set the
encryption policy (which currently is only which encryption key and
the encryption modes to be used) on an empty directory, an ioctl to
fetch the encryption policy, and an ioctl to get a per-file system
password salt[1] (to protect against rainbow tables).

We *know* that the API will be need to be extended to support more
complicated encryption policies (i.e., the use of public key, files
that can be read by possessors of more than one key, etc.), but we
also know we're not smart enough to figure out what that API will look
like right now.  So that is something that will have to be extended
later, almost certainly with a new ioctl.

So I've designed the existing interface to be things that can be
easily supported into the future, and if the existing interfaces are
only supported by ext4, and someone else wants to design a new system
call, and argue about x509 vs GPG certificates for the future use of
public key crypto, that's fine.  I just don't want the patches to be
delayed while the interface bikeshedding party is going on.  :-)

					- Ted

[1] Note that the use of the password salt, and the string-to-key
algorithm specified by NIST SP800-132, is purely optional.  It's what
the e4crypt userspace tool happens to use, but if you want to use your
own string-to-key algorithm, or just use bare AES keys, a userspace
tool can always do that.

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

[Al Viro]

On Tue, Apr 14, 2015 at 01:00:00PM -0400, Theodore Ts'o wrote:

> > Look, either nd_get_link() points inside that page (in which case that
> > kfree() is obviously invalid), or it points at kmalloc'ed buffer.  In
> > which case kfree() is correct, but WTF do you need anything _else_?
> > Such as mapped pages, etc.
> 
> Yes, it's either one or the other.
> 
> 1) In the case of an unencrypted symlink which is too big to fit in
> the inode, we map in the first (only) block of the symlink, and set
> the link to it.

... and that kfree() will bugger us.

> 2) In the case of an encrypted symlink, we allocate memory and decrypt
> from the first block (or the i_block[] array in the inode), and then
> release the page if necessary.

... and that should've dropped that page in ->follow_link().

> I suppose we could have gone from two struct inode_operations (for
> fast and "slow" symlinks), to four struct inodes_operations (for
> [fast, unencrypted symlinks], [fast, encrypted symlinks], [slow,
> unencrypted symlinks], and [slow, encrypted symlinks]), but it was
> simpler to use a single follow_link() and put_link() function to
> handle multiple cases.

Except that you do not handle the slow unencrypted case - you end up with
kfree() on the freshly kunmaped address.

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

[Theodore Ts'o]

On Tue, Apr 14, 2015 at 02:48:55AM +0100, Al Viro wrote:
> On Tue, Apr 14, 2015 at 11:30:25AM +1000, Stephen Rothwell wrote:
> >  +static void ext4_put_link(struct dentry *dentry, struct nameidata *nd,
> >  +			  void *cookie)
> >  +{
> >  +	struct page *page = cookie;
> >  +	char *buf = nd_get_link(nd);
> >  +
> >  +	if (page) {
> >  +		kunmap(page);
> >  +		page_cache_release(page);
> >  +	}
> >  +	if (buf) {
> >  +		nd_set_link(nd, NULL);
> >  +		kfree(buf);
> 
> What the hell is that for?  ->put_link() has no damn reason to call
> nd_set_link(); the whole _point_ of ->put_link() is to free what needs
> to be freed when we discard a stack element.  And why, in the name of
> everything unholy, does it need to keep *any* page mapped?

The nd_set_link(nd, NULL) call was to clear out the link before it was
freed.  No one else seems to be doing it, so I'm happy to drop it.

> Look, either nd_get_link() points inside that page (in which case that
> kfree() is obviously invalid), or it points at kmalloc'ed buffer.  In
> which case kfree() is correct, but WTF do you need anything _else_?
> Such as mapped pages, etc.

Yes, it's either one or the other.

1) In the case of an unencrypted symlink which is too big to fit in
the inode, we map in the first (only) block of the symlink, and set
the link to it.

2) In the case of an encrypted symlink, we allocate memory and decrypt
from the first block (or the i_block[] array in the inode), and then
release the page if necessary.

I suppose we could have gone from two struct inode_operations (for
fast and "slow" symlinks), to four struct inodes_operations (for
[fast, unencrypted symlinks], [fast, encrypted symlinks], [slow,
unencrypted symlinks], and [slow, encrypted symlinks]), but it was
simpler to use a single follow_link() and put_link() function to
handle multiple cases.

Cheers,

					- Ted

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

[Christoph Hellwig]

On Tue, Apr 07, 2015 at 11:26:46PM -0400, Theodore Ts'o wrote:
> On Tue, Apr 07, 2015 at 09:02:14AM +0200, Christoph Hellwig wrote:
> > FYI, the ext4 tree seems to have the crypto support, which I don't think is
> > ready for 4.1 with all the implications of it..
> 
> What sort of implications are you concerned about?  We're no longer
> exposing anything via the xattr interface, and all of the changes are
> not contained strictly within the ext4 tree.
> 
> There are still are some cleanups which I'm still in the process of
> applying, but I considered it more likely than not something that was
> ready for 4.1, so that's why I let it go into the ext4 dev branch for
> testing in linux-next.

A far as I can tell the patches were just posted last week, and it's
still under very active review.  Also for something that while only
implemented in one filesystem has pretty broad API implications I'd
really expect a generalist VFS review (I plan to get to it ASAP..).

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

[Al Viro]

On Tue, Apr 14, 2015 at 11:30:25AM +1000, Stephen Rothwell wrote:
>  +static void ext4_put_link(struct dentry *dentry, struct nameidata *nd,
>  +			  void *cookie)
>  +{
>  +	struct page *page = cookie;
>  +	char *buf = nd_get_link(nd);
>  +
>  +	if (page) {
>  +		kunmap(page);
>  +		page_cache_release(page);
>  +	}
>  +	if (buf) {
>  +		nd_set_link(nd, NULL);
>  +		kfree(buf);

What the hell is that for?  ->put_link() has no damn reason to call
nd_set_link(); the whole _point_ of ->put_link() is to free what needs
to be freed when we discard a stack element.  And why, in the name of
everything unholy, does it need to keep *any* page mapped?

Look, either nd_get_link() points inside that page (in which case that
kfree() is obviously invalid), or it points at kmalloc'ed buffer.  In
which case kfree() is correct, but WTF do you need anything _else_?
Such as mapped pages, etc.

Has anyone reviewed that code?

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 4078 bytes --]

Hi Al,

Today's linux-next merge of the vfs tree got a conflict in
fs/ext4/symlink.c between commit f1195c72c951 ("ext4 crypto: Add
symlink encryption") from the ext4 tree and commit 5dd3dc06371a ("VFS:
normal filesystems (and lustre): d_inode() annotations") from the vfs
tree.

I fixed it up (see below) and can carry the fix as necessary (no action
is required).

P.S. is there some reason that the two copies of
"EXT4_I(dentry->d_inode)" that I fixed up below are not just
"EXT4_I(inode)" ?
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

diff --cc fs/ext4/symlink.c
index 408d15bc7b12,57f50091b8d1..000000000000
--- a/fs/ext4/symlink.c
+++ b/fs/ext4/symlink.c
@@@ -21,112 -21,11 +21,112 @@@
  #include <linux/namei.h>
  #include "ext4.h"
  #include "xattr.h"
 +#include "ext4_crypto.h"
  
 +#ifdef CONFIG_EXT4_FS_ENCRYPTION
  static void *ext4_follow_link(struct dentry *dentry, struct nameidata *nd)
  {
 +	struct page *cpage = NULL;
 +	char *caddr, *paddr = NULL;
 +	struct ext4_str cstr, pstr;
- 	struct inode *inode = dentry->d_inode;
++	struct inode *inode = d_inode(dentry);
 +	struct ext4_fname_crypto_ctx *ctx = NULL;
 +	struct ext4_encrypted_symlink_data *sd;
 +	loff_t size = min_t(loff_t, i_size_read(inode), PAGE_SIZE - 1);
 +	int res;
 +	u32 plen, plen2, max_size = inode->i_sb->s_blocksize;
 +
 +	ctx = ext4_get_fname_crypto_ctx(inode, inode->i_sb->s_blocksize);
 +	if (IS_ERR(ctx))
 +		return ctx;
 +
 +	if (ext4_inode_is_fast_symlink(inode)) {
- 		caddr = (char *) EXT4_I(dentry->d_inode)->i_data;
- 		max_size = sizeof(EXT4_I(dentry->d_inode)->i_data);
++		caddr = (char *) EXT4_I(d_inode(dentry))->i_data;
++		max_size = sizeof(EXT4_I(d_inode(dentry))->i_data);
 +	} else {
 +		cpage = read_mapping_page(inode->i_mapping, 0, NULL);
 +		if (IS_ERR(cpage)) {
 +			ext4_put_fname_crypto_ctx(&ctx);
 +			return cpage;
 +		}
 +		caddr = kmap(cpage);
 +		caddr[size] = 0;
 +	}
 +
 +	if (!ctx) {
 +		/* Symlink is unencrypted */
 +		plen = strnlen((char *)caddr, inode->i_sb->s_blocksize);
 +		plen2 = (plen < max_size) ? plen + 1 : plen;
 +		paddr = kmalloc(plen2, GFP_NOFS);
 +		if (!paddr) {
 +			ext4_put_fname_crypto_ctx(&ctx);
 +			kunmap(cpage);
 +			page_cache_release(cpage);
 +			return ERR_PTR(-ENOMEM);
 +		}
 +		memcpy(paddr, caddr, plen);
 +		if (plen < inode->i_sb->s_blocksize)
 +			paddr[plen] = '\0';
 +	} else {
 +		/* Symlink is encrypted */
 +		sd = (struct ext4_encrypted_symlink_data *)caddr;
 +		cstr.name = sd->encrypted_path;
 +		cstr.len  = le32_to_cpu(sd->len);
 +		if ((cstr.len +
 +		     sizeof(struct ext4_encrypted_symlink_data) - 1) >
 +		    max_size) {
 +			/* Symlink data on the disk is corrupted */
 +			res = -EIO;
 +			goto errout;
 +		}
 +		plen = (cstr.len < EXT4_FNAME_CRYPTO_DIGEST_SIZE*2) ?
 +			EXT4_FNAME_CRYPTO_DIGEST_SIZE*2 : cstr.len;
 +		paddr = kmalloc(plen + 1, GFP_NOFS);
 +		if (!paddr) {
 +			res = -ENOMEM;
 +			goto errout;
 +		}
 +		pstr.name = paddr;
 +		res = _ext4_fname_disk_to_usr(ctx, &cstr, &pstr);
 +		if (res < 0)
 +			goto errout;
 +		/* Null-terminate the name */
 +		if (res <= plen)
 +			paddr[res] = '\0';
 +	}
 +	nd_set_link(nd, paddr);
 +	ext4_put_fname_crypto_ctx(&ctx);
 +	return cpage;
 +errout:
 +	ext4_put_fname_crypto_ctx(&ctx);
 +	if (cpage) {
 +		kunmap(cpage);
 +		page_cache_release(cpage);
 +	}
 +	kfree(paddr);
 +	return ERR_PTR(res);
 +}
 +
 +static void ext4_put_link(struct dentry *dentry, struct nameidata *nd,
 +			  void *cookie)
 +{
 +	struct page *page = cookie;
 +	char *buf = nd_get_link(nd);
 +
 +	if (page) {
 +		kunmap(page);
 +		page_cache_release(page);
 +	}
 +	if (buf) {
 +		nd_set_link(nd, NULL);
 +		kfree(buf);
 +	}
 +}
 +#endif
 +
 +static void *ext4_follow_fast_link(struct dentry *dentry, struct nameidata *nd)
 +{
- 	struct ext4_inode_info *ei = EXT4_I(dentry->d_inode);
+ 	struct ext4_inode_info *ei = EXT4_I(d_inode(dentry));
  	nd_set_link(nd, (char *) ei->i_data);
  	return NULL;
  }

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 3718 bytes --]

Hi Al,

Today's linux-next merge of the vfs tree got a conflict in
fs/ext4/symlink.c between commit 8ee224253787 ("ext4 crypto: Add
symlink encryption") from the ext4 tree and commit 5dd3dc06371a ("VFS:
normal filesystems (and lustre): d_inode() annotations") from the vfs
tree.

I fixed it up (see below) and can carry the fix as necessary (no action
is required).

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

diff --cc fs/ext4/symlink.c
index 839f71e941a9,57f50091b8d1..000000000000
--- a/fs/ext4/symlink.c
+++ b/fs/ext4/symlink.c
@@@ -21,107 -21,11 +21,107 @@@
  #include <linux/namei.h>
  #include "ext4.h"
  #include "xattr.h"
 +#include "ext4_crypto.h"
  
 +#ifdef CONFIG_EXT4_FS_ENCRYPTION
  static void *ext4_follow_link(struct dentry *dentry, struct nameidata *nd)
  {
 +	struct page *cpage = NULL;
 +	char *caddr, *paddr;
 +	struct ext4_str cstr, pstr;
- 	struct inode *inode = dentry->d_inode;
++	struct inode *inode = d_inode(dentry);
 +	struct ext4_fname_crypto_ctx *ctx = NULL;
 +	struct ext4_encrypted_symlink_data *sd;
 +	loff_t size = min(inode->i_size, (loff_t) PAGE_SIZE-1);
 +	int res;
 +	u32 plen, plen2;
 +
 +	ctx = ext4_get_fname_crypto_ctx(inode, inode->i_sb->s_blocksize);
 +	if (IS_ERR(ctx))
 +		return ctx;
 +
 +	cpage = read_mapping_page(inode->i_mapping, 0, NULL);
 +	if (IS_ERR(cpage)) {
 +		ext4_put_fname_crypto_ctx(&ctx);
 +		return cpage;
 +	}
 +	caddr = kmap(cpage);
 +	caddr[size] = 0;
 +
 +	if (!ctx) {
 +		/* Symlink is unencrypted */
 +		plen = strnlen((char *)caddr, inode->i_sb->s_blocksize);
 +		plen2 = (plen < inode->i_sb->s_blocksize) ? plen + 1 : plen;
 +		paddr = kmalloc(plen2, GFP_NOFS);
 +		if (!paddr) {
 +			ext4_put_fname_crypto_ctx(&ctx);
 +			kunmap(cpage);
 +			page_cache_release(cpage);
 +			return ERR_PTR(-ENOMEM);
 +		}
 +		memcpy(paddr, caddr, plen);
 +		if (plen < inode->i_sb->s_blocksize)
 +			paddr[plen] = '\0';
 +	} else {
 +		/* Symlink is encrypted */
 +		sd = (struct ext4_encrypted_symlink_data *)caddr;
 +		cstr.name = sd->encrypted_path;
 +		cstr.len  = le32_to_cpu(sd->len);
 +		if ((cstr.len + sizeof(struct ext4_encrypted_symlink_data) - 1)
 +			> inode->i_sb->s_blocksize) {
 +			/* Symlink data on the disk is corrupted */
 +			kunmap(cpage);
 +			page_cache_release(cpage);
 +			return ERR_PTR(-EIO);
 +		}
 +		plen = (cstr.len < EXT4_FNAME_CRYPTO_DIGEST_SIZE*2)
 +			? EXT4_FNAME_CRYPTO_DIGEST_SIZE*2
 +			: cstr.len;
 +		paddr = kmalloc(plen + 1, GFP_NOFS);
 +		if (!paddr) {
 +			ext4_put_fname_crypto_ctx(&ctx);
 +			kunmap(cpage);
 +			page_cache_release(cpage);
 +			return ERR_PTR(-ENOMEM);
 +		}
 +		pstr.name = paddr;
 +		res = _ext4_fname_disk_to_usr(ctx, &cstr, &pstr);
 +		if (res < 0) {
 +			ext4_put_fname_crypto_ctx(&ctx);
 +			kunmap(cpage);
 +			page_cache_release(cpage);
 +			kfree(paddr);
 +			return ERR_PTR(res);
 +		}
 +		/* Null-terminate the name */
 +		if (res <= plen)
 +			paddr[res] = '\0';
 +	}
 +	nd_set_link(nd, paddr);
 +	ext4_put_fname_crypto_ctx(&ctx);
 +	return cpage;
 +}
 +
 +static void ext4_put_link(struct dentry *dentry, struct nameidata *nd,
 +			  void *cookie)
 +{
 +	struct page *page = cookie;
 +	char *buf = nd_get_link(nd);
 +
 +	if (page) {
 +		kunmap(page);
 +		page_cache_release(page);
 +	}
 +	if (buf) {
 +		nd_set_link(nd, NULL);
 +		kfree(buf);
 +	}
 +}
 +#endif
 +
 +static void *ext4_follow_fast_link(struct dentry *dentry, struct nameidata *nd)
 +{
- 	struct ext4_inode_info *ei = EXT4_I(dentry->d_inode);
+ 	struct ext4_inode_info *ei = EXT4_I(d_inode(dentry));
  	nd_set_link(nd, (char *) ei->i_data);
  	return NULL;
  }

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1691 bytes --]

Hi Al,

Today's linux-next merge of the vfs tree got a conflict in
fs/ext4/namei.c between commits e12fb97222fc ("ext4: make fsync to sync
parent dir in no-journal for real this time") and 5c34f02d301e ("ext4
crypto: partial update to namei.c for fname crypto") from the ext4 tree
and commit 5dd3dc06371a ("VFS: normal filesystems (and lustre):
d_inode() annotations") from the vfs tree.

I fixed it up (see below) and can carry the fix as necessary (no action
is required).

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

diff --cc fs/ext4/namei.c
index c5197176dba4,e086eebe335e..000000000000
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@@ -2058,14 -1732,9 +2058,14 @@@ static int add_dirent_to_buf(handle_t *
  static int make_indexed_dir(handle_t *handle, struct dentry *dentry,
  			    struct inode *inode, struct buffer_head *bh)
  {
- 	struct inode	*dir = dentry->d_parent->d_inode;
+ 	struct inode	*dir = d_inode(dentry->d_parent);
 +#ifdef CONFIG_EXT4_FS_ENCRYPTION
 +	struct ext4_fname_crypto_ctx *ctx = NULL;
 +	int res;
 +#else
  	const char	*name = dentry->d_name.name;
  	int		namelen = dentry->d_name.len;
 +#endif
  	struct buffer_head *bh2;
  	struct dx_root	*root;
  	struct dx_frame	frames[2], *frame;
@@@ -2212,8 -1864,8 +2212,8 @@@ out_frames
  static int ext4_add_entry(handle_t *handle, struct dentry *dentry,
  			  struct inode *inode)
  {
- 	struct inode *dir = dentry->d_parent->d_inode;
+ 	struct inode *dir = d_inode(dentry->d_parent);
 -	struct buffer_head *bh;
 +	struct buffer_head *bh = NULL;
  	struct ext4_dir_entry_2 *de;
  	struct ext4_dir_entry_tail *t;
  	struct super_block *sb;

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

[Theodore Ts'o]

On Tue, Apr 07, 2015 at 09:02:14AM +0200, Christoph Hellwig wrote:
> FYI, the ext4 tree seems to have the crypto support, which I don't think is
> ready for 4.1 with all the implications of it..

What sort of implications are you concerned about?  We're no longer
exposing anything via the xattr interface, and all of the changes are
not contained strictly within the ext4 tree.

There are still are some cleanups which I'm still in the process of
applying, but I considered it more likely than not something that was
ready for 4.1, so that's why I let it go into the ext4 dev branch for
testing in linux-next.

Cheers,

						- Ted

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 987 bytes --]

Hi Christoph,

On Tue, 7 Apr 2015 09:02:14 +0200 Christoph Hellwig <hch@lst.de> wrote:
>
> On Tue, Apr 07, 2015 at 02:00:35PM +1000, Stephen Rothwell wrote:
> > Hi Al,
> > 
> > Today's linux-next merge of the vfs tree got a conflict in fs/ext4/inode.c between commit 72b8e0f9fa8a ("ext4: remove unused header files") from the ext4 tree and commit e2e40f2c1ed4 ("fs: move struct kiocb to fs.h") from the vfs tree.
> 
> FYI, the ext4 tree seems to have the crypto support, which I don't think is
> ready for 4.1 with all the implications of it..

Actually I was going to ping Ted about that ...

Ted,

A few of the files added by the ext4 crypto support do not have
copyright notices or license acknowledgements ... and there was one
place I noticed that has just a template for function documentation
that is not very useful.  I didn't look further, but was wondering if
it had leaked out ...

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

[Christoph Hellwig]

On Tue, Apr 07, 2015 at 02:00:35PM +1000, Stephen Rothwell wrote:
> Hi Al,
> 
> Today's linux-next merge of the vfs tree got a conflict in fs/ext4/inode.c between commit 72b8e0f9fa8a ("ext4: remove unused header files") from the ext4 tree and commit e2e40f2c1ed4 ("fs: move struct kiocb to fs.h") from the vfs tree.

FYI, the ext4 tree seems to have the crypto support, which I don't think is
ready for 4.1 with all the implications of it..

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1573 bytes --]

Hi Al,

Today's linux-next merge of the vfs tree got a conflict in fs/ext4/inode.c between commit 72b8e0f9fa8a ("ext4: remove unused header files") from the ext4 tree and commit e2e40f2c1ed4 ("fs: move struct kiocb to fs.h") from the vfs tree.

I fixed it up (see below) and can carry the fix as necessary (no action
is required).

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

diff --cc fs/ext4/inode.c
index 7eb70b7a5c19,42c942a950e1..000000000000
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@@ -35,12 -36,10 +35,11 @@@
  #include <linux/kernel.h>
  #include <linux/printk.h>
  #include <linux/slab.h>
- #include <linux/aio.h>
 -#include <linux/ratelimit.h>
  #include <linux/bitops.h>
 +#include <linux/prefetch.h>
  
  #include "ext4_jbd2.h"
 +#include "ext4_crypto.h"
  #include "xattr.h"
  #include "acl.h"
  #include "truncate.h"
@@@ -3136,14 -3033,11 +3135,14 @@@ static ssize_t ext4_ext_direct_IO(struc
  		get_block_func = ext4_get_block_write;
  		dio_flags = DIO_LOCKING;
  	}
 +#ifdef CONFIG_EXT4_FS_ENCRYPTION
 +	BUG_ON(ext4_encrypted_inode(inode) && S_ISREG(inode->i_mode));
 +#endif
  	if (IS_DAX(inode))
- 		ret = dax_do_io(rw, iocb, inode, iter, offset, get_block_func,
+ 		ret = dax_do_io(iocb, inode, iter, offset, get_block_func,
  				ext4_end_io_dio, dio_flags);
  	else
- 		ret = __blockdev_direct_IO(rw, iocb, inode,
+ 		ret = __blockdev_direct_IO(iocb, inode,
  					   inode->i_sb->s_bdev, iter, offset,
  					   get_block_func,
  					   ext4_end_io_dio, NULL, dio_flags);

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1812 bytes --]

Hi Al,

Today's linux-next merge of the vfs tree got a conflict in
fs/ext4/file.c between commit 00532604c72e ("ext4: introduce new
i_write_mutex to protect fallocate") from the ext4 tree and commit
9b884164d597 ("convert ext4 to ->write_iter()") from the vfs tree.

I fixed it up (see below) and can carry the fix as necessary (no action
is required).

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

diff --cc fs/ext4/file.c
index 8c39305abc23,708aad768199..000000000000
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@@ -101,13 -97,10 +97,12 @@@ ext4_file_write_iter(struct kiocb *iocb
  	struct blk_plug plug;
  	int o_direct = file->f_flags & O_DIRECT;
  	int overwrite = 0;
- 	size_t length = iov_length(iov, nr_segs);
+ 	size_t length = iov_iter_count(from);
  	ssize_t ret;
- 
- 	BUG_ON(iocb->ki_pos != pos);
+ 	loff_t pos = iocb->ki_pos;
  
 +	mutex_lock(&EXT4_I(inode)->i_write_mutex);
 +
  	/*
  	 * Unaligned direct AIO must be serialized; see comment above
  	 * In the case of O_APPEND, assume that we must always serialize
@@@ -116,8 -109,9 +111,8 @@@
  	    ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS) &&
  	    !is_sync_kiocb(iocb) &&
  	    (file->f_flags & O_APPEND ||
- 	     ext4_unaligned_aio(inode, iov, nr_segs, pos))) {
+ 	     ext4_unaligned_aio(inode, from, pos))) {
 -		aio_mutex = ext4_aio_mutex(inode);
 -		mutex_lock(aio_mutex);
 +		unaligned_direct_aio = true;
  		ext4_unwritten_wait(inode);
  	}
  
@@@ -181,10 -172,8 +174,10 @@@
  		}
  	}
  
- 	ret = __generic_file_aio_write(iocb, iov, nr_segs);
+ 	ret = __generic_file_write_iter(iocb, from);
  	mutex_unlock(&inode->i_mutex);
 +	if (!unaligned_direct_aio)
 +		mutex_unlock(&EXT4_I(inode)->i_write_mutex);
  
  	if (ret > 0) {
  		ssize_t err;

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 442 bytes --]

Hi Al,

Today's linux-next merge of the vfs tree got a conflict in fs/ext4/file.c
between rebased commits from the ext4 tree and commit 29a8196bc41c
("convert ext4 to ->write_iter()") from the vfs tree.

I fixed it up (in this case I used the conflicting hunks from the vfs
tree - I hope its right) and can carry the fix as necessary (no action is
required).

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

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

[Djalal Harouni]

Hello,

(Cc'ed Jan Kara).

On Fri, Jan 06, 2012 at 01:54:42PM +1100, Stephen Rothwell wrote:
> Hi Al,
> 
> Today's linux-next merge of the vfs tree got a conflict in
> fs/ext4/ioctl.c between commits 2a415f129806 ("ext4: add missing
> ext4_resize_end on error paths") and 22cdfca56418 ("ext4: remove unneeded
> file_remove_suid() from ext4_ioctl()") from the ext4 tree and commit
There is also another commit from ext4 involved:
commit ce06c5fa14f1a ("ext{3,4}: Fix potential race when setversion ioctl
updates inode) by Jan Kara.

All these ext4 commits are fine.

> a561be7100cd ("switch a bunch of places to mnt_want_write_file()") from
> the vfs tree.
> 
> I fixed it up (see below) and can carry the fix as necessary.
> -- 
> Cheers,
> Stephen Rothwell                    sfr@canb.auug.org.au
> 
> diff --cc fs/ext4/ioctl.c
> index f2c335b,d37b3bb..0000000
> --- a/fs/ext4/ioctl.c
> +++ b/fs/ext4/ioctl.c
> @@@ -173,11 -170,8 +173,11 @@@ flags_out
>   			err = ext4_mark_iloc_dirty(handle, inode, &iloc);
>   		}
>   		ext4_journal_stop(handle);
>  +
>  +unlock_out:
>  +		mutex_unlock(&inode->i_mutex);
This is the code related to commit: ce06c5fa14f1a

Thanks for the fix.

>   setversion_out:
> - 		mnt_drop_write(filp->f_path.mnt);
> + 		mnt_drop_write_file(filp);
>   		return err;
>   	}
>   	case EXT4_IOC_GROUP_EXTEND: {
> @@@ -197,13 -189,12 +197,13 @@@
>   			       EXT4_FEATURE_RO_COMPAT_BIGALLOC)) {
>   			ext4_msg(sb, KERN_ERR,
>   				 "Online resizing not supported with bigalloc");
>  -			return -EOPNOTSUPP;
>  +			err = -EOPNOTSUPP;
>  +			goto group_extend_out;
>   		}
>   
> - 		err = mnt_want_write(filp->f_path.mnt);
> + 		err = mnt_want_write_file(filp);
>   		if (err)
>  -			return err;
>  +			goto group_extend_out;
>   
>   		err = ext4_group_extend(sb, EXT4_SB(sb)->s_es, n_blocks_count);
>   		if (EXT4_SB(sb)->s_journal) {
> @@@ -213,10 -204,9 +213,10 @@@
>   		}
>   		if (err == 0)
>   			err = err2;
>  +
> - 		mnt_drop_write(filp->f_path.mnt);
> + 		mnt_drop_write_file(filp);
>  +group_extend_out:
>   		ext4_resize_end(sb);
>  -
>   		return err;
>   	}
>   
> @@@ -256,7 -246,9 +256,7 @@@
>   
>   		err = ext4_move_extents(filp, donor_filp, me.orig_start,
>   					me.donor_start, me.len, &me.moved_len);
> - 		mnt_drop_write(filp->f_path.mnt);
> + 		mnt_drop_write_file(filp);
>  -		if (me.moved_len > 0)
>  -			file_remove_suid(donor_filp);
>   
>   		if (copy_to_user((struct move_extent __user *)arg,
>   				 &me, sizeof(me)))
> @@@ -284,13 -274,12 +284,13 @@@ mext_out
>   			       EXT4_FEATURE_RO_COMPAT_BIGALLOC)) {
>   			ext4_msg(sb, KERN_ERR,
>   				 "Online resizing not supported with bigalloc");
>  -			return -EOPNOTSUPP;
>  +			err = -EOPNOTSUPP;
>  +			goto group_add_out;
>   		}
>   
> - 		err = mnt_want_write(filp->f_path.mnt);
> + 		err = mnt_want_write_file(filp);
>   		if (err)
>  -			return err;
>  +			goto group_add_out;
>   
>   		err = ext4_group_add(sb, &input);
>   		if (EXT4_SB(sb)->s_journal) {
> @@@ -300,10 -289,9 +300,10 @@@
>   		}
>   		if (err == 0)
>   			err = err2;
>  +
> - 		mnt_drop_write(filp->f_path.mnt);
> + 		mnt_drop_write_file(filp);
>  +group_add_out:
>   		ext4_resize_end(sb);
>  -
>   		return err;
>   	}
>   



-- 
tixxdz
http://opendz.org

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 2764 bytes --]

Hi Al,

Today's linux-next merge of the vfs tree got a conflict in
fs/ext4/ioctl.c between commits 2a415f129806 ("ext4: add missing
ext4_resize_end on error paths") and 22cdfca56418 ("ext4: remove unneeded
file_remove_suid() from ext4_ioctl()") from the ext4 tree and commit
a561be7100cd ("switch a bunch of places to mnt_want_write_file()") from
the vfs tree.

I fixed it up (see below) and can carry the fix as necessary.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

diff --cc fs/ext4/ioctl.c
index f2c335b,d37b3bb..0000000
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@@ -173,11 -170,8 +173,11 @@@ flags_out
  			err = ext4_mark_iloc_dirty(handle, inode, &iloc);
  		}
  		ext4_journal_stop(handle);
 +
 +unlock_out:
 +		mutex_unlock(&inode->i_mutex);
  setversion_out:
- 		mnt_drop_write(filp->f_path.mnt);
+ 		mnt_drop_write_file(filp);
  		return err;
  	}
  	case EXT4_IOC_GROUP_EXTEND: {
@@@ -197,13 -189,12 +197,13 @@@
  			       EXT4_FEATURE_RO_COMPAT_BIGALLOC)) {
  			ext4_msg(sb, KERN_ERR,
  				 "Online resizing not supported with bigalloc");
 -			return -EOPNOTSUPP;
 +			err = -EOPNOTSUPP;
 +			goto group_extend_out;
  		}
  
- 		err = mnt_want_write(filp->f_path.mnt);
+ 		err = mnt_want_write_file(filp);
  		if (err)
 -			return err;
 +			goto group_extend_out;
  
  		err = ext4_group_extend(sb, EXT4_SB(sb)->s_es, n_blocks_count);
  		if (EXT4_SB(sb)->s_journal) {
@@@ -213,10 -204,9 +213,10 @@@
  		}
  		if (err == 0)
  			err = err2;
 +
- 		mnt_drop_write(filp->f_path.mnt);
+ 		mnt_drop_write_file(filp);
 +group_extend_out:
  		ext4_resize_end(sb);
 -
  		return err;
  	}
  
@@@ -256,7 -246,9 +256,7 @@@
  
  		err = ext4_move_extents(filp, donor_filp, me.orig_start,
  					me.donor_start, me.len, &me.moved_len);
- 		mnt_drop_write(filp->f_path.mnt);
+ 		mnt_drop_write_file(filp);
 -		if (me.moved_len > 0)
 -			file_remove_suid(donor_filp);
  
  		if (copy_to_user((struct move_extent __user *)arg,
  				 &me, sizeof(me)))
@@@ -284,13 -274,12 +284,13 @@@ mext_out
  			       EXT4_FEATURE_RO_COMPAT_BIGALLOC)) {
  			ext4_msg(sb, KERN_ERR,
  				 "Online resizing not supported with bigalloc");
 -			return -EOPNOTSUPP;
 +			err = -EOPNOTSUPP;
 +			goto group_add_out;
  		}
  
- 		err = mnt_want_write(filp->f_path.mnt);
+ 		err = mnt_want_write_file(filp);
  		if (err)
 -			return err;
 +			goto group_add_out;
  
  		err = ext4_group_add(sb, &input);
  		if (EXT4_SB(sb)->s_journal) {
@@@ -300,10 -289,9 +300,10 @@@
  		}
  		if (err == 0)
  			err = err2;
 +
- 		mnt_drop_write(filp->f_path.mnt);
+ 		mnt_drop_write_file(filp);
 +group_add_out:
  		ext4_resize_end(sb);
 -
  		return err;
  	}
  

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 645 bytes --]

Hi Al,

On Wed, 21 Dec 2011 11:18:01 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Today's linux-next merge of the vfs tree got a conflict in
> fs/ext4/ioctl.c between commit 5e8af306ef80 ("ext4: fix potential
> deadlock with setuid files and EXT4_IOC_MOVE_EXT") from theext4  tree and
> commit 99b4f71273d9 ("vfs: mnt_drop_write_file()") from the vfs tree.
> 
> Just context changes.  I fixed it up (see below) and can carry the fix as
> necessary.

Of course, the build still failed (see yesterday's report) :-(
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 932 bytes --]

Hi Al,

Today's linux-next merge of the vfs tree got a conflict in
fs/ext4/ioctl.c between commit 5e8af306ef80 ("ext4: fix potential
deadlock with setuid files and EXT4_IOC_MOVE_EXT") from theext4  tree and
commit 99b4f71273d9 ("vfs: mnt_drop_write_file()") from the vfs tree.

Just context changes.  I fixed it up (see below) and can carry the fix as
necessary.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

diff --cc fs/ext4/ioctl.c
index ff1aab7,d37b3bb..0000000
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@@ -246,7 -246,9 +246,7 @@@ setversion_out
  
  		err = ext4_move_extents(filp, donor_filp, me.orig_start,
  					me.donor_start, me.len, &me.moved_len);
- 		mnt_drop_write(filp->f_path.mnt);
+ 		mnt_drop_write_file(filp);
 -		if (me.moved_len > 0)
 -			file_remove_suid(donor_filp);
  
  		if (copy_to_user((struct move_extent __user *)arg,
  				 &me, sizeof(me)))

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

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

[Stephen Rothwell]

Hi Al, Ted,

Today's linux-next merge of the vfs tree got a conflict in
fs/ext4/super.c between commit cb0765fe4c3db8c9c22f1ed1017cbced2dd68150
("ext4: Add a comprehensive block validity check to ext4_get_blocks()")
from the ext4 tree and various commits from the vfs tree.

Just context changes.  I fixed it up (see below - please check as I am
not sure of the ordering constraints in the first hunk) and can carry the
fixup as necessary.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

diff --cc fs/ext4/super.c
index eca6c05,d789833..0000000
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@@ -568,7 -564,11 +569,12 @@@ static void ext4_put_super(struct super
  	struct ext4_super_block *es = sbi->s_es;
  	int i, err;
  
+ 	lock_super(sb);
+ 	lock_kernel();
+ 	if (sb->s_dirt)
+ 		ext4_write_super_locked(sb);
+ 
 +	ext4_release_system_zone(sb);
  	ext4_mb_release(sb);
  	ext4_ext_release(sb);
  	ext4_xattr_put_super(sb);
@@@ -3322,11 -3268,30 +3325,18 @@@ int ext4_force_commit(struct super_bloc
  	return ret;
  }
  
- static void ext4_write_super(struct super_block *sb)
 -/*
 - * Ext4 always journals updates to the superblock itself, so we don't
 - * have to propagate any other updates to the superblock on disk at this
 - * point.  (We can probably nuke this function altogether, and remove
 - * any mention to sb->s_dirt in all of fs/ext4; eventual cleanup...)
 - */
+ static void ext4_write_super_locked(struct super_block *sb)
  {
 -	if (EXT4_SB(sb)->s_journal) {
 -		if (mutex_trylock(&sb->s_lock) != 0)
 -			BUG();
 -		sb->s_dirt = 0;
 -	} else {
 -		ext4_commit_super(sb, EXT4_SB(sb)->s_es, 1);
 -	}
 +	ext4_commit_super(sb, 1);
  }
  
+ static void ext4_write_super(struct super_block *sb)
+ {
+ 	lock_super(sb);
+ 	ext4_write_super_locked(sb);
+ 	unlock_super(sb);
+ }
+ 
  static int ext4_sync_fs(struct super_block *sb, int wait)
  {
  	int ret = 0;