| Message ID | alpine.DEB.2.00.1308080940000.18359@cobra.newdream.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Pkxvb2tzIGdvb2Q7IEkndmUgYXBwbGllZCB0aGlzIHRvIHRoZSB0cmVlLiAgQ2FueW91IHJldmll dyB0aGUgYmVsb3cgcGF0Y2ggDQo+d2hpbGUgd2UgYXJlIGxvb2tpbmcgYXQgdGhpcyBjb2RlPw0K Pg0KPlRoYW5rcyENCj5zYWdlDQo+DQo+RnJvbSAyNmQwZDdiMjEzZDg3ZGIwZWY0NmU4ODVhZTc0 OWMyNzM5NWMxMWIxIE1vbiBTZXAgMTcgMDA6MDA6MDAgMjAwMQ0KPkZyb206IFNhZ2UgV2VpbCA8 c2FnZUBpbmt0YW5rLmNvbT4NCj5EYXRlOiBUaHUsIDggQXVnIDIwMTMgMDk6Mzk6NDQgLTA3MDAN Cj5TdWJqZWN0OiBbUEFUQ0hdIGNlcGg6IHJlcGxhY2UgaG9sZF9tdXRleCBmbGFnIHdpdGggZ290 bw0KPg0KPkFsbCBvZiB0aGUgZWFybHkgZXhpdCBwYXRocyBuZWVkIHRvIGRyb3AgdGhlIG11dGV4 OyBpdCBpcyBvbmx5IHRoZSBub3JtYWwNCj5wYXRoIHRocm91Z2ggdGhlIGZ1bmN0aW9uIHRoYXQg ZG9lcyBub3QuICBTa2lwIHRoZSB1bmxvY2sgaW4gdGhhdCBjYXNlDQo+d2l0aCBhIGdvdG8gb3V0 X3VubG9ja2VkLg0KPg0KPlNpZ25lZC1vZmYtYnk6IFNhZ2UgV2VpbCA8c2FnZUBpbmt0YW5rLmNv bT4NCj4tLS0NCj4gZnMvY2VwaC9maWxlLmMgfCAxMSArKysrLS0tLS0tLQ0KPiAxIGZpbGUgY2hh bmdlZCwgNCBpbnNlcnRpb25zKCspLCA3IGRlbGV0aW9ucygtKQ0KPg0KPmRpZmYgLS1naXQgYS9m cy9jZXBoL2ZpbGUuYyBiL2ZzL2NlcGgvZmlsZS5jDQo+aW5kZXggNzQ3OGQ1ZC4uYTE3ZmZlNCAx MDA2NDQNCj4tLS0gYS9mcy9jZXBoL2ZpbGUuYw0KPisrKyBiL2ZzL2NlcGgvZmlsZS5jDQo+QEAg LTcxMCwxMyArNzEwLDExIEBAIHN0YXRpYyBzc2l6ZV90IGNlcGhfYWlvX3dyaXRlKHN0cnVjdCBr aW9jYiAqaW9jYiwgY29uc3Qgc3RydWN0IGlvdmVjICppb3YsDQo+IAkJJmNlcGhfc2JfdG9fY2xp ZW50KGlub2RlLT5pX3NiKS0+Y2xpZW50LT5vc2RjOw0KPiAJc3NpemVfdCBjb3VudCwgd3JpdHRl biA9IDA7DQo+IAlpbnQgZXJyLCB3YW50LCBnb3Q7DQo+LQlib29sIGhvbGRfbXV0ZXg7DQo+IA0K PiAJaWYgKGNlcGhfc25hcChpbm9kZSkgIT0gQ0VQSF9OT1NOQVApDQo+IAkJcmV0dXJuIC1FUk9G UzsNCj4gDQo+IAltdXRleF9sb2NrKCZpbm9kZS0+aV9tdXRleCk7DQo+LQlob2xkX211dGV4ID0g dHJ1ZTsNCj4gDQo+IAllcnIgPSBnZW5lcmljX3NlZ21lbnRfY2hlY2tzKGlvdiwgJm5yX3NlZ3Ms ICZjb3VudCwgVkVSSUZZX1JFQUQpOw0KPiAJaWYgKGVycikNCj5AQCAtNzcyLDcgKzc3MCw2IEBA IHJldHJ5X3NuYXA6DQo+IAkJCQlpbm9kZSwgY2VwaF92aW5vcChpbm9kZSksDQo+IAkJCQlwb3Ms ICh1bnNpZ25lZClpb3YtPmlvdl9sZW4pOw0KPiAJCQltdXRleF9sb2NrKCZpbm9kZS0+aV9tdXRl eCk7DQo+LQkJCWhvbGRfbXV0ZXggPSB0cnVlOw0KPiAJCQlnb3RvIHJldHJ5X3NuYXA7DQo+IAkJ fQ0KPiAJfSBlbHNlIHsNCj5AQCAtNzgxLDcgKzc3OCw2IEBAIHJldHJ5X3NuYXA6DQo+IAkJCQkJ CSAgICAgIGNvdW50LCAwKTsNCj4gCQltdXRleF91bmxvY2soJmlub2RlLT5pX211dGV4KTsNCj4g CX0NCj4tCWhvbGRfbXV0ZXggPSBmYWxzZTsNCj4gDQo+IAlpZiAod3JpdHRlbiA+PSAwKSB7DQo+ IAkJaW50IGRpcnR5Ow0KPkBAIC04MDUsMTEgKzgwMSwxMiBAQCByZXRyeV9zbmFwOg0KPiAJCQl3 cml0dGVuID0gZXJyOw0KPiAJfQ0KPiANCj4rCWdvdG8gb3V0X3VubG9ja2VkOw0KPisNCj4gb3V0 Og0KPi0JaWYgKGhvbGRfbXV0ZXgpDQo+LQkJbXV0ZXhfdW5sb2NrKCZpbm9kZS0+aV9tdXRleCk7 DQo+KwltdXRleF91bmxvY2soJmlub2RlLT5pX211dGV4KTsNCj4rb3V0X3VubG9ja2VkOg0KPiAJ Y3VycmVudC0+YmFja2luZ19kZXZfaW5mbyA9IE5VTEw7DQo+LQ0KPiAJcmV0dXJuIHdyaXR0ZW4g PyB3cml0dGVuIDogZXJyOw0KPiB9DQo+IA0KPi0tIA0KPjEuOC4xLjINCj4NCkhpIHNhZ2UsDQoJ SXQncyBvay4NCglCVFcsIGkgaGFkIGEgcXVlc3Rpb24gYWJvdXQgRU9MRFNOQVBDLk5vdyBmb3Ig YSBzeW5jLXdyaXRlLGlmIGl0IG1ldCBFT0xEU05BUEMsIGl0IHdpbGwgcmV0cnkuDQpUaGUgcmV3 cml0ZSBpcyBmb3IgYWxsIGJpby5TdXBwb3J0ZWQgYSBzeW5jLXdyaXRlIGNyb3NzZWQgbXVsdGkg c3RyaXBlLg0KUTE6SXMgdGhlcmUgYSBjaGFuY2UgdGhhdCBmb3Igc29tZSBzdHJpcGUtd3JpdGUg aXQgZGluJ3QgbWV0IEVPTERTTkFQQyx0aGUgbGF0ZXIgbWV0Pw0KUTI6SWYgUTEgb2NjdXJlZCwg Y2FuIHdlIG9ubHkgcmV3cml0ZSB0aGUgc3RyaXBlIHdoaWNoIG1ldCBFT0xEU05BUENFPw0KDQpU aGFua3MhDQpKaWFucGVuZyBNYQ0KDQoJ -- 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
On Fri, 9 Aug 2013, majianpeng wrote: > >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 <sage@inktank.com> > >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 <sage@inktank.com> > >--- > > fs/ceph/file.c | 11 ++++------- > > 1 file changed, 4 insertions(+), 7 deletions(-) > > > >diff --git a/fs/ceph/file.c b/fs/ceph/file.c > >index 7478d5d..a17ffe4 100644 > >--- a/fs/ceph/file.c > >+++ b/fs/ceph/file.c > >@@ -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: > >- if (hold_mutex) > >- mutex_unlock(&inode->i_mutex); > >+ mutex_unlock(&inode->i_mutex); > >+out_unlocked: > > current->backing_dev_info = NULL; > >- > > return written ? written : err; > > } > > > >-- > >1.8.1.2 > > > Hi sage, > It's ok. Thanks for reviewing! > 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? Yes. The writes that cross object boundaries aren't atomic in that sense. > Q2:If Q1 occured, can we only rewrite the stripe which met EOLDSNAPCE? Right. This would mean the write might be 'torn', with half of it in the snapshot and half after. It's not strictly posix, and hard to make atomic without adding a lot of complexity and slowing things down. So far we've decided it's not worth it. sage -- 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
diff --git a/fs/ceph/file.c b/fs/ceph/file.c index 7478d5d..a17ffe4 100644 --- a/fs/ceph/file.c +++ b/fs/ceph/file.c @@ -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: - if (hold_mutex) - mutex_unlock(&inode->i_mutex); + mutex_unlock(&inode->i_mutex); +out_unlocked: current->backing_dev_info = NULL; - return written ? written : err; }