Discussion:
linux-next: manual merge of the vfs tree with the cifs tree
Stephen Rothwell
2014-05-13 03:16:16 UTC
Permalink
Hi Al,

Today's linux-next merge of the vfs tree got a conflict in
fs/cifs/cifsfs.c between commit 485c4e72d0f1 ("cifs: revalidate mapping
prior to satisfying aio_read request") from the cifs tree and commits
aad4f8bb42af ("switch simple generic_file_aio_read() users to
->read_iter()") and 3dae8750c368 ("cifs: switch to ->write_iter()")
from the vfs tree.

I fixed it up (I dropped the cifs commit, so this will need a better
fix ...) and can carry the fix as necessary.
--
Cheers,
Stephen Rothwell sfr-3FnU+***@public.gmane.org
Jeff Layton
2014-05-13 10:49:37 UTC
Permalink
On Tue, 13 May 2014 13:16:16 +1000
Post by Stephen Rothwell
Hi Al,
Today's linux-next merge of the vfs tree got a conflict in
fs/cifs/cifsfs.c between commit 485c4e72d0f1 ("cifs: revalidate mapping
prior to satisfying aio_read request") from the cifs tree and commits
aad4f8bb42af ("switch simple generic_file_aio_read() users to
->read_iter()") and 3dae8750c368 ("cifs: switch to ->write_iter()")
from the vfs tree.
I fixed it up (I dropped the cifs commit, so this will need a better
fix ...) and can carry the fix as necessary.
Thanks Stephen,

Steve, what we probably need to do is replace 485c4e72d0f1 with this
patch. Only tested for compilation so far, but it should do the right
thing. I'll resend to the list once I've had a chance to test it.

--------------------------[snip]-------------------------

[PATCH] cifs: revalidate mapping prior to satisfying read_iter request

Before satisfying a read with cache=loose, we should always check
that the pagecache is valid.

Reported-by: Tetsuo Handa <penguin-***@I-love.SAKURA.ne.jp>
Signed-off-by: Jeff Layton <***@poochiereds.net>
---
fs/cifs/cifsfs.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 078c171c4f1b..149b09ce148d 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -729,6 +729,19 @@ out_nls:
goto out;
}

+static ssize_t
+cifs_loose_read_iter(struct kiocb *iocb, struct iov_iter *to)
+{
+ ssize_t rc;
+ struct inode *inode = file_inode(iocb->ki_filp);
+
+ rc = cifs_revalidate_mapping(inode);
+ if (rc)
+ return rc;
+
+ return generic_file_read_iter(iocb, to);
+}
+
static ssize_t cifs_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
{
struct inode *inode = file_inode(iocb->ki_filp);
@@ -885,7 +898,7 @@ const struct inode_operations cifs_symlink_inode_ops = {
const struct file_operations cifs_file_ops = {
.read = new_sync_read,
.write = new_sync_write,
- .read_iter = generic_file_read_iter,
+ .read_iter = cifs_loose_read_iter,
.write_iter = cifs_file_write_iter,
.open = cifs_open,
.release = cifs_close,
@@ -943,7 +956,7 @@ const struct file_operations cifs_file_direct_ops = {
const struct file_operations cifs_file_nobrl_ops = {
.read = new_sync_read,
.write = new_sync_write,
- .read_iter = generic_file_read_iter,
+ .read_iter = cifs_loose_read_iter,
.write_iter = cifs_file_write_iter,
.open = cifs_open,
.release = cifs_close,
--
1.9.0
Jeff Layton
2014-05-23 10:50:21 UTC
Permalink
Before satisfying a read with cache=loose, we should always check
that the pagecache is valid before allowing a read to be satisfied
out of it.

Reported-by: Tetsuo Handa <penguin-kernel-JPay3/***@public.gmane.org>
Signed-off-by: Jeff Layton <jlayton-vpEMnDpepFuMZCB2o+***@public.gmane.org>
---
fs/cifs/cifsfs.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 2c90d07c0b3a..888398067420 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -725,6 +725,19 @@ out_nls:
goto out;
}

+static ssize_t
+cifs_loose_read_iter(struct kiocb *iocb, struct iov_iter *iter)
+{
+ ssize_t rc;
+ struct inode *inode = file_inode(iocb->ki_filp);
+
+ rc = cifs_revalidate_mapping(inode);
+ if (rc)
+ return rc;
+
+ return generic_file_read_iter(iocb, iter);
+}
+
static ssize_t cifs_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
{
struct inode *inode = file_inode(iocb->ki_filp);
@@ -881,7 +894,7 @@ const struct inode_operations cifs_symlink_inode_ops = {
const struct file_operations cifs_file_ops = {
.read = new_sync_read,
.write = new_sync_write,
- .read_iter = generic_file_read_iter,
+ .read_iter = cifs_loose_read_iter,
.write_iter = cifs_file_write_iter,
.open = cifs_open,
.release = cifs_close,
@@ -939,7 +952,7 @@ const struct file_operations cifs_file_direct_ops = {
const struct file_operations cifs_file_nobrl_ops = {
.read = new_sync_read,
.write = new_sync_write,
- .read_iter = generic_file_read_iter,
+ .read_iter = cifs_loose_read_iter,
.write_iter = cifs_file_write_iter,
.open = cifs_open,
.release = cifs_close,
--
1.9.0
Jeff Layton
2014-05-23 14:44:24 UTC
Permalink
On Fri, 23 May 2014 06:50:21 -0400
Post by Jeff Layton
Before satisfying a read with cache=loose, we should always check
that the pagecache is valid before allowing a read to be satisfied
out of it.
---
fs/cifs/cifsfs.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 2c90d07c0b3a..888398067420 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
goto out;
}
+static ssize_t
+cifs_loose_read_iter(struct kiocb *iocb, struct iov_iter *iter)
+{
+ ssize_t rc;
+ struct inode *inode = file_inode(iocb->ki_filp);
+
+ rc = cifs_revalidate_mapping(inode);
+ if (rc)
+ return rc;
+
+ return generic_file_read_iter(iocb, iter);
+}
+
static ssize_t cifs_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
{
struct inode *inode = file_inode(iocb->ki_filp);
@@ -881,7 +894,7 @@ const struct inode_operations cifs_symlink_inode_ops = {
const struct file_operations cifs_file_ops = {
.read = new_sync_read,
.write = new_sync_write,
- .read_iter = generic_file_read_iter,
+ .read_iter = cifs_loose_read_iter,
.write_iter = cifs_file_write_iter,
.open = cifs_open,
.release = cifs_close,
@@ -939,7 +952,7 @@ const struct file_operations cifs_file_direct_ops = {
const struct file_operations cifs_file_nobrl_ops = {
.read = new_sync_read,
.write = new_sync_write,
- .read_iter = generic_file_read_iter,
+ .read_iter = cifs_loose_read_iter,
.write_iter = cifs_file_write_iter,
.open = cifs_open,
.release = cifs_close,
Steve,

This patch is a replacement for the last patch in the 4 patch series
for handling reads when cache=loose. The reason for the respin is that
aio_read has been replaced by read_iter in linux-next, so this is what
we'll want for v3.16 (once Al's read_iter patches are merged).

Thanks,
--
Jeff Layton <jlayton-vpEMnDpepFuMZCB2o+***@public.gmane.org>
Steve French
2014-05-23 14:52:04 UTC
Permalink
Yes - makes sense. I am rebuilding for-next branch of cifs-2.6.git
now. I plan to put your patch on the tip of the branch - I may create
two branches, one with old and one with new version of the patch since
when I am testing latest cifs patches (and also the proposed SMB3
Posix extensions) don't have Al's series.
Post by Jeff Layton
On Fri, 23 May 2014 06:50:21 -0400
Post by Jeff Layton
Before satisfying a read with cache=loose, we should always check
that the pagecache is valid before allowing a read to be satisfied
out of it.
---
fs/cifs/cifsfs.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 2c90d07c0b3a..888398067420 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
goto out;
}
+static ssize_t
+cifs_loose_read_iter(struct kiocb *iocb, struct iov_iter *iter)
+{
+ ssize_t rc;
+ struct inode *inode = file_inode(iocb->ki_filp);
+
+ rc = cifs_revalidate_mapping(inode);
+ if (rc)
+ return rc;
+
+ return generic_file_read_iter(iocb, iter);
+}
+
static ssize_t cifs_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
{
struct inode *inode = file_inode(iocb->ki_filp);
@@ -881,7 +894,7 @@ const struct inode_operations cifs_symlink_inode_ops = {
const struct file_operations cifs_file_ops = {
.read = new_sync_read,
.write = new_sync_write,
- .read_iter = generic_file_read_iter,
+ .read_iter = cifs_loose_read_iter,
.write_iter = cifs_file_write_iter,
.open = cifs_open,
.release = cifs_close,
@@ -939,7 +952,7 @@ const struct file_operations cifs_file_direct_ops = {
const struct file_operations cifs_file_nobrl_ops = {
.read = new_sync_read,
.write = new_sync_write,
- .read_iter = generic_file_read_iter,
+ .read_iter = cifs_loose_read_iter,
.write_iter = cifs_file_write_iter,
.open = cifs_open,
.release = cifs_close,
Steve,
This patch is a replacement for the last patch in the 4 patch series
for handling reads when cache=loose. The reason for the respin is that
aio_read has been replaced by read_iter in linux-next, so this is what
we'll want for v3.16 (once Al's read_iter patches are merged).
Thanks,
--
--
Thanks,

Steve
Steve French
2014-05-23 15:33:41 UTC
Permalink
I pushed the other cifs patches to cifs-2.6.git for-next (and created
a for-next-without-aio-iter branch that also includes the same set of
cifs patch and also includes the older version of your revalidate
patch that builds on current kernels) but your revalidate read_iter
patch is not going to merge to for-next without me picking up at a
minimum the patch that adds read_iter and write_iter to fs/cifs.
Suggestions?
Post by Steve French
Yes - makes sense. I am rebuilding for-next branch of cifs-2.6.git
now. I plan to put your patch on the tip of the branch - I may create
two branches, one with old and one with new version of the patch since
when I am testing latest cifs patches (and also the proposed SMB3
Posix extensions) don't have Al's series.
Post by Jeff Layton
On Fri, 23 May 2014 06:50:21 -0400
Post by Jeff Layton
Before satisfying a read with cache=loose, we should always check
that the pagecache is valid before allowing a read to be satisfied
out of it.
---
fs/cifs/cifsfs.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 2c90d07c0b3a..888398067420 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
goto out;
}
+static ssize_t
+cifs_loose_read_iter(struct kiocb *iocb, struct iov_iter *iter)
+{
+ ssize_t rc;
+ struct inode *inode = file_inode(iocb->ki_filp);
+
+ rc = cifs_revalidate_mapping(inode);
+ if (rc)
+ return rc;
+
+ return generic_file_read_iter(iocb, iter);
+}
+
static ssize_t cifs_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
{
struct inode *inode = file_inode(iocb->ki_filp);
@@ -881,7 +894,7 @@ const struct inode_operations cifs_symlink_inode_ops = {
const struct file_operations cifs_file_ops = {
.read = new_sync_read,
.write = new_sync_write,
- .read_iter = generic_file_read_iter,
+ .read_iter = cifs_loose_read_iter,
.write_iter = cifs_file_write_iter,
.open = cifs_open,
.release = cifs_close,
@@ -939,7 +952,7 @@ const struct file_operations cifs_file_direct_ops = {
const struct file_operations cifs_file_nobrl_ops = {
.read = new_sync_read,
.write = new_sync_write,
- .read_iter = generic_file_read_iter,
+ .read_iter = cifs_loose_read_iter,
.write_iter = cifs_file_write_iter,
.open = cifs_open,
.release = cifs_close,
Steve,
This patch is a replacement for the last patch in the 4 patch series
for handling reads when cache=loose. The reason for the respin is that
aio_read has been replaced by read_iter in linux-next, so this is what
we'll want for v3.16 (once Al's read_iter patches are merged).
Thanks,
--
--
Thanks,
Steve
--
Thanks,

Steve
Jeff Layton
2014-05-23 15:35:58 UTC
Permalink
On Fri, 23 May 2014 10:33:41 -0500
Post by Steve French
I pushed the other cifs patches to cifs-2.6.git for-next (and created
a for-next-without-aio-iter branch that also includes the same set of
cifs patch and also includes the older version of your revalidate
patch that builds on current kernels) but your revalidate read_iter
patch is not going to merge to for-next without me picking up at a
minimum the patch that adds read_iter and write_iter to fs/cifs.
Suggestions?
Tough call...

I'm not sure how to handle that. One possibility would be to rebase
your for-next on top of Al's tree? Sort of icky, but I'm not sure what
else can be done...
Post by Steve French
Post by Steve French
Yes - makes sense. I am rebuilding for-next branch of cifs-2.6.git
now. I plan to put your patch on the tip of the branch - I may create
two branches, one with old and one with new version of the patch since
when I am testing latest cifs patches (and also the proposed SMB3
Posix extensions) don't have Al's series.
Post by Jeff Layton
On Fri, 23 May 2014 06:50:21 -0400
Post by Jeff Layton
Before satisfying a read with cache=loose, we should always check
that the pagecache is valid before allowing a read to be satisfied
out of it.
---
fs/cifs/cifsfs.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 2c90d07c0b3a..888398067420 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
goto out;
}
+static ssize_t
+cifs_loose_read_iter(struct kiocb *iocb, struct iov_iter *iter)
+{
+ ssize_t rc;
+ struct inode *inode = file_inode(iocb->ki_filp);
+
+ rc = cifs_revalidate_mapping(inode);
+ if (rc)
+ return rc;
+
+ return generic_file_read_iter(iocb, iter);
+}
+
static ssize_t cifs_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
{
struct inode *inode = file_inode(iocb->ki_filp);
@@ -881,7 +894,7 @@ const struct inode_operations cifs_symlink_inode_ops = {
const struct file_operations cifs_file_ops = {
.read = new_sync_read,
.write = new_sync_write,
- .read_iter = generic_file_read_iter,
+ .read_iter = cifs_loose_read_iter,
.write_iter = cifs_file_write_iter,
.open = cifs_open,
.release = cifs_close,
@@ -939,7 +952,7 @@ const struct file_operations cifs_file_direct_ops = {
const struct file_operations cifs_file_nobrl_ops = {
.read = new_sync_read,
.write = new_sync_write,
- .read_iter = generic_file_read_iter,
+ .read_iter = cifs_loose_read_iter,
.write_iter = cifs_file_write_iter,
.open = cifs_open,
.release = cifs_close,
Steve,
This patch is a replacement for the last patch in the 4 patch series
for handling reads when cache=loose. The reason for the respin is that
aio_read has been replaced by read_iter in linux-next, so this is what
we'll want for v3.16 (once Al's read_iter patches are merged).
Thanks,
--
--
Thanks,
Steve
--
Jeff Layton <jlayton-vpEMnDpepFuMZCB2o+***@public.gmane.org>
Loading...