Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize timestamps with per-CPU caching #2349

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 74 additions & 4 deletions lib/common.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,83 @@
#ifndef __LIB_COMMON_H__
#define __LIB_COMMON_H__

/* Get current timestamp in secs. */
static inline long
tfw_current_timestamp(void)
#include <linux/percpu.h>
#include <linux/timer.h>
#include <linux/time.h>

/* Per-CPU cached timestamp */
static DEFINE_PER_CPU(long, tfw_ts_cache);

/* Update timer */
static struct timer_list tfw_ts_timer;

/* Timer interval: 1 second */
#define TFW_TS_UPDATE_INTERVAL (HZ)

/**
* Timer callback to update the cached timestamp across all CPUs.
*/
static void
tfw_ts_update_timer(struct timer_list *t)
{
struct timespec64 ts;
int cpu;

/* Get the current timestamp */
ktime_get_real_ts64(&ts);
return ts.tv_sec;

/* Update the timestamp on all CPUs */
for_each_online_cpu(cpu) {
per_cpu(tfw_ts_cache, cpu) = ts.tv_sec;
Copy link
Contributor

@const-t const-t Mar 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks not good, however we do it only once per second. But I have doubts about current solution anyway. Suggested in the task approach seems more elegant. We can conditionally update per-cpu cached time relying on jiffies inside softirq handler. @krizhanovsky What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@const-t What do you think about this solution ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MorganaFuture In this solution we do check/update timer on each call of tfw_current_timestamp, but we can improve it doing check/update only once per softirq handler, and then in tfw_current_timestamp only receive the value. Solution with timer also not bad, but I would prefer to minimize cache bouncing even once per second. However, maybe I'm trying to over optimize, therefore I would like to hear also your @MorganaFuture opinion as well as @krizhanovsky.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @const-t and think that the problem can be fixed easier on the kernel level. We do our best to minimize the kernel patch, but this case seems should be done on the kernel side.

I think it makes sense to do this on https://github.com/tempesta-tech/linux-6.12.12-tfw/ . @const-t please guide how it'd be more convenient to do: as a branch from your https://github.com/tempesta-tech/linux-6.12.12-tfw/tree/kt-apply-tempesta-patch or will you just add the @MorganaFuture patch to your patch preserving his authorship.

}

/* Reschedule the timer */
mod_timer(&tfw_ts_timer, jiffies + TFW_TS_UPDATE_INTERVAL);
}

/**
* Initialize the timestamp caching system.
* Should be called during module initialization.
*/
static inline int
tfw_ts_cache_init(void)
{
struct timespec64 ts;
int cpu;

/* Initialize the initial timestamp value */
ktime_get_real_ts64(&ts);

/* Set the initial value for all CPUs */
for_each_online_cpu(cpu) {
per_cpu(tfw_ts_cache, cpu) = ts.tv_sec;
}

/* Setup the timer */
timer_setup(&tfw_ts_timer, tfw_ts_update_timer, 0);
mod_timer(&tfw_ts_timer, jiffies + TFW_TS_UPDATE_INTERVAL);

return 0;
}

/**
* Clean up the timestamp caching system.
* Should be called during module cleanup.
*/
static inline void
tfw_ts_cache_exit(void)
{
del_timer_sync(&tfw_ts_timer);
}

/**
* Get current timestamp in seconds.
* Always returns the cached value, which is updated by the timer.
*/
static inline long
tfw_current_timestamp(void)
{
return this_cpu_read(tfw_ts_cache);
}

#endif /* __LIB_COMMON_H__ */
11 changes: 9 additions & 2 deletions lib/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
#include <linux/module.h>
#include <linux/string.h>

#include "common.h"

MODULE_AUTHOR("Tempesta Technologies, INC");
MODULE_VERSION("0.1.1");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a new feature, so let's advance the version to 0.2.0

MODULE_LICENSE("GPL");
Expand Down Expand Up @@ -64,13 +66,18 @@ EXPORT_SYMBOL(bzero_fast);

static int __init
tempesta_lib_init(void)
{
return 0;
{
int ret = tfw_ts_cache_init();
if (ret)
return 1;

return ret;
}

static void __exit
tempesta_lib_exit(void)
{
tfw_ts_cache_exit();
}

module_init(tempesta_lib_init);
Expand Down