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

Groups > linux.kernel > #1356894 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  linux-next: manual merge of the aio tree with the vfs tree Stephen Rothwell <sfr@canb.auug.org.au> - 2016-03-14 05:40 +0100
    Re: linux-next: manual merge of the aio tree with the vfs tree Stephen Rothwell <sfr@canb.auug.org.au> - 2016-03-14 05:40 +0100
      Re: linux-next: manual merge of the aio tree with the vfs tree Stephen Rothwell <sfr@canb.auug.org.au> - 2016-03-14 05:50 +0100
    Re: linux-next: manual merge of the aio tree with the vfs tree Christoph Hellwig <hch@lst.de> - 2016-03-14 08:40 +0100
      Re: linux-next: manual merge of the aio tree with the vfs tree Benjamin LaHaise <bcrl@kvack.org> - 2016-03-14 16:30 +0100
        Re: linux-next: manual merge of the aio tree with the vfs tree Al Viro <viro@ZenIV.linux.org.uk> - 2016-03-15 06:40 +0100
          Re: linux-next: manual merge of the aio tree with the vfs tree Benjamin LaHaise <bcrl@kvack.org> - 2016-03-15 14:30 +0100
        Re: linux-next: manual merge of the aio tree with the vfs tree Christoph Hellwig <hch@lst.de> - 2016-03-15 09:40 +0100

#1356894 — linux-next: manual merge of the aio tree with the vfs tree

Hi Benjamin,

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

  fs/read_write.c

between commit:

  793b80ef14af ("vfs: pass a flags argument to vfs_readv/vfs_writev")

from the vfs tree and commit:

  4047629ed53e ("fs: make do_loop_readv_writev() non-static")

from the aio tree.

I fixed it up (see below - thanks to Al for the heads up) and can carry
the fix as necessary (no action is required).

I also added the following merge fix patch:

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Mon, 14 Mar 2016 15:28:05 +1100
Subject: [PATCH] vfs: do_loop_readv_writev() API change merge fix

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 fs/internal.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/internal.h b/fs/internal.h
index 3bbe63d5eb5e..39f6e9831c5f 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -139,7 +139,7 @@ extern long prune_dcache_sb(struct super_block *sb, struct shrink_control *sc);
  */
 extern int rw_verify_area(int, struct file *, const loff_t *, size_t);
 extern ssize_t do_loop_readv_writev(struct file *filp, struct iov_iter *iter,
-				    loff_t *ppos, io_fn_t fn);
+				    loff_t *ppos, io_fn_t fn, int flags);
 
 
 /*
-- 
2.7.0

-- 
Cheers,
Stephen Rothwell

diff --cc fs/read_write.c
index cf377cf9dfe3,36344ff2991c..000000000000
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@@ -713,8 -665,8 +710,8 @@@ static ssize_t do_iter_readv_writev(str
  }
  
  /* Do it by hand, with file-ops */
- static ssize_t do_loop_readv_writev(struct file *filp, struct iov_iter *iter,
- 		loff_t *ppos, io_fn_t fn, int flags)
+ ssize_t do_loop_readv_writev(struct file *filp, struct iov_iter *iter,
 -		loff_t *ppos, io_fn_t fn)
++		loff_t *ppos, io_fn_t fn , int flags)
  {
  	ssize_t ret = 0;
  

[toc] | [next] | [standalone]

#1356895

Hi all,

On Mon, 14 Mar 2016 15:32:14 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> I also added the following merge fix patch:
> 
> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Mon, 14 Mar 2016 15:28:05 +1100
> Subject: [PATCH] vfs: do_loop_readv_writev() API change merge fix
> 
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
>  fs/internal.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/internal.h b/fs/internal.h
> index 3bbe63d5eb5e..39f6e9831c5f 100644
> --- a/fs/internal.h
> +++ b/fs/internal.h
> @@ -139,7 +139,7 @@ extern long prune_dcache_sb(struct super_block *sb, struct shrink_control *sc);
>   */
>  extern int rw_verify_area(int, struct file *, const loff_t *, size_t);
>  extern ssize_t do_loop_readv_writev(struct file *filp, struct iov_iter *iter,
> -				    loff_t *ppos, io_fn_t fn);
> +				    loff_t *ppos, io_fn_t fn, int flags);
>  
>  
>  /*

And I forgot this bit :-(

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Mon, 14 Mar 2016 15:34:17 +1100
Subject: [PATCH] vfs: do_loop_readv_writev API merge fix up part 2

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 fs/aio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/aio.c b/fs/aio.c
index b079cb65a90f..d2dd7d482ffe 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -1576,7 +1576,7 @@ static long aio_thread_op_read_iter(struct aio_kiocb *iocb)
 	} else if (filp->f_op->read)
 		ret = do_loop_readv_writev(filp, &iocb->ki_iter,
 					   &iocb->common.ki_pos,
-					   filp->f_op->read);
+					   filp->f_op->read, 0);
 	else
 		ret = -EINVAL;
 	unuse_mm(iocb->ki_ctx->mm);
-- 
2.7.0

-- 
Cheers,
Stephen Rothwell

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

#1356900

Hi all,

On Mon, 14 Mar 2016 15:36:47 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> And I forgot this bit :-(

And just to show I should be on holidays, this as well :-(

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Mon, 14 Mar 2016 15:38:22 +1100
Subject: [PATCH] vfs: do_loop_readv_writev API merge fix part 3

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 fs/aio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/aio.c b/fs/aio.c
index d2dd7d482ffe..5652953d73bc 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -1626,7 +1626,7 @@ static long aio_thread_op_write_iter(struct aio_kiocb *iocb)
 	} else if (filp->f_op->write)
 		ret = do_loop_readv_writev(filp, &iocb->ki_iter,
 					   &iocb->common.ki_pos,
-					   (io_fn_t)filp->f_op->write);
+					   (io_fn_t)filp->f_op->write, 0);
 	else
 		ret = -EINVAL;
 	current->signal->rlim[RLIMIT_FSIZE].rlim_cur = saved_rlim_fsize;
-- 
2.7.0

-- 
Cheers,
Stephen Rothwell

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

#1357006

The aio changes have either been reviewed negatively or not at all.  That 
tree should be dropped.

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

#1357351

On Mon, Mar 14, 2016 at 08:35:23AM +0100, Christoph Hellwig wrote:
> The aio changes have either been reviewed negatively or not at all.  That 
> tree should be dropped.

That isn't solely your decision.  If you have comments, please provide 
constructive feedback, as there are users and use-cases that need this 
kind of functionality.

		-ben
-- 
"Thought is the essence of where you are now."

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

#1357809

On Mon, Mar 14, 2016 at 11:24:38AM -0400, Benjamin LaHaise wrote:
> On Mon, Mar 14, 2016 at 08:35:23AM +0100, Christoph Hellwig wrote:
> > The aio changes have either been reviewed negatively or not at all.  That 
> > tree should be dropped.
> 
> That isn't solely your decision.  If you have comments, please provide 
> constructive feedback, as there are users and use-cases that need this 
> kind of functionality.

"This kind of functionality" being what, exactly?  Bypassing code review?
It's not that you've made trivial mistakes; everyone does from time to time.
But failing to post patches with non-trivial interactions outside of the
subsystem you are familiar with (be it on fsdevel or privately to people who
work with the areas involved) *AND* failing to recognize that the lack
of review might be a problem even after having been explicitly told so...

For fuck sake, you should know better.  You are not a newbie with a pet set
of half-baked patches for his pet application, refering to (unspecified)
"users that need this kind of functionality" and getting very surprised when
those mean, rude folks on development lists inform them that code review is
more than just a good idea.

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

#1358038

On Tue, Mar 15, 2016 at 05:35:33AM +0000, Al Viro wrote:
> On Mon, Mar 14, 2016 at 11:24:38AM -0400, Benjamin LaHaise wrote:
> > On Mon, Mar 14, 2016 at 08:35:23AM +0100, Christoph Hellwig wrote:
> > > The aio changes have either been reviewed negatively or not at all.  That 
> > > tree should be dropped.
> > 
> > That isn't solely your decision.  If you have comments, please provide 
> > constructive feedback, as there are users and use-cases that need this 
> > kind of functionality.
> 
> "This kind of functionality" being what, exactly?  Bypassing code review?
> It's not that you've made trivial mistakes; everyone does from time to time.
> But failing to post patches with non-trivial interactions outside of the
> subsystem you are familiar with (be it on fsdevel or privately to people who
> work with the areas involved) *AND* failing to recognize that the lack
> of review might be a problem even after having been explicitly told so...

I'm not bypassing code review.  The code was sent out back in January, 
you were cc'd on it and has been waiting for you to provide some feedback 
on the mailing lists, which you haven't done until yesterday.  Given that 
review has not occurred, I fully well don't expect this series to be 
merged until further work is done.

> For fuck sake, you should know better.  You are not a newbie with a pet set
> of half-baked patches for his pet application, refering to (unspecified)
> "users that need this kind of functionality" and getting very surprised when
> those mean, rude folks on development lists inform them that code review is
> more than just a good idea.

No, my comment was based on HCH essentially saying that exposure in 
linux-next is a bad thing and the tree should be removed.  Quite the 
opposite -- it's useful in that it lets people see what is going on and 
lets me know about conflicts with other work.  Removing the tree from 
linux-next gets rid of those benefits, and that is what I was objecting 
to.  Fwiw, I don't think someone should have a veto over what goes in 
linux-next.  People should provide feedback, not a blanket "drop that".

		-ben
-- 
"Thought is the essence of where you are now."

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

#1357898

On Mon, Mar 14, 2016 at 11:24:38AM -0400, Benjamin LaHaise wrote:
> On Mon, Mar 14, 2016 at 08:35:23AM +0100, Christoph Hellwig wrote:
> > The aio changes have either been reviewed negatively or not at all.  That 
> > tree should be dropped.
> 
> That isn't solely your decision.  If you have comments, please provide 
> constructive feedback, as there are users and use-cases that need this 
> kind of functionality.

Nothing should be a "sole decision".  But you have patches that stomp
all over areas outside your maintainership, and even those in there
aren't reviewed.  This does not fit the Linux-next criteria.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web