linux-kernel - Re: [PATCH v2 1/8] lib/printbuf: New data structure for heap-allocated strings

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 22 Apr 2022 15:30:15 -0400
From:   Kent Overstreet <kent.overstreet@...il.com>
To:     Steven Rostedt <rostedt@...dmis.org>
Cc:     Christoph Hellwig <hch@....de>, linux-kernel@...r.kernel.org,
        linux-mm@...ck.org, linux-fsdevel@...r.kernel.org,
        hannes@...xchg.org, akpm@...ux-foundation.org,
        linux-clk@...r.kernel.org, linux-tegra@...r.kernel.org,
        linux-input@...r.kernel.org, roman.gushchin@...ux.dev
Subject: Re: [PATCH v2 1/8] lib/printbuf: New data structure for
 heap-allocated strings

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@...il.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 :)