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

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

[Stephen Rothwell]

Hi Benjamin,

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

  fs/aio.c

between commit:

  32ec9f249d65 ("io_getevents: Use timespec64 to represent timeouts")

from the vfs tree and commit:

  eb5263749f68 ("aio: handle integer overflow in io_getevents() timespec usage")

from the aio tree.

I fixed it up (I just dropped the change in the latter commit) 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

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

[Benjamin LaHaise]

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

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

[Benjamin LaHaise]

On Tue, Mar 15, 2016 at 05:19:39AM +0000, Al Viro wrote:
> On Tue, Mar 15, 2016 at 05:07:12AM +0000, Al Viro wrote:
> 
> > There *is* a reason for code review.  Or, at least, asking somebody familiar
> > with the code you are working with whether some assumption you are making
> > is true or false.  Me, for example, in our conversation regarding earlier parts
> > of aio.git queue about a week ago.  Or at any other point.
> 
> While we are at it, 150a0b49 ("aio: add support for async openat()") is also
> crap.  fs_struct and files_struct is nowhere near enough.  And yes, I realize
> that your application probably doesn't step into it.  Which means that these
> patches are just fine for your private kernel.  _Not_ for mainline.
> 
> Reviewed-and-NAKed-by: Al Viro <viro@zeniv.linux.org.uk>

You've had two months to make this comment, so I'm glad you've finally 
done so.

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

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

[Christoph Hellwig]

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.

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

[Al Viro]

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.

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

[Al Viro]

On Tue, Mar 15, 2016 at 05:07:12AM +0000, Al Viro wrote:

> There *is* a reason for code review.  Or, at least, asking somebody familiar
> with the code you are working with whether some assumption you are making
> is true or false.  Me, for example, in our conversation regarding earlier parts
> of aio.git queue about a week ago.  Or at any other point.

While we are at it, 150a0b49 ("aio: add support for async openat()") is also
crap.  fs_struct and files_struct is nowhere near enough.  And yes, I realize
that your application probably doesn't step into it.  Which means that these
patches are just fine for your private kernel.  _Not_ for mainline.

Reviewed-and-NAKed-by: Al Viro <viro@zeniv.linux.org.uk>

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

[Al Viro]

On Tue, Mar 15, 2016 at 04:34:48AM +0000, Al Viro wrote:

> Incidentally, why the hell has that thing never landed in my mailbox?  Not
> directly, not Cc'd, not via fsdevel either.
> 
> Ben, what the fuck going on?  OK, you don't feel necessary to mention
> that to me (or have me take a look through it).  Your business.  You also
> don't bother to bring it on fsdevel.  Again, your estimates of the
> usefulness of said list and review there are your business.  But it looks
> like you also do not bother to check what has landed in the mainline two
> months ago.  WTF?

While we are at it, aio.git commit in question is crap anyway.  What is the
semantics of that LOOKUP_NONBLOCK thing and what makes you think that
it will *not* block prior to reaching do_last()?  LOOKUP_RCU that was
originally there?  Sorry, wrong.  RCU pathwalk will happily fall back to
non-RCU one if it can do so without restart from scratch.  And proceed
to lock directories, hit the disk over nbd over wet string, do automounts,
etc.  Anything and everything.  IOW, this is complete BS and had been such
for at least ~5 years.

There *is* a reason for code review.  Or, at least, asking somebody familiar
with the code you are working with whether some assumption you are making
is true or false.  Me, for example, in our conversation regarding earlier parts
of aio.git queue about a week ago.  Or at any other point.

						Al "Really annoyed" Viro

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

[Stephen Rothwell]

Hi Al,

On Tue, 15 Mar 2016 04:34:48 +0000 Al Viro <viro@ZenIV.linux.org.uk> wrote:
>
> > between commit:
> > 
> >   5955102c9984 ("wrappers for ->i_mutex access")
> > 
> > from the vfs tree and commit:
> 
> What.
> 
> The.
> 
> Hell?
> 
> The first commit is in the mainline, not in vfs tree.  And it had been
> there since before -rc1.

Sorry, my mistake.

-- 
Cheers,
Stephen Rothwell

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

[Al Viro]

On Tue, Mar 15, 2016 at 03:06:40PM +1100, Stephen Rothwell wrote:
> Hi Ben,
> 
> Today's linux-next merge of the aio tree got a conflict in:
> 
>   fs/namei.c
> 
> between commit:
> 
>   5955102c9984 ("wrappers for ->i_mutex access")
> 
> from the vfs tree and commit:
> 
>   5d3d80fcf992 ("aio: add support for in-submit openat")
> 
> from the aio tree.

What.

The.

Hell?

The first commit is in the mainline, not in vfs tree.  And it had been
there since before -rc1.

Incidentally, why the hell has that thing never landed in my mailbox?  Not
directly, not Cc'd, not via fsdevel either.

Ben, what the fuck going on?  OK, you don't feel necessary to mention
that to me (or have me take a look through it).  Your business.  You also
don't bother to bring it on fsdevel.  Again, your estimates of the
usefulness of said list and review there are your business.  But it looks
like you also do not bother to check what has landed in the mainline two
months ago.  WTF?

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

[Stephen Rothwell]

Hi Ben,

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

  fs/namei.c

between commit:

  5955102c9984 ("wrappers for ->i_mutex access")

from the vfs tree and commit:

  5d3d80fcf992 ("aio: add support for in-submit openat")

from the aio tree.

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

-- 
Cheers,
Stephen Rothwell

diff --cc fs/namei.c
index 794f81dce766,260782f5a868..000000000000
--- a/fs/namei.c
+++ b/fs/namei.c
@@@ -3125,9 -3079,15 +3125,15 @@@ retry_lookup
  		 * dropping this one anyway.
  		 */
  	}
+ 
+ 	if (nd->flags & LOOKUP_NONBLOCK) {
+ 		error = -EAGAIN;
+ 		goto out;
+ 	}
+ 		
 -	mutex_lock(&dir->d_inode->i_mutex);
 +	inode_lock(dir->d_inode);
  	error = lookup_open(nd, &path, file, op, got_write, opened);
 -	mutex_unlock(&dir->d_inode->i_mutex);
 +	inode_unlock(dir->d_inode);
  
  	if (error <= 0) {
  		if (error)

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

[Benjamin LaHaise]

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

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

[Christoph Hellwig]

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

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

[Stephen Rothwell]

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

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

[Stephen Rothwell]

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

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

[Stephen Rothwell]

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;