From 74ba033d9b3fff536599f8adca25d5ca2cac9f18 Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Wed, 26 Mar 2008 21:42:16 -0700 Subject: [PATCH] fix kclient lease release, client_lease encoding --- src/TODO | 4 ++++ src/client/Client.cc | 16 +++++++------- src/include/ceph_fs.h | 6 +++++ src/kernel/client.c | 1 + src/kernel/inode.c | 11 ++++++---- src/kernel/mds_client.c | 44 +++++++++++++++++-------------------- src/mds/Locker.cc | 13 ++++++----- src/messages/MClientLease.h | 35 ++++++++++++++++------------- 8 files changed, 73 insertions(+), 57 deletions(-) diff --git a/src/TODO b/src/TODO index db58466c1c8c6..2127ef236c535 100644 --- a/src/TODO +++ b/src/TODO @@ -8,6 +8,10 @@ client leases - lease length heuristics - mds lock last_change stamp? - client: preemptively release lease on dentry we are unlinking, renaming from/to, etc. + - ICONTENT on file open + - DN on unlink, rename + - IAUTH on setattr + - ICONTENT on utimes - or inode fields we are chmodding, etc. - lease renewals (to avoid blocking client ops?) diff --git a/src/client/Client.cc b/src/client/Client.cc index 3905ca15121f5..3680bb1e10b75 100644 --- a/src/client/Client.cc +++ b/src/client/Client.cc @@ -1160,25 +1160,25 @@ void Client::handle_lease(MClientLease *m) { dout(10) << "handle_lease " << *m << dendl; - assert(m->action == CEPH_MDS_LEASE_REVOKE); + assert(m->get_action() == CEPH_MDS_LEASE_REVOKE); Inode *in; - if (inode_map.count(m->ino) == 0) { - dout(10) << " don't have inode " << m->ino << dendl; + if (inode_map.count(m->get_ino()) == 0) { + dout(10) << " don't have inode " << m->get_ino() << dendl; goto revoke; } - in = inode_map[m->ino]; + in = inode_map[m->get_ino()]; - if (m->mask & CEPH_LOCK_DN) { + if (m->get_mask() & CEPH_LOCK_DN) { if (!in->dir || in->dir->dentries.count(m->dname) == 0) { - dout(10) << " don't have dir|dentry " << m->ino << "/" << m->dname <get_ino() << "/" << m->dname <dir->dentries[m->dname]; dout(10) << " reset ttl on " << dn << dendl; dn->ttl = utime_t(); } else { - int newmask = in->mask & ~m->mask; + int newmask = in->mask & ~m->get_mask(); dout(10) << " reset inode " << in->ino() << " mask " << in->mask << " -> " << newmask << dendl; in->mask = newmask; @@ -1186,7 +1186,7 @@ void Client::handle_lease(MClientLease *m) revoke: messenger->send_message(new MClientLease(CEPH_MDS_LEASE_RELEASE, - m->mask, m->ino, m->dname), + m->get_mask(), m->get_ino(), m->dname), m->get_source_inst()); delete m; } diff --git a/src/include/ceph_fs.h b/src/include/ceph_fs.h index e3d63e52a33eb..647534a4080cf 100644 --- a/src/include/ceph_fs.h +++ b/src/include/ceph_fs.h @@ -510,6 +510,12 @@ struct ceph_mds_file_caps { #define CEPH_MDS_LEASE_RELEASE 2 /* client -> mds */ #define CEPH_MDS_LEASE_RENEW 3 /* client <-> mds */ +struct ceph_mds_lease { + __u8 action; + __le16 mask; + __le64 ino; +} __attribute__ ((packed)); +/* followed by a __le32+string for dname */ /* client reconnect */ struct ceph_mds_cap_reconnect { diff --git a/src/kernel/client.c b/src/kernel/client.c index 557a294627339..7bcbda3cbf4de 100644 --- a/src/kernel/client.c +++ b/src/kernel/client.c @@ -316,6 +316,7 @@ const char *ceph_msg_type_name(int type) case CEPH_MSG_CLIENT_REQUEST_FORWARD: return "client_request_forward"; case CEPH_MSG_CLIENT_REPLY: return "client_reply"; case CEPH_MSG_CLIENT_FILECAPS: return "client_filecaps"; + case CEPH_MSG_CLIENT_LEASE: return "client_lease"; case CEPH_MSG_OSD_GETMAP: return "osd_getmap"; case CEPH_MSG_OSD_MAP: return "osd_map"; case CEPH_MSG_OSD_OP: return "osd_op"; diff --git a/src/kernel/inode.c b/src/kernel/inode.c index a9ec887c68e7c..07765941bc37e 100644 --- a/src/kernel/inode.c +++ b/src/kernel/inode.c @@ -269,10 +269,13 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req) /* inode */ if (d+1 == rinfo->trace_numi) { dout(10, "fill_trace has dentry but no inode\n"); - err = -ENOENT; - d_instantiate(dn, NULL); - if (d_unhashed(dn)) - d_rehash(dn); + if (dn->d_inode) + d_delete(dn); /* is this right? */ + else { + d_instantiate(dn, NULL); + if (d_unhashed(dn)) + d_rehash(dn); + } break; } ininfo = rinfo->trace_in[d+1].in; diff --git a/src/kernel/mds_client.c b/src/kernel/mds_client.c index 09c864b27bb36..b490fa6206478 100644 --- a/src/kernel/mds_client.c +++ b/src/kernel/mds_client.c @@ -80,7 +80,6 @@ static int parse_reply_info_trace(void **p, void *end, ceph_decode_16(p, numd); info->trace_numi = numi; info->trace_numd = numd; - dout(10, "numi %d numd %d\n", numi, numd); if (numi == 0) { info->trace_in = 0; goto done; /* hrm, this shouldn't actually happen, but.. */ @@ -107,19 +106,16 @@ static int parse_reply_info_trace(void **p, void *end, inode: if (!numi) goto done; - dout(20, "parsing inode at %p/%p, numi %d\n", *p, end, numi); numi--; err = parse_reply_info_in(p, end, &info->trace_in[numi]); if (err < 0) goto bad; - dout(20, "parsing lease at %p/%p\n", *p, end); info->trace_ilease[numi] = *p; *p += sizeof(struct ceph_mds_reply_lease); dentry: if (!numd) goto done; - dout(20, "parsing dentry and dir, numd %d\n", numd); numd--; ceph_decode_32_safe(p, end, info->trace_dname_len[numd], bad); ceph_decode_need(p, end, info->trace_dname_len[numd], bad); @@ -137,7 +133,6 @@ dentry: goto inode; done: - dout(20, "done, %p/%p\n", *p, end); if (unlikely(*p != end)) return -EINVAL; return 0; @@ -1380,23 +1375,21 @@ void ceph_mdsc_handle_lease(struct ceph_mds_client *mdsc, struct ceph_msg *msg) struct ceph_inode_info *ci; struct dentry *parent, *dentry; int mds = le32_to_cpu(msg->hdr.src.name.num); - __u8 action; - __u16 mask; + struct ceph_mds_lease *h = msg->front.iov_base; __u64 ino; + int mask; struct qstr dname; - void *p = msg->front.iov_base; - void *end = p + msg->front.iov_len; ino_t inot; dout(10, "handle_lease from mds%d\n", mds); /* decode */ - ceph_decode_need(&p, end, 1 + 2 + 8, bad); - ceph_decode_8(&p, action); - ceph_decode_16(&p, mask); - ceph_decode_64(&p, ino); - dname.name = p; - dname.len = end-p; + if (msg->front.iov_len < sizeof(*h) + sizeof(__u32)) + goto bad; + ino = le64_to_cpu(h->ino); + mask = le16_to_cpu(h->mask); + dname.name = (void *)(h + 1); + dname.len = msg->front.iov_len - sizeof(*h) - sizeof(__u32); /* lookup inode */ inot = ceph_ino_to_ino(ino); @@ -1405,37 +1398,40 @@ void ceph_mdsc_handle_lease(struct ceph_mds_client *mdsc, struct ceph_msg *msg) #else inode = ilookup5(sb, inot, ceph_ino_compare, &ino); #endif - dout(20, "action is %d, ino %llx %p\n", action, ino, inode); + dout(20, "action is %d, ino %llx %p\n", h->action, ino, inode); - BUG_ON(action != CEPH_MDS_LEASE_REVOKE); /* for now */ + BUG_ON(h->action != CEPH_MDS_LEASE_REVOKE); /* for now */ if (inode == NULL) { dout(10, "i don't have inode %llx\n", ino); - goto revoke; + goto release; } if (mask & CEPH_LOCK_DN) { /* dentry */ parent = d_find_alias(inode); - if (!parent) - goto revoke; /* hrm... */ + if (!parent) { + dout(10, "no parent dentry on inode %p\n", inode); + goto release; /* hrm... */ + } dname.hash = full_name_hash(dname.name, dname.len); dentry = d_lookup(parent, &dname); if (!dentry) - goto revoke; + goto release; dentry->d_time = 0; dout(10, "lease revoked on dentry %p\n", dentry); } else { /* inode */ ci = ceph_inode(inode); - ci->i_lease_mask &= mask; + ci->i_lease_mask &= ~mask; dout(10, "lease mask %d revoked on inode %p, still have %d\n", mask, inode, ci->i_lease_mask); } -revoke: +release: dout(10, "sending release\n"); - *(__u8*)(msg->front.iov_base) = CEPH_MDS_LEASE_RELEASE; + h->action = CEPH_MDS_LEASE_RELEASE; + ceph_msg_get(msg); send_msg_mds(mdsc, msg, mds); return; diff --git a/src/mds/Locker.cc b/src/mds/Locker.cc index 07825b7d1befb..b8fd8986009ef 100644 --- a/src/mds/Locker.cc +++ b/src/mds/Locker.cc @@ -962,21 +962,21 @@ void Locker::handle_client_lease(MClientLease *m) assert(m->get_source().is_client()); int client = m->get_source().num(); - CInode *in = mdcache->get_inode(m->ino); + CInode *in = mdcache->get_inode(m->get_ino()); if (!in) { - dout(7) << "handle_client_lease don't have ino " << m->ino << dendl; + dout(7) << "handle_client_lease don't have ino " << m->get_ino() << dendl; delete m; return; } CDentry *dn = 0; MDSCacheObject *p; - if (m->mask & CEPH_LOCK_DN) { + if (m->get_mask() & CEPH_LOCK_DN) { frag_t fg = in->pick_dirfrag(m->dname); CDir *dir = in->get_dirfrag(fg); if (dir) p = dn = dir->lookup(m->dname); if (!dn) { - dout(7) << "handle_client_lease don't have dn " << m->ino << " " << m->dname << dendl; + dout(7) << "handle_client_lease don't have dn " << m->get_ino() << " " << m->dname << dendl; delete m; return; } @@ -993,11 +993,11 @@ void Locker::handle_client_lease(MClientLease *m) return; } - switch (m->action) { + switch (m->get_action()) { case CEPH_MDS_LEASE_RELEASE: { dout(7) << "handle_client_lease client" << client - << " release mask " << m->mask + << " release mask " << m->get_mask() << " on " << *p << dendl; int left = p->remove_client_lease(l, l->mask, this); dout(10) << " remaining mask is " << left << " on " << *p << dendl; @@ -1005,6 +1005,7 @@ void Locker::handle_client_lease(MClientLease *m) break; case CEPH_MDS_LEASE_RENEW: + default: assert(0); // implement me break; } diff --git a/src/messages/MClientLease.h b/src/messages/MClientLease.h index dc6472a32f1cf..879d3bedd6a9b 100644 --- a/src/messages/MClientLease.h +++ b/src/messages/MClientLease.h @@ -30,24 +30,33 @@ static const char *get_lease_action_name(int a) { struct MClientLease : public Message { - __u8 action; - __u16 mask; - __u64 ino; + struct ceph_mds_lease h; string dname; + + int get_action() { return h.action; } + int get_mask() { return le16_to_cpu(h.mask); } + __u64 get_ino() { return le64_to_cpu(h.ino); } MClientLease() : Message(CEPH_MSG_CLIENT_LEASE) {} MClientLease(int ac, int m, __u64 i) : - Message(CEPH_MSG_CLIENT_LEASE), - action(ac), mask(m), ino(i) {} + Message(CEPH_MSG_CLIENT_LEASE) { + h.action = ac; + h.mask = cpu_to_le16(m); + h.ino = cpu_to_le64(i); + } MClientLease(int ac, int m, __u64 i, const string& d) : Message(CEPH_MSG_CLIENT_LEASE), - action(ac), mask(m), ino(i), dname(d) {} + dname(d) { + h.action = ac; + h.mask = cpu_to_le16(m); + h.ino = cpu_to_le64(i); + } const char *get_type_name() { return "client_lease"; } void print(ostream& out) { - out << "client_lease(a=" << get_lease_action_name(action) - << " mask " << mask; - out << " " << inodeno_t(ino); + out << "client_lease(a=" << get_lease_action_name(get_action()) + << " mask " << get_mask(); + out << " " << inodeno_t(get_ino()); if (dname.length()) out << "/" << dname; out << ")"; @@ -55,15 +64,11 @@ struct MClientLease : public Message { void decode_payload() { int off = 0; - ::_decode(mask, payload, off); - ::_decode(action, payload, off); - ::_decode(ino, payload, off); + ::_decode(h, payload, off); ::_decode(dname, payload, off); } virtual void encode_payload() { - ::_encode(mask, payload); - ::_encode(action, payload); - ::_encode(ino, payload); + ::_encode(h, payload); ::_encode(dname, payload); } -- 2.39.5