* [PATCH] uuidd: add cont_clock persistence
@ 2023-12-15 22:18 Michael Trapp
2024-01-04 12:12 ` Karel Zak
0 siblings, 1 reply; 3+ messages in thread
From: Michael Trapp @ 2023-12-15 22:18 UTC (permalink / raw
To: util-linux; +Cc: kzak
cont_clock requires a correct time setup and therefore it
must be possible to detect a step back between uuidd starts.
Reserving the next 10 seconds in clock-cont.txt is sufficient
and should not have a noticeable performance impact.
It will also provide the possibility to start with the clock_reg
from the previous session when the system was rebooted.
Whith that, the early cont_clock initialization in uuidd
should be removed because writing the cont_clock persitence
when -C was not set is useless and might be confusing.
---
libuuid/src/gen_uuid.c | 78 ++++++++++++++++++++++++++++++++++++------
libuuid/src/uuidP.h | 1 +
misc-utils/uuidd.c | 9 -----
3 files changed, 69 insertions(+), 19 deletions(-)
diff --git a/libuuid/src/gen_uuid.c b/libuuid/src/gen_uuid.c
index 826cd2245..94b99f1bd 100644
--- a/libuuid/src/gen_uuid.c
+++ b/libuuid/src/gen_uuid.c
@@ -355,44 +355,102 @@ static uint64_t get_clock_counter(void)
/*
* Get continuous clock value.
*
- * Return -1 if there is no further clock counter available,
+ * Return -1 if there is no valid clock counter available,
* otherwise return 0.
*
* This implementation doesn't deliver clock counters based on
* the current time because last_clock_reg is only incremented
* by the number of requested UUIDs.
* max_clock_offset is used to limit the offset of last_clock_reg.
+ * used/reserved UUIDs are written to LIBUUID_CLOCK_CONT_FILE.
*/
static int get_clock_cont(uint32_t *clock_high,
uint32_t *clock_low,
int num,
uint32_t max_clock_offset)
{
- /* 100ns based time offset according to RFC 4122. 4.1.4. */
+ /* all 64bit clock_reg values in this function represent '100ns ticks'
+ * due to the combination of tv_usec + MAX_ADJUSTMENT */
+
+ enum { fd_init = -2, fd_error = -1 };
+ /* time offset according to RFC 4122. 4.1.4. */
const uint64_t reg_offset = (((uint64_t) 0x01B21DD2) << 32) + 0x13814000;
static uint64_t last_clock_reg = 0;
- uint64_t clock_reg;
+ static uint64_t saved_clock_reg = 0;
+ static int state_fd = fd_init;
+ static FILE *state_f = NULL;
+ uint64_t clock_reg, next_clock_reg;
- if (last_clock_reg == 0)
- last_clock_reg = get_clock_counter();
+ if (state_fd == fd_error)
+ return -1;
clock_reg = get_clock_counter();
+
+ if (state_fd == fd_init) {
+ mode_t save_umask;
+ struct stat st;
+
+ save_umask = umask(0);
+ state_fd = open(LIBUUID_CLOCK_CONT_FILE, O_RDWR|O_CREAT|O_CLOEXEC, 0660);
+ (void) umask(save_umask);
+ if (state_fd == fd_error)
+ return -1;
+
+ state_f = fdopen(state_fd, "r+" UL_CLOEXECSTR);
+ if (!state_f)
+ goto error;
+
+ if (fstat(state_fd, &st))
+ goto error;
+
+ if (st.st_size) {
+ rewind(state_f);
+ if (fscanf(state_f, "cont: %lu\n", &last_clock_reg) != 1)
+ goto error;
+ } else
+ last_clock_reg = clock_reg;
+
+ saved_clock_reg = last_clock_reg;
+ }
+
if (max_clock_offset) {
- uint64_t clock_offset = max_clock_offset * 10000000ULL;
- if (last_clock_reg < (clock_reg - clock_offset))
- last_clock_reg = clock_reg - clock_offset;
+ uint64_t co = 10000000ULL * (uint64_t)max_clock_offset; // clock_offset in [100ns]
+
+ if ((last_clock_reg + co) < clock_reg)
+ last_clock_reg = clock_reg - co;
}
clock_reg += MAX_ADJUSTMENT;
- if ((last_clock_reg + num) >= clock_reg)
+ next_clock_reg = last_clock_reg + (uint64_t)num;
+ if (next_clock_reg >= clock_reg)
return -1;
+ if (next_clock_reg >= saved_clock_reg) {
+ uint64_t cl = next_clock_reg + 100000000ULL; // 10s interval in [100ns]
+ int l;
+
+ rewind(state_f);
+ l = fprintf(state_f, "cont: %020lu \n", cl);
+ if (l < 30 || fflush(state_f))
+ goto error;
+ saved_clock_reg = cl;
+ }
+
*clock_high = (last_clock_reg + reg_offset) >> 32;
*clock_low = last_clock_reg + reg_offset;
- last_clock_reg += num;
+ last_clock_reg = next_clock_reg;
return 0;
+
+error:
+ if (state_fd >= 0)
+ close(state_fd);
+ if (state_f)
+ fclose(state_f);
+ state_fd = fd_error;
+ state_f = NULL;
+ return -1;
}
#if defined(HAVE_UUIDD) && defined(HAVE_SYS_UN_H)
diff --git a/libuuid/src/uuidP.h b/libuuid/src/uuidP.h
index 200702c1e..fef7e6cb5 100644
--- a/libuuid/src/uuidP.h
+++ b/libuuid/src/uuidP.h
@@ -40,6 +40,7 @@
#include "uuid.h"
#define LIBUUID_CLOCK_FILE "/var/lib/libuuid/clock.txt"
+#define LIBUUID_CLOCK_CONT_FILE "/var/lib/libuuid/clock-cont.txt"
/*
* Offset between 15-Oct-1582 and 1-Jan-70
diff --git a/misc-utils/uuidd.c b/misc-utils/uuidd.c
index fd121c5bc..42a252dd0 100644
--- a/misc-utils/uuidd.c
+++ b/misc-utils/uuidd.c
@@ -442,15 +442,6 @@ static void server_loop(const char *socket_path, const char *pidfile_path,
pfd[POLLFD_SOCKET].fd = s;
pfd[POLLFD_SIGNAL].events = pfd[POLLFD_SOCKET].events = POLLIN | POLLERR | POLLHUP;
- num = 1;
- if (uuidd_cxt->cont_clock_offset) {
- /* trigger initialization */
- (void) __uuid_generate_time_cont(uu, &num, uuidd_cxt->cont_clock_offset);
- if (uuidd_cxt->debug)
- fprintf(stderr, _("max_clock_offset = %u sec\n"),
- uuidd_cxt->cont_clock_offset);
- }
-
while (1) {
ret = poll(pfd, ARRAY_SIZE(pfd),
uuidd_cxt->timeout ?
--
2.39.3 (Apple Git-145)
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] uuidd: add cont_clock persistence
2023-12-15 22:18 [PATCH] uuidd: add cont_clock persistence Michael Trapp
@ 2024-01-04 12:12 ` Karel Zak
2024-01-10 15:42 ` Trapp, Michael
0 siblings, 1 reply; 3+ messages in thread
From: Karel Zak @ 2024-01-04 12:12 UTC (permalink / raw
To: Michael Trapp; +Cc: util-linux
On Fri, Dec 15, 2023 at 11:18:29PM +0100, Michael Trapp wrote:
> cont_clock requires a correct time setup and therefore it
> must be possible to detect a step back between uuidd starts.
>
> Reserving the next 10 seconds in clock-cont.txt is sufficient
> and should not have a noticeable performance impact.
> It will also provide the possibility to start with the clock_reg
> from the previous session when the system was rebooted.
>
> Whith that, the early cont_clock initialization in uuidd
> should be removed because writing the cont_clock persitence
> when -C was not set is useless and might be confusing.
Hi Michael,
that is an interesting idea; I have only a few pedantic notes ;-)
> ---
> libuuid/src/gen_uuid.c | 78 ++++++++++++++++++++++++++++++++++++------
> libuuid/src/uuidP.h | 1 +
> misc-utils/uuidd.c | 9 -----
> 3 files changed, 69 insertions(+), 19 deletions(-)
>
> diff --git a/libuuid/src/gen_uuid.c b/libuuid/src/gen_uuid.c
> index 826cd2245..94b99f1bd 100644
> --- a/libuuid/src/gen_uuid.c
> +++ b/libuuid/src/gen_uuid.c
> @@ -355,44 +355,102 @@ static uint64_t get_clock_counter(void)
> /*
> * Get continuous clock value.
> *
> - * Return -1 if there is no further clock counter available,
> + * Return -1 if there is no valid clock counter available,
> * otherwise return 0.
> *
> * This implementation doesn't deliver clock counters based on
> * the current time because last_clock_reg is only incremented
> * by the number of requested UUIDs.
> * max_clock_offset is used to limit the offset of last_clock_reg.
> + * used/reserved UUIDs are written to LIBUUID_CLOCK_CONT_FILE.
> */
> static int get_clock_cont(uint32_t *clock_high,
> uint32_t *clock_low,
> int num,
> uint32_t max_clock_offset)
> {
> - /* 100ns based time offset according to RFC 4122. 4.1.4. */
> + /* all 64bit clock_reg values in this function represent '100ns ticks'
> + * due to the combination of tv_usec + MAX_ADJUSTMENT */
> +
> + enum { fd_init = -2, fd_error = -1 };
In the code (below) the enum items seems like variables, a little bit
confusing. It would be better use upper-case, STATE_FD_INIT, STATE_FD_ERROR.
> + /* time offset according to RFC 4122. 4.1.4. */
> const uint64_t reg_offset = (((uint64_t) 0x01B21DD2) << 32) + 0x13814000;
> static uint64_t last_clock_reg = 0;
> - uint64_t clock_reg;
> + static uint64_t saved_clock_reg = 0;
> + static int state_fd = fd_init;
> + static FILE *state_f = NULL;
> + uint64_t clock_reg, next_clock_reg;
>
> - if (last_clock_reg == 0)
> - last_clock_reg = get_clock_counter();
> + if (state_fd == fd_error)
> + return -1;
>
> clock_reg = get_clock_counter();
> +
> + if (state_fd == fd_init) {
> + mode_t save_umask;
> + struct stat st;
> +
> + save_umask = umask(0);
> + state_fd = open(LIBUUID_CLOCK_CONT_FILE, O_RDWR|O_CREAT|O_CLOEXEC, 0660);
> + (void) umask(save_umask);
> + if (state_fd == fd_error)
> + return -1;
> +
> + state_f = fdopen(state_fd, "r+" UL_CLOEXECSTR);
> + if (!state_f)
> + goto error;
Seems it duplicates code from get_clock(), what about introduce a generic
state_fd_init(LIBUUID_CLOCK_CONT_FILE, &state_fd, &state_f);
and use the same in get_clock() for LIBUUID_CLOCK_FILE?
> + if (fstat(state_fd, &st))
> + goto error;
> +
> + if (st.st_size) {
> + rewind(state_f);
> + if (fscanf(state_f, "cont: %lu\n", &last_clock_reg) != 1)
> + goto error;
> + } else
> + last_clock_reg = clock_reg;
For LIBUUID_CLOCK_FILE we use flock(), I guess it's unnecessary for
LIBUUID_CLOCK_CONT_FILE as we assume only one uuidd instance, right?
Thanks!
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] uuidd: add cont_clock persistence
2024-01-04 12:12 ` Karel Zak
@ 2024-01-10 15:42 ` Trapp, Michael
0 siblings, 0 replies; 3+ messages in thread
From: Trapp, Michael @ 2024-01-10 15:42 UTC (permalink / raw
To: Karel Zak; +Cc: util-linux@vger.kernel.org
Hi Karel,
> On 4. Jan 2024, at 13:12, Karel Zak <kzak@redhat.com> wrote:
>
> On Fri, Dec 15, 2023 at 11:18:29PM +0100, Michael Trapp wrote:
>> cont_clock requires a correct time setup and therefore it
>> must be possible to detect a step back between uuidd starts.
>> ...
>> + enum { fd_init = -2, fd_error = -1 };
>
> In the code (below) the enum items seems like variables, a little bit
> confusing. It would be better use upper-case, STATE_FD_INIT, STATE_FD_ERROR.
>> + static int state_fd = fd_init;
>> ...
>> + if (state_fd == fd_error)
Makes sense and will be fixed.
>> ...
>> + save_umask = umask(0);
>> + state_fd = open(LIBUUID_CLOCK_CONT_FILE, O_RDWR|O_CREAT|O_CLOEXEC, 0660);
>> + (void) umask(save_umask);
>> + if (state_fd == fd_error)
>> + return -1;
>> +
>> + state_f = fdopen(state_fd, "r+" UL_CLOEXECSTR);
>> + if (!state_f)
>> + goto error;
>
> Seems it duplicates code from get_clock(), what about introduce a generic
>
> state_fd_init(LIBUUID_CLOCK_CONT_FILE, &state_fd, &state_f);
>
> and use the same in get_clock() for LIBUUID_CLOCK_FILE?
That’s the case for the initialization with open()/fdopen().
I’ll move this into the new function state_fd_init().
>> + if (fstat(state_fd, &st))
>> + goto error;
>> +
>> + if (st.st_size) {
>> + rewind(state_f);
>> + if (fscanf(state_f, "cont: %lu\n", &last_clock_reg) != 1)
>> + goto error;
>> + } else
>> + last_clock_reg = clock_reg;
>
> For LIBUUID_CLOCK_FILE we use flock(), I guess it's unnecessary for
> LIBUUID_CLOCK_CONT_FILE as we assume only one uuidd instance, right?
That’s correct, based on the check in uuidd.c, there can't be multiple instances.
Michael
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-01-10 15:42 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-15 22:18 [PATCH] uuidd: add cont_clock persistence Michael Trapp
2024-01-04 12:12 ` Karel Zak
2024-01-10 15:42 ` Trapp, Michael
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).