[PATCH 0/7] ceph: misc fixes

[PATCH 0/7] ceph: misc fixes

[Yan, Zheng]

From: "Yan, Zheng" <zheng.z.yan@intel.com>

These patches are also in:
  git://github.com/ukernel/linux.git wip-ceph

Regards
Yan, Zheng

[PATCH 1/7] ceph: fix LSSNAP regression

[Yan, Zheng]

From: "Yan, Zheng" <zheng.z.yan@intel.com>

commit 6e8575faa8 makes parse_reply_info_extra() return -EIO for LSSNAP

Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
---
 fs/ceph/mds_client.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index d958420..608ffcf 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -265,7 +265,8 @@ static int parse_reply_info_extra(void **p, void *end,
 {
 	if (info->head->op == CEPH_MDS_OP_GETFILELOCK)
 		return parse_reply_info_filelock(p, end, info, features);
-	else if (info->head->op == CEPH_MDS_OP_READDIR)
+	else if (info->head->op == CEPH_MDS_OP_READDIR ||
+		 info->head->op == CEPH_MDS_OP_LSSNAP)
 		return parse_reply_info_dir(p, end, info, features);
 	else if (info->head->op == CEPH_MDS_OP_CREATE)
 		return parse_reply_info_create(p, end, info, features);
-- 
1.7.11.7

[PATCH 2/7] ceph: queue cap release when trimming cap

[Yan, Zheng]

From: "Yan, Zheng" <zheng.z.yan@intel.com>

So the client will later send cap release message to MDS

Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
---
 fs/ceph/caps.c       | 6 +++---
 fs/ceph/mds_client.c | 2 ++
 fs/ceph/super.h      | 2 ++
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
index 1e1e020..5d5c32b 100644
--- a/fs/ceph/caps.c
+++ b/fs/ceph/caps.c
@@ -997,9 +997,9 @@ static int send_cap_msg(struct ceph_mds_session *session,
 	return 0;
 }
 
-static void __queue_cap_release(struct ceph_mds_session *session,
-				u64 ino, u64 cap_id, u32 migrate_seq,
-				u32 issue_seq)
+void __queue_cap_release(struct ceph_mds_session *session,
+			 u64 ino, u64 cap_id, u32 migrate_seq,
+			 u32 issue_seq)
 {
 	struct ceph_msg *msg;
 	struct ceph_mds_cap_release *head;
diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index 608ffcf..ccc68b0 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -1197,6 +1197,8 @@ static int trim_caps_cb(struct inode *inode, struct ceph_cap *cap, void *arg)
 	session->s_trim_caps--;
 	if (oissued) {
 		/* we aren't the only cap.. just remove us */
+		__queue_cap_release(session, ceph_ino(inode), cap->cap_id,
+				    cap->mseq, cap->issue_seq);
 		__ceph_remove_cap(cap);
 	} else {
 		/* try to drop referring dentries */
diff --git a/fs/ceph/super.h b/fs/ceph/super.h
index 604526a..4353ebc 100644
--- a/fs/ceph/super.h
+++ b/fs/ceph/super.h
@@ -755,6 +755,8 @@ static inline void ceph_remove_cap(struct ceph_cap *cap)
 extern void ceph_put_cap(struct ceph_mds_client *mdsc,
 			 struct ceph_cap *cap);
 
+extern void __queue_cap_release(struct ceph_mds_session *session, u64 ino,
+				u64 cap_id, u32 migrate_seq, u32 issue_seq);
 extern void ceph_queue_caps_release(struct inode *inode);
 extern int ceph_write_inode(struct inode *inode, struct writeback_control *wbc);
 extern int ceph_fsync(struct file *file, loff_t start, loff_t end,
-- 
1.7.11.7

[PATCH 3/7] ceph: set mds_want according to cap import message

[Yan, Zheng]

From: "Yan, Zheng" <zheng.z.yan@intel.com>

MDS ignores cap update message if migrate_seq mismatch, so when
receiving a cap import message with higher migrate_seq, set mds_want
according to the cap import message.

Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
---
 fs/ceph/caps.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
index 5d5c32b..61f3833 100644
--- a/fs/ceph/caps.c
+++ b/fs/ceph/caps.c
@@ -553,6 +553,7 @@ retry:
 		cap->implemented = 0;
 		cap->mds = mds;
 		cap->mds_wanted = 0;
+		cap->mseq = 0;
 
 		cap->ci = ci;
 		__insert_cap_node(ci, cap);
@@ -628,7 +629,10 @@ retry:
 	cap->cap_id = cap_id;
 	cap->issued = issued;
 	cap->implemented |= issued;
-	cap->mds_wanted |= wanted;
+	if (mseq > cap->mseq)
+		cap->mds_wanted = wanted;
+	else
+		cap->mds_wanted |= wanted;
 	cap->seq = seq;
 	cap->issue_seq = seq;
 	cap->mseq = mseq;
-- 
1.7.11.7

[PATCH 4/7] ceph: use I_COMPLETE inode flag instead of D_COMPLETE flag

[Yan, Zheng]

From: "Yan, Zheng" <zheng.z.yan@intel.com>

commit c6ffe10015 moved the flag that tracks if the dcache contents
for a directory are complete to dentry. The problem is there are
lots of places that use ceph_dir_{set,clear,test}_complete() while
holding i_ceph_lock. but ceph_dir_{set,clear,test}_complete() may
sleep because they call dput().

This patch basically reverts that commit. For ceph_d_prune(), it's
called with both the dentry to prune and the parent dentry are
locked. So it's safe to access the parent dentry's d_inode and
clear I_COMPLETE flag.

Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
---
 fs/ceph/caps.c       |  8 ++++---
 fs/ceph/dir.c        | 62 ++++++++++------------------------------------------
 fs/ceph/inode.c      | 30 +++++++++++--------------
 fs/ceph/mds_client.c |  6 ++---
 fs/ceph/super.h      | 23 ++-----------------
 5 files changed, 34 insertions(+), 95 deletions(-)

diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
index 61f3833..76634f4 100644
--- a/fs/ceph/caps.c
+++ b/fs/ceph/caps.c
@@ -490,15 +490,17 @@ static void __check_cap_issue(struct ceph_inode_info *ci, struct ceph_cap *cap,
 		ci->i_rdcache_gen++;
 
 	/*
-	 * if we are newly issued FILE_SHARED, clear D_COMPLETE; we
+	 * if we are newly issued FILE_SHARED, clear I_COMPLETE; we
 	 * don't know what happened to this directory while we didn't
 	 * have the cap.
 	 */
 	if ((issued & CEPH_CAP_FILE_SHARED) &&
 	    (had & CEPH_CAP_FILE_SHARED) == 0) {
 		ci->i_shared_gen++;
-		if (S_ISDIR(ci->vfs_inode.i_mode))
-			ceph_dir_clear_complete(&ci->vfs_inode);
+		if (S_ISDIR(ci->vfs_inode.i_mode)) {
+			dout(" marking %p NOT complete\n", &ci->vfs_inode);
+			ci->i_ceph_flags &= ~CEPH_I_COMPLETE;
+		}
 	}
 }
 
diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
index 8c1aabe..76821be 100644
--- a/fs/ceph/dir.c
+++ b/fs/ceph/dir.c
@@ -107,7 +107,7 @@ static unsigned fpos_off(loff_t p)
  * falling back to a "normal" sync readdir if any dentries in the dir
  * are dropped.
  *
- * D_COMPLETE tells indicates we have all dentries in the dir.  It is
+ * I_COMPLETE tells indicates we have all dentries in the dir.  It is
  * defined IFF we hold CEPH_CAP_FILE_SHARED (which will be revoked by
  * the MDS if/when the directory is modified).
  */
@@ -198,8 +198,8 @@ more:
 	filp->f_pos++;
 
 	/* make sure a dentry wasn't dropped while we didn't have parent lock */
-	if (!ceph_dir_test_complete(dir)) {
-		dout(" lost D_COMPLETE on %p; falling back to mds\n", dir);
+	if (!ceph_i_test(dir, CEPH_I_COMPLETE)) {
+		dout(" lost I_COMPLETE on %p; falling back to mds\n", dir);
 		err = -EAGAIN;
 		goto out;
 	}
@@ -284,7 +284,7 @@ static int ceph_readdir(struct file *filp, void *dirent, filldir_t filldir)
 	if ((filp->f_pos == 2 || fi->dentry) &&
 	    !ceph_test_mount_opt(fsc, NOASYNCREADDIR) &&
 	    ceph_snap(inode) != CEPH_SNAPDIR &&
-	    ceph_dir_test_complete(inode) &&
+	    (ci->i_ceph_flags & CEPH_I_COMPLETE) &&
 	    __ceph_caps_issued_mask(ci, CEPH_CAP_FILE_SHARED, 1)) {
 		spin_unlock(&ci->i_ceph_lock);
 		err = __dcache_readdir(filp, dirent, filldir);
@@ -350,7 +350,7 @@ more:
 
 		if (!req->r_did_prepopulate) {
 			dout("readdir !did_prepopulate");
-			fi->dir_release_count--;    /* preclude D_COMPLETE */
+			fi->dir_release_count--;    /* preclude I_COMPLETE */
 		}
 
 		/* note next offset and last dentry name */
@@ -429,7 +429,8 @@ more:
 	 */
 	spin_lock(&ci->i_ceph_lock);
 	if (ci->i_release_count == fi->dir_release_count) {
-		ceph_dir_set_complete(inode);
+		dout(" marking %p complete\n", inode);
+		ci->i_ceph_flags |= CEPH_I_COMPLETE;
 		ci->i_max_offset = filp->f_pos;
 	}
 	spin_unlock(&ci->i_ceph_lock);
@@ -604,7 +605,7 @@ static struct dentry *ceph_lookup(struct inode *dir, struct dentry *dentry,
 			    fsc->mount_options->snapdir_name,
 			    dentry->d_name.len) &&
 		    !is_root_ceph_dentry(dir, dentry) &&
-		    ceph_dir_test_complete(dir) &&
+		    (ci->i_ceph_flags & CEPH_I_COMPLETE) &&
 		    (__ceph_caps_issued_mask(ci, CEPH_CAP_FILE_SHARED, 1))) {
 			spin_unlock(&ci->i_ceph_lock);
 			dout(" dir %p complete, -ENOENT\n", dir);
@@ -908,7 +909,7 @@ static int ceph_rename(struct inode *old_dir, struct dentry *old_dentry,
 		 */
 
 		/* d_move screws up d_subdirs order */
-		ceph_dir_clear_complete(new_dir);
+		ceph_i_clear(new_dir, CEPH_I_COMPLETE);
 
 		d_move(old_dentry, new_dentry);
 
@@ -1065,44 +1066,6 @@ static int ceph_snapdir_d_revalidate(struct dentry *dentry,
 }
 
 /*
- * Set/clear/test dir complete flag on the dir's dentry.
- */
-void ceph_dir_set_complete(struct inode *inode)
-{
-	struct dentry *dentry = d_find_any_alias(inode);
-	
-	if (dentry && ceph_dentry(dentry) &&
-	    ceph_test_mount_opt(ceph_sb_to_client(dentry->d_sb), DCACHE)) {
-		dout(" marking %p (%p) complete\n", inode, dentry);
-		set_bit(CEPH_D_COMPLETE, &ceph_dentry(dentry)->flags);
-	}
-	dput(dentry);
-}
-
-void ceph_dir_clear_complete(struct inode *inode)
-{
-	struct dentry *dentry = d_find_any_alias(inode);
-
-	if (dentry && ceph_dentry(dentry)) {
-		dout(" marking %p (%p) complete\n", inode, dentry);
-		set_bit(CEPH_D_COMPLETE, &ceph_dentry(dentry)->flags);
-	}
-	dput(dentry);
-}
-
-bool ceph_dir_test_complete(struct inode *inode)
-{
-	struct dentry *dentry = d_find_any_alias(inode);
-
-	if (dentry && ceph_dentry(dentry)) {
-		dout(" marking %p (%p) NOT complete\n", inode, dentry);
-		clear_bit(CEPH_D_COMPLETE, &ceph_dentry(dentry)->flags);
-	}
-	dput(dentry);
-	return false;
-}
-
-/*
  * When the VFS prunes a dentry from the cache, we need to clear the
  * complete flag on the parent directory.
  *
@@ -1110,15 +1073,13 @@ bool ceph_dir_test_complete(struct inode *inode)
  */
 static void ceph_d_prune(struct dentry *dentry)
 {
-	struct ceph_dentry_info *di;
-
 	dout("ceph_d_prune %p\n", dentry);
 
 	/* do we have a valid parent? */
 	if (IS_ROOT(dentry))
 		return;
 
-	/* if we are not hashed, we don't affect D_COMPLETE */
+	/* if we are not hashed, we don't affect I_COMPLETE */
 	if (d_unhashed(dentry))
 		return;
 
@@ -1126,8 +1087,7 @@ static void ceph_d_prune(struct dentry *dentry)
 	 * we hold d_lock, so d_parent is stable, and d_fsdata is never
 	 * cleared until d_release
 	 */
-	di = ceph_dentry(dentry->d_parent);
-	clear_bit(CEPH_D_COMPLETE, &di->flags);
+	ceph_i_clear(dentry->d_parent->d_inode, CEPH_I_COMPLETE);
 }
 
 /*
diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
index 2971eaa..42c5769 100644
--- a/fs/ceph/inode.c
+++ b/fs/ceph/inode.c
@@ -561,7 +561,6 @@ static int fill_inode(struct inode *inode,
 	struct ceph_inode_info *ci = ceph_inode(inode);
 	int i;
 	int issued = 0, implemented;
-	int updating_inode = 0;
 	struct timespec mtime, atime, ctime;
 	u32 nsplits;
 	struct ceph_buffer *xattr_blob = NULL;
@@ -601,7 +600,6 @@ static int fill_inode(struct inode *inode,
 	    (ci->i_version & ~1) >= le64_to_cpu(info->version))
 		goto no_change;
 	
-	updating_inode = 1;
 	issued = __ceph_caps_issued(ci, &implemented);
 	issued |= implemented | __ceph_caps_dirty(ci);
 
@@ -716,6 +714,17 @@ static int fill_inode(struct inode *inode,
 		       ceph_vinop(inode), inode->i_mode);
 	}
 
+	/* set dir completion flag? */
+	if (S_ISDIR(inode->i_mode) &&
+	    ci->i_files == 0 && ci->i_subdirs == 0 &&
+	    ceph_snap(inode) == CEPH_NOSNAP &&
+	    (le32_to_cpu(info->cap.caps) & CEPH_CAP_FILE_SHARED) &&
+	    (issued & CEPH_CAP_FILE_EXCL) == 0 &&
+	    (ci->i_ceph_flags & CEPH_I_COMPLETE) == 0) {
+		dout(" marking %p complete (empty)\n", inode);
+		ci->i_ceph_flags |= CEPH_I_COMPLETE;
+		ci->i_max_offset = 2;
+	}
 no_change:
 	spin_unlock(&ci->i_ceph_lock);
 
@@ -766,19 +775,6 @@ no_change:
 		__ceph_get_fmode(ci, cap_fmode);
 	}
 
-	/* set dir completion flag? */
-	if (S_ISDIR(inode->i_mode) &&
-	    updating_inode &&                 /* didn't jump to no_change */
-	    ci->i_files == 0 && ci->i_subdirs == 0 &&
-	    ceph_snap(inode) == CEPH_NOSNAP &&
-	    (le32_to_cpu(info->cap.caps) & CEPH_CAP_FILE_SHARED) &&
-	    (issued & CEPH_CAP_FILE_EXCL) == 0 &&
-	    !ceph_dir_test_complete(inode)) {
-		dout(" marking %p complete (empty)\n", inode);
-		ceph_dir_set_complete(inode);
-		ci->i_max_offset = 2;
-	}
-
 	/* update delegation info? */
 	if (dirinfo)
 		ceph_fill_dirfrag(inode, dirinfo);
@@ -860,7 +856,7 @@ static void ceph_set_dentry_offset(struct dentry *dn)
 	di = ceph_dentry(dn);
 
 	spin_lock(&ci->i_ceph_lock);
-	if (!ceph_dir_test_complete(inode)) {
+	if ((ceph_inode(inode)->i_ceph_flags & CEPH_I_COMPLETE) == 0) {
 		spin_unlock(&ci->i_ceph_lock);
 		return;
 	}
@@ -1065,7 +1061,7 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req,
 			 * d_move() puts the renamed dentry at the end of
 			 * d_subdirs.  We need to assign it an appropriate
 			 * directory offset so we can behave when holding
-			 * D_COMPLETE.
+			 * I_COMPLETE.
 			 */
 			ceph_set_dentry_offset(req->r_old_dentry);
 			dout("dn %p gets new offset %lld\n", req->r_old_dentry, 
diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index ccc68b0..e52b0fb 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -2029,7 +2029,7 @@ out:
 }
 
 /*
- * Invalidate dir D_COMPLETE, dentry lease state on an aborted MDS
+ * Invalidate dir I_COMPLETE, dentry lease state on an aborted MDS
  * namespace request.
  */
 void ceph_invalidate_dir_request(struct ceph_mds_request *req)
@@ -2037,9 +2037,9 @@ void ceph_invalidate_dir_request(struct ceph_mds_request *req)
 	struct inode *inode = req->r_locked_dir;
 	struct ceph_inode_info *ci = ceph_inode(inode);
 
-	dout("invalidate_dir_request %p (D_COMPLETE, lease(s))\n", inode);
+	dout("invalidate_dir_request %p (I_COMPLETE, lease(s))\n", inode);
 	spin_lock(&ci->i_ceph_lock);
-	ceph_dir_clear_complete(inode);
+	ci->i_ceph_flags &= ~CEPH_I_COMPLETE;
 	ci->i_release_count++;
 	spin_unlock(&ci->i_ceph_lock);
 
diff --git a/fs/ceph/super.h b/fs/ceph/super.h
index 4353ebc..efbcb56 100644
--- a/fs/ceph/super.h
+++ b/fs/ceph/super.h
@@ -204,7 +204,6 @@ struct ceph_inode_xattr {
  * Ceph dentry state
  */
 struct ceph_dentry_info {
-	unsigned long flags;
 	struct ceph_mds_session *lease_session;
 	u32 lease_gen, lease_shared_gen;
 	u32 lease_seq;
@@ -215,18 +214,6 @@ struct ceph_dentry_info {
 	u64 offset;
 };
 
-/*
- * dentry flags
- *
- * The locking for D_COMPLETE is a bit odd:
- *  - we can clear it at almost any time (see ceph_d_prune)
- *  - it is only meaningful if:
- *    - we hold dir inode i_ceph_lock
- *    - we hold dir FILE_SHARED caps
- *    - the dentry D_COMPLETE is set
- */
-#define CEPH_D_COMPLETE 1  /* if set, d_u.d_subdirs is complete directory */
-
 struct ceph_inode_xattrs_info {
 	/*
 	 * (still encoded) xattr blob. we avoid the overhead of parsing
@@ -267,7 +254,7 @@ struct ceph_inode_info {
 	struct timespec i_rctime;
 	u64 i_rbytes, i_rfiles, i_rsubdirs;
 	u64 i_files, i_subdirs;
-	u64 i_max_offset;  /* largest readdir offset, set with D_COMPLETE */
+	u64 i_max_offset;  /* largest readdir offset, set with I_COMPLETE */
 
 	struct rb_root i_fragtree;
 	struct mutex i_fragtree_mutex;
@@ -432,6 +419,7 @@ static inline struct inode *ceph_find_inode(struct super_block *sb,
 /*
  * Ceph inode.
  */
+#define CEPH_I_COMPLETE  1  /* we have complete directory cached */
 #define CEPH_I_NODELAY   4  /* do not delay cap release */
 #define CEPH_I_FLUSH     8  /* do not delay flush of dirty metadata */
 #define CEPH_I_NOFLUSH  16  /* do not flush dirty caps */
@@ -489,13 +477,6 @@ static inline loff_t ceph_make_fpos(unsigned frag, unsigned off)
 }
 
 /*
- * set/clear directory D_COMPLETE flag
- */
-void ceph_dir_set_complete(struct inode *inode);
-void ceph_dir_clear_complete(struct inode *inode);
-bool ceph_dir_test_complete(struct inode *inode);
-
-/*
  * caps helpers
  */
 static inline bool __ceph_is_any_real_caps(struct ceph_inode_info *ci)
-- 
1.7.11.7

[PATCH 5/7] ceph: revert commit 22cddde104

[Yan, Zheng]

From: "Yan, Zheng" <zheng.z.yan@intel.com>

commit 22cddde104 breaks the atomicity of write operation, it also
introduces a deadlock between write and truncate.

Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
---
 fs/ceph/addr.c       | 51 +++---------------------------------
 fs/ceph/file.c       | 73 +++++++++++++++++++++++++++++++---------------------
 fs/ceph/mds_client.c |  1 +
 3 files changed, 48 insertions(+), 77 deletions(-)

diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c
index cfef3e0..d662025 100644
--- a/fs/ceph/addr.c
+++ b/fs/ceph/addr.c
@@ -1067,51 +1067,23 @@ static int ceph_write_begin(struct file *file, struct address_space *mapping,
 			    struct page **pagep, void **fsdata)
 {
 	struct inode *inode = file->f_dentry->d_inode;
-	struct ceph_inode_info *ci = ceph_inode(inode);
-	struct ceph_file_info *fi = file->private_data;
 	struct page *page;
 	pgoff_t index = pos >> PAGE_CACHE_SHIFT;
-	int r, want, got = 0;
-
-	if (fi->fmode & CEPH_FILE_MODE_LAZY)
-		want = CEPH_CAP_FILE_BUFFER | CEPH_CAP_FILE_LAZYIO;
-	else
-		want = CEPH_CAP_FILE_BUFFER;
-
-	dout("write_begin %p %llx.%llx %llu~%u getting caps. i_size %llu\n",
-	     inode, ceph_vinop(inode), pos, len, inode->i_size);
-	r = ceph_get_caps(ci, CEPH_CAP_FILE_WR, want, &got, pos+len);
-	if (r < 0)
-		return r;
-	dout("write_begin %p %llx.%llx %llu~%u  got cap refs on %s\n",
-	     inode, ceph_vinop(inode), pos, len, ceph_cap_string(got));
-	if (!(got & (CEPH_CAP_FILE_BUFFER|CEPH_CAP_FILE_LAZYIO))) {
-		ceph_put_cap_refs(ci, got);
-		return -EAGAIN;
-	}
+	int r;
 
 	do {
 		/* get a page */
 		page = grab_cache_page_write_begin(mapping, index, 0);
-		if (!page) {
-			r = -ENOMEM;
-			break;
-		}
+		if (!page)
+			return -ENOMEM;
+		*pagep = page;
 
 		dout("write_begin file %p inode %p page %p %d~%d\n", file,
 		     inode, page, (int)pos, (int)len);
 
 		r = ceph_update_writeable_page(file, pos, len, page);
-		if (r)
-			page_cache_release(page);
 	} while (r == -EAGAIN);
 
-	if (r) {
-		ceph_put_cap_refs(ci, got);
-	} else {
-		*pagep = page;
-		*(int *)fsdata = got;
-	}
 	return r;
 }
 
@@ -1125,12 +1097,10 @@ static int ceph_write_end(struct file *file, struct address_space *mapping,
 			  struct page *page, void *fsdata)
 {
 	struct inode *inode = file->f_dentry->d_inode;
-	struct ceph_inode_info *ci = ceph_inode(inode);
 	struct ceph_fs_client *fsc = ceph_inode_to_client(inode);
 	struct ceph_mds_client *mdsc = fsc->mdsc;
 	unsigned from = pos & (PAGE_CACHE_SIZE - 1);
 	int check_cap = 0;
-	int got = (unsigned long)fsdata;
 
 	dout("write_end file %p inode %p page %p %d~%d (%d)\n", file,
 	     inode, page, (int)pos, (int)copied, (int)len);
@@ -1153,19 +1123,6 @@ static int ceph_write_end(struct file *file, struct address_space *mapping,
 	up_read(&mdsc->snap_rwsem);
 	page_cache_release(page);
 
-	if (copied > 0) {
-		int dirty;
-		spin_lock(&ci->i_ceph_lock);
-		dirty = __ceph_mark_dirty_caps(ci, CEPH_CAP_FILE_WR);
-		spin_unlock(&ci->i_ceph_lock);
-		if (dirty)
-			__mark_inode_dirty(inode, dirty);
-	}
-
-	dout("write_end %p %llx.%llx %llu~%u  dropping cap refs on %s\n",
-	     inode, ceph_vinop(inode), pos, len, ceph_cap_string(got));
-	ceph_put_cap_refs(ci, got);
-
 	if (check_cap)
 		ceph_check_caps(ceph_inode(inode), CHECK_CAPS_AUTHONLY, NULL);
 
diff --git a/fs/ceph/file.c b/fs/ceph/file.c
index 9c4325e..a949805 100644
--- a/fs/ceph/file.c
+++ b/fs/ceph/file.c
@@ -718,53 +718,63 @@ static ssize_t ceph_aio_write(struct kiocb *iocb, const struct iovec *iov,
 	struct ceph_osd_client *osdc =
 		&ceph_sb_to_client(inode->i_sb)->client->osdc;
 	loff_t endoff = pos + iov->iov_len;
-	int got = 0;
-	int ret, err, written;
+	int want, got = 0;
+	int ret, err;
 
 	if (ceph_snap(inode) != CEPH_NOSNAP)
 		return -EROFS;
 
 retry_snap:
-	written = 0;
 	if (ceph_osdmap_flag(osdc->osdmap, CEPH_OSDMAP_FULL))
 		return -ENOSPC;
 	__ceph_do_pending_vmtruncate(inode);
+	dout("aio_write %p %llx.%llx %llu~%u getting caps. i_size %llu\n",
+	     inode, ceph_vinop(inode), pos, (unsigned)iov->iov_len,
+	     inode->i_size);
+	if (fi->fmode & CEPH_FILE_MODE_LAZY)
+		want = CEPH_CAP_FILE_BUFFER | CEPH_CAP_FILE_LAZYIO;
+	else
+		want = CEPH_CAP_FILE_BUFFER;
+	ret = ceph_get_caps(ci, CEPH_CAP_FILE_WR, want, &got, endoff);
+	if (ret < 0)
+		goto out_put;
 
-	/*
-	 * try to do a buffered write.  if we don't have sufficient
-	 * caps, we'll get -EAGAIN from generic_file_aio_write, or a
-	 * short write if we only get caps for some pages.
-	 */
-	if (!(iocb->ki_filp->f_flags & O_DIRECT) &&
-	    !(inode->i_sb->s_flags & MS_SYNCHRONOUS) &&
-	    !(fi->flags & CEPH_F_SYNC)) {
-		ret = generic_file_aio_write(iocb, iov, nr_segs, pos);
-		if (ret >= 0)
-			written = ret;
+	dout("aio_write %p %llx.%llx %llu~%u  got cap refs on %s\n",
+	     inode, ceph_vinop(inode), pos, (unsigned)iov->iov_len,
+	     ceph_cap_string(got));
+
+	if ((got & (CEPH_CAP_FILE_BUFFER|CEPH_CAP_FILE_LAZYIO)) == 0 ||
+	    (iocb->ki_filp->f_flags & O_DIRECT) ||
+	    (inode->i_sb->s_flags & MS_SYNCHRONOUS) ||
+	    (fi->flags & CEPH_F_SYNC)) {
+		ret = ceph_sync_write(file, iov->iov_base, iov->iov_len,
+			&iocb->ki_pos);
+	} else {
+		/*
+		 * buffered write; drop Fw early to avoid slow
+		 * revocation if we get stuck on balance_dirty_pages
+		 */
+		int dirty;
 
+		spin_lock(&ci->i_ceph_lock);
+		dirty = __ceph_mark_dirty_caps(ci, CEPH_CAP_FILE_WR);
+		spin_unlock(&ci->i_ceph_lock);
+		ceph_put_cap_refs(ci, got);
+
+		ret = generic_file_aio_write(iocb, iov, nr_segs, pos);
 		if ((ret >= 0 || ret == -EIOCBQUEUED) &&
 		    ((file->f_flags & O_SYNC) || IS_SYNC(file->f_mapping->host)
 		     || ceph_osdmap_flag(osdc->osdmap, CEPH_OSDMAP_NEARFULL))) {
-			err = vfs_fsync_range(file, pos, pos + written - 1, 1);
+			err = vfs_fsync_range(file, pos, pos + ret - 1, 1);
 			if (err < 0)
 				ret = err;
 		}
-		if ((ret < 0 && ret != -EAGAIN) || pos + written >= endoff)
-			goto out;
-	}
 
-	dout("aio_write %p %llx.%llx %llu~%u getting caps. i_size %llu\n",
-	     inode, ceph_vinop(inode), pos + written,
-	     (unsigned)iov->iov_len - written, inode->i_size);
-	ret = ceph_get_caps(ci, CEPH_CAP_FILE_WR, 0, &got, endoff);
-	if (ret < 0)
+		if (dirty)
+			__mark_inode_dirty(inode, dirty);
 		goto out;
+	}
 
-	dout("aio_write %p %llx.%llx %llu~%u  got cap refs on %s\n",
-	     inode, ceph_vinop(inode), pos + written,
-	     (unsigned)iov->iov_len - written, ceph_cap_string(got));
-	ret = ceph_sync_write(file, iov->iov_base + written,
-			      iov->iov_len - written, &iocb->ki_pos);
 	if (ret >= 0) {
 		int dirty;
 		spin_lock(&ci->i_ceph_lock);
@@ -773,10 +783,13 @@ retry_snap:
 		if (dirty)
 			__mark_inode_dirty(inode, dirty);
 	}
+
+out_put:
 	dout("aio_write %p %llx.%llx %llu~%u  dropping cap refs on %s\n",
-	     inode, ceph_vinop(inode), pos + written,
-	     (unsigned)iov->iov_len - written, ceph_cap_string(got));
+	     inode, ceph_vinop(inode), pos, (unsigned)iov->iov_len,
+	     ceph_cap_string(got));
 	ceph_put_cap_refs(ci, got);
+
 out:
 	if (ret == -EOLDSNAPC) {
 		dout("aio_write %p %llx.%llx %llu~%u got EOLDSNAPC, retrying\n",
diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index e52b0fb..ab899c8 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -1916,6 +1916,7 @@ static void __wake_requests(struct ceph_mds_client *mdsc,
 		req = list_entry(tmp_list.next,
 				 struct ceph_mds_request, r_wait);
 		list_del_init(&req->r_wait);
+		dout(" wake request %p tid %llu\n", req, req->r_tid);
 		__do_request(mdsc, req);
 	}
 }
-- 
1.7.11.7

[PATCH 6/7] ceph: don't early drop Fw cap

[Yan, Zheng]

From: "Yan, Zheng" <zheng.z.yan@intel.com>

ceph_aio_write() has an optimization that marks CEPH_CAP_FILE_WR
cap dirty before data is copied to page cache and inode size is
updated. The optimization avoids slow cap revocation caused by
balance_dirty_pages(), but introduces inode size update race. If
ceph_check_caps() flushes the dirty cap before the inode size is
updated, MDS can miss the new inode size. So just remove the
optimization.

Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
---
 fs/ceph/file.c | 42 +++++++++++++++++-------------------------
 1 file changed, 17 insertions(+), 25 deletions(-)

diff --git a/fs/ceph/file.c b/fs/ceph/file.c
index a949805..28ef273 100644
--- a/fs/ceph/file.c
+++ b/fs/ceph/file.c
@@ -724,9 +724,12 @@ static ssize_t ceph_aio_write(struct kiocb *iocb, const struct iovec *iov,
 	if (ceph_snap(inode) != CEPH_NOSNAP)
 		return -EROFS;
 
+	sb_start_write(inode->i_sb);
 retry_snap:
-	if (ceph_osdmap_flag(osdc->osdmap, CEPH_OSDMAP_FULL))
-		return -ENOSPC;
+	if (ceph_osdmap_flag(osdc->osdmap, CEPH_OSDMAP_FULL)) {
+		ret = -ENOSPC;
+		goto out;
+	}
 	__ceph_do_pending_vmtruncate(inode);
 	dout("aio_write %p %llx.%llx %llu~%u getting caps. i_size %llu\n",
 	     inode, ceph_vinop(inode), pos, (unsigned)iov->iov_len,
@@ -750,29 +753,10 @@ retry_snap:
 		ret = ceph_sync_write(file, iov->iov_base, iov->iov_len,
 			&iocb->ki_pos);
 	} else {
-		/*
-		 * buffered write; drop Fw early to avoid slow
-		 * revocation if we get stuck on balance_dirty_pages
-		 */
-		int dirty;
-
-		spin_lock(&ci->i_ceph_lock);
-		dirty = __ceph_mark_dirty_caps(ci, CEPH_CAP_FILE_WR);
-		spin_unlock(&ci->i_ceph_lock);
-		ceph_put_cap_refs(ci, got);
-
-		ret = generic_file_aio_write(iocb, iov, nr_segs, pos);
-		if ((ret >= 0 || ret == -EIOCBQUEUED) &&
-		    ((file->f_flags & O_SYNC) || IS_SYNC(file->f_mapping->host)
-		     || ceph_osdmap_flag(osdc->osdmap, CEPH_OSDMAP_NEARFULL))) {
-			err = vfs_fsync_range(file, pos, pos + ret - 1, 1);
-			if (err < 0)
-				ret = err;
-		}
-
-		if (dirty)
-			__mark_inode_dirty(inode, dirty);
-		goto out;
+		mutex_lock(&inode->i_mutex);
+		ret = __generic_file_aio_write(iocb, iov, nr_segs,
+					       &iocb->ki_pos);
+		mutex_unlock(&inode->i_mutex);
 	}
 
 	if (ret >= 0) {
@@ -790,12 +774,20 @@ out_put:
 	     ceph_cap_string(got));
 	ceph_put_cap_refs(ci, got);
 
+	if (ret >= 0 &&
+	    ((file->f_flags & O_SYNC) || IS_SYNC(file->f_mapping->host) ||
+	     ceph_osdmap_flag(osdc->osdmap, CEPH_OSDMAP_NEARFULL))) {
+		err = vfs_fsync_range(file, pos, pos + ret - 1, 1);
+		if (err < 0)
+			ret = err;
+	}
 out:
 	if (ret == -EOLDSNAPC) {
 		dout("aio_write %p %llx.%llx %llu~%u got EOLDSNAPC, retrying\n",
 		     inode, ceph_vinop(inode), pos, (unsigned)iov->iov_len);
 		goto retry_snap;
 	}
+	sb_end_write(inode->i_sb);
 
 	return ret;
 }
-- 
1.7.11.7

[PATCH 7/7] ceph: acquire i_mutex in __ceph_do_pending_vmtruncate

[Yan, Zheng]

From: "Yan, Zheng" <zheng.z.yan@intel.com>

make __ceph_do_pending_vmtruncate() acquire i_mutex if the caller
does not hold the mutex, so ceph_aio_read() can call it safely.

Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
---
 fs/ceph/file.c  |  6 +++---
 fs/ceph/inode.c | 18 +++++++++---------
 fs/ceph/super.h |  2 +-
 3 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/fs/ceph/file.c b/fs/ceph/file.c
index 28ef273..b9eedd4 100644
--- a/fs/ceph/file.c
+++ b/fs/ceph/file.c
@@ -653,7 +653,7 @@ static ssize_t ceph_aio_read(struct kiocb *iocb, const struct iovec *iov,
 	dout("aio_read %p %llx.%llx %llu~%u trying to get caps on %p\n",
 	     inode, ceph_vinop(inode), pos, (unsigned)len, inode);
 again:
-	__ceph_do_pending_vmtruncate(inode);
+	__ceph_do_pending_vmtruncate(inode, true);
 	if (fi->fmode & CEPH_FILE_MODE_LAZY)
 		want = CEPH_CAP_FILE_CACHE | CEPH_CAP_FILE_LAZYIO;
 	else
@@ -730,7 +730,7 @@ retry_snap:
 		ret = -ENOSPC;
 		goto out;
 	}
-	__ceph_do_pending_vmtruncate(inode);
+	__ceph_do_pending_vmtruncate(inode, true);
 	dout("aio_write %p %llx.%llx %llu~%u getting caps. i_size %llu\n",
 	     inode, ceph_vinop(inode), pos, (unsigned)iov->iov_len,
 	     inode->i_size);
@@ -801,7 +801,7 @@ static loff_t ceph_llseek(struct file *file, loff_t offset, int whence)
 	int ret;
 
 	mutex_lock(&inode->i_mutex);
-	__ceph_do_pending_vmtruncate(inode);
+	__ceph_do_pending_vmtruncate(inode, false);
 
 	if (whence == SEEK_END || whence == SEEK_DATA || whence == SEEK_HOLE) {
 		ret = ceph_do_getattr(inode, CEPH_STAT_CAP_SIZE);
diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
index 42c5769..2b3fee7 100644
--- a/fs/ceph/inode.c
+++ b/fs/ceph/inode.c
@@ -1416,7 +1416,7 @@ out:
 
 
 /*
- * called by trunc_wq; take i_mutex ourselves
+ * called by trunc_wq;
  *
  * We also truncate in a separate thread as well.
  */
@@ -1427,9 +1427,7 @@ static void ceph_vmtruncate_work(struct work_struct *work)
 	struct inode *inode = &ci->vfs_inode;
 
 	dout("vmtruncate_work %p\n", inode);
-	mutex_lock(&inode->i_mutex);
-	__ceph_do_pending_vmtruncate(inode);
-	mutex_unlock(&inode->i_mutex);
+	__ceph_do_pending_vmtruncate(inode, true);
 	iput(inode);
 }
 
@@ -1453,12 +1451,10 @@ void ceph_queue_vmtruncate(struct inode *inode)
 }
 
 /*
- * called with i_mutex held.
- *
  * Make sure any pending truncation is applied before doing anything
  * that may depend on it.
  */
-void __ceph_do_pending_vmtruncate(struct inode *inode)
+void __ceph_do_pending_vmtruncate(struct inode *inode, bool needlock)
 {
 	struct ceph_inode_info *ci = ceph_inode(inode);
 	u64 to;
@@ -1491,7 +1487,11 @@ retry:
 	     ci->i_truncate_pending, to);
 	spin_unlock(&ci->i_ceph_lock);
 
+	if (needlock)
+		mutex_lock(&inode->i_mutex);
 	truncate_inode_pages(inode->i_mapping, to);
+	if (needlock)
+		mutex_unlock(&inode->i_mutex);
 
 	spin_lock(&ci->i_ceph_lock);
 	if (to == ci->i_truncate_size) {
@@ -1544,7 +1544,7 @@ int ceph_setattr(struct dentry *dentry, struct iattr *attr)
 	if (ceph_snap(inode) != CEPH_NOSNAP)
 		return -EROFS;
 
-	__ceph_do_pending_vmtruncate(inode);
+	__ceph_do_pending_vmtruncate(inode, false);
 
 	err = inode_change_ok(inode, attr);
 	if (err != 0)
@@ -1722,7 +1722,7 @@ int ceph_setattr(struct dentry *dentry, struct iattr *attr)
 	     ceph_cap_string(dirtied), mask);
 
 	ceph_mdsc_put_request(req);
-	__ceph_do_pending_vmtruncate(inode);
+	__ceph_do_pending_vmtruncate(inode, false);
 	return err;
 out:
 	spin_unlock(&ci->i_ceph_lock);
diff --git a/fs/ceph/super.h b/fs/ceph/super.h
index efbcb56..e5f1875 100644
--- a/fs/ceph/super.h
+++ b/fs/ceph/super.h
@@ -694,7 +694,7 @@ extern int ceph_readdir_prepopulate(struct ceph_mds_request *req,
 extern int ceph_inode_holds_cap(struct inode *inode, int mask);
 
 extern int ceph_inode_set_size(struct inode *inode, loff_t size);
-extern void __ceph_do_pending_vmtruncate(struct inode *inode);
+extern void __ceph_do_pending_vmtruncate(struct inode *inode, bool needlock);
 extern void ceph_queue_vmtruncate(struct inode *inode);
 
 extern void ceph_queue_invalidate(struct inode *inode);
-- 
1.7.11.7

Re: [PATCH 4/7] ceph: use I_COMPLETE inode flag instead of D_COMPLETE flag

[Sage Weil]

On Fri, 1 Mar 2013, Yan, Zheng wrote:
> From: "Yan, Zheng" <zheng.z.yan@intel.com>
> 
> commit c6ffe10015 moved the flag that tracks if the dcache contents
> for a directory are complete to dentry. The problem is there are
> lots of places that use ceph_dir_{set,clear,test}_complete() while
> holding i_ceph_lock. but ceph_dir_{set,clear,test}_complete() may
> sleep because they call dput().
> 
> This patch basically reverts that commit. For ceph_d_prune(), it's
> called with both the dentry to prune and the parent dentry are
> locked. So it's safe to access the parent dentry's d_inode and
> clear I_COMPLETE flag.

I'm trying to remember why I thought the D_COMPETE flag was necessary.  
Maybe I didn't think that i_ceph_lock could safely nest inside of d_lock?  
Or that the parent was locked?

Anyway, assuming both of those things are in fact true, this looks good 
(and simpler :).

sage


> 
> Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
> ---
>  fs/ceph/caps.c       |  8 ++++---
>  fs/ceph/dir.c        | 62 ++++++++++------------------------------------------
>  fs/ceph/inode.c      | 30 +++++++++++--------------
>  fs/ceph/mds_client.c |  6 ++---
>  fs/ceph/super.h      | 23 ++-----------------
>  5 files changed, 34 insertions(+), 95 deletions(-)
> 
> diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
> index 61f3833..76634f4 100644
> --- a/fs/ceph/caps.c
> +++ b/fs/ceph/caps.c
> @@ -490,15 +490,17 @@ static void __check_cap_issue(struct ceph_inode_info *ci, struct ceph_cap *cap,
>  		ci->i_rdcache_gen++;
>  
>  	/*
> -	 * if we are newly issued FILE_SHARED, clear D_COMPLETE; we
> +	 * if we are newly issued FILE_SHARED, clear I_COMPLETE; we
>  	 * don't know what happened to this directory while we didn't
>  	 * have the cap.
>  	 */
>  	if ((issued & CEPH_CAP_FILE_SHARED) &&
>  	    (had & CEPH_CAP_FILE_SHARED) == 0) {
>  		ci->i_shared_gen++;
> -		if (S_ISDIR(ci->vfs_inode.i_mode))
> -			ceph_dir_clear_complete(&ci->vfs_inode);
> +		if (S_ISDIR(ci->vfs_inode.i_mode)) {
> +			dout(" marking %p NOT complete\n", &ci->vfs_inode);
> +			ci->i_ceph_flags &= ~CEPH_I_COMPLETE;
> +		}
>  	}
>  }
>  
> diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
> index 8c1aabe..76821be 100644
> --- a/fs/ceph/dir.c
> +++ b/fs/ceph/dir.c
> @@ -107,7 +107,7 @@ static unsigned fpos_off(loff_t p)
>   * falling back to a "normal" sync readdir if any dentries in the dir
>   * are dropped.
>   *
> - * D_COMPLETE tells indicates we have all dentries in the dir.  It is
> + * I_COMPLETE tells indicates we have all dentries in the dir.  It is
>   * defined IFF we hold CEPH_CAP_FILE_SHARED (which will be revoked by
>   * the MDS if/when the directory is modified).
>   */
> @@ -198,8 +198,8 @@ more:
>  	filp->f_pos++;
>  
>  	/* make sure a dentry wasn't dropped while we didn't have parent lock */
> -	if (!ceph_dir_test_complete(dir)) {
> -		dout(" lost D_COMPLETE on %p; falling back to mds\n", dir);
> +	if (!ceph_i_test(dir, CEPH_I_COMPLETE)) {
> +		dout(" lost I_COMPLETE on %p; falling back to mds\n", dir);
>  		err = -EAGAIN;
>  		goto out;
>  	}
> @@ -284,7 +284,7 @@ static int ceph_readdir(struct file *filp, void *dirent, filldir_t filldir)
>  	if ((filp->f_pos == 2 || fi->dentry) &&
>  	    !ceph_test_mount_opt(fsc, NOASYNCREADDIR) &&
>  	    ceph_snap(inode) != CEPH_SNAPDIR &&
> -	    ceph_dir_test_complete(inode) &&
> +	    (ci->i_ceph_flags & CEPH_I_COMPLETE) &&
>  	    __ceph_caps_issued_mask(ci, CEPH_CAP_FILE_SHARED, 1)) {
>  		spin_unlock(&ci->i_ceph_lock);
>  		err = __dcache_readdir(filp, dirent, filldir);
> @@ -350,7 +350,7 @@ more:
>  
>  		if (!req->r_did_prepopulate) {
>  			dout("readdir !did_prepopulate");
> -			fi->dir_release_count--;    /* preclude D_COMPLETE */
> +			fi->dir_release_count--;    /* preclude I_COMPLETE */
>  		}
>  
>  		/* note next offset and last dentry name */
> @@ -429,7 +429,8 @@ more:
>  	 */
>  	spin_lock(&ci->i_ceph_lock);
>  	if (ci->i_release_count == fi->dir_release_count) {
> -		ceph_dir_set_complete(inode);
> +		dout(" marking %p complete\n", inode);
> +		ci->i_ceph_flags |= CEPH_I_COMPLETE;
>  		ci->i_max_offset = filp->f_pos;
>  	}
>  	spin_unlock(&ci->i_ceph_lock);
> @@ -604,7 +605,7 @@ static struct dentry *ceph_lookup(struct inode *dir, struct dentry *dentry,
>  			    fsc->mount_options->snapdir_name,
>  			    dentry->d_name.len) &&
>  		    !is_root_ceph_dentry(dir, dentry) &&
> -		    ceph_dir_test_complete(dir) &&
> +		    (ci->i_ceph_flags & CEPH_I_COMPLETE) &&
>  		    (__ceph_caps_issued_mask(ci, CEPH_CAP_FILE_SHARED, 1))) {
>  			spin_unlock(&ci->i_ceph_lock);
>  			dout(" dir %p complete, -ENOENT\n", dir);
> @@ -908,7 +909,7 @@ static int ceph_rename(struct inode *old_dir, struct dentry *old_dentry,
>  		 */
>  
>  		/* d_move screws up d_subdirs order */
> -		ceph_dir_clear_complete(new_dir);
> +		ceph_i_clear(new_dir, CEPH_I_COMPLETE);
>  
>  		d_move(old_dentry, new_dentry);
>  
> @@ -1065,44 +1066,6 @@ static int ceph_snapdir_d_revalidate(struct dentry *dentry,
>  }
>  
>  /*
> - * Set/clear/test dir complete flag on the dir's dentry.
> - */
> -void ceph_dir_set_complete(struct inode *inode)
> -{
> -	struct dentry *dentry = d_find_any_alias(inode);
> -	
> -	if (dentry && ceph_dentry(dentry) &&
> -	    ceph_test_mount_opt(ceph_sb_to_client(dentry->d_sb), DCACHE)) {
> -		dout(" marking %p (%p) complete\n", inode, dentry);
> -		set_bit(CEPH_D_COMPLETE, &ceph_dentry(dentry)->flags);
> -	}
> -	dput(dentry);
> -}
> -
> -void ceph_dir_clear_complete(struct inode *inode)
> -{
> -	struct dentry *dentry = d_find_any_alias(inode);
> -
> -	if (dentry && ceph_dentry(dentry)) {
> -		dout(" marking %p (%p) complete\n", inode, dentry);
> -		set_bit(CEPH_D_COMPLETE, &ceph_dentry(dentry)->flags);
> -	}
> -	dput(dentry);
> -}
> -
> -bool ceph_dir_test_complete(struct inode *inode)
> -{
> -	struct dentry *dentry = d_find_any_alias(inode);
> -
> -	if (dentry && ceph_dentry(dentry)) {
> -		dout(" marking %p (%p) NOT complete\n", inode, dentry);
> -		clear_bit(CEPH_D_COMPLETE, &ceph_dentry(dentry)->flags);
> -	}
> -	dput(dentry);
> -	return false;
> -}
> -
> -/*
>   * When the VFS prunes a dentry from the cache, we need to clear the
>   * complete flag on the parent directory.
>   *
> @@ -1110,15 +1073,13 @@ bool ceph_dir_test_complete(struct inode *inode)
>   */
>  static void ceph_d_prune(struct dentry *dentry)
>  {
> -	struct ceph_dentry_info *di;
> -
>  	dout("ceph_d_prune %p\n", dentry);
>  
>  	/* do we have a valid parent? */
>  	if (IS_ROOT(dentry))
>  		return;
>  
> -	/* if we are not hashed, we don't affect D_COMPLETE */
> +	/* if we are not hashed, we don't affect I_COMPLETE */
>  	if (d_unhashed(dentry))
>  		return;
>  
> @@ -1126,8 +1087,7 @@ static void ceph_d_prune(struct dentry *dentry)
>  	 * we hold d_lock, so d_parent is stable, and d_fsdata is never
>  	 * cleared until d_release
>  	 */
> -	di = ceph_dentry(dentry->d_parent);
> -	clear_bit(CEPH_D_COMPLETE, &di->flags);
> +	ceph_i_clear(dentry->d_parent->d_inode, CEPH_I_COMPLETE);
>  }
>  
>  /*
> diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
> index 2971eaa..42c5769 100644
> --- a/fs/ceph/inode.c
> +++ b/fs/ceph/inode.c
> @@ -561,7 +561,6 @@ static int fill_inode(struct inode *inode,
>  	struct ceph_inode_info *ci = ceph_inode(inode);
>  	int i;
>  	int issued = 0, implemented;
> -	int updating_inode = 0;
>  	struct timespec mtime, atime, ctime;
>  	u32 nsplits;
>  	struct ceph_buffer *xattr_blob = NULL;
> @@ -601,7 +600,6 @@ static int fill_inode(struct inode *inode,
>  	    (ci->i_version & ~1) >= le64_to_cpu(info->version))
>  		goto no_change;
>  	
> -	updating_inode = 1;
>  	issued = __ceph_caps_issued(ci, &implemented);
>  	issued |= implemented | __ceph_caps_dirty(ci);
>  
> @@ -716,6 +714,17 @@ static int fill_inode(struct inode *inode,
>  		       ceph_vinop(inode), inode->i_mode);
>  	}
>  
> +	/* set dir completion flag? */
> +	if (S_ISDIR(inode->i_mode) &&
> +	    ci->i_files == 0 && ci->i_subdirs == 0 &&
> +	    ceph_snap(inode) == CEPH_NOSNAP &&
> +	    (le32_to_cpu(info->cap.caps) & CEPH_CAP_FILE_SHARED) &&
> +	    (issued & CEPH_CAP_FILE_EXCL) == 0 &&
> +	    (ci->i_ceph_flags & CEPH_I_COMPLETE) == 0) {
> +		dout(" marking %p complete (empty)\n", inode);
> +		ci->i_ceph_flags |= CEPH_I_COMPLETE;
> +		ci->i_max_offset = 2;
> +	}
>  no_change:
>  	spin_unlock(&ci->i_ceph_lock);
>  
> @@ -766,19 +775,6 @@ no_change:
>  		__ceph_get_fmode(ci, cap_fmode);
>  	}
>  
> -	/* set dir completion flag? */
> -	if (S_ISDIR(inode->i_mode) &&
> -	    updating_inode &&                 /* didn't jump to no_change */
> -	    ci->i_files == 0 && ci->i_subdirs == 0 &&
> -	    ceph_snap(inode) == CEPH_NOSNAP &&
> -	    (le32_to_cpu(info->cap.caps) & CEPH_CAP_FILE_SHARED) &&
> -	    (issued & CEPH_CAP_FILE_EXCL) == 0 &&
> -	    !ceph_dir_test_complete(inode)) {
> -		dout(" marking %p complete (empty)\n", inode);
> -		ceph_dir_set_complete(inode);
> -		ci->i_max_offset = 2;
> -	}
> -
>  	/* update delegation info? */
>  	if (dirinfo)
>  		ceph_fill_dirfrag(inode, dirinfo);
> @@ -860,7 +856,7 @@ static void ceph_set_dentry_offset(struct dentry *dn)
>  	di = ceph_dentry(dn);
>  
>  	spin_lock(&ci->i_ceph_lock);
> -	if (!ceph_dir_test_complete(inode)) {
> +	if ((ceph_inode(inode)->i_ceph_flags & CEPH_I_COMPLETE) == 0) {
>  		spin_unlock(&ci->i_ceph_lock);
>  		return;
>  	}
> @@ -1065,7 +1061,7 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req,
>  			 * d_move() puts the renamed dentry at the end of
>  			 * d_subdirs.  We need to assign it an appropriate
>  			 * directory offset so we can behave when holding
> -			 * D_COMPLETE.
> +			 * I_COMPLETE.
>  			 */
>  			ceph_set_dentry_offset(req->r_old_dentry);
>  			dout("dn %p gets new offset %lld\n", req->r_old_dentry, 
> diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
> index ccc68b0..e52b0fb 100644
> --- a/fs/ceph/mds_client.c
> +++ b/fs/ceph/mds_client.c
> @@ -2029,7 +2029,7 @@ out:
>  }
>  
>  /*
> - * Invalidate dir D_COMPLETE, dentry lease state on an aborted MDS
> + * Invalidate dir I_COMPLETE, dentry lease state on an aborted MDS
>   * namespace request.
>   */
>  void ceph_invalidate_dir_request(struct ceph_mds_request *req)
> @@ -2037,9 +2037,9 @@ void ceph_invalidate_dir_request(struct ceph_mds_request *req)
>  	struct inode *inode = req->r_locked_dir;
>  	struct ceph_inode_info *ci = ceph_inode(inode);
>  
> -	dout("invalidate_dir_request %p (D_COMPLETE, lease(s))\n", inode);
> +	dout("invalidate_dir_request %p (I_COMPLETE, lease(s))\n", inode);
>  	spin_lock(&ci->i_ceph_lock);
> -	ceph_dir_clear_complete(inode);
> +	ci->i_ceph_flags &= ~CEPH_I_COMPLETE;
>  	ci->i_release_count++;
>  	spin_unlock(&ci->i_ceph_lock);
>  
> diff --git a/fs/ceph/super.h b/fs/ceph/super.h
> index 4353ebc..efbcb56 100644
> --- a/fs/ceph/super.h
> +++ b/fs/ceph/super.h
> @@ -204,7 +204,6 @@ struct ceph_inode_xattr {
>   * Ceph dentry state
>   */
>  struct ceph_dentry_info {
> -	unsigned long flags;
>  	struct ceph_mds_session *lease_session;
>  	u32 lease_gen, lease_shared_gen;
>  	u32 lease_seq;
> @@ -215,18 +214,6 @@ struct ceph_dentry_info {
>  	u64 offset;
>  };
>  
> -/*
> - * dentry flags
> - *
> - * The locking for D_COMPLETE is a bit odd:
> - *  - we can clear it at almost any time (see ceph_d_prune)
> - *  - it is only meaningful if:
> - *    - we hold dir inode i_ceph_lock
> - *    - we hold dir FILE_SHARED caps
> - *    - the dentry D_COMPLETE is set
> - */
> -#define CEPH_D_COMPLETE 1  /* if set, d_u.d_subdirs is complete directory */
> -
>  struct ceph_inode_xattrs_info {
>  	/*
>  	 * (still encoded) xattr blob. we avoid the overhead of parsing
> @@ -267,7 +254,7 @@ struct ceph_inode_info {
>  	struct timespec i_rctime;
>  	u64 i_rbytes, i_rfiles, i_rsubdirs;
>  	u64 i_files, i_subdirs;
> -	u64 i_max_offset;  /* largest readdir offset, set with D_COMPLETE */
> +	u64 i_max_offset;  /* largest readdir offset, set with I_COMPLETE */
>  
>  	struct rb_root i_fragtree;
>  	struct mutex i_fragtree_mutex;
> @@ -432,6 +419,7 @@ static inline struct inode *ceph_find_inode(struct super_block *sb,
>  /*
>   * Ceph inode.
>   */
> +#define CEPH_I_COMPLETE  1  /* we have complete directory cached */
>  #define CEPH_I_NODELAY   4  /* do not delay cap release */
>  #define CEPH_I_FLUSH     8  /* do not delay flush of dirty metadata */
>  #define CEPH_I_NOFLUSH  16  /* do not flush dirty caps */
> @@ -489,13 +477,6 @@ static inline loff_t ceph_make_fpos(unsigned frag, unsigned off)
>  }
>  
>  /*
> - * set/clear directory D_COMPLETE flag
> - */
> -void ceph_dir_set_complete(struct inode *inode);
> -void ceph_dir_clear_complete(struct inode *inode);
> -bool ceph_dir_test_complete(struct inode *inode);
> -
> -/*
>   * caps helpers
>   */
>  static inline bool __ceph_is_any_real_caps(struct ceph_inode_info *ci)
> -- 
> 1.7.11.7
> 
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

Re: [PATCH 6/7] ceph: don't early drop Fw cap

[Gregory Farnum]

On Thu, Feb 28, 2013 at 10:46 PM, Yan, Zheng <zheng.z.yan@intel.com> wrote:
> From: "Yan, Zheng" <zheng.z.yan@intel.com>
>
> ceph_aio_write() has an optimization that marks CEPH_CAP_FILE_WR
> cap dirty before data is copied to page cache and inode size is
> updated. The optimization avoids slow cap revocation caused by
> balance_dirty_pages(), but introduces inode size update race. If
> ceph_check_caps() flushes the dirty cap before the inode size is
> updated, MDS can miss the new inode size. So just remove the
> optimization.
>
> Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
> ---
>  fs/ceph/file.c | 42 +++++++++++++++++-------------------------
>  1 file changed, 17 insertions(+), 25 deletions(-)
>
> diff --git a/fs/ceph/file.c b/fs/ceph/file.c
> index a949805..28ef273 100644
> --- a/fs/ceph/file.c
> +++ b/fs/ceph/file.c
> @@ -724,9 +724,12 @@ static ssize_t ceph_aio_write(struct kiocb *iocb, const struct iovec *iov,
>         if (ceph_snap(inode) != CEPH_NOSNAP)
>                 return -EROFS;
>
> +       sb_start_write(inode->i_sb);
>  retry_snap:
> -       if (ceph_osdmap_flag(osdc->osdmap, CEPH_OSDMAP_FULL))
> -               return -ENOSPC;
> +       if (ceph_osdmap_flag(osdc->osdmap, CEPH_OSDMAP_FULL)) {
> +               ret = -ENOSPC;
> +               goto out;
> +       }
>         __ceph_do_pending_vmtruncate(inode);
>         dout("aio_write %p %llx.%llx %llu~%u getting caps. i_size %llu\n",
>              inode, ceph_vinop(inode), pos, (unsigned)iov->iov_len,
> @@ -750,29 +753,10 @@ retry_snap:
>                 ret = ceph_sync_write(file, iov->iov_base, iov->iov_len,
>                         &iocb->ki_pos);
>         } else {
> -               /*
> -                * buffered write; drop Fw early to avoid slow
> -                * revocation if we get stuck on balance_dirty_pages
> -                */
> -               int dirty;
> -
> -               spin_lock(&ci->i_ceph_lock);
> -               dirty = __ceph_mark_dirty_caps(ci, CEPH_CAP_FILE_WR);
> -               spin_unlock(&ci->i_ceph_lock);
> -               ceph_put_cap_refs(ci, got);
> -
> -               ret = generic_file_aio_write(iocb, iov, nr_segs, pos);
> -               if ((ret >= 0 || ret == -EIOCBQUEUED) &&
> -                   ((file->f_flags & O_SYNC) || IS_SYNC(file->f_mapping->host)
> -                    || ceph_osdmap_flag(osdc->osdmap, CEPH_OSDMAP_NEARFULL))) {
> -                       err = vfs_fsync_range(file, pos, pos + ret - 1, 1);
> -                       if (err < 0)
> -                               ret = err;
> -               }
> -
> -               if (dirty)
> -                       __mark_inode_dirty(inode, dirty);
> -               goto out;
> +               mutex_lock(&inode->i_mutex);
> +               ret = __generic_file_aio_write(iocb, iov, nr_segs,
> +                                              &iocb->ki_pos);
> +               mutex_unlock(&inode->i_mutex);

Hmm, you're here passing in a different value than the removed
generic_file_aio_write() call did — &iocb->ki_pos instead of pos.
Everything else is using the pos parameter so I rather expect that
should still be used here?
Also a quick skim of the interfaces makes me think that the two
versions aren't interchangeable — __generic_file_aio_write() also
handles O_SYNC in addition to grabbing i_mutex. Why'd you switch them?
(I haven't checked deep enough to see if the unlocked version is
correct or not, but it does say that's what most filesystems should
use.)
-Greg


>         }
>
>         if (ret >= 0) {
> @@ -790,12 +774,20 @@ out_put:
>              ceph_cap_string(got));
>         ceph_put_cap_refs(ci, got);
>
> +       if (ret >= 0 &&
> +           ((file->f_flags & O_SYNC) || IS_SYNC(file->f_mapping->host) ||
> +            ceph_osdmap_flag(osdc->osdmap, CEPH_OSDMAP_NEARFULL))) {
> +               err = vfs_fsync_range(file, pos, pos + ret - 1, 1);
> +               if (err < 0)
> +                       ret = err;
> +       }
>  out:
>         if (ret == -EOLDSNAPC) {
>                 dout("aio_write %p %llx.%llx %llu~%u got EOLDSNAPC, retrying\n",
>                      inode, ceph_vinop(inode), pos, (unsigned)iov->iov_len);
>                 goto retry_snap;
>         }
> +       sb_end_write(inode->i_sb);
>
>         return ret;
>  }
> --
> 1.7.11.7
>
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/7] ceph: misc fixes

[Gregory Farnum]

On Thu, Feb 28, 2013 at 10:46 PM, Yan, Zheng <zheng.z.yan@intel.com> wrote:
> From: "Yan, Zheng" <zheng.z.yan@intel.com>
>
> These patches are also in:
>   git://github.com/ukernel/linux.git wip-ceph

1, 2, 3, 5, 7 all look good to me. If you can double-check Sage's
concerns on 4 and my questions on 6 I'll be happy to pull these in. :)
-Greg

Re: [PATCH 6/7] ceph: don't early drop Fw cap

[Yan, Zheng]

On 03/05/2013 02:26 AM, Gregory Farnum wrote:
> On Thu, Feb 28, 2013 at 10:46 PM, Yan, Zheng <zheng.z.yan@intel.com> wrote:
>> From: "Yan, Zheng" <zheng.z.yan@intel.com>
>>
>> ceph_aio_write() has an optimization that marks CEPH_CAP_FILE_WR
>> cap dirty before data is copied to page cache and inode size is
>> updated. The optimization avoids slow cap revocation caused by
>> balance_dirty_pages(), but introduces inode size update race. If
>> ceph_check_caps() flushes the dirty cap before the inode size is
>> updated, MDS can miss the new inode size. So just remove the
>> optimization.
>>
>> Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
>> ---
>>  fs/ceph/file.c | 42 +++++++++++++++++-------------------------
>>  1 file changed, 17 insertions(+), 25 deletions(-)
>>
>> diff --git a/fs/ceph/file.c b/fs/ceph/file.c
>> index a949805..28ef273 100644
>> --- a/fs/ceph/file.c
>> +++ b/fs/ceph/file.c
>> @@ -724,9 +724,12 @@ static ssize_t ceph_aio_write(struct kiocb *iocb, const struct iovec *iov,
>>         if (ceph_snap(inode) != CEPH_NOSNAP)
>>                 return -EROFS;
>>
>> +       sb_start_write(inode->i_sb);
>>  retry_snap:
>> -       if (ceph_osdmap_flag(osdc->osdmap, CEPH_OSDMAP_FULL))
>> -               return -ENOSPC;
>> +       if (ceph_osdmap_flag(osdc->osdmap, CEPH_OSDMAP_FULL)) {
>> +               ret = -ENOSPC;
>> +               goto out;
>> +       }
>>         __ceph_do_pending_vmtruncate(inode);
>>         dout("aio_write %p %llx.%llx %llu~%u getting caps. i_size %llu\n",
>>              inode, ceph_vinop(inode), pos, (unsigned)iov->iov_len,
>> @@ -750,29 +753,10 @@ retry_snap:
>>                 ret = ceph_sync_write(file, iov->iov_base, iov->iov_len,
>>                         &iocb->ki_pos);
>>         } else {
>> -               /*
>> -                * buffered write; drop Fw early to avoid slow
>> -                * revocation if we get stuck on balance_dirty_pages
>> -                */
>> -               int dirty;
>> -
>> -               spin_lock(&ci->i_ceph_lock);
>> -               dirty = __ceph_mark_dirty_caps(ci, CEPH_CAP_FILE_WR);
>> -               spin_unlock(&ci->i_ceph_lock);
>> -               ceph_put_cap_refs(ci, got);
>> -
>> -               ret = generic_file_aio_write(iocb, iov, nr_segs, pos);
>> -               if ((ret >= 0 || ret == -EIOCBQUEUED) &&
>> -                   ((file->f_flags & O_SYNC) || IS_SYNC(file->f_mapping->host)
>> -                    || ceph_osdmap_flag(osdc->osdmap, CEPH_OSDMAP_NEARFULL))) {
>> -                       err = vfs_fsync_range(file, pos, pos + ret - 1, 1);
>> -                       if (err < 0)
>> -                               ret = err;
>> -               }
>> -
>> -               if (dirty)
>> -                       __mark_inode_dirty(inode, dirty);
>> -               goto out;
>> +               mutex_lock(&inode->i_mutex);
>> +               ret = __generic_file_aio_write(iocb, iov, nr_segs,
>> +                                              &iocb->ki_pos);
>> +               mutex_unlock(&inode->i_mutex);
> 
> Hmm, you're here passing in a different value than the removed
> generic_file_aio_write() call did — &iocb->ki_pos instead of pos.
> Everything else is using the pos parameter so I rather expect that
> should still be used here?

They always have the same value, see the BUG_ON in generic_file_aio_write()

> Also a quick skim of the interfaces makes me think that the two
> versions aren't interchangeable — __generic_file_aio_write() also
> handles O_SYNC in addition to grabbing i_mutex. Why'd you switch them?

ceph has its own code that handles O_SYNC case. I want to make sb_start_write()
covers ceph_sync_write(), that's the reason I use __generic_file_aio_write() here.

Regards
Yan, Zheng

> (I haven't checked deep enough to see if the unlocked version is
> correct or not, but it does say that's what most filesystems should
> use.)
> -Greg
> 
> 
>>         }
>>
>>         if (ret >= 0) {
>> @@ -790,12 +774,20 @@ out_put:
>>              ceph_cap_string(got));
>>         ceph_put_cap_refs(ci, got);
>>
>> +       if (ret >= 0 &&
>> +           ((file->f_flags & O_SYNC) || IS_SYNC(file->f_mapping->host) ||
>> +            ceph_osdmap_flag(osdc->osdmap, CEPH_OSDMAP_NEARFULL))) {
>> +               err = vfs_fsync_range(file, pos, pos + ret - 1, 1);
>> +               if (err < 0)
>> +                       ret = err;
>> +       }
>>  out:
>>         if (ret == -EOLDSNAPC) {
>>                 dout("aio_write %p %llx.%llx %llu~%u got EOLDSNAPC, retrying\n",
>>                      inode, ceph_vinop(inode), pos, (unsigned)iov->iov_len);
>>                 goto retry_snap;
>>         }
>> +       sb_end_write(inode->i_sb);
>>
>>         return ret;
>>  }
>> --
>> 1.7.11.7
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 6/7] ceph: don't early drop Fw cap

[Greg Farnum]

On Monday, March 4, 2013 at 5:57 PM, Yan, Zheng wrote:
> On 03/05/2013 02:26 AM, Gregory Farnum wrote:
> > On Thu, Feb 28, 2013 at 10:46 PM, Yan, Zheng <zheng.z.yan@intel.com (mailto:zheng.z.yan@intel.com)> wrote:
> > > From: "Yan, Zheng" <zheng.z.yan@intel.com (mailto:zheng.z.yan@intel.com)>
> > >  
> > > ceph_aio_write() has an optimization that marks CEPH_CAP_FILE_WR
> > > cap dirty before data is copied to page cache and inode size is
> > > updated. The optimization avoids slow cap revocation caused by
> > > balance_dirty_pages(), but introduces inode size update race. If
> > > ceph_check_caps() flushes the dirty cap before the inode size is
> > > updated, MDS can miss the new inode size. So just remove the
> > > optimization.
> > >  
> > > Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com (mailto:zheng.z.yan@intel.com)>
> > > ---
> > > fs/ceph/file.c | 42 +++++++++++++++++-------------------------
> > > 1 file changed, 17 insertions(+), 25 deletions(-)
> > >  
> > > diff --git a/fs/ceph/file.c b/fs/ceph/file.c
> > > index a949805..28ef273 100644
> > > --- a/fs/ceph/file.c
> > > +++ b/fs/ceph/file.c
> > > @@ -724,9 +724,12 @@ static ssize_t ceph_aio_write(struct kiocb *iocb, const struct iovec *iov,
> > > if (ceph_snap(inode) != CEPH_NOSNAP)
> > > return -EROFS;
> > >  
> > > + sb_start_write(inode->i_sb);
> > > retry_snap:
> > > - if (ceph_osdmap_flag(osdc->osdmap, CEPH_OSDMAP_FULL))
> > > - return -ENOSPC;
> > > + if (ceph_osdmap_flag(osdc->osdmap, CEPH_OSDMAP_FULL)) {
> > > + ret = -ENOSPC;
> > > + goto out;
> > > + }
> > > __ceph_do_pending_vmtruncate(inode);
> > > dout("aio_write %p %llx.%llx %llu~%u getting caps. i_size %llu\n",
> > > inode, ceph_vinop(inode), pos, (unsigned)iov->iov_len,
> > > @@ -750,29 +753,10 @@ retry_snap:
> > > ret = ceph_sync_write(file, iov->iov_base, iov->iov_len,
> > > &iocb->ki_pos);
> > > } else {
> > > - /*
> > > - * buffered write; drop Fw early to avoid slow
> > > - * revocation if we get stuck on balance_dirty_pages
> > > - */
> > > - int dirty;
> > > -
> > > - spin_lock(&ci->i_ceph_lock);
> > > - dirty = __ceph_mark_dirty_caps(ci, CEPH_CAP_FILE_WR);
> > > - spin_unlock(&ci->i_ceph_lock);
> > > - ceph_put_cap_refs(ci, got);
> > > -
> > > - ret = generic_file_aio_write(iocb, iov, nr_segs, pos);
> > > - if ((ret >= 0 || ret == -EIOCBQUEUED) &&
> > > - ((file->f_flags & O_SYNC) || IS_SYNC(file->f_mapping->host)
> > > - || ceph_osdmap_flag(osdc->osdmap, CEPH_OSDMAP_NEARFULL))) {
> > > - err = vfs_fsync_range(file, pos, pos + ret - 1, 1);
> > > - if (err < 0)
> > > - ret = err;
> > > - }
> > > -
> > > - if (dirty)
> > > - __mark_inode_dirty(inode, dirty);
> > > - goto out;
> > > + mutex_lock(&inode->i_mutex);
> > > + ret = __generic_file_aio_write(iocb, iov, nr_segs,
> > > + &iocb->ki_pos);
> > > + mutex_unlock(&inode->i_mutex);
> >  
> >  
> >  
> > Hmm, you're here passing in a different value than the removed
> > generic_file_aio_write() call did — &iocb->ki_pos instead of pos.
> > Everything else is using the pos parameter so I rather expect that
> > should still be used here?
>  
>  
>  
> They always have the same value, see the BUG_ON in generic_file_aio_write()
>  
> > Also a quick skim of the interfaces makes me think that the two
> > versions aren't interchangeable — __generic_file_aio_write() also
> > handles O_SYNC in addition to grabbing i_mutex. Why'd you switch them?
>  
>  
>  
> ceph has its own code that handles O_SYNC case. I want to make sb_start_write()
> covers ceph_sync_write(), that's the reason I use __generic_file_aio_write() here.
>  
> Regards
> Yan, Zheng
>  
Ah, yep — sounds good!
-Greg  


--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/7] ceph: misc fixes

[Greg Farnum]

I've merged this series into the testing branch, with appropriate Reviewed-by tags from me (and Sage on #4). Thanks much for the code and helping me go through it. :) 
-Greg
Software Engineer #42 @ http://inktank.com | http://ceph.com


On Monday, March 4, 2013 at 7:38 PM, Yan, Zheng wrote:

> On 03/05/2013 02:49 AM, Gregory Farnum wrote:
> > On Thu, Feb 28, 2013 at 10:46 PM, Yan, Zheng <zheng.z.yan@intel.com> wrote:
> > > From: "Yan, Zheng" <zheng.z.yan@intel.com>
> > > 
> > > These patches are also in:
> > > git://github.com/ukernel/linux.git (http://github.com/ukernel/linux.git) wip-ceph
> > 
> > 
> > 
> > 1, 2, 3, 5, 7 all look good to me. If you can double-check Sage's
> > concerns on 4 and my questions on 6 I'll be happy to pull these in. :)
> 
> 
> 
> I rechecked locking assumption in patch 4, nothing goes wrong.
> 
> Regards
> Yan, Zheng
> 
> > -Greg