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 20A40C433F5 for ; Fri, 22 Apr 2022 20:31:02 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A9BDF6B0073; Fri, 22 Apr 2022 16:31:01 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id A4AAE6B0075; Fri, 22 Apr 2022 16:31:01 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8D1C86B0073; Fri, 22 Apr 2022 16:31:01 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (relay.a.hostedemail.com [64.99.140.24]) by kanga.kvack.org (Postfix) with ESMTP id 7EAC46B0073 for ; Fri, 22 Apr 2022 16:31:01 -0400 (EDT) Received: from smtpin22.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 53592F00 for ; Fri, 22 Apr 2022 20:31:01 +0000 (UTC) X-FDA: 79385659122.22.A303564 Received: from mail-qv1-f53.google.com (mail-qv1-f53.google.com [209.85.219.53]) by imf17.hostedemail.com (Postfix) with ESMTP id ED1AA40036 for ; Fri, 22 Apr 2022 20:30:56 +0000 (UTC) Received: by mail-qv1-f53.google.com with SMTP id a5so6919474qvx.1 for ; Fri, 22 Apr 2022 13:31:00 -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=3NGyHwNzcPpl7RBhf4au6sQueS2Jmki+EL6hXZnP8PQ=; b=QDq+OMzhXiDZE3ekAGb1VjqbjweR6q15vhiF8oMbAkui5VdjtN0MnvTvkPzXwTNfid wp+k4NSNy3zdWHIgDyAm2OCTr7k1D7/aotkTNki3fw8w796Pmo7fMMf+Wi0VkdbsY6gT TtP5jIFZLS146fJvbNfZJR2IEUqMEAmO8rF2Y0WhgolbJS9+5Wm9tXxgIUyP2wgfGzdk 56LkNxicQnznIOk+Z63Zc2ynlTh8ZtDGTR4CtFvXBaDLHP/ZSsNrqyzyI9mF3W2uoO0b 7l8q01HtgPOTX21HWNDPxfWs9iqsrMXPDmO4JFG9H/KchRQNvq50cgw8FfRVJIYwRo21 n+xQ== 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=3NGyHwNzcPpl7RBhf4au6sQueS2Jmki+EL6hXZnP8PQ=; b=vmM/mpc3uW6POJd4N9ruqR5eUXc1QRBHOpAOSeXPMoFzA4Mha0oFfudBjQo1t3rnKT 1IGlPZ22xDzYN9xSmaAcAkJ+w4Vk/7BUwWIKU+IW4bSiia+os1rZmX/yzjNxrxtImmQ4 bSlUJEv4d+2PpzdOZF8wahvlqaKuGG3jKPEdrHgq7MTOwE/w9JYQDiNHNjpf9AkSQLvH TbNOPxEv4drDMCqvRvbuA2nhGJIOs+ZnoI5m3cS4KkAPgxGJ8AlyFRPYvz3MGXV5B30N 4d86In4a0xvgi78i0JcaS2sbTUzOvAKZp28QbBTdh+iFHqCQe+ozsn6C1iowXyoe6u92 J6rw== X-Gm-Message-State: AOAM531mMjhvmsshYGjv6z7RYDMEzuXUwTG422buDO3ccS9jA1P5EdtY CvQevAH8Q3bccUaCvF372Q== X-Google-Smtp-Source: ABdhPJxzISRgHHODXKrrwyPqtpJH5WzT+DrLyG8K/Zt/m0UNqJazuD3i0Bd7jVRhJazqucNnus+6QQ== X-Received: by 2002:a0c:d7cb:0:b0:444:2b27:80d3 with SMTP id g11-20020a0cd7cb000000b004442b2780d3mr5088064qvj.57.1650659460120; Fri, 22 Apr 2022 13:31:00 -0700 (PDT) Received: from moria.home.lan (c-73-219-103-14.hsd1.vt.comcast.net. [73.219.103.14]) by smtp.gmail.com with ESMTPSA id n22-20020ac85b56000000b002f1d7a2867dsm1790112qtw.67.2022.04.22.13.30.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 22 Apr 2022 13:30:59 -0700 (PDT) Date: Fri, 22 Apr 2022 16:30:57 -0400 From: Kent Overstreet To: Steven Rostedt Cc: Christoph Hellwig , linux-kernel@vger.kernel.org, linux-mm@kvack.org, linux-fsdevel@vger.kernel.org, hannes@cmpxchg.org, akpm@linux-foundation.org, linux-clk@vger.kernel.org, linux-tegra@vger.kernel.org, linux-input@vger.kernel.org, roman.gushchin@linux.dev Subject: Re: [PATCH v2 1/8] lib/printbuf: New data structure for heap-allocated strings Message-ID: <20220422203057.iscsmurtrmwkpwnq@moria.home.lan> References: <20220421234837.3629927-1-kent.overstreet@gmail.com> <20220421234837.3629927-7-kent.overstreet@gmail.com> <20220422042017.GA9946@lst.de> <20220422052208.GA10745@lst.de> <20220422113736.460058cc@gandalf.local.home> <20220422193015.2rs2wvqwdlczreh3@moria.home.lan> <20220422153916.7ebf20c3@gandalf.local.home> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220422153916.7ebf20c3@gandalf.local.home> Authentication-Results: imf17.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=QDq+OMzh; spf=pass (imf17.hostedemail.com: domain of kent.overstreet@gmail.com designates 209.85.219.53 as permitted sender) smtp.mailfrom=kent.overstreet@gmail.com; dmarc=pass (policy=none) header.from=gmail.com X-Rspam-User: X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: ED1AA40036 X-Stat-Signature: usstp6jyb6tn18wwuy4zom4jbzx7zj3e X-HE-Tag: 1650659456-850707 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 Fri, Apr 22, 2022 at 03:39:16PM -0400, Steven Rostedt wrote: > I do not consider Facebook an open source company. One reason I turned them > down. Surely you can see how NIH syndrome isn't something that just happens at closed-source companies? How a default cultural assumption of "we do things the way we've always done" leads to things getting insular? > > The reason I bring that up is that in this case, printbuf is the more evolved, > > more widely used implementation, and you're asking me to discard it so the > > kernel can stick with its more primitive, less widely used implementation. > > > > $ git grep -w seq_buf|wc -l > > 86 > > > > $ git grep -w printbuf|wc -l > > 366 > > $ git grep printbuf > drivers/media/i2c/ccs/ccs-reg-access.c: char printbuf[(MAX_WRITE_LEN << 1) + > drivers/media/i2c/ccs/ccs-reg-access.c: bin2hex(printbuf, regdata, msg.len); > drivers/media/i2c/ccs/ccs-reg-access.c: regs->addr + j, printbuf); > > I don't see it. Here: https://evilpiepirate.org/git/bcachefs.git/ It may not be merged yet, but it is actively developed open source code with active users that's intended to be merged! > I'd like to know more to why seq_buf is not good for you. And just telling > me that you never seriously tried to make it work because you were afraid > of causing tracing regressions without ever asking the tracing maintainer > is not going to cut it. I didn't know about seq_buf until a day or two ago, that's literally all it was. And Steve, apologies if I've come across as being a dick about this, that wasn't my intent. I've got nothing against you or your code - I'd love it if we could just have a discussion about them on their merits, and if it feels like I'm making an issue about this unnecessarily that's because I think there's something about kernel process and culture worth improving that I want to raise, so I'm sticking my neck out a bit here. So here's the story of how I got from where seq_buf is now to where printbuf is now: - Printbuf started out as almost an exact duplicate of seq_buf (like I said, not intentionally), with an external buffer typically allocated on the stack. - As error/log messages got to be bigger and more structured, stack usage eventually became an issue, so eventually I added the heap allocations. - This made them a lot more convenient to use, and made possible entirely new ways of using them - so I started using them more, and converting everything that outputted to strings to them. - This lead to the realization that when pretty-printers are easy and convenient to write, that leads to writing pretty-printers for _more_ stuff, which makes it easy to stay in the habit of adding anything relevant to sysfs/debugfs - and log/error messages got a _whole_ lot better when I realized instead of writing format strings for every basic C type I can just use the .to_text() methods of the high level objects I'm working with. Basically, my debugging life has gotten _drastically_ easier because of this change in process and approach - deadlocks that I used to have to attach a debugger for are now trivial because all the relevant state is in debugfs and greppable, and filesystem inconsistencies that used to suck to debug I now just take what's in the error message and grep through the journal for. I can't understate how invaluable all this stuff has been, and I'm excited to take the lessons I've learned and apply them to the wider kernel and make other people's lives easier too. The shrinkers-OOM-reporting patch was an obvious starting point because - shrinkers have internal state that's definitely worth reporting on - we shouldn't just be logging this on OOM, we should also make this available in sysfs or debugfs. Feature wise, printbufs have: - heap allocation - extra state for formatting: indent level, tabstops, and a way of specifying units. That's basically it. Heap allocation adds very little code and eliminates a _lot_ of headaches in playing the "how much do I need to/can I put on the stack" game, and you'll want the formatting options as soon as you start formatting multi line output and writing pretty printers that call other pretty printers.