linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Kent Overstreet <kent.overstreet@gmail.com>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Christoph Hellwig <hch@lst.de>,
	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
Date: Fri, 22 Apr 2022 15:30:15 -0400	[thread overview]
Message-ID: <20220422193015.2rs2wvqwdlczreh3@moria.home.lan> (raw)
In-Reply-To: <20220422113736.460058cc@gandalf.local.home>

Hi Steve!

On Fri, Apr 22, 2022 at 11:37:36AM -0400, Steven Rostedt wrote:
> On Fri, 22 Apr 2022 01:40:15 -0400
> Kent Overstreet <kent.overstreet@gmail.com> wrote:
> 
> > So I'm honestly not super eager to start modifying tricky arch code that I can't
> > test, and digging into what looked like non trivial interactions between the way
> > the traceing code using seq_buf (naturally, given that's where it originates).
> 
> Yes, seq_buf came from the tracing system but was to be used in a more
> broader way. I had originally pushed trace_seq into the lib directory, but
> Andrew Morton said it was too specific to tracing. Thus, I gutted the
> generic parts out of it and created seq_buf, which looks to be something
> that you could use. I had patches to convert seq_file to it, but ran out of
> time. I probably can pull them out of the closet and start that again.
> 
> > 
> > Now yes, I _could_ do a wholesale conversion of seq_buf to printbuf and delete
> > that code, but doing that job right, to be confident that I'm not introducing
> > bugs, is going to take more time than I really want to invest right now. I
> > really don't like to play fast and loose with that stuff.
> 
> I would be happy to work with you to convert to seq_buf. If there's
> something missing from it, I can help you change it so that it doesn't
> cause any regressions with the tracing subsystem.
> 
> This is how open source programming is suppose to work ;-)

Is it though? :)

One of the things I've been meaning to talk more about, that
came out of a recent Rust discussion, is that we in the kernel community could
really do a better job with how we interact with the outside world, particularly
with regards to the sharing of code.

The point was made to me when another long standing kernel dev was complaining
about Facebook being a large, insular, difficult to work with organization, that
likes to pretend it is the center of the universe and not bend to the outside
world, while doing the exact same thing with respect to new concerns brought by
the Rust community. The irony was illuminating :)

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

So, going to have to push back on that one :)

Printbufs aren't new code; everything in them is there because I've found it
valuable, which is why I decided to try promoting them to the kernel proper (and
more importantly, the idea of a standard way to pretty-print anything).

I'm happy to discuss the merits of the code more, and try to convince you why
you'll like them :)


  reply	other threads:[~2022-04-22 19:30 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-21 23:48 [PATCH 0/4] Printbufs & shrinker OOM reporting Kent Overstreet
2022-04-21 23:48 ` [PATCH 1/4] lib/printbuf: New data structure for heap-allocated strings Kent Overstreet
2022-04-21 23:48 ` [PATCH 2/4] mm: Add a .to_text() method for shrinkers Kent Overstreet
2022-04-22 12:21   ` Michal Hocko
2022-04-21 23:48 ` [PATCH 3/4] mm: Centralize & improve oom reporting in show_mem.c Kent Overstreet
2022-04-21 23:48 ` [PATCH 4/4] bcachefs: shrinker.to_text() methods Kent Overstreet
2022-04-21 23:48 ` [PATCH v2 0/8] Printbufs & improved shrinker debugging Kent Overstreet
2022-04-21 23:48 ` [PATCH v2 1/8] lib/printbuf: New data structure for heap-allocated strings Kent Overstreet
2022-04-22  4:20   ` Christoph Hellwig
2022-04-22  5:14     ` Kent Overstreet
2022-04-22  5:22       ` Christoph Hellwig
2022-04-22  5:40         ` Kent Overstreet
2022-04-22  5:52           ` Christoph Hellwig
2022-04-22  6:06             ` Kent Overstreet
2022-04-22  6:11               ` Christoph Hellwig
2022-04-22  6:18                 ` Kent Overstreet
2022-04-22 15:37           ` Steven Rostedt
2022-04-22 19:30             ` Kent Overstreet [this message]
2022-04-22 19:39               ` Steven Rostedt
2022-04-22 20:30                 ` Kent Overstreet
2022-04-22 20:47                   ` Steven Rostedt
2022-04-22 21:51                     ` Kent Overstreet
2022-04-22 22:20                       ` Steven Rostedt
2022-04-22 20:03               ` James Bottomley
2022-04-22 21:13                 ` Kent Overstreet
2022-04-23 14:16                   ` Rust and Kernel Vendoring [Was Re: [PATCH v2 1/8] lib/printbuf: New data structure for heap-allocated strings] James Bottomley
2022-04-24 20:36                     ` Kent Overstreet
2022-04-26  2:22                       ` James Bottomley
2022-04-24 23:46   ` [PATCH v2 1/8] lib/printbuf: New data structure for heap-allocated strings Joe Perches
2022-04-25  0:45     ` Kent Overstreet
2022-04-25  2:44     ` Matthew Wilcox
2022-04-25  4:19       ` Kent Overstreet
2022-04-25  4:48         ` Joe Perches
2022-04-25  4:59           ` Kent Overstreet
2022-04-25  5:00             ` Joe Perches
2022-04-25  5:56               ` Kent Overstreet
2022-04-21 23:48 ` [PATCH v2 2/8] Input/joystick/analog: Convert from seq_buf -> printbuf Kent Overstreet
2022-04-21 23:48 ` [PATCH v2 3/8] mm/memcontrol.c: Convert to printbuf Kent Overstreet
2022-04-22 12:28   ` Michal Hocko
2022-04-21 23:48 ` [PATCH v2 4/8] clk: tegra: bpmp: " Kent Overstreet
2022-04-21 23:48 ` [PATCH v2 5/8] mm: Add a .to_text() method for shrinkers Kent Overstreet
2022-04-21 23:48 ` [PATCH v2 6/8] mm: Count requests to free & nr freed per shrinker Kent Overstreet
2022-04-21 23:48 ` [PATCH v2 7/8] mm: Move lib/show_mem.c to mm/ Kent Overstreet
2022-04-22 12:32   ` Michal Hocko
2022-04-21 23:48 ` [PATCH v2 8/8] mm: Centralize & improve oom reporting in show_mem.c Kent Overstreet
2022-04-22 12:58   ` Michal Hocko
2022-04-22 15:09     ` Roman Gushchin
2022-04-22 23:48       ` Kent Overstreet
2022-04-23  0:27         ` Roman Gushchin
2022-04-23  0:46           ` Kent Overstreet
2022-04-23  1:25             ` Roman Gushchin
2022-04-23 11:48               ` Tetsuo Handa
2022-04-25  9:28             ` Michal Hocko
2022-04-25 15:28               ` Kent Overstreet
2022-04-26  7:17                 ` Michal Hocko
2022-04-26  7:26                   ` Kent Overstreet
2022-04-26  7:40                     ` Michal Hocko
2022-04-30  4:00 ` [PATCH 0/4] Printbufs & shrinker OOM reporting Dave Young

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=20220422193015.2rs2wvqwdlczreh3@moria.home.lan \
    --to=kent.overstreet@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=hannes@cmpxchg.org \
    --cc=hch@lst.de \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=roman.gushchin@linux.dev \
    --cc=rostedt@goodmis.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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox