From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 92043C43334 for ; Tue, 21 Jun 2022 07:52:03 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 278588E0001; Tue, 21 Jun 2022 03:52:03 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 229766B007B; Tue, 21 Jun 2022 03:52:03 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0F10F8E0001; Tue, 21 Jun 2022 03:52:03 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id EFD956B0072 for ; Tue, 21 Jun 2022 03:52:02 -0400 (EDT) Received: from smtpin26.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id C97C3285 for ; Tue, 21 Jun 2022 07:52:02 +0000 (UTC) X-FDA: 79601474484.26.640DA03 Received: from mail-qv1-f44.google.com (mail-qv1-f44.google.com [209.85.219.44]) by imf17.hostedemail.com (Postfix) with ESMTP id 6D5864001A for ; Tue, 21 Jun 2022 07:52:02 +0000 (UTC) Received: by mail-qv1-f44.google.com with SMTP id p31so19175392qvp.5 for ; Tue, 21 Jun 2022 00:52:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=oBi1GtdcKMWmT1bDHc11If7BFUljocWb98H7N/H6xs0=; b=SGPJ5m4tvIHRyjwwdQIIal8VjxuNUxpv23Qai8KBUFCze0qz9KMZRvu8ZUqU3eN3Ku fl4o56SOitD/EI4WXS5RhK3oGUbWPgnOyKgfDrGciUttk7igTulNAjflhz1fpUnQN4lX aRimKlcstTq4qTa0Gf9IOTzFsnalN0nboEJh3CcPDXUjN0h8kDOh8EpqDRKAC+ZOB7cn FHXXrWylD4MypmamELDdF1j65iVrsu5e9H3g2wQ6mOaVmFOdv6gw/bEleMWXTm7wQea1 2u+2YtFlgloF51zN4dc7ehszwNHr+Vw4YfOlPRw55uWxOJjFdXbhGorhz6P8E1OuNeTd 91sQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=oBi1GtdcKMWmT1bDHc11If7BFUljocWb98H7N/H6xs0=; b=uwzpEo5eZ8dgPTXBus10E6cLpqjGxtQ1xpp7GuaDaJmfh14i26Xed6jETx+K4LYO2j /xtW0vFvOnxjJVPJ7RphNIlOLfsEPs38dgV6e6CwpStDzQCMwMIkfj2En0jBfh7uAGPL 4U3trPuu0keDSWQa6WyWiwvJsDX1Hei9qgWGsU2hVZ6urSXh/gpO0OghT2bukWY2+Lxs N4FqUNDGK0FB8VD8/9SgOjTPwFg12apcxVIsdc4EalCD7TPzVia3lsks7VAFcPdRCGIA Vc5fpM071FQ3lmHvZYPdVD1uWXnl5EVudmjCYOU17btuASACoPQJ7G7vF+JyHBpvdPmp e9BA== X-Gm-Message-State: AJIora+WgOeYSWRf6Wl0cuXACjRAGjwWb/mT2VPFQEKmN3ArljJQwRq1 6zjiOqmrxdlhe+YR7wWMIA== X-Google-Smtp-Source: AGRyM1uldbQ+FF0ReOT1dCERxSwSqnUWwf/gHG/zCBzt2IGMN+ewK96Jq0luXf2dgWYAW+ncZVhwDw== X-Received: by 2002:a05:6214:5199:b0:464:58c0:3926 with SMTP id kl25-20020a056214519900b0046458c03926mr21833952qvb.48.1655797921656; Tue, 21 Jun 2022 00:52:01 -0700 (PDT) Received: from localhost (c-73-219-103-14.hsd1.vt.comcast.net. [73.219.103.14]) by smtp.gmail.com with ESMTPSA id l16-20020a05620a28d000b006a6cadd89efsm14678198qkp.82.2022.06.21.00.52.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 21 Jun 2022 00:52:00 -0700 (PDT) Date: Tue, 21 Jun 2022 03:51:59 -0400 From: Kent Overstreet To: Rasmus Villemoes Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, pmladek@suse.com, rostedt@goodmis.org, enozhatsky@chromium.org, willy@infradead.org Subject: Re: [PATCH v4 05/34] vsprintf: %pf(%p) Message-ID: <20220621075159.m67qzftqulvphivw@moria.home.lan> References: <20220620004233.3805-1-kent.overstreet@gmail.com> <20220620004233.3805-6-kent.overstreet@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1655797922; a=rsa-sha256; cv=none; b=7Mt7So/47aQzc4P96+OjnbwlQBlZy9GLXr+mScPuDSEQecxPGUf4kcJCdidO/HybUfELrb KFLff21Xqdk2WDC8BhfTio+dLj0zyDtaZBGhqw6/qRLcpBoDlkduqUKY0DXpXTfT3kmVJD 65QWLs7nwpNTNT+2dBdgCi0c3hDe8Zw= ARC-Authentication-Results: i=1; imf17.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=SGPJ5m4t; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf17.hostedemail.com: domain of kent.overstreet@gmail.com designates 209.85.219.44 as permitted sender) smtp.mailfrom=kent.overstreet@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1655797922; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=oBi1GtdcKMWmT1bDHc11If7BFUljocWb98H7N/H6xs0=; b=VG58FCOEaWdje6tmax0NZsR7u/oCRE2OuHiZ5skVItU32zP30/k4HEBxwdkYb0hydR0W+C v/R5sqFvwVffSA7M3TgFbQAQ6r09gEzmniiG154SEzps2nLEodY2sWj5U+hIFo3VKndGRy U20C2ic6jUTL+soYO2eY19BQv5jAij8= Authentication-Results: imf17.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=SGPJ5m4t; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf17.hostedemail.com: domain of kent.overstreet@gmail.com designates 209.85.219.44 as permitted sender) smtp.mailfrom=kent.overstreet@gmail.com X-Stat-Signature: wh64aakpm7x9q11t69a4c8swa38czzoo X-Rspamd-Queue-Id: 6D5864001A X-Rspam-User: X-Rspamd-Server: rspam03 X-HE-Tag: 1655797922-94062 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Tue, Jun 21, 2022 at 09:04:38AM +0200, Rasmus Villemoes wrote: > On 20/06/2022 02.42, Kent Overstreet wrote: > > +Note that a pretty-printer may not sleep, if called from printk(). If called > > +from pr_buf() or sprintf() there are no such restrictions. > > I know what you're trying to say, but if the sprintf() call itself is > from a non-sleepable context this is obviously not true. So please just > make the rule "A pretty-printer must not sleep.". That's much simpler > and less error-prone. Otherwise I guarantee you that somebody is going > to add a sleeping pretty-printer for their own need, use it in a couple > of safe places, and then somebody wants to add a printk() in that driver > and sees "hey, I can get all this state dumped very easily with this > pretty-printer". Kernel programmers are used to having to consider the context they're in and what the functions they're calling might do, a pretty-printer being called indirectly via sprintf() is absolutely no different. > > struct printf_spec { > > @@ -2520,7 +2521,16 @@ int format_decode(const char *fmt, struct printf_spec *spec) > > return ++fmt - start; > > > > case 'p': > > - spec->type = FORMAT_TYPE_PTR; > > + fmt++; > > + if (fmt[0] == 'f' && > > + fmt[1] == '(') { > > + fmt += 2; > > + spec->type = FORMAT_TYPE_FN; > > + } else > > + spec->type = FORMAT_TYPE_PTR; > > + return fmt - start; > > + case '(': > > + spec->type = FORMAT_TYPE_FN; > > return ++fmt - start; > > NAK. Don't implement something that will never be tested nor used. > There's not a snowball's chance in hell that we'll ever build the kernel > without -Wformat. We're not stopping here. Matthew is taking this to WG14 and I'll be working on adding this functionality to glibc next, and %() is the syntax we intend to take to the working group. But the working group is naturally going to want to see that a working implementation of it exists. > Sorry, but this is way too ugly, and the prospect of at some point in > the future invoking libffi to do something even naster... eww. We do not > need more functions with completely generic prototypes with no > typechecking and making it extremely hard to teach one of our static > analyzers (smatch has some %pX checking) to do that typechecking. > > There are at least two ways you can achieve this passing of a variable > number of arguments with proper types. > > (1) Each pretty-printer comes with a struct wrapping up its real > arguments and a macro for creating a compound literal passing those > arguments. > > struct foo_pp { > void (*func)(struct printbuf *pb, void *ctx); /* always first */ > int x; > long y; > }; > void foo_pp(struct printbuf *pb, void *ctx) > { > struct foo_pp *f = ctx; > pr_printf(pb, "%d %ld", f->x, f->y); > } > > #define FOO_PP(_x, _y) (struct foo_pp){.func = foo_pp, .x = (_x), .y = (_y)} > > printk("bla bla %pf\n", &FOO_PP(aa, bb)); Hellllllllll no. All that's missing right now is gcc checking that the function signature matches the args specified within the (). That will come. No way in hell am I going to implement some half baked hacked up macro crap - I intend to do this right. > (2) Let the pretty-printer itself extract the varargs it expects. To > portably pass around a va_list by reference it needs to be wrapped, so > this would be > > struct wva { va_list ap; }; > > void foo_pp(struct printbuf *pb, struct wva *w) > { > int x = va_arg(w->ap, int); > long y = va_arg(w->ap, long); > pr_printf(pb, "%d %ld", x, y); > } > > printk("bla bla %pf(%d, %ld)\n", foo_pp, aa, bb) > > with the core printf implementation internally using such a wrapped > va_list, and after a %pf( relying on the pretty-printer having consumed > the arguments up until the closing ). It would probably be a good idea > to give the pretty-printer a pointer to that opening '(' or one-past-it > as well so it could do a sanity check. Also no. Varargs is terrible in most situations, and should only be used for functions that actually want to take variable numbers of arguments - that doesn't apply to pretty printers.