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  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 
> 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 
> 
> 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

[Stephen Rothwell]

Hi Arnd,

On Wed, 18 May 2016 16:25:39 +0200 Arnd Bergmann  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 
> 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 
> 
> 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 
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 

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

[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 
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 

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

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..
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
diff --cc fs/ext4/indirect.c
index bc15c2c17633,627b7e8f9ef3..
--- a/fs/ext4/indirect.c
+++ b/fs/ext4/indirect.c
diff --cc fs/ext4/inode.c
index f9ab1e8cc416,79b298d397b4..
--- 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 

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..
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
diff --cc fs/ext4/indirect.c
index bc15c2c17633,627b7e8f9ef3..
--- a/fs/ext4/indirect.c
+++ b/fs/ext4/indirect.c
diff --cc fs/ext4/inode.c
index f9ab1e8cc416,79b298d397b4..
--- 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 

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/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 Rothwells...@canb.auug.org.au

diff --cc fs/ext4/symlink.c
index 68e915aac0fe,ba5bd18a9825..
--- 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();
 +  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();
if (cpage) {
kunmap(cpage);
page_cache_release(cpage);
}
-   return NULL;
+   return *cookie = paddr;
  errout:
 -  ext4_put_fname_crypto_ctx();
if (cpage) {
kunmap(cpage);
page_cache_release(cpage);


pgpB6y8OlHZo7.pgp
Description: OpenPGP digital signature

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/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 Rothwells...@canb.auug.org.au

diff --cc fs/ext4/symlink.c
index 68e915aac0fe,ba5bd18a9825..
--- 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);


pgpB6y8OlHZo7.pgp
Description: OpenPGP digital signature

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/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 Rothwells...@canb.auug.org.au

diff --cc fs/ext4/symlink.c
index 32870881188e,ba5bd18a9825..
--- 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();
 +  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();
if (cpage) {
kunmap(cpage);
page_cache_release(cpage);
}
-   return NULL;
+   return *cookie = paddr;
  errout:
 -  ext4_put_fname_crypto_ctx();
if (cpage) {
kunmap(cpage);
page_cache_release(cpage);


pgper49fSWtbK.pgp
Description: OpenPGP digital signature

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/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 Rothwells...@canb.auug.org.au

diff --cc fs/ext4/symlink.c
index 32870881188e,ba5bd18a9825..
--- 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);


pgper49fSWtbK.pgp
Description: OpenPGP digital signature

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/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 Rothwells...@canb.auug.org.au

diff --cc fs/ext4/symlink.c
index a7b3a646dde0,57f50091b8d1..
--- a/fs/ext4/symlink.c
+++ b/fs/ext4/symlink.c
@@@ -21,100 -21,11 +21,100 @@@
  #include 
  #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();
 +  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, , );
 +  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();
 +  if (cpage) {
 +  kunmap(cpage);
 +  page_cache_release(cpage);
 +  }
 +  return NULL;
 +errout:
 +  ext4_put_fname_crypto_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;
  }


pgp9s5UaJtXo4.pgp
Description: OpenPGP digital signature

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...
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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..).

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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..).

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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/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 Rothwells...@canb.auug.org.au

diff --cc fs/ext4/symlink.c
index a7b3a646dde0,57f50091b8d1..
--- 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;
  }


pgp9s5UaJtXo4.pgp
Description: OpenPGP digital signature

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.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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...
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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/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 Rothwells...@canb.auug.org.au

diff --cc fs/ext4/symlink.c
index 408d15bc7b12,57f50091b8d1..
--- a/fs/ext4/symlink.c
+++ b/fs/ext4/symlink.c
@@@ -21,112 -21,11 +21,112 @@@
  #include 
  #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();
 +  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();
 +  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, , );
 +  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();
 +  return cpage;
 +errout:
 +  ext4_put_fname_crypto_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;
  }


pgpOJymdHRkTF.pgp
Description: OpenPGP digital signature

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/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 Rothwells...@canb.auug.org.au

diff --cc fs/ext4/symlink.c
index 408d15bc7b12,57f50091b8d1..
--- 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;
  }


pgpOJymdHRkTF.pgp
Description: OpenPGP digital signature

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?
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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/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 Rothwells...@canb.auug.org.au

diff --cc fs/ext4/symlink.c
index 839f71e941a9,57f50091b8d1..
--- a/fs/ext4/symlink.c
+++ b/fs/ext4/symlink.c
@@@ -21,107 -21,11 +21,107 @@@
  #include 
  #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();
 +  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();
 +  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();
 +  kunmap(cpage);
 +  page_cache_release(cpage);
 +  return ERR_PTR(-ENOMEM);
 +  }
 +  pstr.name = paddr;
 +  res = _ext4_fname_disk_to_usr(ctx, , );
 +  if (res < 0) {
 +  ext4_put_fname_crypto_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();
 +  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;
  }


pgpm0HB1xbW3A.pgp
Description: OpenPGP digital signature

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/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 Rothwells...@canb.auug.org.au

diff --cc fs/ext4/namei.c
index c5197176dba4,e086eebe335e..
--- 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;


pgpFhuMAiWVOY.pgp
Description: OpenPGP digital signature

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/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 Rothwells...@canb.auug.org.au

diff --cc fs/ext4/symlink.c
index 839f71e941a9,57f50091b8d1..
--- 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;
  }


pgpm0HB1xbW3A.pgp
Description: OpenPGP digital signature

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/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 Rothwells...@canb.auug.org.au

diff --cc fs/ext4/namei.c
index c5197176dba4,e086eebe335e..
--- 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;


pgpFhuMAiWVOY.pgp
Description: OpenPGP digital signature

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

[Stephen Rothwell]

Hi Christoph,

On Tue, 7 Apr 2015 09:02:14 +0200 Christoph Hellwig  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 Rothwells...@canb.auug.org.au


pgpwiK9o0zDfO.pgp
Description: OpenPGP digital signature

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..
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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..
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

[Stephen Rothwell]

Hi Christoph,

On Tue, 7 Apr 2015 09:02:14 +0200 Christoph Hellwig h...@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 Rothwells...@canb.auug.org.au


pgpwiK9o0zDfO.pgp
Description: OpenPGP digital signature

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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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 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 Rothwells...@canb.auug.org.au

diff --cc fs/ext4/inode.c
index 7eb70b7a5c19,42c942a950e1..
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@@ -35,12 -36,10 +35,11 @@@
  #include 
  #include 
  #include 
- #include 
 -#include 
  #include 
 +#include 
  
  #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);


pgpc6rWu57mIk.pgp
Description: OpenPGP digital signature

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 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 Rothwells...@canb.auug.org.au

diff --cc fs/ext4/inode.c
index 7eb70b7a5c19,42c942a950e1..
--- 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);


pgpc6rWu57mIk.pgp
Description: OpenPGP digital signature

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/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 Rothwells...@canb.auug.org.au

diff --cc fs/ext4/file.c
index 8c39305abc23,708aad768199..
--- 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(_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(>i_mutex);
 +  if (!unaligned_direct_aio)
 +  mutex_unlock(_I(inode)->i_write_mutex);
  
if (ret > 0) {
ssize_t err;


signature.asc
Description: PGP signature

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/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 Rothwells...@canb.auug.org.au

diff --cc fs/ext4/file.c
index 8c39305abc23,708aad768199..
--- 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;


signature.asc
Description: PGP signature

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/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 Rothwells...@canb.auug.org.au


pgpuBObCPyb86.pgp
Description: PGP signature

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/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 Rothwells...@canb.auug.org.au


pgpuBObCPyb86.pgp
Description: PGP signature