All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mateusz Guzik <mjguzik@gmail.com>
To: Dave Chinner <david@fromorbit.com>
Cc: Christian Brauner <brauner@kernel.org>,
	Dave Chinner <dchinner@redhat.com>,
	linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	linux-bcachefs@vger.kernel.org,
	Kent Overstreet <kent.overstreet@linux.dev>,
	Alexander Viro <viro@zeniv.linux.org.uk>
Subject: Re: (subset) [PATCH 22/32] vfs: inode cache conversion to hash-bl
Date: Fri, 27 Oct 2023 19:13:11 +0200	[thread overview]
Message-ID: <CAGudoHGzX2H4pUuDNYzYOf8s-HaZuAi7Dttpg_SqtXAgTw8tiw@mail.gmail.com> (raw)
In-Reply-To: <ZTYAUyiTYsX43O9F@dread.disaster.area>

On 10/23/23, Dave Chinner <david@fromorbit.com> wrote:
> On Fri, Oct 20, 2023 at 07:49:18PM +0200, Mateusz Guzik wrote:
>> On 10/20/23, Dave Chinner <david@fromorbit.com> wrote:
>> > On Thu, Oct 19, 2023 at 05:59:58PM +0200, Mateusz Guzik wrote:
>> >> > To be clear there is no urgency as far as I'm concerned, but I did
>> >> > run
>> >> > into something which is primarily bottlenecked by inode hash lock
>> >> > and
>> >> > looks like the above should sort it out.
>> >> >
>> >> > Looks like the patch was simply forgotten.
>> >> >
>> >> > tl;dr can this land in -next please
>> >>
>> >> In case you can't be arsed, here is something funny which may convince
>> >> you to expedite. ;)
>> >>
>> >> I did some benching by running 20 processes in parallel, each doing
>> >> stat
>> >> on a tree of 1 million files (one tree per proc, 1000 dirs x 1000
>> >> files,
>> >> so 20 mln inodes in total).  Box had 24 cores and 24G RAM.
>> >>
>> >> Best times:
>> >> Linux:          7.60s user 1306.90s system 1863% cpu 1:10.55 total
>> >> FreeBSD:        3.49s user 345.12s system 1983% cpu 17.573 total
>> >> OpenBSD:        5.01s user 6463.66s system 2000% cpu 5:23.42 total
>> >> DragonflyBSD:   11.73s user 1316.76s system 1023% cpu 2:09.78 total
>> >> OmniosCE:       9.17s user 516.53s system 1550% cpu 33.905 total
>> >>
>> >> NetBSD failed to complete the run, OOM-killing workers:
>> >> http://mail-index.netbsd.org/tech-kern/2023/10/19/msg029242.html
>> >> OpenBSD is shafted by a big kernel lock, so no surprise it takes a
>> >> long
>> >> time.
>> >>
>> >> So what I find funny is that Linux needed more time than OmniosCE (an
>> >> Illumos variant, fork of Solaris).
>> >>
>> >> It also needed more time than FreeBSD, which is not necessarily funny
>> >> but not that great either.
>> >>
>> >> All systems were mostly busy contending on locks and in particular
>> >> Linux
>> >> was almost exclusively busy waiting on inode hash lock.
>> >
>> > Did you bother to test the patch, or are you just complaining
>> > that nobody has already done the work for you?
>>
>> Why are you giving me attitude?
>
> Look in the mirror, mate.
>
> Starting off with a derogatory statement like:
>
> "In case you can't be arsed, ..."
>
> is a really good way to start a fight.
>
> I don't think anyone working on this stuff couldn't be bothered to
> get their lazy arses off their couches to get it merged. Though you
> may not have intended it that way, that's exactly what "can't be
> arsed" means.
>
> I have not asked for this code to be merged because I'm not ready to
> ask for it to be merged. I'm trying to be careful and cautious about
> changing core kernel code that every linux installation out there
> uses because I care about this code being robust and stable. That's
> the exact opposite of "can't be arsed"....
>
> Further, you have asked for code that is not ready to be merged to
> be merged without reviewing it or even testing it to see if it
> solved your reported problem. This is pretty basic stuff - it you
> want it merged, then *you also need to put effort into getting it
> merged* regardless of who wrote the code. TANSTAAFL.
>
> But you've done neither - you've just made demands and thrown
> hypocritical shade implying busy people working on complex code are
> lazy arses.
>

So I took few days to take a look at this with a fresh eye and I see
where the major disconnect is coming from, albeit still don't see how
it came to be nor why it persists.

To my understanding your understanding is that I demand you carry the
hash bl patch over the finish line and I'm rude about it as well.

That is not my position here though.

For starters my opening e-mail was to Christian, not you. You are
CC'ed as the patch author. It is responding to an e-mail which claimed
the patch would land in -next, which to my poking around did not
happen (and I checked it's not in master either). Since there was no
other traffic about it that I could find, I figured it was probably
forgotten. You may also notice the e-mail explicitly states:
1. I have a case which runs into inode hash being a problem
2. *there is no urgency*, I'm just asking what's up with the patch not
getting anywhere.

The follow up including a statement about "being arsed" once more was
to Christian, not you and was rather "tongue in cheek".

If you know about Illumos, it is mostly slow and any serious
performance work stopped there when Oracle closed the codebase over a
decade ago. Or to put it differently, one has to be doing something
really bad to not be faster today. And there was this bad -- the inode
hash. I found it amusing and decided to share in addition to asking
about the patch.

So no Dave, I'm not claiming the patch is not in because anyone is lazy.

Whether the patch is ready for reviews and whatnot is your call to
make as the author.

To repeat from my previous e-mail I note the lock causes real problems
in a real-world setting, it's not just microbenchmarks, but I'm in no
position to test it against the actual workload (only the part I
carved out into a benchmark, where it does help -- gets rid of the
nasty back-to-back lock acquire, first to search for the inode and
then to insert a new one).

If your assessment is that more testing is needed, that makes sense
and is again your call to make. I repeat again I can't help with this
bit though. And if you don't think the effort is justified at the
moment (or there are other things with higher priority), so be it.

It may be I'll stick around in general and if so it may be I'm going
to run into you again.
With this in mind:

> Perhaps you should consider your words more carefully in future?
>

On that front perhaps you could refrain from assuming someone is
trying to call you names or whatnot. But more importantly if you
consider an e-mail to be rude, you can call it out instead of
escalating or responding in what you consider to be the same tone.

All that said I'm bailing from this patchset.

Cheers,
-- 
Mateusz Guzik <mjguzik gmail.com>

  reply	other threads:[~2023-10-27 17:13 UTC|newest]

Thread overview: 207+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-09 16:56 [PATCH 00/32] bcachefs - a new COW filesystem Kent Overstreet
2023-05-09 16:56 ` [PATCH 01/32] Compiler Attributes: add __flatten Kent Overstreet
2023-05-09 17:04   ` Miguel Ojeda
2023-05-09 17:24     ` Kent Overstreet
2023-05-09 16:56 ` [PATCH 02/32] locking/lockdep: lock_class_is_held() Kent Overstreet
2023-05-09 19:30   ` Peter Zijlstra
2023-05-09 20:11     ` Kent Overstreet
2023-05-09 16:56 ` [PATCH 03/32] locking/lockdep: lockdep_set_no_check_recursion() Kent Overstreet
2023-05-09 19:31   ` Peter Zijlstra
2023-05-09 19:57     ` Kent Overstreet
2023-05-09 20:18     ` Kent Overstreet
2023-05-09 20:27       ` Waiman Long
2023-05-09 20:35         ` Kent Overstreet
2023-05-09 21:37           ` Waiman Long
2023-05-10  8:59       ` Peter Zijlstra
2023-05-10 20:38         ` Kent Overstreet
2023-05-11  8:25           ` Peter Zijlstra
2023-05-11  9:32             ` Kent Overstreet
2023-05-12 20:49         ` Kent Overstreet
2023-05-09 16:56 ` [PATCH 04/32] locking: SIX locks (shared/intent/exclusive) Kent Overstreet
2023-05-11 12:14   ` Jan Engelhardt
2023-05-12 20:58     ` Kent Overstreet
2023-05-12 22:39       ` Jan Engelhardt
2023-05-12 23:26         ` Kent Overstreet
2023-05-12 23:49           ` Randy Dunlap
2023-05-13  0:17             ` Kent Overstreet
2023-05-13  0:45               ` Eric Biggers
2023-05-13  0:51                 ` Kent Overstreet
2023-05-14 12:15   ` Jeff Layton
2023-05-15  2:39     ` Kent Overstreet
2023-05-09 16:56 ` [PATCH 05/32] MAINTAINERS: Add entry for six locks Kent Overstreet
2023-05-09 16:56 ` [PATCH 06/32] sched: Add task_struct->faults_disabled_mapping Kent Overstreet
2023-05-10  1:07   ` Jan Kara
2023-05-10  6:18     ` Kent Overstreet
2023-05-23 13:34       ` Jan Kara
2023-05-23 13:34         ` [Cluster-devel] " Jan Kara
2023-05-23 16:21         ` Christoph Hellwig
2023-05-23 16:21           ` Christoph Hellwig
2023-05-23 16:35           ` Kent Overstreet
2023-05-23 16:35             ` Kent Overstreet
2023-05-24  6:43             ` Christoph Hellwig
2023-05-24  6:43               ` Christoph Hellwig
2023-05-24  8:09               ` Kent Overstreet
2023-05-24  8:09                 ` Kent Overstreet
2023-05-25  8:58                 ` Christoph Hellwig
2023-05-25  8:58                   ` Christoph Hellwig
2023-05-25 20:50                   ` Kent Overstreet
2023-05-25 20:50                     ` Kent Overstreet
2023-05-26  8:06                     ` Christoph Hellwig
2023-05-26  8:06                       ` Christoph Hellwig
2023-05-26  8:34                       ` Kent Overstreet
2023-05-26  8:34                         ` Kent Overstreet
2023-05-25 21:40                   ` Kent Overstreet
2023-05-25 21:40                     ` Kent Overstreet
2023-05-25 22:25           ` Andreas Grünbacher
2023-05-25 22:25             ` Andreas Grünbacher
2023-05-25 23:20             ` Kent Overstreet
2023-05-25 23:20               ` Kent Overstreet
2023-05-26  0:05               ` Andreas Grünbacher
2023-05-26  0:05                 ` Andreas Grünbacher
2023-05-26  0:39                 ` Kent Overstreet
2023-05-26  0:39                   ` Kent Overstreet
2023-05-26  8:10               ` Christoph Hellwig
2023-05-26  8:10                 ` Christoph Hellwig
2023-05-26  8:38                 ` Kent Overstreet
2023-05-26  8:38                   ` Kent Overstreet
2023-05-23 16:49         ` Kent Overstreet
2023-05-23 16:49           ` [Cluster-devel] " Kent Overstreet
2023-05-25  8:47           ` Jan Kara
2023-05-25  8:47             ` [Cluster-devel] " Jan Kara
2023-05-25 21:36             ` Kent Overstreet
2023-05-25 21:36               ` [Cluster-devel] " Kent Overstreet
2023-05-25 22:45             ` Andreas Grünbacher
2023-05-25 22:45               ` [Cluster-devel] " Andreas Grünbacher
2023-05-25 22:04         ` Andreas Grünbacher
2023-05-25 22:04           ` [Cluster-devel] " Andreas Grünbacher
2023-05-09 16:56 ` [PATCH 07/32] mm: Bring back vmalloc_exec Kent Overstreet
2023-05-09 18:19   ` Lorenzo Stoakes
2023-05-09 20:15     ` Kent Overstreet
2023-05-09 20:46   ` Christoph Hellwig
2023-05-09 21:12     ` Lorenzo Stoakes
2023-05-09 21:29       ` Kent Overstreet
2023-05-10  6:48         ` Eric Biggers
2023-05-12 18:36           ` Kent Overstreet
2023-05-13  1:57             ` Eric Biggers
2023-05-13 19:28               ` Kent Overstreet
2023-05-14  5:45               ` Kent Overstreet
2023-05-14 18:43                 ` Eric Biggers
2023-05-15  5:38                   ` Kent Overstreet
2023-05-15  6:13                     ` Eric Biggers
2023-05-15  6:18                       ` Kent Overstreet
2023-05-15  7:13                         ` Eric Biggers
2023-05-15  7:26                           ` Kent Overstreet
2023-05-21 21:33                             ` Eric Biggers
2023-05-21 22:04                               ` Kent Overstreet
2023-05-15 10:29                 ` David Laight
2023-05-10 11:56         ` David Laight
2023-05-09 21:43       ` Darrick J. Wong
2023-05-09 21:54         ` Kent Overstreet
2023-05-11  5:33           ` Theodore Ts'o
2023-05-11  5:44             ` Kent Overstreet
2023-05-13 13:25       ` Lorenzo Stoakes
2023-05-14 18:39         ` Christophe Leroy
2023-05-14 23:43           ` Kent Overstreet
2023-05-15  4:45             ` Christophe Leroy
2023-05-15  5:02               ` Kent Overstreet
2023-05-10 14:18   ` Christophe Leroy
2023-05-10 15:05   ` Johannes Thumshirn
2023-05-11 22:28     ` Kees Cook
2023-05-12 18:41       ` Kent Overstreet
2023-05-16 21:02         ` Kees Cook
2023-05-16 21:20           ` Kent Overstreet
2023-05-16 21:47             ` Matthew Wilcox
2023-05-16 21:57               ` Kent Overstreet
2023-05-17  5:28               ` Kent Overstreet
2023-05-17 14:04                 ` Mike Rapoport
2023-05-17 14:18                   ` Kent Overstreet
2023-05-17 15:44                     ` Mike Rapoport
2023-05-17 15:59                       ` Kent Overstreet
2023-06-17  4:13             ` Andy Lutomirski
2023-06-17 15:34               ` Kent Overstreet
2023-06-17 19:19                 ` Andy Lutomirski
2023-06-17 20:08                   ` Kent Overstreet
2023-06-17 20:35                     ` Andy Lutomirski
2023-06-19 19:45                 ` Kees Cook
2023-06-20  0:39                   ` Kent Overstreet
2023-06-19  9:19   ` Mark Rutland
2023-06-19 10:47     ` Kent Overstreet
2023-06-19 12:47       ` Mark Rutland
2023-06-19 19:17         ` Kent Overstreet
2023-06-20 17:42           ` Andy Lutomirski
2023-06-20 18:08             ` Kent Overstreet
2023-06-20 18:15               ` Andy Lutomirski
2023-06-20 18:48                 ` Dave Hansen
2023-06-20 20:18                   ` Kent Overstreet
2023-06-20 20:42                   ` Andy Lutomirski
2023-06-20 22:32                     ` Andy Lutomirski
2023-06-20 22:43                       ` Nadav Amit
2023-06-21  1:27                         ` Andy Lutomirski
2023-05-09 16:56 ` [PATCH 08/32] fs: factor out d_mark_tmpfile() Kent Overstreet
2023-05-09 16:56 ` [PATCH 09/32] block: Add some exports for bcachefs Kent Overstreet
2023-05-09 16:56 ` [PATCH 10/32] block: Allow bio_iov_iter_get_pages() with bio->bi_bdev unset Kent Overstreet
2023-05-09 16:56 ` [PATCH 11/32] block: Bring back zero_fill_bio_iter Kent Overstreet
2023-05-09 16:56 ` [PATCH 12/32] block: Rework bio_for_each_segment_all() Kent Overstreet
2023-05-09 16:56 ` [PATCH 13/32] block: Rework bio_for_each_folio_all() Kent Overstreet
2023-05-09 16:56 ` [PATCH 14/32] block: Don't block on s_umount from __invalidate_super() Kent Overstreet
2023-05-09 16:56 ` [PATCH 15/32] bcache: move closures to lib/ Kent Overstreet
2023-05-10  1:10   ` Randy Dunlap
2023-05-09 16:56 ` [PATCH 16/32] MAINTAINERS: Add entry for closures Kent Overstreet
2023-05-09 17:05   ` Coly Li
2023-05-09 21:03   ` Randy Dunlap
2023-05-09 16:56 ` [PATCH 17/32] closures: closure_wait_event() Kent Overstreet
2023-05-09 16:56 ` [PATCH 18/32] closures: closure_nr_remaining() Kent Overstreet
2023-05-09 16:56 ` [PATCH 19/32] closures: Add a missing include Kent Overstreet
2023-05-09 16:56 ` [PATCH 20/32] vfs: factor out inode hash head calculation Kent Overstreet
2023-05-23  9:27   ` (subset) " Christian Brauner
2023-05-23 22:53     ` Dave Chinner
2023-05-24  6:44       ` Christoph Hellwig
2023-05-24  7:35         ` Dave Chinner
2023-05-24  8:31           ` Christian Brauner
2023-05-24  8:41             ` Kent Overstreet
2023-05-09 16:56 ` [PATCH 21/32] hlist-bl: add hlist_bl_fake() Kent Overstreet
2023-05-10  4:48   ` Dave Chinner
2023-05-23  9:27   ` (subset) " Christian Brauner
2023-05-09 16:56 ` [PATCH 22/32] vfs: inode cache conversion to hash-bl Kent Overstreet
2023-05-10  4:45   ` Dave Chinner
2023-05-16 15:45     ` Christian Brauner
2023-05-16 16:17       ` Kent Overstreet
2023-05-16 23:15         ` Dave Chinner
2023-05-22 13:04           ` Christian Brauner
2023-05-23  9:28   ` (subset) " Christian Brauner
2023-10-19 15:30     ` Mateusz Guzik
2023-10-19 15:59       ` Mateusz Guzik
2023-10-20 11:38         ` Dave Chinner
2023-10-20 17:49           ` Mateusz Guzik
2023-10-21 12:13             ` Mateusz Guzik
2023-10-23  5:10             ` Dave Chinner
2023-10-27 17:13               ` Mateusz Guzik [this message]
2023-10-27 18:36                 ` Darrick J. Wong
2023-10-31 11:02                 ` Christian Brauner
2023-10-31 11:31                   ` Mateusz Guzik
2023-11-02  2:36                   ` Kent Overstreet
2023-11-04 20:51                     ` Dave Chinner
2023-05-09 16:56 ` [PATCH 23/32] iov_iter: copy_folio_from_iter_atomic() Kent Overstreet
2023-05-10  2:20   ` kernel test robot
2023-05-11  2:08   ` kernel test robot
2023-05-09 16:56 ` [PATCH 24/32] MAINTAINERS: Add entry for generic-radix-tree Kent Overstreet
2023-05-09 21:03   ` Randy Dunlap
2023-05-09 16:56 ` [PATCH 25/32] lib/generic-radix-tree.c: Don't overflow in peek() Kent Overstreet
2023-05-09 16:56 ` [PATCH 26/32] lib/generic-radix-tree.c: Add a missing include Kent Overstreet
2023-05-09 16:56 ` [PATCH 27/32] lib/generic-radix-tree.c: Add peek_prev() Kent Overstreet
2023-05-09 16:56 ` [PATCH 28/32] stacktrace: Export stack_trace_save_tsk Kent Overstreet
2023-06-19  9:10   ` Mark Rutland
2023-06-19 11:16     ` Kent Overstreet
2023-05-09 16:56 ` [PATCH 29/32] lib/string_helpers: string_get_size() now returns characters wrote Kent Overstreet
2023-07-12 19:58   ` Kees Cook
2023-07-12 20:19     ` Kent Overstreet
2023-07-12 22:38       ` Kees Cook
2023-07-12 23:53         ` Kent Overstreet
2023-07-12 20:23     ` Kent Overstreet
2023-05-09 16:56 ` [PATCH 30/32] lib: Export errname Kent Overstreet
2023-05-09 16:56 ` [PATCH 31/32] lib: add mean and variance module Kent Overstreet
2023-05-09 16:56 ` [PATCH 32/32] MAINTAINERS: Add entry for bcachefs Kent Overstreet
2023-05-09 21:04   ` Randy Dunlap
2023-05-09 21:07     ` Kent Overstreet
2023-06-15 20:41 ` [PATCH 00/32] bcachefs - a new COW filesystem Pavel Machek
2023-06-15 21:26   ` Kent Overstreet

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=CAGudoHGzX2H4pUuDNYzYOf8s-HaZuAi7Dttpg_SqtXAgTw8tiw@mail.gmail.com \
    --to=mjguzik@gmail.com \
    --cc=brauner@kernel.org \
    --cc=david@fromorbit.com \
    --cc=dchinner@redhat.com \
    --cc=kent.overstreet@linux.dev \
    --cc=linux-bcachefs@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.