[LU-9397] Inconsistence use of cfs_time_current() and ktime_get_real_seconds()

• Type: Bug
• Resolution: Fixed
• Priority: Critical
• Fix Version/s: Lustre 2.10.0
• Severity: 3
• Assignee: James A Simmons
• Reporter: Amir Shehata (Inactive)

Description

"LU-6245 libcfs: add ktime_get_real_seconds support" introduced a problem where sometimes we use cfs_time_current() and others ktime_get_real_seconds(). This leads to wrong comparison, leading to strange timing issues.

One behavior observed were routers considering peers dead, due to these inconsistent comparison.

We'll need to make sure that the LNet and LND code follows a consistent and correct approach.

Issue Links

• is related to: LU-10707 TCP eth routed LNet traffic broken

Comments

[Peter Jones]

Landed for 2.10

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch https://review.whamcloud.com/26813/
Subject: LU-9397 ksocklnd: move remaining time handling to 64 bits
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 59c25356208706b4dd0920bc7754f21e2db14a0f

[James A Simmons]

Okay just wanted to verify that was the case for jiffies. The issue with jiffies they can also vary between nodes (LU-5443) depending on what the user selected for HZ so I like to migrate away as much as possible from them. Currently I'm working on the patch but I have to go over it very carefully. To easy to break things.

[Olaf Weber (Inactive)]

As far as I can tell, the main argument for jiffies is that they're cheaper to get and use. The extra resolution doesn't matter for these cases.

Using ktime_get_real_seconds() in particular should get you (assuming proper time sync in place) time stamp values that can (easily) be compared between nodes. That by itself might justify this.

[James A Simmons]

I have been looking at the socklnd code and it looks like everything is pretty much needs only second precision. It looks like everything could be changed into time64_t. Olaf do you see cases where we will need higher resolution? When we want to go to jiffies I can use cfs_time_seconds() as the case for the call to lnet_notify() and ksocknal_query(). Other than that I don't see the need for ktime_t.

[James A Simmons]

Good we are on the right track. I have to look over the ksocklnd driver and do a proper 64 bit time port.

[Amir Shehata (Inactive)]

aside from Olaf's comments, I tested the fix and I don't see the issue any longer

[Olaf Weber (Inactive)]

And in ksocknal_destroy_conn() the calculation in the CERROR() involving last_rcv is wrong. last_rcv is in jiffies, is subtracted from ktime_get_real_seconds(), with the result then being converted from jiffies to seconds.

[Olaf Weber (Inactive)]

It looks like a similar issue exists for ksock_conn::ksnc_tx_last_post.

[Gerrit Updater]

James Simmons ([e-mail]) uploaded a new patch: https://review.whamcloud.com/26813
Subject: LU-9397 ksocklnd: make last_alive in units of jiffies
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 7006890fcc484a8074a033517a6607e6ace15b8b