From ea51623345b1a1bed0db72e836d07b27c72daae8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 8 Jun 2025 13:21:28 +0200 Subject: [PATCH 1/5] sd-event: drop some bitfield specifiers from struct sd_event_source This does not change the size of the structure, because the size is determined by .child, which has a 128-byte siginfo_t field. But by dropping the specifiers we let the compiler generate code that operates on full bytes instead of having to play with bitmasks, see second diff below. Also move the bools in .memory_pressure into a gap to save a few bytes on initialization. $ diff -U100 <(pahole build/src/shared/libsystemd-shared-258.so.0 | awk '/struct sd_event_source/,/^}/') \ <(pahole build/src/shared/libsystemd-shared-258.so | awk '/struct sd_event_source/,/^}/') --- /proc/self/fd/11 2025-06-08 13:16:55.614738334 +0200 +++ /proc/self/fd/12 2025-06-08 13:16:55.615738386 +0200 @@ -1,109 +1,109 @@ struct sd_event_source { WakeupType wakeup; /* 0 4 */ unsigned int n_ref; /* 4 4 */ sd_event * event; /* 8 8 */ void * userdata; /* 16 8 */ sd_event_handler_t prepare; /* 24 8 */ char * description; /* 32 8 */ EventSourceType type; /* 40 4 */ signed int enabled:3; /* 44: 0 4 */ _Bool pending:1; /* 44: 3 1 */ _Bool dispatching:1; /* 44: 4 1 */ _Bool floating:1; /* 44: 5 1 */ _Bool exit_on_failure:1; /* 44: 6 1 */ _Bool ratelimited:1; /* 44: 7 1 */ /* XXX 24 bits hole, try to pack */ int64_t priority; /* 48 8 */ unsigned int pending_index; /* 56 4 */ unsigned int prepare_index; /* 60 4 */ /* --- cacheline 1 boundary (64 bytes) --- */ uint64_t pending_iteration; /* 64 8 */ uint64_t prepare_iteration; /* 72 8 */ sd_event_destroy_t destroy_callback; /* 80 8 */ sd_event_handler_t ratelimit_expire_callback; /* 88 8 */ sd_event_source * sources_next; /* 96 8 */ sd_event_source * sources_prev; /* 104 8 */ RateLimit rate_limit; /* 112 24 */ /* --- cacheline 2 boundary (128 bytes) was 8 bytes ago --- */ unsigned int earliest_index; /* 136 4 */ unsigned int latest_index; /* 140 4 */ union { struct { sd_event_io_handler_t callback; /* 144 8 */ int fd; /* 152 4 */ uint32_t events; /* 156 4 */ uint32_t revents; /* 160 4 */ - _Bool registered:1; /* 164: 0 1 */ - _Bool owned:1; /* 164: 1 1 */ + _Bool registered; /* 164 1 */ + _Bool owned; /* 165 1 */ } io; /* 144 24 */ struct { sd_event_time_handler_t callback; /* 144 8 */ usec_t next; /* 152 8 */ usec_t accuracy; /* 160 8 */ } time; /* 144 24 */ struct { sd_event_signal_handler_t callback; /* 144 8 */ struct signalfd_siginfo siginfo; /* 152 128 */ /* --- cacheline 4 boundary (256 bytes) was 24 bytes ago --- */ int sig; /* 280 4 */ _Bool unblock; /* 284 1 */ } signal; /* 144 144 */ struct { sd_event_child_handler_t callback; /* 144 8 */ siginfo_t siginfo; /* 152 128 */ /* --- cacheline 4 boundary (256 bytes) was 24 bytes ago --- */ pid_t pid; /* 280 4 */ int options; /* 284 4 */ int pidfd; /* 288 4 */ _Bool registered:1; /* 292: 0 1 */ _Bool pidfd_owned:1; /* 292: 1 1 */ _Bool process_owned:1; /* 292: 2 1 */ _Bool exited:1; /* 292: 3 1 */ _Bool waited:1; /* 292: 4 1 */ } child; /* 144 152 */ struct { sd_event_handler_t callback; /* 144 8 */ } defer; /* 144 8 */ struct { sd_event_handler_t callback; /* 144 8 */ } post; /* 144 8 */ struct { sd_event_handler_t callback; /* 144 8 */ unsigned int prioq_index; /* 152 4 */ } exit; /* 144 16 */ struct { sd_event_inotify_handler_t callback; /* 144 8 */ uint32_t mask; /* 152 4 */ /* XXX 4 bytes hole, try to pack */ struct inode_data * inode_data; /* 160 8 */ sd_event_source * by_inode_data_next; /* 168 8 */ sd_event_source * by_inode_data_prev; /* 176 8 */ } inotify; /* 144 40 */ struct { int fd; /* 144 4 */ + _Bool registered; /* 148 1 */ + _Bool locked; /* 149 1 */ + _Bool in_write_list; /* 150 1 */ - /* XXX 4 bytes hole, try to pack */ + /* XXX 1 byte hole, try to pack */ sd_event_handler_t callback; /* 152 8 */ void * write_buffer; /* 160 8 */ size_t write_buffer_size; /* 168 8 */ uint32_t events; /* 176 4 */ uint32_t revents; /* 180 4 */ sd_event_source * write_list_next; /* 184 8 */ /* --- cacheline 3 boundary (192 bytes) --- */ sd_event_source * write_list_prev; /* 192 8 */ - _Bool registered:1; /* 200: 0 1 */ - _Bool locked:1; /* 200: 1 1 */ - _Bool in_write_list:1; /* 200: 2 1 */ - } memory_pressure; /* 144 64 */ + } memory_pressure; /* 144 56 */ }; /* 144 152 */ /* size: 296, cachelines: 5, members: 26 */ /* sum members: 292 */ /* sum bitfield members: 8 bits, bit holes: 1, sum bit holes: 24 bits */ /* last cacheline: 40 bytes */ }; Example diff in assembly: $ diff -u <(objdump -S build/src/shared/libsystemd-shared-258.so.0|awk '/^static void event_source_time_prioq_reshuffle/,/^\}/') \ <(objdump -S build/src/shared/libsystemd-shared-258.so|awk '/^static void event_source_time_prioq_reshuffle/,/^\}/') d->needs_rearm = true; - 34d80e: 48 8b 45 f8 mov -0x8(%rbp),%rax - 34d812: 0f b6 50 20 movzbl 0x20(%rax),%edx - 34d816: 83 ca 01 or $0x1,%edx - 34d819: 88 50 20 mov %dl,0x20(%rax) - 34d81c: eb 01 jmp 34d81f + 34d7c3: 48 8b 45 f8 mov -0x8(%rbp),%rax + 34d7c7: c6 40 20 01 movb $0x1,0x20(%rax) + 34d7cb: eb 01 jmp 34d7ce return; /* no-op for an event source which is neither a timer nor ratelimited. */ - 34d81e: 90 nop + 34d7cd: 90 nop --- src/libsystemd/sd-event/event-source.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/libsystemd/sd-event/event-source.h b/src/libsystemd/sd-event/event-source.h index 5fef3b1ac9..385779df2c 100644 --- a/src/libsystemd/sd-event/event-source.h +++ b/src/libsystemd/sd-event/event-source.h @@ -87,8 +87,8 @@ struct sd_event_source { int fd; uint32_t events; uint32_t revents; - bool registered:1; - bool owned:1; + bool registered; + bool owned; } io; struct { sd_event_time_handler_t callback; @@ -130,14 +130,14 @@ struct sd_event_source { } inotify; struct { int fd; + bool registered; + bool locked; + bool in_write_list; sd_event_handler_t callback; void *write_buffer; size_t write_buffer_size; uint32_t events, revents; LIST_FIELDS(sd_event_source, write_list); - bool registered:1; - bool locked:1; - bool in_write_list:1; } memory_pressure; }; }; @@ -157,7 +157,7 @@ struct clock_data { Prioq *latest; usec_t next; - bool needs_rearm:1; + bool needs_rearm; }; struct signal_data { From c9ea39689f5e0f24ba0882d268bb9808ab27541a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 8 Jun 2025 13:41:31 +0200 Subject: [PATCH 2/5] sd-event: make some more bools non-bitfield In sd_event_source.child, we have 5 bools. If we make them each take one byte, the structure size increases. So let's do that for the three of them, and leave the other two (less frequently used) squished into the last byte. This allows more efficient code to be generated, without changing the size of the struct: $ diff -u <(objdump -S build/src/shared/libsystemd-shared-258.so.0|awk '/^static void source_io_unregister/,/^\}/') \ <(objdump -S build/src/shared/libsystemd-shared-258.so|awk '/^static void source_io_unregister/,/^\}/') s->io.registered = false; - 34d46f: 48 8b 45 d8 mov -0x28(%rbp),%rax - 34d473: 0f b6 90 a4 00 00 00 movzbl 0xa4(%rax),%edx - 34d47a: 83 e2 fe and $0xfffffffe,%edx - 34d47d: 88 90 a4 00 00 00 mov %dl,0xa4(%rax) - 34d483: eb 04 jmp 34d489 + 34bffe: 48 8b 45 d8 mov -0x28(%rbp),%rax + 34c002: c6 80 a4 00 00 00 00 movb $0x0,0xa4(%rax) + 34c009: eb 04 jmp 34c00f return; --- src/libsystemd/sd-event/event-source.h | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/libsystemd/sd-event/event-source.h b/src/libsystemd/sd-event/event-source.h index 385779df2c..a7c1807cd4 100644 --- a/src/libsystemd/sd-event/event-source.h +++ b/src/libsystemd/sd-event/event-source.h @@ -106,11 +106,16 @@ struct sd_event_source { pid_t pid; int options; int pidfd; - bool registered:1; /* whether the pidfd is registered in the epoll */ + /* We have five bools, and we want to fit them into 4 bytes so the whole struct + * remains 32 bytes. Thus, use bitfields for two of them and single bytes for the + * other three. */ + bool registered; /* whether the pidfd is registered in the epoll */ bool pidfd_owned:1; /* close pidfd when event source is freed */ bool process_owned:1; /* kill+reap process when event source is freed */ - bool exited:1; /* true if process exited (i.e. if there's value in SIGKILLing it if we want to get rid of it) */ - bool waited:1; /* true if process was waited for (i.e. if there's value in waitid(P_PID)'ing it if we want to get rid of it) */ + bool exited; /* true if process exited (i.e. if there's value in SIGKILLing it if + * we want to get rid of it) */ + bool waited; /* true if process was waited for (i.e. if there's value in + * waitid(P_PID)'ing it if we want to get rid of it) */ } child; struct { sd_event_handler_t callback; From 7eb10d69df0f8e1eabebabb4bd74f2fc92400132 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 8 Jun 2025 13:52:59 +0200 Subject: [PATCH 3/5] sd-event: typedef struct inode_data to InodeData This is an internal definition, so use the usual CamelCase typedef. --- src/libsystemd/sd-event/event-source.h | 12 +++---- src/libsystemd/sd-event/sd-event.c | 48 +++++++++++--------------- 2 files changed, 27 insertions(+), 33 deletions(-) diff --git a/src/libsystemd/sd-event/event-source.h b/src/libsystemd/sd-event/event-source.h index a7c1807cd4..6ffb440ce5 100644 --- a/src/libsystemd/sd-event/event-source.h +++ b/src/libsystemd/sd-event/event-source.h @@ -42,7 +42,7 @@ typedef enum WakeupType { _WAKEUP_TYPE_INVALID = -EINVAL, } WakeupType; -struct inode_data; +typedef struct inode_data InodeData; struct sd_event_source { WakeupType wakeup; @@ -130,7 +130,7 @@ struct sd_event_source { struct { sd_event_inotify_handler_t callback; uint32_t mask; - struct inode_data *inode_data; + InodeData *inode_data; LIST_FIELDS(sd_event_source, by_inode_data); } inotify; struct { @@ -187,7 +187,7 @@ struct inode_data { /* An fd of the inode to watch. The fd is kept open until the next iteration of the loop, so that we can * rearrange the priority still until then, as we need the original inode to change the priority as we need to * add a watch descriptor to the right inotify for the priority which we can only do if we have a handle to the - * original inode. We keep a list of all inode_data objects with an open fd in the to_close list (see below) of + * original inode. We keep a list of all InodeData objects with an open fd in the to_close list (see below) of * the sd-event object, so that it is efficient to close everything, before entering the next event loop * iteration. */ int fd; @@ -209,7 +209,7 @@ struct inode_data { struct inotify_data *inotify_data; /* A linked list of all inode data objects with fds to close (see above) */ - LIST_FIELDS(struct inode_data, to_close); + LIST_FIELDS(InodeData, to_close); }; /* A structure encapsulating an inotify fd */ @@ -222,8 +222,8 @@ struct inotify_data { int fd; int64_t priority; - Hashmap *inodes; /* The inode_data structures keyed by dev+ino */ - Hashmap *wd; /* The inode_data structures keyed by the watch descriptor for each */ + Hashmap *inodes; /* The InodeData structures keyed by dev+ino */ + Hashmap *wd; /* The InodeData structures keyed by the watch descriptor for each */ /* How many event sources are currently marked pending for this inotify. We won't read new events off the * inotify fd as long as there are still pending events on the inotify (because we have no strategy of queuing diff --git a/src/libsystemd/sd-event/sd-event.c b/src/libsystemd/sd-event/sd-event.c index 6d8a207b5d..d1733e2419 100644 --- a/src/libsystemd/sd-event/sd-event.c +++ b/src/libsystemd/sd-event/sd-event.c @@ -139,7 +139,7 @@ struct sd_event { Hashmap *inotify_data; /* indexed by priority */ /* A list of inode structures that still have an fd open, that we need to close before the next loop iteration */ - LIST_HEAD(struct inode_data, inode_data_to_close_list); + LIST_HEAD(InodeData, inode_data_to_close_list); /* A list of inotify objects that already have events buffered which aren't processed yet */ LIST_HEAD(struct inotify_data, buffered_inotify_data_list); @@ -182,7 +182,7 @@ DEFINE_PRIVATE_ORIGIN_ID_HELPERS(sd_event, event); static thread_local sd_event *default_event = NULL; static void source_disconnect(sd_event_source *s); -static void event_gc_inode_data(sd_event *e, struct inode_data *d); +static void event_gc_inode_data(sd_event *e, InodeData *d); static sd_event* event_resolve(sd_event *e) { return e == SD_EVENT_DEFAULT ? default_event : e; @@ -1011,7 +1011,7 @@ static void source_disconnect(sd_event_source *s) { break; case SOURCE_INOTIFY: { - struct inode_data *inode_data; + InodeData *inode_data; inode_data = s->inotify.inode_data; if (inode_data) { @@ -2201,7 +2201,7 @@ static int event_make_inotify_data( return 1; } -static int inode_data_compare(const struct inode_data *x, const struct inode_data *y) { +static int inode_data_compare(const InodeData *x, const InodeData *y) { int r; assert(x); @@ -2214,19 +2214,16 @@ static int inode_data_compare(const struct inode_data *x, const struct inode_dat return CMP(x->ino, y->ino); } -static void inode_data_hash_func(const struct inode_data *d, struct siphash *state) { +static void inode_data_hash_func(const InodeData *d, struct siphash *state) { assert(d); siphash24_compress_typesafe(d->dev, state); siphash24_compress_typesafe(d->ino, state); } -DEFINE_PRIVATE_HASH_OPS(inode_data_hash_ops, struct inode_data, inode_data_hash_func, inode_data_compare); - -static void event_free_inode_data( - sd_event *e, - struct inode_data *d) { +DEFINE_PRIVATE_HASH_OPS(inode_data_hash_ops, InodeData, inode_data_hash_func, inode_data_compare); +static void event_free_inode_data(sd_event *e, InodeData *d) { assert(e); if (!d) @@ -2285,10 +2282,7 @@ static void event_gc_inotify_data( event_free_inotify_data(e, d); } -static void event_gc_inode_data( - sd_event *e, - struct inode_data *d) { - +static void event_gc_inode_data(sd_event *e, InodeData *d) { struct inotify_data *inotify_data; assert(e); @@ -2310,15 +2304,15 @@ static int event_make_inode_data( struct inotify_data *inotify_data, dev_t dev, ino_t ino, - struct inode_data **ret) { + InodeData **ret) { - struct inode_data *d, key; + InodeData *d, key; int r; assert(e); assert(inotify_data); - key = (struct inode_data) { + key = (InodeData) { .ino = ino, .dev = dev, }; @@ -2335,11 +2329,11 @@ static int event_make_inode_data( if (r < 0) return r; - d = new(struct inode_data, 1); + d = new(InodeData, 1); if (!d) return -ENOMEM; - *d = (struct inode_data) { + *d = (InodeData) { .dev = dev, .ino = ino, .wd = -1, @@ -2359,7 +2353,7 @@ static int event_make_inode_data( return 1; } -static uint32_t inode_data_determine_mask(struct inode_data *d) { +static uint32_t inode_data_determine_mask(InodeData *d) { bool excl_unlink = true; uint32_t combined = 0; @@ -2384,7 +2378,7 @@ static uint32_t inode_data_determine_mask(struct inode_data *d) { return (combined & ~(IN_ONESHOT|IN_DONT_FOLLOW|IN_ONLYDIR|IN_EXCL_UNLINK)) | (excl_unlink ? IN_EXCL_UNLINK : 0); } -static int inode_data_realize_watch(sd_event *e, struct inode_data *d) { +static int inode_data_realize_watch(sd_event *e, InodeData *d) { uint32_t combined_mask; int wd, r; @@ -2442,7 +2436,7 @@ static int event_add_inotify_fd_internal( _cleanup_close_ int donated_fd = donate ? fd : -EBADF; _cleanup_(source_freep) sd_event_source *s = NULL; struct inotify_data *inotify_data = NULL; - struct inode_data *inode_data = NULL; + InodeData *inode_data = NULL; struct stat st; int r; @@ -2755,7 +2749,7 @@ _public_ int sd_event_source_get_priority(sd_event_source *s, int64_t *ret) { _public_ int sd_event_source_set_priority(sd_event_source *s, int64_t priority) { bool rm_inotify = false, rm_inode = false; struct inotify_data *new_inotify_data = NULL; - struct inode_data *new_inode_data = NULL; + InodeData *new_inode_data = NULL; int r; assert_return(s, -EINVAL); @@ -2766,7 +2760,7 @@ _public_ int sd_event_source_set_priority(sd_event_source *s, int64_t priority) return 0; if (s->type == SOURCE_INOTIFY) { - struct inode_data *old_inode_data; + InodeData *old_inode_data; assert(s->inotify.inode_data); old_inode_data = s->inotify.inode_data; @@ -3922,7 +3916,7 @@ static int event_inotify_data_process(sd_event *e, struct inotify_data *d) { return -EIO; if (d->buffer.ev.mask & IN_Q_OVERFLOW) { - struct inode_data *inode_data; + InodeData *inode_data; /* The queue overran, let's pass this event to all event sources connected to this inotify * object */ @@ -3938,7 +3932,7 @@ static int event_inotify_data_process(sd_event *e, struct inotify_data *d) { return r; } } else { - struct inode_data *inode_data; + InodeData *inode_data; /* Find the inode object for this watch descriptor. If IN_IGNORED is set we also remove it from * our watch descriptor table. */ @@ -4397,7 +4391,7 @@ static int process_watchdog(sd_event *e) { } static void event_close_inode_data_fds(sd_event *e) { - struct inode_data *d; + InodeData *d; assert(e); From 391b507257686330db75c267d7786d29b78b169e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 8 Jun 2025 14:01:16 +0200 Subject: [PATCH 4/5] sd-event: typedef struct inotify_data to InotifyData --- src/libsystemd/sd-event/event-source.h | 9 ++--- src/libsystemd/sd-event/sd-event.c | 48 +++++++++++--------------- 2 files changed, 26 insertions(+), 31 deletions(-) diff --git a/src/libsystemd/sd-event/event-source.h b/src/libsystemd/sd-event/event-source.h index 6ffb440ce5..d25a0958ed 100644 --- a/src/libsystemd/sd-event/event-source.h +++ b/src/libsystemd/sd-event/event-source.h @@ -43,6 +43,7 @@ typedef enum WakeupType { } WakeupType; typedef struct inode_data InodeData; +typedef struct inotify_data InotifyData; struct sd_event_source { WakeupType wakeup; @@ -206,7 +207,7 @@ struct inode_data { LIST_HEAD(sd_event_source, event_sources); /* The inotify object we watch this inode with */ - struct inotify_data *inotify_data; + InotifyData *inotify_data; /* A linked list of all inode data objects with fds to close (see above) */ LIST_FIELDS(InodeData, to_close); @@ -235,9 +236,9 @@ struct inotify_data { * is gone. */ unsigned n_busy; - /* A linked list of all inotify objects with data already read, that still need processing. We keep this list - * to make it efficient to figure out what inotify objects to process data on next. */ - LIST_FIELDS(struct inotify_data, buffered); + /* A linked list of all InotifyData objects with data already read, that still need processing. We + * keep this list to make it efficient to figure out what inotify objects to process data on next. */ + LIST_FIELDS(InotifyData, buffered); /* The buffer we read inotify events into */ size_t buffer_filled; /* fill level of the buffer */ diff --git a/src/libsystemd/sd-event/sd-event.c b/src/libsystemd/sd-event/sd-event.c index d1733e2419..b477e47c56 100644 --- a/src/libsystemd/sd-event/sd-event.c +++ b/src/libsystemd/sd-event/sd-event.c @@ -142,7 +142,7 @@ struct sd_event { LIST_HEAD(InodeData, inode_data_to_close_list); /* A list of inotify objects that already have events buffered which aren't processed yet */ - LIST_HEAD(struct inotify_data, buffered_inotify_data_list); + LIST_HEAD(InotifyData, buffered_inotify_data_list); /* A list of memory pressure event sources that still need their subscription string written */ LIST_HEAD(sd_event_source, memory_pressure_write_list); @@ -1015,7 +1015,7 @@ static void source_disconnect(sd_event_source *s) { inode_data = s->inotify.inode_data; if (inode_data) { - struct inotify_data *inotify_data; + InotifyData *inotify_data; assert_se(inotify_data = inode_data->inotify_data); /* Detach this event source from the inode object */ @@ -2113,7 +2113,7 @@ _public_ int sd_event_add_memory_pressure( return 0; } -static void event_free_inotify_data(sd_event *e, struct inotify_data *d) { +static void event_free_inotify_data(sd_event *e, InotifyData *d) { assert(e); if (!d) @@ -2140,13 +2140,9 @@ static void event_free_inotify_data(sd_event *e, struct inotify_data *d) { free(d); } -static int event_make_inotify_data( - sd_event *e, - int64_t priority, - struct inotify_data **ret) { - +static int event_make_inotify_data(sd_event *e, int64_t priority, InotifyData **ret) { _cleanup_close_ int fd = -EBADF; - struct inotify_data *d; + InotifyData *d; int r; assert(e); @@ -2164,11 +2160,11 @@ static int event_make_inotify_data( fd = fd_move_above_stdio(fd); - d = new(struct inotify_data, 1); + d = new(InotifyData, 1); if (!d) return -ENOMEM; - *d = (struct inotify_data) { + *d = (InotifyData) { .wakeup = WAKEUP_INOTIFY_DATA, .fd = TAKE_FD(fd), .priority = priority, @@ -2259,16 +2255,14 @@ static void event_free_inode_data(sd_event *e, InodeData *d) { free(d); } -static void event_gc_inotify_data( - sd_event *e, - struct inotify_data *d) { - +static void event_gc_inotify_data(sd_event *e, InotifyData *d) { assert(e); - /* GCs the inotify data object if we don't need it anymore. That's the case if we don't want to watch - * any inode with it anymore, which in turn happens if no event source of this priority is interested - * in any inode any longer. That said, we maintain an extra busy counter: if non-zero we'll delay GC - * (under the expectation that the GC is called again once the counter is decremented). */ + /* Collects the InotifyData object if we don't need it anymore. That's the case if we don't want to + * watch any inode with it anymore, which in turn happens if no event source of this priority is + * interested in any inode any longer. That said, we maintain an extra busy counter: if non-zero + * we'll delay GC (under the expectation that the GC is called again once the counter is + * decremented). */ if (!d) return; @@ -2283,7 +2277,7 @@ static void event_gc_inotify_data( } static void event_gc_inode_data(sd_event *e, InodeData *d) { - struct inotify_data *inotify_data; + InotifyData *inotify_data; assert(e); @@ -2301,7 +2295,7 @@ static void event_gc_inode_data(sd_event *e, InodeData *d) { static int event_make_inode_data( sd_event *e, - struct inotify_data *inotify_data, + InotifyData *inotify_data, dev_t dev, ino_t ino, InodeData **ret) { @@ -2435,7 +2429,7 @@ static int event_add_inotify_fd_internal( _cleanup_close_ int donated_fd = donate ? fd : -EBADF; _cleanup_(source_freep) sd_event_source *s = NULL; - struct inotify_data *inotify_data = NULL; + InotifyData *inotify_data = NULL; InodeData *inode_data = NULL; struct stat st; int r; @@ -2748,7 +2742,7 @@ _public_ int sd_event_source_get_priority(sd_event_source *s, int64_t *ret) { _public_ int sd_event_source_set_priority(sd_event_source *s, int64_t priority) { bool rm_inotify = false, rm_inode = false; - struct inotify_data *new_inotify_data = NULL; + InotifyData *new_inotify_data = NULL; InodeData *new_inode_data = NULL; int r; @@ -3844,7 +3838,7 @@ static int process_signal(sd_event *e, struct signal_data *d, uint32_t events, i } } -static int event_inotify_data_read(sd_event *e, struct inotify_data *d, uint32_t revents, int64_t threshold) { +static int event_inotify_data_read(sd_event *e, InotifyData *d, uint32_t revents, int64_t threshold) { ssize_t n; assert(e); @@ -3878,7 +3872,7 @@ static int event_inotify_data_read(sd_event *e, struct inotify_data *d, uint32_t return 1; } -static void event_inotify_data_drop(sd_event *e, struct inotify_data *d, size_t sz) { +static void event_inotify_data_drop(sd_event *e, InotifyData *d, size_t sz) { assert(e); assert(d); assert(sz <= d->buffer_filled); @@ -3894,7 +3888,7 @@ static void event_inotify_data_drop(sd_event *e, struct inotify_data *d, size_t LIST_REMOVE(buffered, e->buffered_inotify_data_list, d); } -static int event_inotify_data_process(sd_event *e, struct inotify_data *d) { +static int event_inotify_data_process(sd_event *e, InotifyData *d) { int r; assert(e); @@ -4216,7 +4210,7 @@ static int source_dispatch(sd_event_source *s) { case SOURCE_INOTIFY: { struct sd_event *e = s->event; - struct inotify_data *d; + InotifyData *d; size_t sz; assert(s->inotify.inode_data); From 9f145c2a9ad730eb6d607408b9688cbf20abdde7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 9 Jun 2025 16:11:17 +0200 Subject: [PATCH 5/5] sd-event: extend comment about a flex member MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up for dbef4dd4f23517abfc73b35f0bdf004d2f8f4805. Everything that that commit says is true, but — at least for me — it wasn't obvious why the code is correct and we can do fixed-size allocations like new(struct inotify_data, 1). --- src/libsystemd/sd-event/event-source.h | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/libsystemd/sd-event/event-source.h b/src/libsystemd/sd-event/event-source.h index d25a0958ed..7f6d5b8b50 100644 --- a/src/libsystemd/sd-event/event-source.h +++ b/src/libsystemd/sd-event/event-source.h @@ -242,6 +242,12 @@ struct inotify_data { /* The buffer we read inotify events into */ size_t buffer_filled; /* fill level of the buffer */ - union inotify_event_buffer buffer; /* struct inotify_event in union inotify_event_buffer has flex - * array. Hence, this must be at the end. */ + union inotify_event_buffer buffer; /* We use a union to allow type-punning. One of the variants in + * the union — struct inotify_event — has a flex array, so C99 + * only allows this union to be used at the end of the structure. + * The other variant in the union defines a fixed-size buffer that + * covers the maximum size that can be used for the flex variant, + * so in fact this field has a fixed size and could be safely + * placed in the middle of the struct. Unfortunately, there is no + * mechanism to let the compiler know that. */ };