Linux-mm Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: "Theodore Tso" <tytso@mit.edu>
To: Jaegeuk Kim <jaegeuk@kernel.org>
Cc: Christoph Hellwig <hch@infradead.org>,
	linux-api@vger.kernel.org, linux-kernel@vger.kernel.org,
	Matthew Wilcox <willy@infradead.org>,
	linux-f2fs-devel@lists.sourceforge.net, linux-mm@kvack.org,
	linux-fsdevel@vger.kernel.org,
	Akilesh Kailash <akailash@google.com>,
	Christian Brauner <christian@brauner.io>
Subject: Re: [f2fs-dev] [PATCH v2] f2fs: another way to set large folio by remembering inode number
Date: Fri, 22 May 2026 10:11:15 -0400	[thread overview]
Message-ID: <20260522141115.GA8258@macsyma-wired.lan> (raw)
In-Reply-To: <ag_OVwPF49LSZ7rz@google.com>

On Fri, May 22, 2026 at 03:32:39AM +0000, Jaegeuk Kim wrote:
> I went this route because Android heavily restricts ioctl() permissions
> and we needed broader access for this to work within the framework. It’s
> definitely a pragmatic choice just to get it running in production.
> 
> If ioctl() is a right way for upstream, I'm happy to change this patch. By
> the way, I really don't understand why all the messages are so offensive,
> even without trying to understand the problem or guiding right directions.

The reason why some people were getting annoyed was because as a Linux
file system maintainer, there was an assumption that you would
understand that extended attributes --- especially in the user.*
namespace --- have an intended use case of storing a user-chosen small
piece of metadata that would be stored in the file system.

Hijacking user.fadvise such that it no longer persistent stores an
extended attribute for one specific file system --- such that if a
hypothetical user application might decide to store a piece of
application data in the extended attribute named "fadvise" would do
something completely different on a single mainline file system is in
such poor taste that I would have *hoped* that any Linux file system
maintainer would know that this a Really Bad Thing, such that if
someone in your development community suggested such an idea, you
would reject it.

And then, when people complained that it was a bad idea, and you
decided to put in the f2fs branch, such that it would show up in
linux-next, and there was no way for other file system developers to
object (short of appealing to Linus) --- well, that's basically you
taking advantage of your file system maintainer privileges.  And this
is why I started proposing whether we needed to appeal this to Linus
so he could make the call to reject something that the community had
already told you was in terrible, terrible taste.

As far as trying to understand why you were doing this --- I have to
turn that question around.  Why didn't *you* explain why you needed to
do this thing?  I went through the e-mail history, and I couldn't find
an explanation of why you decided to do this thing.  

Perhaps we need to add an explanation the Documentation directory
explaining what the intended use of the extended attribute case, and
perhaps referencing past cases were people tried to use this to bypass
the linux-api review process (f2fs's user.fadvise is not the first
time someone has tried to do this) thing), so that automated review
bots like Sashiko can explain why it's in such terrible taste to patch
authors, perhaps we need to do this.  Up until now, I think the
assumption is that file system maintainers would know something this
self-evident, and if not, if it was pointed out, they wouldn't try to
force such an ill-advised interface to Linus.

						- Ted

P.S.  As an exmaple of how I hanlded a somewhat similar scenario in
the past, my employer's cluster file system needed
FALLOC_FL_NO_HIDE_STALE to save $$$$ in TCO storage costs.  But the
concern was this would be an attractive nuisance for enterprise distro
users, who would see the massive performance increase, not realize
that this would leak stale data, which could result in user PII being
exposed, thus making life hard for Enterprise Linux's reputation.
(This wasn't an issue at $WORK because we enrypt all data at rest, and
the cluster file system daemon was a privileged server who (a) knew
what it was doing, and (b) only it would have access to set
FALLOC_FL_NO_HIDE_STALE.)

I disclosed *why* $WORK needed such a thing (it made a huge difference
to storage TCO costss for Google's Cluster Filesystem), and after
discussion and negotiation, we came to a compromise which involved my
keeping the (very small) patch out of tree, but reserving the code
point upstream to avoid future bitfield collisions.  They key here was
that I *knew* it was controversial, and I understood what problems it
might cause in the rest of the ecosystem.  That's part of the job of a
maintainer, and it's also why a company might want to hire a
maintainer.  They can represent the needs of multiple stakeholders ---
the upstream community, upstream users and the greater Linux
ecosystem, as well as their employer.


  parent reply	other threads:[~2026-05-22 14:11 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20260409134538.3692605-1-jaegeuk@kernel.org>
     [not found] ` <adhPZxtbZxgU-37v@google.com>
2026-04-14  8:02   ` [PATCH v2] f2fs: another way to set large folio by remembering inode number Christoph Hellwig
2026-04-15 16:44     ` Jaegeuk Kim
2026-04-15 17:15       ` Matthew Wilcox
2026-04-15 22:02         ` Jaegeuk Kim
2026-04-15 23:49           ` Darrick J. Wong
2026-04-16  1:19             ` Jaegeuk Kim
2026-05-21  8:51         ` Christoph Hellwig
2026-05-21 15:57           ` Theodore Tso
2026-05-21 17:42             ` Matthew Wilcox
2026-05-22  3:59               ` Jaegeuk Kim
2026-05-22 12:55                 ` Matthew Wilcox
2026-05-22 14:04                   ` [f2fs-dev] " Jaegeuk Kim
2026-05-22  3:32             ` Jaegeuk Kim
2026-05-22  3:53               ` Eric Biggers
2026-05-22  4:02                 ` Jaegeuk Kim
2026-05-22 10:01                 ` Christian Brauner
2026-05-22 14:11               ` Theodore Tso [this message]
2026-05-22 17:08                 ` Jaegeuk Kim
2026-05-22 22:41                   ` Theodore Tso
2026-05-22  9:59             ` Christian Brauner

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=20260522141115.GA8258@macsyma-wired.lan \
    --to=tytso@mit.edu \
    --cc=akailash@google.com \
    --cc=christian@brauner.io \
    --cc=hch@infradead.org \
    --cc=jaegeuk@kernel.org \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-f2fs-devel@lists.sourceforge.net \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=willy@infradead.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