QuarkContainer / Quark

A secure container runtime with CRI/OCI interface
Apache License 2.0
323 stars 47 forks source link

Bug in clock_gettime syscall #1333

Open mhomidi opened 3 months ago

mhomidi commented 3 months ago

Problem

Hi everyone, When we call clock_gettime syscall, we get the userTime and systemTime for CPU state and make their scale to nano-second in qkernel/src/qlib/kernel/threadmgr/task_sched.rs:154. Then in the qkernel/src/qlib/kernel/threadmgr/task_sched.rs:454 and/or qkernel/src/qlib/kernel/threadmgr/task_sched.rs:457, we again scale and multiply them to 1000 to get nano-second again while their are in nano-second scale by themselves. This make some issue in wait syscall and probably other places.

If you want to see the issue, try to run Quark in two machines with different clock frequency. (Actually it just be accurate when clock freq is 1GHz)

Solution

We just need to change function Now in line qkernel/src/qlib/kernel/threadmgr/task_sched.rs:451 and remove the Scale and *1000:

pub fn Now(&self) -> Time {
    let stats = self.t.Upgrade().unwrap().CPUStats();
    if self.includeSys {
        return Time::FromNs(stats.UserTime + stats.SysTime);
    }

    return Time::FromNs(stats.UserTime);
}
shrik3 commented 2 months ago

Hi mhomidi:

since you already have a solution, perhaps it's better to just go ahead and open an PR, mark as draft or RFC if you are not sure.

It's an overhead to create a patch from your snippet; Also you should be properly credited for the contribution.