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 <[email protected]>
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 <[email protected]>
---
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;
Hi all,
On Mon, 14 Mar 2016 15:32:14 +1100 Stephen Rothwell <[email protected]> wrote:
>
> I also added the following merge fix patch:
>
> From: Stephen Rothwell <[email protected]>
> 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 <[email protected]>
> ---
> 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 <[email protected]>
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 <[email protected]>
---
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
Hi all,
On Mon, 14 Mar 2016 15:36:47 +1100 Stephen Rothwell <[email protected]> wrote:
>
> And I forgot this bit :-(
And just to show I should be on holidays, this as well :-(
From: Stephen Rothwell <[email protected]>
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 <[email protected]>
---
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
The aio changes have either been reviewed negatively or not at all. That
tree should be dropped.
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."
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.
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.
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."