ceph: Move the place for EOLDSNAPC handle in ceph_aio_write to easily understand

Only for ceph_sync_write, the osd can return EOLDSNAPC.so move the related codes after the call ceph_sync_write.

Signed-off-by Jianpeng Ma <[email protected]>

---
 fs/ceph/file.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

fs/ceph/file.c +10 -8

index 2ddf061..30e959f 100644
@@ -768,6 +768,15 @@ retry_snap:
 		mutex_unlock(&inode->i_mutex);
 		written = ceph_sync_write(file, iov->iov_base, count,
 					  pos, &iocb->ki_pos);
+		if (written == -EOLDSNAPC) {
+			dout("aio_write %p %llx.%llx %llu~%u"
+				"got EOLDSNAPC, retrying\n",
+				inode, ceph_vinop(inode),
+				pos, (unsigned)iov->iov_len);
+			mutex_lock(&inode->i_mutex);
+			hold_mutex = true;
+			goto retry_snap;
+		}
 	} else {
 		written = generic_file_buffered_write(iocb, iov, nr_segs,
 						      pos, &iocb->ki_pos,

@@ -797,14 +806,7 @@ retry_snap:
 		if (err < 0)
 			written = err;
 	}
-
-	if (written == -EOLDSNAPC) {
-		dout("aio_write %p %llx.%llx %llu~%u got EOLDSNAPC, retrying\n",
-		     inode, ceph_vinop(inode), pos, (unsigned)iov->iov_len);
-		mutex_lock(&inode->i_mutex);
-		hold_mutex = true;
-		goto retry_snap;
-	}
+
 out:
 	if (hold_mutex)
 		mutex_unlock(&inode->i_mutex);

Signature · 1 line

1.8.3.rc1.44.gb387c77

Looks good; I've applied this to the tree. Canyou review the below patch while we are looking at this code?

Thanks!
sage

From 26d0d7b213d87db0ef46e885ae749c27395c11b1 Mon Sep 17 00:00:00 2001
From: Sage Weil <[email protected]>
Date: Thu, 8 Aug 2013 09:39:44 -0700
Subject: [PATCH] ceph: replace hold_mutex flag with goto

All of the early exit paths need to drop the mutex; it is only the normal path through the function that does not. Skip the unlock in that case with a goto out_unlocked.

Signed-off-by Sage Weil <[email protected]>

---
fs/ceph/file.c | 11 ++++-------

1 file changed, 4 insertions(+), 7 deletions(-)

fs/ceph/file.c +2 -4

index 7478d5d..a17ffe4 100644
@@ -710,13 +710,11 @@ static ssize_t ceph_aio_write(struct kiocb *iocb, const struct iovec *iov,
 		&ceph_sb_to_client(inode->i_sb)->client->osdc;
 	ssize_t count, written = 0;
 	int err, want, got;
-	bool hold_mutex;
 
 	if (ceph_snap(inode) != CEPH_NOSNAP)
 		return -EROFS;
 
 	mutex_lock(&inode->i_mutex);
-	hold_mutex = true;
 
 	err = generic_segment_checks(iov, &nr_segs, &count, VERIFY_READ);
 	if (err)

@@ -772,7 +770,6 @@ retry_snap:
 				inode, ceph_vinop(inode),
 				pos, (unsigned)iov->iov_len);
 			mutex_lock(&inode->i_mutex);
-			hold_mutex = true;
 			goto retry_snap;
 		}
 	} else {

@@ -781,7 +778,6 @@ retry_snap:
 						      count, 0);
 		mutex_unlock(&inode->i_mutex);
 	}
-	hold_mutex = false;
 
 	if (written >= 0) {
 		int dirty;

@@ -805,11 +801,12 @@ retry_snap:
 			written = err;
 	}
 
+	goto out_unlocked;
+

out:

fs/ceph/file.c +2 -2

-	if (hold_mutex)
-		mutex_unlock(&inode->i_mutex);
+	mutex_unlock(&inode->i_mutex);
+out_unlocked:

	current->backing_dev_info = NULL;
-
	return written ? written : err;
}

Signature · 1 line

1.8.1.2

Quoted 70 lines

Looks good; I've applied this to the tree. Canyou review the below patch while we are looking at this code?

Thanks!
sage

From 26d0d7b213d87db0ef46e885ae749c27395c11b1 Mon Sep 17 00:00:00 2001
From: Sage Weil <[email protected]>
Date: Thu, 8 Aug 2013 09:39:44 -0700
Subject: [PATCH] ceph: replace hold_mutex flag with goto

All of the early exit paths need to drop the mutex; it is only the normal path through the function that does not. Skip the unlock in that case with a goto out_unlocked.

Signed-off-by Sage Weil <[email protected]>

---
fs/ceph/file.c | 11 ++++-------

1 file changed, 4 insertions(+), 7 deletions(-)

fs/ceph/file.c +2 -4

index 7478d5d..a17ffe4 100644
@@ -710,13 +710,11 @@ static ssize_t ceph_aio_write(struct kiocb *iocb, const struct iovec *iov,
 		&ceph_sb_to_client(inode->i_sb)->client->osdc;
 	ssize_t count, written = 0;
 	int err, want, got;
-	bool hold_mutex;
 
 	if (ceph_snap(inode) != CEPH_NOSNAP)
 		return -EROFS;
 
 	mutex_lock(&inode->i_mutex);
-	hold_mutex = true;
 
 	err = generic_segment_checks(iov, &nr_segs, &count, VERIFY_READ);
 	if (err)

@@ -772,7 +770,6 @@ retry_snap:
 				inode, ceph_vinop(inode),
 				pos, (unsigned)iov->iov_len);
 			mutex_lock(&inode->i_mutex);
-			hold_mutex = true;
 			goto retry_snap;
 		}
 	} else {

@@ -781,7 +778,6 @@ retry_snap:
 						      count, 0);
 		mutex_unlock(&inode->i_mutex);
 	}
-	hold_mutex = false;
 
 	if (written >= 0) {
 		int dirty;

@@ -805,11 +801,12 @@ retry_snap:
 			written = err;
 	}
 
+	goto out_unlocked;
+

out:

fs/ceph/file.c +2 -2

-	if (hold_mutex)
-		mutex_unlock(&inode->i_mutex);
+	mutex_unlock(&inode->i_mutex);
+out_unlocked:

	current->backing_dev_info = NULL;
-
	return written ? written : err;
}

Quoted 3 lines

Signature · 1 line

1.8.1.2

Hi sage,
It's ok.

BTW, i had a question about EOLDSNAPC.Now for a sync-write,if it met EOLDSNAPC, it will retry.
The rewrite is for all bio.Supported a sync-write crossed multi stripe.
Q1:Is there a chance that for some stripe-write it din't met EOLDSNAPC,the later met?

Q2:If Q1 occured, can we only rewrite the stripe which met EOLDSNAPCE?