Yep, I think you're right, Alex.. don't need the atomics here, either. On Fri, 13 Jan 2012, Alex Elder wrote:
> The ceph_mds_session s_cap_ttl field is 64 bits that can be accessed > concurrently and is used for synchronization. Change it to be an > atomic64_t to make guarantee it is operated on atomicly. > > Signed-off-by: Alex Elder <[email protected]> > --- > fs/ceph/caps.c | 2 +- > fs/ceph/dir.c | 2 +- > fs/ceph/mds_client.c | 25 +++++++++++++++---------- > fs/ceph/mds_client.h | 2 +- > 4 files changed, 18 insertions(+), 13 deletions(-) > > diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c > index fd240e3..9b87e48 100644 > --- a/fs/ceph/caps.c > +++ b/fs/ceph/caps.c > @@ -643,7 +643,7 @@ static int __cap_is_valid(struct ceph_cap *cap) > > spin_lock(&cap->session->s_gen_ttl_lock); > gen = atomic_read(&cap->session->s_cap_gen); > - ttl = cap->session->s_cap_ttl; > + ttl = atomic64_read(&cap->session->s_cap_ttl); > spin_unlock(&cap->session->s_gen_ttl_lock); spinlcok > > if (cap->cap_gen < gen || time_after_eq(jiffies, ttl)) { > diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c > index da9427d..a182907 100644 > --- a/fs/ceph/dir.c > +++ b/fs/ceph/dir.c > @@ -979,7 +979,7 @@ static int dentry_lease_is_valid(struct dentry > *dentry) > > spin_lock(&s->s_gen_ttl_lock); > gen = atomic_read(&s->s_cap_gen); > - ttl = s->s_cap_ttl; > + ttl = atomic64_read(&s->s_cap_ttl); > spin_unlock(&s->s_gen_ttl_lock); spin > > if (di->lease_gen == gen && > diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c > index cd567c3..a1ecd1a 100644 > --- a/fs/ceph/mds_client.c > +++ b/fs/ceph/mds_client.c > @@ -400,7 +400,7 @@ static struct ceph_mds_session > *register_session(struct ceph_mds_client *mdsc, > > spin_lock_init(&s->s_gen_ttl_lock); > atomic_set(&s->s_cap_gen, 0); > - s->s_cap_ttl = jiffies - 1; > + atomic64_set(&s->s_cap_ttl, jiffies - 1); no sync needed > > spin_lock_init(&s->s_cap_lock); > s->s_renew_requested = 0; > @@ -1044,9 +1044,11 @@ static int send_renew_caps(struct ceph_mds_client > *mdsc, > { > struct ceph_msg *msg; > int state; > + unsigned long cap_ttl; > > - if (time_after_eq(jiffies, session->s_cap_ttl) && > - time_after_eq(session->s_cap_ttl, session->s_renew_requested)) > + cap_ttl = atomic64_read(&session->s_cap_ttl); > + if (time_after_eq(jiffies, cap_ttl) && > + time_after_eq(cap_ttl, session->s_renew_requested)) this is under session mutex, i think. if not, spin > pr_info("mds%d caps stale\n", session->s_mds); > session->s_renew_requested = jiffies; > > @@ -1079,15 +1081,18 @@ static void renewed_caps(struct ceph_mds_client > *mdsc, > { > int was_stale; > int wake = 0; > + unsigned long cap_ttl; > > spin_lock(&session->s_cap_lock); > - was_stale = is_renew && time_after_eq(jiffies, session->s_cap_ttl); > + cap_ttl = atomic64_read(&session->s_cap_ttl); > + was_stale = is_renew && time_after_eq(jiffies, cap_ttl); > > - session->s_cap_ttl = session->s_renew_requested + > - mdsc->mdsmap->m_session_timeout*HZ; > + cap_ttl = session->s_renew_requested + > + mdsc->mdsmap->m_session_timeout*HZ; > + atomic64_set(&session->s_cap_ttl, cap_ttl); spinlock is ok > > if (was_stale) { > - if (time_before(jiffies, session->s_cap_ttl)) { > + if (time_before(jiffies, cap_ttl)) { > pr_info("mds%d caps renewed\n", session->s_mds); > wake = 1; > } else { > @@ -1095,8 +1100,8 @@ static void renewed_caps(struct ceph_mds_client > *mdsc, > } > } > dout("renewed_caps mds%d ttl now %lu, was %s, now %s\n", > - session->s_mds, session->s_cap_ttl, was_stale ? "stale" : > "fresh", > - time_before(jiffies, session->s_cap_ttl) ? "stale" : "fresh"); > + session->s_mds, cap_ttl, was_stale ? "stale" : "fresh", > + time_before(jiffies, cap_ttl) ? "stale" : "fresh"); > spin_unlock(&session->s_cap_lock); > > if (wake) > @@ -2329,7 +2334,7 @@ static void handle_session(struct ceph_mds_session > *session, > session->s_mds); > spin_lock(&session->s_gen_ttl_lock); > atomic_inc(&session->s_cap_gen); > - session->s_cap_ttl = jiffies - 1; > + atomic64_set(&session->s_cap_ttl, jiffies - 1); session mutex > spin_unlock(&session->s_gen_ttl_lock); > send_renew_caps(mdsc, session); > break; > diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h > index 20ca17e..f3c32fb 100644 > --- a/fs/ceph/mds_client.h > +++ b/fs/ceph/mds_client.h > @@ -120,7 +120,7 @@ struct ceph_mds_session { > /* protected by s_gen_ttl_lock */ > spinlock_t s_gen_ttl_lock; > atomic_t s_cap_gen; /* inc each time we get mds stale msg */ > - unsigned long s_cap_ttl; /* when session caps expire */ > + atomic64_t s_cap_ttl; /* when session caps expire */ > > /* protected by s_cap_lock */ > spinlock_t s_cap_lock; > -- > 1.7.5.4 > > > > -- > To unsubscribe from this list: send the line "unsubscribe ceph-devel" in > the body of a message to [email protected] > 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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
