| Message ID | 20210618142740.3345-1-yejune.deng@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | net: ceph: Use CLOCK_MONOTONIC ktime_get_ts64() | expand |
On Fri, 2021-06-18 at 22:27 +0800, Yejune Deng wrote: > The Documentation/core-api/timekeeping.rst recommend that we should use > monotonic time ktime_get_ts64(), to avoid glitches with a concurrent > settimeofday(). > > Signed-off-by: Yejune Deng <yejune.deng@gmail.com> > --- > net/ceph/messenger.c | 2 +- > net/ceph/messenger_v1.c | 2 +- > net/ceph/messenger_v2.c | 2 +- > net/ceph/osd_client.c | 4 ++-- > 4 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c > index 57d043b..2d07ab5 100644 > --- a/net/ceph/messenger.c > +++ b/net/ceph/messenger.c > @@ -1809,7 +1809,7 @@ bool ceph_con_keepalive_expired(struct ceph_connection *con, > (con->peer_features & CEPH_FEATURE_MSGR_KEEPALIVE2)) { > struct timespec64 now; > struct timespec64 ts; > - ktime_get_real_ts64(&now); > + ktime_get_ts64(&now); > jiffies_to_timespec64(interval, &ts); > ts = timespec64_add(con->last_keepalive_ack, ts); > return timespec64_compare(&now, &ts) >= 0; > diff --git a/net/ceph/messenger_v1.c b/net/ceph/messenger_v1.c > index 2cb5ffd..2ec7b1d 100644 > --- a/net/ceph/messenger_v1.c > +++ b/net/ceph/messenger_v1.c > @@ -310,7 +310,7 @@ static void prepare_write_keepalive(struct ceph_connection *con) > if (con->peer_features & CEPH_FEATURE_MSGR_KEEPALIVE2) { > struct timespec64 now; > > - ktime_get_real_ts64(&now); > + ktime_get_ts64(&now); > con_out_kvec_add(con, sizeof(tag_keepalive2), &tag_keepalive2); > ceph_encode_timespec64(&con->v1.out_temp_keepalive2, &now); > con_out_kvec_add(con, sizeof(con->v1.out_temp_keepalive2), > diff --git a/net/ceph/messenger_v2.c b/net/ceph/messenger_v2.c > index cc40ce4..2125e77 100644 > --- a/net/ceph/messenger_v2.c > +++ b/net/ceph/messenger_v2.c > @@ -1439,7 +1439,7 @@ static int prepare_keepalive2(struct ceph_connection *con) > struct ceph_timespec *ts = CTRL_BODY(con->v2.out_buf); > struct timespec64 now; > > - ktime_get_real_ts64(&now); > + ktime_get_ts64(&now); > dout("%s con %p timestamp %lld.%09ld\n", __func__, con, now.tv_sec, > now.tv_nsec); > > diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c > index ff8624a..5192a8a 100644 > --- a/net/ceph/osd_client.c > +++ b/net/ceph/osd_client.c > @@ -4717,7 +4717,7 @@ ceph_osdc_watch(struct ceph_osd_client *osdc, > ceph_oid_copy(&lreq->t.base_oid, oid); > ceph_oloc_copy(&lreq->t.base_oloc, oloc); > lreq->t.flags = CEPH_OSD_FLAG_WRITE; > - ktime_get_real_ts64(&lreq->mtime); > + ktime_get_ts64(&lreq->mtime); > > lreq->reg_req = alloc_watch_request(lreq, CEPH_OSD_WATCH_OP_WATCH); > if (!lreq->reg_req) { > @@ -4767,7 +4767,7 @@ int ceph_osdc_unwatch(struct ceph_osd_client *osdc, > ceph_oid_copy(&req->r_base_oid, &lreq->t.base_oid); > ceph_oloc_copy(&req->r_base_oloc, &lreq->t.base_oloc); > req->r_flags = CEPH_OSD_FLAG_WRITE; > - ktime_get_real_ts64(&req->r_mtime); > + ktime_get_ts64(&req->r_mtime); > osd_req_op_watch_init(req, 0, lreq->linger_id, > CEPH_OSD_WATCH_OP_UNWATCH); > I think this is OK. Most of these timestamps get marshalled onto the wire to use as identifiers. Those are almost certainly better off with CLOCK_MONOTONIC since you could have a clock jump that caused you to send (e.g.) 2 keepalives with the same timestamp otherwise. Reviewed-by: Jeff Layton <jlayton@kernel.org>
On Sat, Jun 19, 2021 at 12:51 PM Jeff Layton <jlayton@kernel.org> wrote: > > On Fri, 2021-06-18 at 22:27 +0800, Yejune Deng wrote: > > The Documentation/core-api/timekeeping.rst recommend that we should use > > monotonic time ktime_get_ts64(), to avoid glitches with a concurrent > > settimeofday(). > > > > Signed-off-by: Yejune Deng <yejune.deng@gmail.com> > > --- > > net/ceph/messenger.c | 2 +- > > net/ceph/messenger_v1.c | 2 +- > > net/ceph/messenger_v2.c | 2 +- > > net/ceph/osd_client.c | 4 ++-- > > 4 files changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c > > index 57d043b..2d07ab5 100644 > > --- a/net/ceph/messenger.c > > +++ b/net/ceph/messenger.c > > @@ -1809,7 +1809,7 @@ bool ceph_con_keepalive_expired(struct ceph_connection *con, > > (con->peer_features & CEPH_FEATURE_MSGR_KEEPALIVE2)) { > > struct timespec64 now; > > struct timespec64 ts; > > - ktime_get_real_ts64(&now); > > + ktime_get_ts64(&now); > > jiffies_to_timespec64(interval, &ts); > > ts = timespec64_add(con->last_keepalive_ack, ts); > > return timespec64_compare(&now, &ts) >= 0; > > diff --git a/net/ceph/messenger_v1.c b/net/ceph/messenger_v1.c > > index 2cb5ffd..2ec7b1d 100644 > > --- a/net/ceph/messenger_v1.c > > +++ b/net/ceph/messenger_v1.c > > @@ -310,7 +310,7 @@ static void prepare_write_keepalive(struct ceph_connection *con) > > if (con->peer_features & CEPH_FEATURE_MSGR_KEEPALIVE2) { > > struct timespec64 now; > > > > - ktime_get_real_ts64(&now); > > + ktime_get_ts64(&now); > > con_out_kvec_add(con, sizeof(tag_keepalive2), &tag_keepalive2); > > ceph_encode_timespec64(&con->v1.out_temp_keepalive2, &now); > > con_out_kvec_add(con, sizeof(con->v1.out_temp_keepalive2), > > diff --git a/net/ceph/messenger_v2.c b/net/ceph/messenger_v2.c > > index cc40ce4..2125e77 100644 > > --- a/net/ceph/messenger_v2.c > > +++ b/net/ceph/messenger_v2.c > > @@ -1439,7 +1439,7 @@ static int prepare_keepalive2(struct ceph_connection *con) > > struct ceph_timespec *ts = CTRL_BODY(con->v2.out_buf); > > struct timespec64 now; > > > > - ktime_get_real_ts64(&now); > > + ktime_get_ts64(&now); > > dout("%s con %p timestamp %lld.%09ld\n", __func__, con, now.tv_sec, > > now.tv_nsec); > > > > diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c > > index ff8624a..5192a8a 100644 > > --- a/net/ceph/osd_client.c > > +++ b/net/ceph/osd_client.c > > @@ -4717,7 +4717,7 @@ ceph_osdc_watch(struct ceph_osd_client *osdc, > > ceph_oid_copy(&lreq->t.base_oid, oid); > > ceph_oloc_copy(&lreq->t.base_oloc, oloc); > > lreq->t.flags = CEPH_OSD_FLAG_WRITE; > > - ktime_get_real_ts64(&lreq->mtime); > > + ktime_get_ts64(&lreq->mtime); > > > > lreq->reg_req = alloc_watch_request(lreq, CEPH_OSD_WATCH_OP_WATCH); > > if (!lreq->reg_req) { > > @@ -4767,7 +4767,7 @@ int ceph_osdc_unwatch(struct ceph_osd_client *osdc, > > ceph_oid_copy(&req->r_base_oid, &lreq->t.base_oid); > > ceph_oloc_copy(&req->r_base_oloc, &lreq->t.base_oloc); > > req->r_flags = CEPH_OSD_FLAG_WRITE; > > - ktime_get_real_ts64(&req->r_mtime); > > + ktime_get_ts64(&req->r_mtime); > > osd_req_op_watch_init(req, 0, lreq->linger_id, > > CEPH_OSD_WATCH_OP_UNWATCH); > > > > I think this is OK. Most of these timestamps get marshalled onto the > wire to use as identifiers. Those are almost certainly better off with > CLOCK_MONOTONIC since you could have a clock jump that caused you to > send (e.g.) 2 keepalives with the same timestamp otherwise. While I generally agree that for keepalive (i.e. the first free instances) using the monotonic clock would be better, in userspace CLOCK_REALTIME is used and I don't want to diverge. Thanks, Ilya
diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c index 57d043b..2d07ab5 100644 --- a/net/ceph/messenger.c +++ b/net/ceph/messenger.c @@ -1809,7 +1809,7 @@ bool ceph_con_keepalive_expired(struct ceph_connection *con, (con->peer_features & CEPH_FEATURE_MSGR_KEEPALIVE2)) { struct timespec64 now; struct timespec64 ts; - ktime_get_real_ts64(&now); + ktime_get_ts64(&now); jiffies_to_timespec64(interval, &ts); ts = timespec64_add(con->last_keepalive_ack, ts); return timespec64_compare(&now, &ts) >= 0; diff --git a/net/ceph/messenger_v1.c b/net/ceph/messenger_v1.c index 2cb5ffd..2ec7b1d 100644 --- a/net/ceph/messenger_v1.c +++ b/net/ceph/messenger_v1.c @@ -310,7 +310,7 @@ static void prepare_write_keepalive(struct ceph_connection *con) if (con->peer_features & CEPH_FEATURE_MSGR_KEEPALIVE2) { struct timespec64 now; - ktime_get_real_ts64(&now); + ktime_get_ts64(&now); con_out_kvec_add(con, sizeof(tag_keepalive2), &tag_keepalive2); ceph_encode_timespec64(&con->v1.out_temp_keepalive2, &now); con_out_kvec_add(con, sizeof(con->v1.out_temp_keepalive2), diff --git a/net/ceph/messenger_v2.c b/net/ceph/messenger_v2.c index cc40ce4..2125e77 100644 --- a/net/ceph/messenger_v2.c +++ b/net/ceph/messenger_v2.c @@ -1439,7 +1439,7 @@ static int prepare_keepalive2(struct ceph_connection *con) struct ceph_timespec *ts = CTRL_BODY(con->v2.out_buf); struct timespec64 now; - ktime_get_real_ts64(&now); + ktime_get_ts64(&now); dout("%s con %p timestamp %lld.%09ld\n", __func__, con, now.tv_sec, now.tv_nsec); diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c index ff8624a..5192a8a 100644 --- a/net/ceph/osd_client.c +++ b/net/ceph/osd_client.c @@ -4717,7 +4717,7 @@ ceph_osdc_watch(struct ceph_osd_client *osdc, ceph_oid_copy(&lreq->t.base_oid, oid); ceph_oloc_copy(&lreq->t.base_oloc, oloc); lreq->t.flags = CEPH_OSD_FLAG_WRITE; - ktime_get_real_ts64(&lreq->mtime); + ktime_get_ts64(&lreq->mtime); lreq->reg_req = alloc_watch_request(lreq, CEPH_OSD_WATCH_OP_WATCH); if (!lreq->reg_req) { @@ -4767,7 +4767,7 @@ int ceph_osdc_unwatch(struct ceph_osd_client *osdc, ceph_oid_copy(&req->r_base_oid, &lreq->t.base_oid); ceph_oloc_copy(&req->r_base_oloc, &lreq->t.base_oloc); req->r_flags = CEPH_OSD_FLAG_WRITE; - ktime_get_real_ts64(&req->r_mtime); + ktime_get_ts64(&req->r_mtime); osd_req_op_watch_init(req, 0, lreq->linger_id, CEPH_OSD_WATCH_OP_UNWATCH);
The Documentation/core-api/timekeeping.rst recommend that we should use monotonic time ktime_get_ts64(), to avoid glitches with a concurrent settimeofday(). Signed-off-by: Yejune Deng <yejune.deng@gmail.com> --- net/ceph/messenger.c | 2 +- net/ceph/messenger_v1.c | 2 +- net/ceph/messenger_v2.c | 2 +- net/ceph/osd_client.c | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-)