From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on sa.local.altlinux.org X-Spam-Level: X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM autolearn=ham autolearn_force=no version=3.4.1 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1684730769; x=1687322769; h=content-transfer-encoding:in-reply-to:from:content-language :references:to:subject:user-agent:mime-version:date:message-id:from :to:cc:subject:date:message-id:reply-to; bh=qrZ7brKakaTwunntpVnFny0/rSC/301Bjr9MsIdW9KQ=; b=XgaOLByO/uuRWIS4M9DOdnhy5mkX0gCXx5PhkrEF43oSyl2BFzIlOewkGXeHOPzqXr FObXhgFAJ6WZhbUvv6vTWG9F4j9e0UBrOG+oTrfBHqEsy5mHyM46tXt6f/CpuZ+3400R X+q/RDoe4BUaiCEuKBHe6zqOBcGMLaAbctEiv19IP95m8gQ7A4pxBQqFnmJ+dSf1e4vh /idpxqzGK/z+8FeE10eWuXp2a/3Nk4hwj/l8m5CL+Cavpevu4VP0AqeSOrRCwVUYw541 aRlZeKeQtaOPrU0YG/dTO1SqJ7aYJm+pznFO645NR0YIUBw483jANUdkvR/LbYnsWGoz 3RQw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1684730769; x=1687322769; h=content-transfer-encoding:in-reply-to:from:content-language :references:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=qrZ7brKakaTwunntpVnFny0/rSC/301Bjr9MsIdW9KQ=; b=J46HnVuuRYCiQd++W3md+RsJQr+JC/u8gxDQpDvY8o4IBPEZSxSzPbvpLIXsVSRCBG u2FtXZlH+Wl0+MD9BbmonriR6D4yhlyz83e0/b6LrvvrVaduqXBUf2VJ+9urBOgJ+5CB P5dOnTsZWobqFSfClXhIe9IyvRSq8xBr1YNpNXKVZc+R3aBQFWxu+G7QZ4u5uAqfaSvg cv5eKQDLN2Pbc8Vnd6az2VwFIKQRfRbzzWdHjg0ix25xHGnnarwBHj5T4ZOODtyYUUhq Pf409yJarUp3Go6iZl+dqkV6Gh5cE+vpkBy5Ii7HJsDm//PHqmv9kkpeQLLqciqdp8kZ 65KQ== X-Gm-Message-State: AC+VfDwVIc5ox7M6DB06gKhL7vNVKJTGpdHHhmtNFBSGcATbIon/0Ehd RS8J0mmc31G3j+ovIHLD7OfzxDeq1e4= X-Google-Smtp-Source: ACHHUZ4o/5H/H8QMdbIvlw/Y3WTBEt/8+1+KfY5d5vhlVdLBzUTt9AXWBmXYfgAlY7yV3OCrQuBTtA== X-Received: by 2002:ac2:5930:0:b0:4f4:b41c:a8c1 with SMTP id v16-20020ac25930000000b004f4b41ca8c1mr54571lfi.69.1684730769139; Sun, 21 May 2023 21:46:09 -0700 (PDT) Message-ID: <6258d160-9ffc-30de-abda-d6aec405559a@gmail.com> Date: Mon, 22 May 2023 07:46:07 +0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Firefox/102.0 Thunderbird/102.11.0 To: make-initrd@lists.altlinux.org References: <5522f0c1-adac-3db7-e569-05cda6d05d58@gmail.com> <20230521085326.308119-1-gladkov.alexey@gmail.com> Content-Language: ru, en-US From: Leonid Krivoshein In-Reply-To: <20230521085326.308119-1-gladkov.alexey@gmail.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [make-initrd] [PATCH] ueventd: Don't use a epoll timeout when it's not needed X-BeenThere: make-initrd@lists.altlinux.org X-Mailman-Version: 2.1.12 Precedence: list Reply-To: make-initrd@lists.altlinux.org List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 22 May 2023 04:46:12 -0000 Archived-At: List-Archive: Привет! Глянул финальную версию в 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 > --- > 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); > }