[PATCH 1/5] fs/9p: Fix revalidate to return correct value

[PATCH 1/5] fs/9p: Fix revalidate to return correct value

[Aneesh Kumar K.V]

revalidate should return > 0 on success.

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 fs/9p/vfs_dentry.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/9p/vfs_dentry.c b/fs/9p/vfs_dentry.c
index b6a3b9f..59c5ddc 100644
--- a/fs/9p/vfs_dentry.c
+++ b/fs/9p/vfs_dentry.c
@@ -126,7 +126,7 @@ static int v9fs_lookup_revalidate(struct dentry *dentry, struct nameidata *nd)
 			retval = v9fs_refresh_inode_dotl(fid, inode);
 		else
 			retval = v9fs_refresh_inode(fid, inode);
-		if (retval <= 0)
+		if (retval < 0)
 			return retval;
 	}
 out_valid:
-- 
1.7.1

[PATCH 2/5] fs/9p: Use write_inode for data sync on server

[Aneesh Kumar K.V]

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 fs/9p/vfs_super.c |   43 +++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 43 insertions(+), 0 deletions(-)

diff --git a/fs/9p/vfs_super.c b/fs/9p/vfs_super.c
index f3eed33..d94adcb 100644
--- a/fs/9p/vfs_super.c
+++ b/fs/9p/vfs_super.c
@@ -307,6 +307,47 @@ static int v9fs_drop_inode(struct inode *inode)
 	return 1;
 }
 
+static int v9fs_write_inode(struct inode *inode,
+			    struct writeback_control *wbc)
+{
+	int ret;
+	struct p9_wstat wstat;
+	struct v9fs_inode *v9inode;
+	/*
+	 * send an fsync request to server irrespective of
+	 * wbc->sync_mode.
+	 */
+	P9_DPRINTK(P9_DEBUG_VFS, "%s: inode %p\n", __func__, inode);
+	v9inode = V9FS_I(inode);
+	v9fs_blank_wstat(&wstat);
+
+	ret = p9_client_wstat(v9inode->writeback_fid, &wstat);
+	if (ret < 0) {
+		__mark_inode_dirty(inode, I_DIRTY_DATASYNC);
+		return ret;
+	}
+	return 0;
+}
+
+static int v9fs_write_inode_dotl(struct inode *inode,
+				 struct writeback_control *wbc)
+{
+	int ret;
+	struct v9fs_inode *v9inode;
+	/*
+	 * send an fsync request to server irrespective of
+	 * wbc->sync_mode.
+	 */
+	P9_DPRINTK(P9_DEBUG_VFS, "%s: inode %p\n", __func__, inode);
+	v9inode = V9FS_I(inode);
+	ret = p9_client_fsync(v9inode->writeback_fid, 0);
+	if (ret < 0) {
+		__mark_inode_dirty(inode, I_DIRTY_DATASYNC);
+		return ret;
+	}
+	return 0;
+}
+
 static const struct super_operations v9fs_super_ops = {
 	.alloc_inode = v9fs_alloc_inode,
 	.destroy_inode = v9fs_destroy_inode,
@@ -314,6 +355,7 @@ static const struct super_operations v9fs_super_ops = {
 	.evict_inode = v9fs_evict_inode,
 	.show_options = generic_show_options,
 	.umount_begin = v9fs_umount_begin,
+	.write_inode = v9fs_write_inode,
 };
 
 static const struct super_operations v9fs_super_ops_dotl = {
@@ -325,6 +367,7 @@ static const struct super_operations v9fs_super_ops_dotl = {
 	.evict_inode = v9fs_evict_inode,
 	.show_options = generic_show_options,
 	.umount_begin = v9fs_umount_begin,
+	.write_inode = v9fs_write_inode_dotl,
 };
 
 struct file_system_type v9fs_fs_type = {
-- 
1.7.1

Re: [V9fs-developer] [PATCH 2/5] fs/9p: Use write_inode for data sync on server

[Venkateswararao Jujjuri]

On 03/25/2011 04:30 AM, Aneesh Kumar K.V wrote:
> Signed-off-by: Aneesh Kumar K.V<aneesh.kumar@linux.vnet.ibm.com>

Reviewed-by: Venkateswararao Jujjuri <jvrao@linux.vnet.ibm.com>
> ---
>   fs/9p/vfs_super.c |   43 +++++++++++++++++++++++++++++++++++++++++++
>   1 files changed, 43 insertions(+), 0 deletions(-)
>
> diff --git a/fs/9p/vfs_super.c b/fs/9p/vfs_super.c
> index f3eed33..d94adcb 100644
> --- a/fs/9p/vfs_super.c
> +++ b/fs/9p/vfs_super.c
> @@ -307,6 +307,47 @@ static int v9fs_drop_inode(struct inode *inode)
>   	return 1;
>   }
>
> +static int v9fs_write_inode(struct inode *inode,
> +			    struct writeback_control *wbc)
> +{
> +	int ret;
> +	struct p9_wstat wstat;
> +	struct v9fs_inode *v9inode;
> +	/*
> +	 * send an fsync request to server irrespective of
> +	 * wbc->sync_mode.
> +	 */
> +	P9_DPRINTK(P9_DEBUG_VFS, "%s: inode %p\n", __func__, inode);
> +	v9inode = V9FS_I(inode);
> +	v9fs_blank_wstat(&wstat);
> +
> +	ret = p9_client_wstat(v9inode->writeback_fid,&wstat);
> +	if (ret<  0) {
> +		__mark_inode_dirty(inode, I_DIRTY_DATASYNC);
> +		return ret;
> +	}
> +	return 0;
> +}
> +
> +static int v9fs_write_inode_dotl(struct inode *inode,
> +				 struct writeback_control *wbc)
> +{
> +	int ret;
> +	struct v9fs_inode *v9inode;
> +	/*
> +	 * send an fsync request to server irrespective of
> +	 * wbc->sync_mode.
> +	 */
> +	P9_DPRINTK(P9_DEBUG_VFS, "%s: inode %p\n", __func__, inode);
> +	v9inode = V9FS_I(inode);
> +	ret = p9_client_fsync(v9inode->writeback_fid, 0);
> +	if (ret<  0) {
> +		__mark_inode_dirty(inode, I_DIRTY_DATASYNC);
> +		return ret;
> +	}
> +	return 0;
> +}
> +
>   static const struct super_operations v9fs_super_ops = {
>   	.alloc_inode = v9fs_alloc_inode,
>   	.destroy_inode = v9fs_destroy_inode,
> @@ -314,6 +355,7 @@ static const struct super_operations v9fs_super_ops = {
>   	.evict_inode = v9fs_evict_inode,
>   	.show_options = generic_show_options,
>   	.umount_begin = v9fs_umount_begin,
> +	.write_inode = v9fs_write_inode,
>   };
>
>   static const struct super_operations v9fs_super_ops_dotl = {
> @@ -325,6 +367,7 @@ static const struct super_operations v9fs_super_ops_dotl = {
>   	.evict_inode = v9fs_evict_inode,
>   	.show_options = generic_show_options,
>   	.umount_begin = v9fs_umount_begin,
> +	.write_inode = v9fs_write_inode_dotl,
>   };
>
>   struct file_system_type v9fs_fs_type = {

[PATCH 3/5] 9p: revert tsyncfs related changes

[Aneesh Kumar K.V]

Now that we use write_inode to flush server
cache related to fid, we don't need tsyncfs.
This help us to do a more efficient server flush
for dotu protocol

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 fs/9p/fid.c             |   15 ++-------------
 fs/9p/v9fs.h            |    1 -
 fs/9p/vfs_super.c       |   33 +++++++++------------------------
 include/net/9p/9p.h     |    2 --
 include/net/9p/client.h |    1 -
 net/9p/client.c         |   21 ---------------------
 6 files changed, 11 insertions(+), 62 deletions(-)

diff --git a/fs/9p/fid.c b/fs/9p/fid.c
index 0ee5945..85b67ff 100644
--- a/fs/9p/fid.c
+++ b/fs/9p/fid.c
@@ -286,11 +286,9 @@ static struct p9_fid *v9fs_fid_clone_with_uid(struct dentry *dentry, uid_t uid)
 
 struct p9_fid *v9fs_writeback_fid(struct dentry *dentry)
 {
-	int err, flags;
+	int err;
 	struct p9_fid *fid;
-	struct v9fs_session_info *v9ses;
 
-	v9ses = v9fs_dentry2v9ses(dentry);
 	fid = v9fs_fid_clone_with_uid(dentry, 0);
 	if (IS_ERR(fid))
 		goto error_out;
@@ -299,17 +297,8 @@ struct p9_fid *v9fs_writeback_fid(struct dentry *dentry)
 	 * dirty pages. We always request for the open fid in read-write
 	 * mode so that a partial page write which result in page
 	 * read can work.
-	 *
-	 * we don't have a tsyncfs operation for older version
-	 * of protocol. So make sure the write back fid is
-	 * opened in O_SYNC mode.
 	 */
-	if (!v9fs_proto_dotl(v9ses))
-		flags = O_RDWR | O_SYNC;
-	else
-		flags = O_RDWR;
-
-	err = p9_client_open(fid, flags);
+	err = p9_client_open(fid, O_RDWR);
 	if (err < 0) {
 		p9_client_clunk(fid);
 		fid = ERR_PTR(err);
diff --git a/fs/9p/v9fs.h b/fs/9p/v9fs.h
index 9665c2b..e5ebedf 100644
--- a/fs/9p/v9fs.h
+++ b/fs/9p/v9fs.h
@@ -116,7 +116,6 @@ struct v9fs_session_info {
 	struct list_head slist; /* list of sessions registered with v9fs */
 	struct backing_dev_info bdi;
 	struct rw_semaphore rename_sem;
-	struct p9_fid *root_fid; /* Used for file system sync */
 };
 
 /* cache_validity flags */
diff --git a/fs/9p/vfs_super.c b/fs/9p/vfs_super.c
index d94adcb..118078d 100644
--- a/fs/9p/vfs_super.c
+++ b/fs/9p/vfs_super.c
@@ -154,6 +154,7 @@ static struct dentry *v9fs_mount(struct file_system_type *fs_type, int flags,
 		retval = PTR_ERR(inode);
 		goto release_sb;
 	}
+
 	root = d_alloc_root(inode);
 	if (!root) {
 		iput(inode);
@@ -185,21 +186,10 @@ static struct dentry *v9fs_mount(struct file_system_type *fs_type, int flags,
 		p9stat_free(st);
 		kfree(st);
 	}
-	v9fs_fid_add(root, fid);
 	retval = v9fs_get_acl(inode, fid);
 	if (retval)
 		goto release_sb;
-	/*
-	 * Add the root fid to session info. This is used
-	 * for file system sync. We want a cloned fid here
-	 * so that we can do a sync_filesystem after a
-	 * shrink_dcache_for_umount
-	 */
-	v9ses->root_fid = v9fs_fid_clone(root);
-	if (IS_ERR(v9ses->root_fid)) {
-		retval = PTR_ERR(v9ses->root_fid);
-		goto release_sb;
-	}
+	v9fs_fid_add(root, fid);
 
 	P9_DPRINTK(P9_DEBUG_VFS, " simple set mount, return 0\n");
 	return dget(sb->s_root);
@@ -210,11 +200,15 @@ close_session:
 	v9fs_session_close(v9ses);
 	kfree(v9ses);
 	return ERR_PTR(retval);
+
 release_sb:
 	/*
-	 * we will do the session_close and root dentry
-	 * release in the below call.
+	 * we will do the session_close and root dentry release
+	 * in the below call. But we need to clunk fid, because we haven't
+	 * attached the fid to dentry so it won't get clunked
+	 * automatically.
 	 */
+	p9_client_clunk(fid);
 	deactivate_locked_super(sb);
 	return ERR_PTR(retval);
 }
@@ -232,7 +226,7 @@ static void v9fs_kill_super(struct super_block *s)
 	P9_DPRINTK(P9_DEBUG_VFS, " %p\n", s);
 
 	kill_anon_super(s);
-	p9_client_clunk(v9ses->root_fid);
+
 	v9fs_session_cancel(v9ses);
 	v9fs_session_close(v9ses);
 	kfree(v9ses);
@@ -285,14 +279,6 @@ done:
 	return res;
 }
 
-static int v9fs_sync_fs(struct super_block *sb, int wait)
-{
-	struct v9fs_session_info *v9ses = sb->s_fs_info;
-
-	P9_DPRINTK(P9_DEBUG_VFS, "v9fs_sync_fs: super_block %p\n", sb);
-	return p9_client_sync_fs(v9ses->root_fid);
-}
-
 static int v9fs_drop_inode(struct inode *inode)
 {
 	struct v9fs_session_info *v9ses;
@@ -361,7 +347,6 @@ static const struct super_operations v9fs_super_ops = {
 static const struct super_operations v9fs_super_ops_dotl = {
 	.alloc_inode = v9fs_alloc_inode,
 	.destroy_inode = v9fs_destroy_inode,
-	.sync_fs = v9fs_sync_fs,
 	.statfs = v9fs_statfs,
 	.drop_inode = v9fs_drop_inode,
 	.evict_inode = v9fs_evict_inode,
diff --git a/include/net/9p/9p.h b/include/net/9p/9p.h
index 6b75a69..eaa45f9 100644
--- a/include/net/9p/9p.h
+++ b/include/net/9p/9p.h
@@ -139,8 +139,6 @@ do { \
  */
 
 enum p9_msg_t {
-	P9_TSYNCFS = 0,
-	P9_RSYNCFS,
 	P9_TLERROR = 6,
 	P9_RLERROR,
 	P9_TSTATFS = 8,
diff --git a/include/net/9p/client.h b/include/net/9p/client.h
index 0a30977..83ba6a4 100644
--- a/include/net/9p/client.h
+++ b/include/net/9p/client.h
@@ -230,7 +230,6 @@ int p9_client_create_dotl(struct p9_fid *ofid, char *name, u32 flags, u32 mode,
 		gid_t gid, struct p9_qid *qid);
 int p9_client_clunk(struct p9_fid *fid);
 int p9_client_fsync(struct p9_fid *fid, int datasync);
-int p9_client_sync_fs(struct p9_fid *fid);
 int p9_client_remove(struct p9_fid *fid);
 int p9_client_read(struct p9_fid *fid, char *data, char __user *udata,
 							u64 offset, u32 count);
diff --git a/net/9p/client.c b/net/9p/client.c
index 2ccbf04..fd7fb37 100644
--- a/net/9p/client.c
+++ b/net/9p/client.c
@@ -1220,27 +1220,6 @@ error:
 }
 EXPORT_SYMBOL(p9_client_fsync);
 
-int p9_client_sync_fs(struct p9_fid *fid)
-{
-	int err = 0;
-	struct p9_req_t *req;
-	struct p9_client *clnt;
-
-	P9_DPRINTK(P9_DEBUG_9P, ">>> TSYNC_FS fid %d\n", fid->fid);
-
-	clnt = fid->clnt;
-	req = p9_client_rpc(clnt, P9_TSYNCFS, "d", fid->fid);
-	if (IS_ERR(req)) {
-		err = PTR_ERR(req);
-		goto error;
-	}
-	P9_DPRINTK(P9_DEBUG_9P, "<<< RSYNCFS fid %d\n", fid->fid);
-	p9_free_req(clnt, req);
-error:
-	return err;
-}
-EXPORT_SYMBOL(p9_client_sync_fs);
-
 int p9_client_clunk(struct p9_fid *fid)
 {
 	int err;
-- 
1.7.1

Re: [V9fs-developer] [PATCH 3/5] 9p: revert tsyncfs related changes

[Venkateswararao Jujjuri]

On 03/25/2011 04:30 AM, Aneesh Kumar K.V wrote:
> Now that we use write_inode to flush server
> cache related to fid, we don't need tsyncfs.
> This help us to do a more efficient server flush
> for dotu protocol
Why are you singling out dotu only? won't it be applicable to dotl too?

- JV
> Signed-off-by: Aneesh Kumar K.V<aneesh.kumar@linux.vnet.ibm.com>
> ---
>   fs/9p/fid.c             |   15 ++-------------
>   fs/9p/v9fs.h            |    1 -
>   fs/9p/vfs_super.c       |   33 +++++++++------------------------
>   include/net/9p/9p.h     |    2 --
>   include/net/9p/client.h |    1 -
>   net/9p/client.c         |   21 ---------------------
>   6 files changed, 11 insertions(+), 62 deletions(-)
>
> diff --git a/fs/9p/fid.c b/fs/9p/fid.c
> index 0ee5945..85b67ff 100644
> --- a/fs/9p/fid.c
> +++ b/fs/9p/fid.c
> @@ -286,11 +286,9 @@ static struct p9_fid *v9fs_fid_clone_with_uid(struct dentry *dentry, uid_t uid)
>
>   struct p9_fid *v9fs_writeback_fid(struct dentry *dentry)
>   {
> -	int err, flags;
> +	int err;
>   	struct p9_fid *fid;
> -	struct v9fs_session_info *v9ses;
>
> -	v9ses = v9fs_dentry2v9ses(dentry);
>   	fid = v9fs_fid_clone_with_uid(dentry, 0);
>   	if (IS_ERR(fid))
>   		goto error_out;
> @@ -299,17 +297,8 @@ struct p9_fid *v9fs_writeback_fid(struct dentry *dentry)
>   	 * dirty pages. We always request for the open fid in read-write
>   	 * mode so that a partial page write which result in page
>   	 * read can work.
> -	 *
> -	 * we don't have a tsyncfs operation for older version
> -	 * of protocol. So make sure the write back fid is
> -	 * opened in O_SYNC mode.
>   	 */
> -	if (!v9fs_proto_dotl(v9ses))
> -		flags = O_RDWR | O_SYNC;
> -	else
> -		flags = O_RDWR;
> -
> -	err = p9_client_open(fid, flags);
> +	err = p9_client_open(fid, O_RDWR);
>   	if (err<  0) {
>   		p9_client_clunk(fid);
>   		fid = ERR_PTR(err);
> diff --git a/fs/9p/v9fs.h b/fs/9p/v9fs.h
> index 9665c2b..e5ebedf 100644
> --- a/fs/9p/v9fs.h
> +++ b/fs/9p/v9fs.h
> @@ -116,7 +116,6 @@ struct v9fs_session_info {
>   	struct list_head slist; /* list of sessions registered with v9fs */
>   	struct backing_dev_info bdi;
>   	struct rw_semaphore rename_sem;
> -	struct p9_fid *root_fid; /* Used for file system sync */
>   };
>
>   /* cache_validity flags */
> diff --git a/fs/9p/vfs_super.c b/fs/9p/vfs_super.c
> index d94adcb..118078d 100644
> --- a/fs/9p/vfs_super.c
> +++ b/fs/9p/vfs_super.c
> @@ -154,6 +154,7 @@ static struct dentry *v9fs_mount(struct file_system_type *fs_type, int flags,
>   		retval = PTR_ERR(inode);
>   		goto release_sb;
>   	}
> +
>   	root = d_alloc_root(inode);
>   	if (!root) {
>   		iput(inode);
> @@ -185,21 +186,10 @@ static struct dentry *v9fs_mount(struct file_system_type *fs_type, int flags,
>   		p9stat_free(st);
>   		kfree(st);
>   	}
> -	v9fs_fid_add(root, fid);
>   	retval = v9fs_get_acl(inode, fid);
>   	if (retval)
>   		goto release_sb;
> -	/*
> -	 * Add the root fid to session info. This is used
> -	 * for file system sync. We want a cloned fid here
> -	 * so that we can do a sync_filesystem after a
> -	 * shrink_dcache_for_umount
> -	 */
> -	v9ses->root_fid = v9fs_fid_clone(root);
> -	if (IS_ERR(v9ses->root_fid)) {
> -		retval = PTR_ERR(v9ses->root_fid);
> -		goto release_sb;
> -	}
> +	v9fs_fid_add(root, fid);
>
>   	P9_DPRINTK(P9_DEBUG_VFS, " simple set mount, return 0\n");
>   	return dget(sb->s_root);
> @@ -210,11 +200,15 @@ close_session:
>   	v9fs_session_close(v9ses);
>   	kfree(v9ses);
>   	return ERR_PTR(retval);
> +
>   release_sb:
>   	/*
> -	 * we will do the session_close and root dentry
> -	 * release in the below call.
> +	 * we will do the session_close and root dentry release
> +	 * in the below call. But we need to clunk fid, because we haven't
> +	 * attached the fid to dentry so it won't get clunked
> +	 * automatically.
>   	 */
> +	p9_client_clunk(fid);
>   	deactivate_locked_super(sb);
>   	return ERR_PTR(retval);
>   }
> @@ -232,7 +226,7 @@ static void v9fs_kill_super(struct super_block *s)
>   	P9_DPRINTK(P9_DEBUG_VFS, " %p\n", s);
>
>   	kill_anon_super(s);
> -	p9_client_clunk(v9ses->root_fid);
> +
>   	v9fs_session_cancel(v9ses);
>   	v9fs_session_close(v9ses);
>   	kfree(v9ses);
> @@ -285,14 +279,6 @@ done:
>   	return res;
>   }
>
> -static int v9fs_sync_fs(struct super_block *sb, int wait)
> -{
> -	struct v9fs_session_info *v9ses = sb->s_fs_info;
> -
> -	P9_DPRINTK(P9_DEBUG_VFS, "v9fs_sync_fs: super_block %p\n", sb);
> -	return p9_client_sync_fs(v9ses->root_fid);
> -}
> -
>   static int v9fs_drop_inode(struct inode *inode)
>   {
>   	struct v9fs_session_info *v9ses;
> @@ -361,7 +347,6 @@ static const struct super_operations v9fs_super_ops = {
>   static const struct super_operations v9fs_super_ops_dotl = {
>   	.alloc_inode = v9fs_alloc_inode,
>   	.destroy_inode = v9fs_destroy_inode,
> -	.sync_fs = v9fs_sync_fs,
>   	.statfs = v9fs_statfs,
>   	.drop_inode = v9fs_drop_inode,
>   	.evict_inode = v9fs_evict_inode,
> diff --git a/include/net/9p/9p.h b/include/net/9p/9p.h
> index 6b75a69..eaa45f9 100644
> --- a/include/net/9p/9p.h
> +++ b/include/net/9p/9p.h
> @@ -139,8 +139,6 @@ do { \
>    */
>
>   enum p9_msg_t {
> -	P9_TSYNCFS = 0,
> -	P9_RSYNCFS,
>   	P9_TLERROR = 6,
>   	P9_RLERROR,
>   	P9_TSTATFS = 8,
> diff --git a/include/net/9p/client.h b/include/net/9p/client.h
> index 0a30977..83ba6a4 100644
> --- a/include/net/9p/client.h
> +++ b/include/net/9p/client.h
> @@ -230,7 +230,6 @@ int p9_client_create_dotl(struct p9_fid *ofid, char *name, u32 flags, u32 mode,
>   		gid_t gid, struct p9_qid *qid);
>   int p9_client_clunk(struct p9_fid *fid);
>   int p9_client_fsync(struct p9_fid *fid, int datasync);
> -int p9_client_sync_fs(struct p9_fid *fid);
>   int p9_client_remove(struct p9_fid *fid);
>   int p9_client_read(struct p9_fid *fid, char *data, char __user *udata,
>   							u64 offset, u32 count);
> diff --git a/net/9p/client.c b/net/9p/client.c
> index 2ccbf04..fd7fb37 100644
> --- a/net/9p/client.c
> +++ b/net/9p/client.c
> @@ -1220,27 +1220,6 @@ error:
>   }
>   EXPORT_SYMBOL(p9_client_fsync);
>
> -int p9_client_sync_fs(struct p9_fid *fid)
> -{
> -	int err = 0;
> -	struct p9_req_t *req;
> -	struct p9_client *clnt;
> -
> -	P9_DPRINTK(P9_DEBUG_9P, ">>>  TSYNC_FS fid %d\n", fid->fid);
> -
> -	clnt = fid->clnt;
> -	req = p9_client_rpc(clnt, P9_TSYNCFS, "d", fid->fid);
> -	if (IS_ERR(req)) {
> -		err = PTR_ERR(req);
> -		goto error;
> -	}
> -	P9_DPRINTK(P9_DEBUG_9P, "<<<  RSYNCFS fid %d\n", fid->fid);
> -	p9_free_req(clnt, req);
> -error:
> -	return err;
> -}
> -EXPORT_SYMBOL(p9_client_sync_fs);
> -
>   int p9_client_clunk(struct p9_fid *fid)
>   {
>   	int err;

Re: [V9fs-developer] [PATCH 3/5] 9p: revert tsyncfs related changes

[Aneesh Kumar K. V]

On Fri, 25 Mar 2011 14:20:04 -0700, Venkateswararao Jujjuri <jvrao@linux.vnet.ibm.com> wrote:
> On 03/25/2011 04:30 AM, Aneesh Kumar K.V wrote:
> > Now that we use write_inode to flush server
> > cache related to fid, we don't need tsyncfs.
> > This help us to do a more efficient server flush
> > for dotu protocol
> Why are you singling out dotu only? won't it be applicable to dotl too?
> 

With dotl we can have new operations and so we added tsyncfs. The
primary goal is to add an operation that can flush server cache. We
hooked that to sync(2) on the client. With dotu we cannot add new
operations so we always forced the write on the server in case of dotu
to O_SYNC. That is much slower than doing an fsync on write_inode. But
whether doing an fsync on write inode is better than doing tsyncfs on
sync(2) on client is something i haven't yet measured. Stefan Hajnoczi wants to
see some numbers before we push tsyncfs in the server(qemu). We also don't
want a kernel release with 9p operation which we may remove later. So
the plan now is to get write_inode changes upstream in this merge window
and later get numbers against tsyncfs/write_inode -> fsync and add tsyncfs only
if we see a benefit. BTW NFS use the write_inode approach.

-aneesh

Re: [V9fs-developer] [PATCH 3/5] 9p: revert tsyncfs related changes

[Venkateswararao Jujjuri]

On 03/27/2011 01:28 AM, Aneesh Kumar K. V wrote:
> On Fri, 25 Mar 2011 14:20:04 -0700, Venkateswararao Jujjuri<jvrao@linux.vnet.ibm.com>  wrote:
>> On 03/25/2011 04:30 AM, Aneesh Kumar K.V wrote:
>>> Now that we use write_inode to flush server
>>> cache related to fid, we don't need tsyncfs.
>>> This help us to do a more efficient server flush
>>> for dotu protocol
>> Why are you singling out dotu only? won't it be applicable to dotl too?
>>
> With dotl we can have new operations and so we added tsyncfs. The
> primary goal is to add an operation that can flush server cache. We
> hooked that to sync(2) on the client. With dotu we cannot add new
> operations so we always forced the write on the server in case of dotu
> to O_SYNC. That is much slower than doing an fsync on write_inode. But
> whether doing an fsync on write inode is better than doing tsyncfs on
> sync(2) on client is something i haven't yet measured. Stefan Hajnoczi wants to
> see some numbers before we push tsyncfs in the server(qemu). We also don't
> want a kernel release with 9p operation which we may remove later. So
> the plan now is to get write_inode changes upstream in this merge window
> and later get numbers against tsyncfs/write_inode ->  fsync and add tsyncfs only
> if we see a benefit. BTW NFS use the write_inode approach.

Nice explanation. I looked at NFS and realized that they also follow 
write_inode approach.
So I think you should make it explict that this will be helpful to dotl 
also and may and TFSYNCFS
in the future if needed. With that I ack this.

Reviewed-by : Venkateswararao Jujjuri <jvrao@linux.vnet.ibm.com>

> -aneesh

Re: [V9fs-developer] [PATCH 3/5] 9p: revert tsyncfs related changes

[Eric Van Hensbergen]

On Sun, Mar 27, 2011 at 5:24 PM, Venkateswararao Jujjuri
<jvrao@linux.vnet.ibm.com> wrote:
>
> Nice explanation. I looked at NFS and realized that they also follow
> write_inode approach.
> So I think you should make it explict that this will be helpful to dotl
> also and may and TFSYNCFS
> in the future if needed. With that I ack this.
>

If this is something we really think we'll be adding back in the
future, is there someway we can conditionalize its use (default off
perhaps) so that if a particular server wanted to take advantage of
it, they could.  This would seem preferable to just backing out the
whole patch.

Another aspect which I didn't consider when we added it is what it
would do to older versions of the servers which didn't have TFSYNCFS
-- maybe this is a good case study for the .L graceful degredation
plan we had talked about in the past where you try a tfsyncfs and if
the server returns an error that it doesn't implement it you back off
to another solution.

Thoughts?  Sorry if I'm being dense -- still adjusting to new sleep
schedule with new baby and spent 16 hours yesterday cranking out two
publication submissions, so folks will have to bear with me for a bit.

        -eric

Re: [V9fs-developer] [PATCH 3/5] 9p: revert tsyncfs related changes

[Aneesh Kumar K.V]

On Tue, 29 Mar 2011 11:05:08 -0500, Eric Van Hensbergen <ericvh@gmail.com> wrote:
> On Sun, Mar 27, 2011 at 5:24 PM, Venkateswararao Jujjuri
> <jvrao@linux.vnet.ibm.com> wrote:
> >
> > Nice explanation. I looked at NFS and realized that they also follow
> > write_inode approach.
> > So I think you should make it explict that this will be helpful to dotl
> > also and may and TFSYNCFS
> > in the future if needed. With that I ack this.
> >
> 
> If this is something we really think we'll be adding back in the
> future, is there someway we can conditionalize its use (default off
> perhaps) so that if a particular server wanted to take advantage of
> it, they could.  This would seem preferable to just backing out the
> whole patch.

It would be a nice feature to request the server for feature bits and
then use different 9p operations with .L. ie if we can query the server
and find out whether it supports tsyncfs then we can probably skip fsync
during write_inode. The feature set is also useful to figure out which
acl model the client should enforce if we start supporting multiple acl
models and also to find out whether server also acl evaluation.
So i guess we should do this once we have something similar to
TFEATURE 9p operation

> 
> Another aspect which I didn't consider when we added it is what it
> would do to older versions of the servers which didn't have TFSYNCFS
> -- maybe this is a good case study for the .L graceful degredation
> plan we had talked about in the past where you try a tfsyncfs and if
> the server returns an error that it doesn't implement it you back off
> to another solution.
> 

With the current Qemu server, the server aborts when it gets a 9p
request that it didn't implement. That need to be fixed. Do you like to
see the above done as a part of new set of operations that we are going
to do, or are you suggesting that we need to do it for the existing set
looking at when we added the 9p operations ?.

-aneesh

[PATCH 4/5] fs/9p: Fix error reported by coccicheck

[Aneesh Kumar K.V]

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 fs/9p/vfs_inode_dotl.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/9p/vfs_inode_dotl.c b/fs/9p/vfs_inode_dotl.c
index ffbb113..82a7c38 100644
--- a/fs/9p/vfs_inode_dotl.c
+++ b/fs/9p/vfs_inode_dotl.c
@@ -811,7 +811,7 @@ v9fs_vfs_follow_link_dotl(struct dentry *dentry, struct nameidata *nd)
 	fid = v9fs_fid_lookup(dentry);
 	if (IS_ERR(fid)) {
 		__putname(link);
-		link = ERR_PTR(PTR_ERR(fid));
+		link = ERR_CAST(fid);
 		goto ndset;
 	}
 	retval = p9_client_readlink(fid, &target);
-- 
1.7.1

Re: [V9fs-developer] [PATCH 4/5] fs/9p: Fix error reported by coccicheck

[Venkateswararao Jujjuri]

On 03/25/2011 04:30 AM, Aneesh Kumar K.V wrote:
> Signed-off-by: Aneesh Kumar K.V<aneesh.kumar@linux.vnet.ibm.com>
Reviewed-by : Venkateswararao Jujjuri <jvrao@linux.vnet.ibm.com>
> ---
>   fs/9p/vfs_inode_dotl.c |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/fs/9p/vfs_inode_dotl.c b/fs/9p/vfs_inode_dotl.c
> index ffbb113..82a7c38 100644
> --- a/fs/9p/vfs_inode_dotl.c
> +++ b/fs/9p/vfs_inode_dotl.c
> @@ -811,7 +811,7 @@ v9fs_vfs_follow_link_dotl(struct dentry *dentry, struct nameidata *nd)
>   	fid = v9fs_fid_lookup(dentry);
>   	if (IS_ERR(fid)) {
>   		__putname(link);
> -		link = ERR_PTR(PTR_ERR(fid));
> +		link = ERR_CAST(fid);
>   		goto ndset;
>   	}
>   	retval = p9_client_readlink(fid,&target);

[PATCH 5/5] 9p: Fix sparse error

[Aneesh Kumar K.V]

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 net/9p/protocol.c     |    3 ++-
 net/9p/trans_common.c |    2 +-
 net/9p/trans_virtio.c |   15 +++++++++++----
 3 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/net/9p/protocol.c b/net/9p/protocol.c
index 8a4084f..a7e8997 100644
--- a/net/9p/protocol.c
+++ b/net/9p/protocol.c
@@ -468,7 +468,8 @@ p9pdu_vwritef(struct p9_fcall *pdu, int proto_version, const char *fmt,
 		case 'E':{
 				 int32_t cnt = va_arg(ap, int32_t);
 				 const char *k = va_arg(ap, const void *);
-				 const char *u = va_arg(ap, const void *);
+				 const char __user *u = va_arg(ap,
+							const void __user *);
 				 errcode = p9pdu_writef(pdu, proto_version, "d",
 						 cnt);
 				 if (!errcode && pdu_write_urw(pdu, k, u, cnt))
diff --git a/net/9p/trans_common.c b/net/9p/trans_common.c
index 9172ab7..14265e8 100644
--- a/net/9p/trans_common.c
+++ b/net/9p/trans_common.c
@@ -66,7 +66,7 @@ p9_payload_gup(struct p9_req_t *req, size_t *pdata_off, int *pdata_len,
 	uint32_t pdata_mapped_pages;
 	struct trans_rpage_info  *rpinfo;
 
-	*pdata_off = (size_t)req->tc->pubuf & (PAGE_SIZE-1);
+	*pdata_off = (__force size_t)req->tc->pubuf & (PAGE_SIZE-1);
 
 	if (*pdata_off)
 		first_page_bytes = min(((size_t)PAGE_SIZE - *pdata_off),
diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c
index e8f046b..244e707 100644
--- a/net/9p/trans_virtio.c
+++ b/net/9p/trans_virtio.c
@@ -326,8 +326,11 @@ req_retry_pinned:
 			outp = pack_sg_list_p(chan->sg, out, VIRTQUEUE_NUM,
 					pdata_off, rpinfo->rp_data, pdata_len);
 		} else {
-			char *pbuf = req->tc->pubuf ? req->tc->pubuf :
-								req->tc->pkbuf;
+			char *pbuf;
+			if (req->tc->pubuf)
+				pbuf = (__force char *) req->tc->pubuf;
+			else
+				pbuf = req->tc->pkbuf;
 			outp = pack_sg_list(chan->sg, out, VIRTQUEUE_NUM, pbuf,
 					req->tc->pbuf_size);
 		}
@@ -352,8 +355,12 @@ req_retry_pinned:
 			in = pack_sg_list_p(chan->sg, out+inp, VIRTQUEUE_NUM,
 					pdata_off, rpinfo->rp_data, pdata_len);
 		} else {
-			char *pbuf = req->tc->pubuf ? req->tc->pubuf :
-								req->tc->pkbuf;
+			char *pbuf;
+			if (req->tc->pubuf)
+				pbuf = (__force char *) req->tc->pubuf;
+			else
+				pbuf = req->tc->pkbuf;
+
 			in = pack_sg_list(chan->sg, out+inp, VIRTQUEUE_NUM,
 					pbuf, req->tc->pbuf_size);
 		}
-- 
1.7.1

Re: [V9fs-developer] [PATCH 5/5] 9p: Fix sparse error

[Venkateswararao Jujjuri]

On 03/25/2011 04:30 AM, Aneesh Kumar K.V wrote:
> Signed-off-by: Aneesh Kumar K.V<aneesh.kumar@linux.vnet.ibm.com>
Reviewed-by : Venkateswararao Jujjuri <jvrao@linux.vnet.ibm.com>
> ---
>   net/9p/protocol.c     |    3 ++-
>   net/9p/trans_common.c |    2 +-
>   net/9p/trans_virtio.c |   15 +++++++++++----
>   3 files changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/net/9p/protocol.c b/net/9p/protocol.c
> index 8a4084f..a7e8997 100644
> --- a/net/9p/protocol.c
> +++ b/net/9p/protocol.c
> @@ -468,7 +468,8 @@ p9pdu_vwritef(struct p9_fcall *pdu, int proto_version, const char *fmt,
>   		case 'E':{
>   				 int32_t cnt = va_arg(ap, int32_t);
>   				 const char *k = va_arg(ap, const void *);
> -				 const char *u = va_arg(ap, const void *);
> +				 const char __user *u = va_arg(ap,
> +							const void __user *);
>   				 errcode = p9pdu_writef(pdu, proto_version, "d",
>   						 cnt);
>   				 if (!errcode&&  pdu_write_urw(pdu, k, u, cnt))
> diff --git a/net/9p/trans_common.c b/net/9p/trans_common.c
> index 9172ab7..14265e8 100644
> --- a/net/9p/trans_common.c
> +++ b/net/9p/trans_common.c
> @@ -66,7 +66,7 @@ p9_payload_gup(struct p9_req_t *req, size_t *pdata_off, int *pdata_len,
>   	uint32_t pdata_mapped_pages;
>   	struct trans_rpage_info  *rpinfo;
>
> -	*pdata_off = (size_t)req->tc->pubuf&  (PAGE_SIZE-1);
> +	*pdata_off = (__force size_t)req->tc->pubuf&  (PAGE_SIZE-1);
>
>   	if (*pdata_off)
>   		first_page_bytes = min(((size_t)PAGE_SIZE - *pdata_off),
> diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c
> index e8f046b..244e707 100644
> --- a/net/9p/trans_virtio.c
> +++ b/net/9p/trans_virtio.c
> @@ -326,8 +326,11 @@ req_retry_pinned:
>   			outp = pack_sg_list_p(chan->sg, out, VIRTQUEUE_NUM,
>   					pdata_off, rpinfo->rp_data, pdata_len);
>   		} else {
> -			char *pbuf = req->tc->pubuf ? req->tc->pubuf :
> -								req->tc->pkbuf;
> +			char *pbuf;
> +			if (req->tc->pubuf)
> +				pbuf = (__force char *) req->tc->pubuf;
> +			else
> +				pbuf = req->tc->pkbuf;
>   			outp = pack_sg_list(chan->sg, out, VIRTQUEUE_NUM, pbuf,
>   					req->tc->pbuf_size);
>   		}
> @@ -352,8 +355,12 @@ req_retry_pinned:
>   			in = pack_sg_list_p(chan->sg, out+inp, VIRTQUEUE_NUM,
>   					pdata_off, rpinfo->rp_data, pdata_len);
>   		} else {
> -			char *pbuf = req->tc->pubuf ? req->tc->pubuf :
> -								req->tc->pkbuf;
> +			char *pbuf;
> +			if (req->tc->pubuf)
> +				pbuf = (__force char *) req->tc->pubuf;
> +			else
> +				pbuf = req->tc->pkbuf;
> +
>   			in = pack_sg_list(chan->sg, out+inp, VIRTQUEUE_NUM,
>   					pbuf, req->tc->pbuf_size);
>   		}

Re: [V9fs-developer] [PATCH 1/5] fs/9p: Fix revalidate to return correct value

[Venkateswararao Jujjuri]

On 03/25/2011 04:30 AM, Aneesh Kumar K.V wrote:
> revalidate should return>  0 on success.
>
> Signed-off-by: Aneesh Kumar K.V<aneesh.kumar@linux.vnet.ibm.com>

Reviewed-by: Venkateswararao Jujjuri <jvrao@linux.vnet.ibm.com>

> ---
>   fs/9p/vfs_dentry.c |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/fs/9p/vfs_dentry.c b/fs/9p/vfs_dentry.c
> index b6a3b9f..59c5ddc 100644
> --- a/fs/9p/vfs_dentry.c
> +++ b/fs/9p/vfs_dentry.c
> @@ -126,7 +126,7 @@ static int v9fs_lookup_revalidate(struct dentry *dentry, struct nameidata *nd)
>   			retval = v9fs_refresh_inode_dotl(fid, inode);
>   		else
>   			retval = v9fs_refresh_inode(fid, inode);
> -		if (retval<= 0)
> +		if (retval<  0)
>   			return retval;
>   	}
>   out_valid:

Re: [V9fs-developer] [PATCH 1/5] fs/9p: Fix revalidate to return correct value

[Venkateswararao Jujjuri]

On 03/25/2011 01:50 PM, Venkateswararao Jujjuri wrote:
> On 03/25/2011 04:30 AM, Aneesh Kumar K.V wrote:
>> revalidate should return>  0 on success.
>>
>> Signed-off-by: Aneesh Kumar K.V<aneesh.kumar@linux.vnet.ibm.com>
>
> Reviewed-by: Venkateswararao Jujjuri <jvrao@linux.vnet.ibm.com>
>
>> ---
>>   fs/9p/vfs_dentry.c |    2 +-
>>   1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/fs/9p/vfs_dentry.c b/fs/9p/vfs_dentry.c
>> index b6a3b9f..59c5ddc 100644
>> --- a/fs/9p/vfs_dentry.c
>> +++ b/fs/9p/vfs_dentry.c
>> @@ -126,7 +126,7 @@ static int v9fs_lookup_revalidate(struct dentry 
>> *dentry, struct nameidata *nd)
>>               retval = v9fs_refresh_inode_dotl(fid, inode);
>>           else
>>               retval = v9fs_refresh_inode(fid, inode);
>> -        if (retval<= 0)
>> +        if (retval<  0)
This change is causing tuxera rename test failure with acls enabled.

/root/pjd-fstest-20080816/tests/rename/10.t  (Wstat: 0 Tests: 188 Failed: 6)
   Failed tests:  65, 71, 74, 77, 79-80
The comment in namei says:
"  If d_revalidate returned 0 attempt to invalidate  the dentry 
otherwise d_revalidate is asking us
  to return a fail status." I guess we need to return 0 in the case of 
rename.
Need more closer look at this patch.

- JV



>>               return retval;
>>       }
>>   out_valid:
>

Re: [V9fs-developer] [PATCH 1/5] fs/9p: Fix revalidate to return correct value

[Aneesh Kumar K. V]

On Mon, 28 Mar 2011 13:49:44 -0700, Venkateswararao Jujjuri <jvrao@linux.vnet.ibm.com> wrote:
> On 03/25/2011 01:50 PM, Venkateswararao Jujjuri wrote:
> > On 03/25/2011 04:30 AM, Aneesh Kumar K.V wrote:
> >> revalidate should return>  0 on success.
> >>
> >> Signed-off-by: Aneesh Kumar K.V<aneesh.kumar@linux.vnet.ibm.com>
> >
> > Reviewed-by: Venkateswararao Jujjuri <jvrao@linux.vnet.ibm.com>
> >
> >> ---
> >>   fs/9p/vfs_dentry.c |    2 +-
> >>   1 files changed, 1 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/fs/9p/vfs_dentry.c b/fs/9p/vfs_dentry.c
> >> index b6a3b9f..59c5ddc 100644
> >> --- a/fs/9p/vfs_dentry.c
> >> +++ b/fs/9p/vfs_dentry.c
> >> @@ -126,7 +126,7 @@ static int v9fs_lookup_revalidate(struct dentry 
> >> *dentry, struct nameidata *nd)
> >>               retval = v9fs_refresh_inode_dotl(fid, inode);
> >>           else
> >>               retval = v9fs_refresh_inode(fid, inode);
> >> -        if (retval<= 0)
> >> +        if (retval<  0)
> This change is causing tuxera rename test failure with acls enabled.
> 
> /root/pjd-fstest-20080816/tests/rename/10.t  (Wstat: 0 Tests: 188 Failed: 6)
>    Failed tests:  65, 71, 74, 77, 79-80
> The comment in namei says:
> "  If d_revalidate returned 0 attempt to invalidate  the dentry 
> otherwise d_revalidate is asking us
>   to return a fail status." I guess we need to return 0 in the case of 
> rename.
> Need more closer look at this patch.
> 

The failure is due to the VFS issue which I already reported. The
patch for that is here
http://article.gmane.org/gmane.linux.file-systems/51123

Previously we worked around that in 9p. But with the recent VFS changes
the work around applies no more, lookup_create fails on error in
lookup_hash and look_hash -> do_revalidate returns ENOENT.

-aneesh