Make-initrd development discussion
 help / color / mirror / Atom feed
From: Leonid Krivoshein <klark.devel@gmail.com>
To: make-initrd@lists.altlinux.org
Subject: Re: [make-initrd] [PATCH] ueventd: Don't use a epoll timeout when it's not needed
Date: Mon, 22 May 2023 07:46:07 +0300
Message-ID: <6258d160-9ffc-30de-abda-d6aec405559a@gmail.com> (raw)
In-Reply-To: <20230521085326.308119-1-gladkov.alexey@gmail.com>

Привет!


Глянул финальную версию в for-master. Было неплохо, а стало намного 
лучше. :-)

В ueventd.c(108): утечка path. А в строках 124, 196, 324, 330 и 336 нет 
предупреждений? В строке 178 получается странный путь для отслеживания 
("%s/queue/pause/."), поскольку параметр name="." -- так задумано или 
тут д.б. ".all"?

В memory.c несколько удивляет неудобство интерфейса rd_asprintf_or_die() 
-- зачем тащить указатель на переменную в первом параметре и потом её 
возвращать? Мне кажется, тут можно обойтись локальной переменной и не 
тащить указатель через стек, к тому же 'char *'.

В queue-processor.c вызовы open() в 51, 55 и 59 так и остались не 
обёрнуты, на хорошей нагрузке это может приводить к произвольным 
фатальным вылетам с Interrupted system call. К слову, абсолютно все 
close() в коде теоретически могут быть этому подвержены.


On 5/21/23 11:53, Alexey Gladkov wrote:
> The timeout is used to process queues that have unprocessed events
> (dirty ones) even when no new events have arrived in the queue. When
> there are no dirty queues, then there is no point in extra iterations of
> the main loop.
>
> Signed-off-by: Alexey Gladkov <gladkov.alexey@gmail.com>
> ---
>   datasrc/ueventd/ueventd.c | 23 ++++++++++++++++++-----
>   1 file changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/datasrc/ueventd/ueventd.c b/datasrc/ueventd/ueventd.c
> index 857d3b4f..afe050d0 100644
> --- a/datasrc/ueventd/ueventd.c
> +++ b/datasrc/ueventd/ueventd.c
> @@ -56,7 +56,8 @@ enum {
>   	PIPE_MAX,
>   };
>   
> -int pipefd[PIPE_MAX];
> +static int pipefd[PIPE_MAX];
> +static int dirty_queues = 0;
>   
>   #define EV_PAUSE_MASK (IN_CREATE | IN_DELETE | IN_ONLYDIR)
>   #define EV_ROOT_MASK  (IN_CREATE | IN_ONLYDIR)
> @@ -121,8 +122,10 @@ int watch_path(int inotifyfd, const char *dir, const char *name, uint32_t mask,
>   	new->next      = watch_list;
>   
>   	if (flags & F_QUEUE_DIR) {
> -		if (!empty_directory(path))
> +		if (!empty_directory(path)) {
>   			new->q_flags |= F_DIRTY;
> +			dirty_queues++;
> +		}
>   		new->q_parentfd = pipefd[PIPE_WRITE];
>   	}
>   
> @@ -339,8 +342,10 @@ int process_inotify_events(int inotifyfd)
>   				continue;
>   			}
>   
> -			if (!(p->q_flags & F_DIRTY))
> +			if (!(p->q_flags & F_DIRTY)) {
>   				p->q_flags |= F_DIRTY;
> +				dirty_queues++;
> +			}
>   		}
>   	}
>   	return 0;
> @@ -375,8 +380,10 @@ int process_pipefd_events(int readfd)
>   		for (p = watch_list; p; p = p->next) {
>   			if (!(p->q_flags & F_QUEUE_DIR) || p->q_watchfd != value)
>   				continue;
> -			if (!(p->q_flags & F_DIRTY))
> +			if (!(p->q_flags & F_DIRTY)) {
>   				p->q_flags |= F_DIRTY;
> +				dirty_queues++;
> +			}
>   			break;
>   		}
>   	}
> @@ -521,7 +528,7 @@ int main(int argc, char **argv)
>   		int fdcount;
>   
>   		errno = 0;
> -		fdcount = epoll_wait(epollfd, ev, EV_MAX, 250);
> +		fdcount = epoll_wait(epollfd, ev, EV_MAX, (dirty_queues ? 250 : -1));
>   
>   		if (fdcount < 0) {
>   			if (errno == EINTR)
> @@ -542,6 +549,12 @@ int main(int argc, char **argv)
>   		for (e = watch_list; e; e = e->next) {
>   			if (!(e->q_flags & F_QUEUE_DIR) || !(e->q_flags & F_DIRTY) || (e->q_pid != 0) || (e->q_flags & F_PAUSED))
>   				continue;
> +
> +			if (dirty_queues == 0)
> +				rd_fatal("BUG: dirty_queues == 0, but dirty queues are present");
> +
> +			dirty_queues--;
> +
>   			e->q_flags &= ~F_DIRTY;
>   			e->q_pid = spawn_worker(epollfd, e);
>   		}



  reply	other threads:[~2023-05-22  4:46 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-04 13:42 [make-initrd] [PATCH 0/3] Reimplement ueventd Alexey Gladkov
2023-05-04 13:42 ` [make-initrd] [PATCH 1/3] " Alexey Gladkov
2023-05-05  3:08   ` Leonid Krivoshein
2023-05-05 17:02     ` Alexey Gladkov
2023-05-05  4:03   ` Leonid Krivoshein
2023-05-05 17:16     ` Alexey Gladkov
2023-05-05  5:21   ` Leonid Krivoshein
2023-05-05 17:24     ` Alexey Gladkov
2023-05-14 20:12   ` Leonid Krivoshein
2023-05-14 20:45     ` Alexey Gladkov
2023-05-14 20:57   ` Leonid Krivoshein
2023-05-15  8:52     ` Alexey Gladkov
2023-05-14 23:08   ` Leonid Krivoshein
2023-05-15  9:05     ` Alexey Gladkov
2023-05-15  0:47   ` Leonid Krivoshein
2023-05-15  9:12     ` Alexey Gladkov
2023-05-15  4:38   ` Leonid Krivoshein
2023-05-15 10:43     ` Alexey Gladkov
2023-05-18  6:39         ` Leonid Krivoshein
2023-05-18  7:05           ` Alexey Gladkov
2023-05-20  9:00   ` Leonid Krivoshein
2023-05-20 13:18     ` Alexey Gladkov
2023-05-20 15:17       ` Vladimir D. Seleznev
2023-05-20 17:23       ` Leonid Krivoshein
2023-05-20 18:51         ` Alexey Gladkov
2023-05-21  8:53         ` [make-initrd] [PATCH] ueventd: Don't use a epoll timeout when it's not needed Alexey Gladkov
2023-05-22  4:46           ` Leonid Krivoshein [this message]
2023-05-22  7:54             ` Alexey Gladkov
2023-05-22  9:19               ` Alexey Gladkov
2023-05-22  7:57             ` [make-initrd] [PATCH 1/2] ueventd: Fix memory leak Alexey Gladkov
2023-05-22  7:57             ` [make-initrd] [PATCH 2/2] ueventd: Change interface rd_asprintf_or_die Alexey Gladkov
2023-05-22  9:36               ` [make-initrd] [PATCH v2] " Alexey Gladkov
2023-05-20 16:37     ` [make-initrd] [PATCH 1/3] ueventd: Simplify call of the queue handler Alexey Gladkov
2023-05-20 16:37     ` [make-initrd] [PATCH 2/3] ueventd: Rename fd_list to fd_handler_list Alexey Gladkov
2023-05-20 16:37     ` [make-initrd] [PATCH 3/3] ueventd: Drop obsolete declarations Alexey Gladkov
2023-05-04 13:42 ` [make-initrd] [PATCH 2/3] Replace polld by uevent queue Alexey Gladkov
2023-05-04 13:42 ` [make-initrd] [PATCH 3/3] feature/kickstart: Reset rootdelay timer after kickstart Alexey Gladkov
2023-05-06 19:45 ` [make-initrd] ueventd: Add a prefix to the logging functions to avoid name collisions Alexey Gladkov
2023-05-06 19:45 ` [make-initrd] ueventd: Allow to configure the log destination Alexey Gladkov
2023-05-06 19:45 ` [make-initrd] ueventd: Pass the program name when initializing the logger Alexey Gladkov
2023-05-06 19:45 ` [make-initrd] ueventd: Rewrite logging function to make it more atomic Alexey Gladkov
2023-05-07 12:48 ` [make-initrd] [PATCH 0/3] Reimplement ueventd Alexey Gladkov
2023-05-13 11:50 ` Alexey Gladkov
2023-05-14 20:15   ` Leonid Krivoshein
2023-05-14 20:49     ` Alexey Gladkov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6258d160-9ffc-30de-abda-d6aec405559a@gmail.com \
    --to=klark.devel@gmail.com \
    --cc=make-initrd@lists.altlinux.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Make-initrd development discussion

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://lore.altlinux.org/make-initrd/0 make-initrd/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 make-initrd make-initrd/ http://lore.altlinux.org/make-initrd \
		make-initrd@lists.altlinux.org make-initrd@lists.altlinux.ru make-initrd@lists.altlinux.com
	public-inbox-index make-initrd

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://lore.altlinux.org/org.altlinux.lists.make-initrd


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git