From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-1.0 required=5.0 tests=MAILING_LIST_MULTI, RCVD_IN_DNSWL_NONE autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 10356 invoked from network); 22 Dec 2020 21:22:15 -0000 Received: from krantz.zx2c4.com (192.95.5.69) by inbox.vuxu.org with ESMTPUTF8; 22 Dec 2020 21:22:15 -0000 Received: by krantz.zx2c4.com (ZX2C4 Mail Server) with ESMTP id c8363924; Tue, 22 Dec 2020 21:13:00 +0000 (UTC) Return-Path: Received: from sdaoden.eu (sdaoden.eu [217.144.132.164]) by krantz.zx2c4.com (ZX2C4 Mail Server) with ESMTPS id d23212e6 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO) for ; Tue, 22 Dec 2020 21:12:57 +0000 (UTC) Received: by sdaoden.eu (Postfix, from userid 1000) id 78E9916057; Tue, 22 Dec 2020 22:22:08 +0100 (CET) Date: Tue, 22 Dec 2020 22:22:07 +0100 From: Steffen Nurpmeso To: gs-cgit-lists.zx2c4.com@gluelogic.com Cc: cgit@lists.zx2c4.com Subject: Re: cgit 1.2.3: lighttpd 1.4.57, AlpineLinux [edge]: using cache breaks delivery Message-ID: <20201222212207.3qb-M%steffen@sdaoden.eu> In-Reply-To: <20201222150957._aAW5%steffen@sdaoden.eu> References: <20201222061206.GA54419@xps13> <20201222135537.ovyl4%steffen@sdaoden.eu> <20201222150957._aAW5%steffen@sdaoden.eu> Mail-Followup-To: gs-cgit-lists.zx2c4.com@gluelogic.com, cgit@lists.zx2c4.com User-Agent: s-nail v14.9.20-84-g7268a84d OpenPGP: id=EE19E1C1F2F7054F8D3954D8308964B51883A0DD; url=https://ftp.sdaoden.eu/steffen.asc; preference=signencrypt BlahBlahBlah: Any stupid boy can crush a beetle. But all the professors in the world can make no bugs. X-BeenThere: cgit@lists.zx2c4.com X-Mailman-Version: 2.1.30rc1 Precedence: list List-Id: List for cgit developers and users List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: cgit-bounces@lists.zx2c4.com Sender: "CGit" Once more, hey, Steffen Nurpmeso wrote in <20201222150957._aAW5%steffen@sdaoden.eu>: |Steffen Nurpmeso wrote in | <20201222135537.ovyl4%steffen@sdaoden.eu>: ||gs-cgit-lists.zx2c4.com@gluelogic.com wrote in || <20201222061206.GA54419@xps13>: |||>Steffen Nurpmeso wrote in |||> <20201221193127.zbZeP%steffen at sdaoden.eu>: |||>|John Keeping wrote in |||>| : |||>||On Mon, Dec 21, 2020 at 05:26:19PM +0100, Steffen Nurpmeso wrote: |||>||> I discovered today that cgit no longer delivers pages, and it must |||>||> have been like that for some time. The server looks show |||>||> successful delivery, the cgit cache is populated and rotated just |||>||> correctly, but all cgit delivers is that final error of main() as ... |||>||> I am pretty sure cgit delivered some weeks ago, the most notable |||>||> difference is that AlpineLinux switched to Lighttpd 1.4.56 then |||>||> .57, which seems to have brought tremendous changes under the |||> ... |||>|But the file was generated normally: ... |||>||and this may be caused by sendfile(2) failing due to some difference \ |||>||in |||>||how the web server is setting up the output file descriptor. You may ... Looking at lighttpd commit history it seems he now uses splice(2) for CGI if possible. ... |||I would like to propose an alternative and more portable solution: ||| |||cgit cache should fallback to lseek, xread, xwrite if sendfile fails. It would be cool if the sendfile(2) could be avoided via configuration. It seems so wasteful to always have that system-call, context-switch and such. What do you think of (on [master] aka adcc4f822fe11836e5f942fc1ae0f00db4eb8d5f plus the other patch), the following runs on my server VM now and works: diff --git a/cache.c b/cache.c index b09769e..8a66db9 100644 --- a/cache.c +++ b/cache.c @@ -25,6 +25,7 @@ struct cache_slot { const char *key; size_t keylen; + int sndfpok; int ttl; cache_fill_fn fn; int cache_fd; @@ -92,7 +93,7 @@ static int print_slot(struct cache_slot *slot) start_off = slot->keylen + 1; - do { + if (slot->sndfpok) for(;;) { ret = sendfile(STDOUT_FILENO, slot->cache_fd, &start_off, slot->cache_st.st_size - start_off); if (ret < 0) { @@ -103,7 +104,7 @@ static int print_slot(struct cache_slot *slot) return errno; } return 0; - } while (1); + } #endif i = lseek(slot->cache_fd, slot->keylen + 1, SEEK_SET); @@ -350,7 +351,7 @@ static int process_slot(struct cache_slot *slot) /* Print cached content to stdout, generate the content if necessary. */ int cache_process(int size, const char *path, const char *key, int ttl, - cache_fill_fn fn) + cache_fill_fn fn, int sndfpok) { unsigned long hash; int i; @@ -383,6 +384,7 @@ int cache_process(int size, const char *path, const char *key, int ttl, strbuf_addbuf(&lockname, &filename); strbuf_addstr(&lockname, ".lock"); slot.fn = fn; + slot.sndfpok = sndfpok; slot.ttl = ttl; slot.stdout_fd = -1; slot.cache_name = filename.buf; diff --git a/cache.h b/cache.h index 470da4f..3e25daa 100644 --- a/cache.h +++ b/cache.h @@ -17,12 +17,13 @@ typedef void (*cache_fill_fn)(void); * key the key used to lookup cache files * ttl max cache time in seconds for this key * fn content generator function for this key + * sndfpok allowed to use sendfile(2) * * Return value * 0 indicates success, everything else is an error */ extern int cache_process(int size, const char *path, const char *key, int ttl, - cache_fill_fn fn); + cache_fill_fn fn, int sndfpok); /* List info about all cache entries on stdout */ diff --git a/cgit.c b/cgit.c index 08d81a1..684f97a 100644 --- a/cgit.c +++ b/cgit.c @@ -197,6 +197,8 @@ static void config_cb(const char *name, const char *value) ctx.cfg.enable_git_config = atoi(value); else if (!strcmp(name, "max-stats")) ctx.cfg.max_stats = cgit_find_stats_period(value, NULL); + else if (!strcmp(name, "cache-sendfile")) + ctx.cfg.cache_sendfile = atoi(value); else if (!strcmp(name, "cache-size")) ctx.cfg.cache_size = atoi(value); else if (!strcmp(name, "cache-root")) @@ -363,6 +365,7 @@ static void prepare_context(void) { memset(&ctx, 0, sizeof(ctx)); ctx.cfg.agefile = "info/web/last-modified"; + ctx.cfg.cache_sendfile = 1; ctx.cfg.cache_size = 0; ctx.cfg.cache_max_create_time = 5; ctx.cfg.cache_root = CGIT_CACHE_ROOT; @@ -1103,7 +1106,8 @@ int cmd_main(int argc, const char **argv) if (!ctx.env.authenticated || (ctx.env.request_method && !strcmp(ctx.env.request_method, "HEAD"))) ctx.cfg.cache_size = 0; err = cache_process(ctx.cfg.cache_size, ctx.cfg.cache_root, - ctx.qry.raw, ttl, process_request); + ctx.qry.raw, ttl, process_request, + ctx.cfg.cache_sendfile); cgit_cleanup_filters(); if (err) cgit_print_error("Error processing page: %s (%d)", diff --git a/cgit.h b/cgit.h index 69b5c13..21677a6 100644 --- a/cgit.h +++ b/cgit.h @@ -215,6 +215,7 @@ struct cgit_config { char *repository_sort; char *virtual_root; /* Always ends with '/'. */ char *strict_export; + int cache_sendfile; int cache_size; int cache_dynamic_ttl; int cache_max_create_time; diff --git a/cgitrc.5.txt b/cgitrc.5.txt index 33a6a8c..7ea1b81 100644 --- a/cgitrc.5.txt +++ b/cgitrc.5.txt @@ -83,6 +83,10 @@ cache-scanrc-ttl:: of scanning a path for git repositories. See also: "CACHE". Default value: "15". +cache-sendfile:: + Whether sendfile(2) optimization may be used for CGI outputs, when + available. Default value: "1". + case-sensitive-sort:: Sort items in the repo list case sensitively. Default value: "1". See also: repository-sort, section-sort. --steffen | |Der Kragenbaer, The moon bear, |der holt sich munter he cheerfully and one by one |einen nach dem anderen runter wa.ks himself off |(By Robert Gernhardt)