* Re: [PATCH 32/32] fs: Drop i_private_list from address_space
From: Christoph Hellwig @ 2026-03-04 13:43 UTC (permalink / raw)
To: Jan Kara
Cc: linux-fsdevel, Christian Brauner, Al Viro, linux-ext4, Ted Tso,
Tigran A. Aivazian, David Sterba, OGAWA Hirofumi, Muchun Song,
Oscar Salvador, David Hildenbrand, linux-mm, linux-aio,
Benjamin LaHaise
In-Reply-To: <20260303103406.4355-64-jack@suse.cz>
On Tue, Mar 03, 2026 at 11:34:21AM +0100, Jan Kara wrote:
> Nobody is using i_private_list anymore. Remove it.
Thanks. If my math isn't off, this should allow us to fit an extra inode
per slab for xfs for typical configurations.
^ permalink raw reply
* Re: [PATCH 31/32] kvm: Use private inode list instead of i_private_list
From: Christoph Hellwig @ 2026-03-04 13:42 UTC (permalink / raw)
To: Jan Kara
Cc: linux-fsdevel, Christian Brauner, Al Viro, linux-ext4, Ted Tso,
Tigran A. Aivazian, David Sterba, OGAWA Hirofumi, Muchun Song,
Oscar Salvador, David Hildenbrand, linux-mm, linux-aio,
Benjamin LaHaise
In-Reply-To: <20260303103406.4355-63-jack@suse.cz>
On Tue, Mar 03, 2026 at 11:34:20AM +0100, Jan Kara wrote:
> Instead of using mapping->i_private_list use a list in private part of
> the inode.
Similarly here, I'd be this toward the front with all the other
switching from i_private_list to private members.
^ permalink raw reply
* Re: [PATCH 19/32] ntfs3: Drop pointless sync_mapping_buffers() call
From: Christoph Hellwig @ 2026-03-04 13:41 UTC (permalink / raw)
To: Jan Kara
Cc: linux-fsdevel, Christian Brauner, Al Viro, linux-ext4, Ted Tso,
Tigran A. Aivazian, David Sterba, OGAWA Hirofumi, Muchun Song,
Oscar Salvador, David Hildenbrand, linux-mm, linux-aio,
Benjamin LaHaise, Konstantin Komarov, ntfs3
In-Reply-To: <20260303103406.4355-51-jack@suse.cz>
Can you move the various cleanups that don't really depend on any of
the refactoring to the front of the list? i.e., drop all the useless
code first before doing real changes?
^ permalink raw reply
* Re: [PATCH 31/32] kvm: Use private inode list instead of i_private_list
From: Christian Brauner @ 2026-03-04 13:40 UTC (permalink / raw)
To: Jan Kara
Cc: linux-fsdevel, Al Viro, linux-ext4, Ted Tso, Tigran A. Aivazian,
David Sterba, OGAWA Hirofumi, Muchun Song, Oscar Salvador,
David Hildenbrand, linux-mm, linux-aio, Benjamin LaHaise
In-Reply-To: <20260303103406.4355-63-jack@suse.cz>
On Tue, Mar 03, 2026 at 11:34:20AM +0100, Jan Kara wrote:
> Instead of using mapping->i_private_list use a list in private part of
> the inode.
>
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
> virt/kvm/guest_memfd.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index 017d84a7adf3..6d36a7827870 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
> @@ -30,6 +30,7 @@ struct gmem_file {
> struct gmem_inode {
> struct shared_policy policy;
> struct inode vfs_inode;
> + struct list_head gem_file_list;
I think that needs to be gMem_file_list not, gem_file_list.
^ permalink raw reply
* Re: [PATCH 17/32] fs: Move metadata bhs tracking to a separate struct
From: Christoph Hellwig @ 2026-03-04 13:40 UTC (permalink / raw)
To: Jan Kara
Cc: linux-fsdevel, Christian Brauner, Al Viro, linux-ext4, Ted Tso,
Tigran A. Aivazian, David Sterba, OGAWA Hirofumi, Muchun Song,
Oscar Salvador, David Hildenbrand, linux-mm, linux-aio,
Benjamin LaHaise
In-Reply-To: <20260303103406.4355-49-jack@suse.cz>
On Tue, Mar 03, 2026 at 11:34:06AM +0100, Jan Kara wrote:
> +static void remove_assoc_queue(struct buffer_head *bh)
> +{
> + struct address_space *mapping;
> + struct mapping_metadata_bhs *mmb;
> +
> + /*
> + * The locking dance is ugly here. We need to acquire lock
> + * protecting metadata bh list while possibly racing with bh
> + * being removed from the list or moved to a different one. We
> + * use RCU to pin mapping_metadata_bhs in memory to
> + * opportunistically acquire the lock and then recheck the bh
> + * didn't move under us.
> + */
Should the buffer_head simply have a pointer to the metadata bh list,
as that would avoid all this and keep a lot of the references to the
list self-contained?
^ permalink raw reply
* Re: [PATCH 21/32] bdev: Drop pointless invalidate_mapping_buffers() call
From: Christian Brauner @ 2026-03-04 13:39 UTC (permalink / raw)
To: Jan Kara
Cc: linux-fsdevel, Al Viro, linux-ext4, Ted Tso, Tigran A. Aivazian,
David Sterba, OGAWA Hirofumi, Muchun Song, Oscar Salvador,
David Hildenbrand, linux-mm, linux-aio, Benjamin LaHaise,
Jens Axboe, linux-block
In-Reply-To: <20260303103406.4355-53-jack@suse.cz>
On Tue, Mar 03, 2026 at 11:34:10AM +0100, Jan Kara wrote:
> Nobody is calling mark_buffer_dirty_inode() with internal bdev inode and
> it doesn't make sense for internal bdev inode to have any metadata
> buffer heads. Just drop the pointless invalidate_mapping_buffers() call.
s/invalidate_mapping_buffers/invalidate_inode_buffers/g?
>
> CC: Jens Axboe <axboe@kernel.dk>
> CC: linux-block@vger.kernel.org
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
> block/bdev.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/block/bdev.c b/block/bdev.c
> index ed022f8c48c7..ad1660b6b324 100644
> --- a/block/bdev.c
> +++ b/block/bdev.c
> @@ -420,7 +420,6 @@ static void init_once(void *data)
> static void bdev_evict_inode(struct inode *inode)
> {
> truncate_inode_pages_final(&inode->i_data);
> - invalidate_inode_buffers(inode); /* is it needed here? */
> clear_inode(inode);
> }
>
> --
> 2.51.0
>
^ permalink raw reply
* Re: [PATCH 16/32] fs: Fold fsync_buffers_list() into sync_mapping_buffers()
From: Christian Brauner @ 2026-03-04 13:38 UTC (permalink / raw)
To: Jan Kara
Cc: linux-fsdevel, Al Viro, linux-ext4, Ted Tso, Tigran A. Aivazian,
David Sterba, OGAWA Hirofumi, Muchun Song, Oscar Salvador,
David Hildenbrand, linux-mm, linux-aio, Benjamin LaHaise
In-Reply-To: <20260303103406.4355-48-jack@suse.cz>
On Tue, Mar 03, 2026 at 11:34:05AM +0100, Jan Kara wrote:
> There's only single caller of fsync_buffers_list() so untangle the code
> a bit by folding fsync_buffers_list() into sync_mapping_buffers(). Also
> merge the comments and update them to reflect current state of code.
>
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
> fs/buffer.c | 180 +++++++++++++++++++++++-----------------------------
> 1 file changed, 80 insertions(+), 100 deletions(-)
>
> diff --git a/fs/buffer.c b/fs/buffer.c
> index 1c0e7c81a38b..18012afb8289 100644
> --- a/fs/buffer.c
> +++ b/fs/buffer.c
> @@ -54,7 +54,6 @@
>
> #include "internal.h"
>
> -static int fsync_buffers_list(spinlock_t *lock, struct list_head *list);
> static void submit_bh_wbc(blk_opf_t opf, struct buffer_head *bh,
> enum rw_hint hint, struct writeback_control *wbc);
>
> @@ -531,22 +530,96 @@ EXPORT_SYMBOL_GPL(inode_has_buffers);
> * @mapping: the mapping which wants those buffers written
> *
> * Starts I/O against the buffers at mapping->i_private_list, and waits upon
> - * that I/O.
> + * that I/O. Basically, this is a convenience function for fsync(). @mapping
> + * is a file or directory which needs those buffers to be written for a
> + * successful fsync().
> *
> - * Basically, this is a convenience function for fsync().
> - * @mapping is a file or directory which needs those buffers to be written for
> - * a successful fsync().
> + * We have conflicting pressures: we want to make sure that all
> + * initially dirty buffers get waited on, but that any subsequently
> + * dirtied buffers don't. After all, we don't want fsync to last
> + * forever if somebody is actively writing to the file.
> + *
> + * Do this in two main stages: first we copy dirty buffers to a
> + * temporary inode list, queueing the writes as we go. Then we clean
> + * up, waiting for those writes to complete. mark_buffer_dirty_inode()
> + * doesn't touch b_assoc_buffers list if b_assoc_map is not NULL so we
> + * are sure the buffer stays on our list until IO completes (at which point
> + * it can be reaped).
> */
> int sync_mapping_buffers(struct address_space *mapping)
> {
> struct address_space *buffer_mapping =
> mapping->host->i_sb->s_bdev->bd_mapping;
> + struct buffer_head *bh;
> + int err = 0;
> + struct blk_plug plug;
> + LIST_HEAD(tmp);
>
> if (list_empty(&mapping->i_private_list))
> return 0;
>
> - return fsync_buffers_list(&buffer_mapping->i_private_lock,
> - &mapping->i_private_list);
> + blk_start_plug(&plug);
> +
> + spin_lock(&buffer_mapping->i_private_lock);
> + while (!list_empty(&mapping->i_private_list)) {
> + bh = BH_ENTRY(list->next);
Stray "list" reference? Shouldn't this be
BH_ENTRY(mapping->i_private_list.next)?
^ permalink raw reply
* Re: [PATCH 18/32] fs: Provide operation for fetching mapping_metadata_bhs
From: Jan Kara @ 2026-03-04 13:38 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jan Kara, linux-fsdevel, Christian Brauner, Al Viro, linux-ext4,
Ted Tso, Tigran A. Aivazian, David Sterba, OGAWA Hirofumi,
Muchun Song, Oscar Salvador, David Hildenbrand, linux-mm,
linux-aio, Benjamin LaHaise
In-Reply-To: <aagxU5f1AI3y0wrw@infradead.org>
On Wed 04-03-26 05:19:15, Christoph Hellwig wrote:
> On Tue, Mar 03, 2026 at 11:34:07AM +0100, Jan Kara wrote:
> > When we move mapping_metadata_bhs to fs-private part of an inode the
> > generic code will need a way to get to this struct from general struct
> > inode. Add inode operation for this similarly to operation for grabbing
> > offset_ctx.
>
> Do we even need this? With your previous cleanups almost all of the
> places that need the buffers list are called more or less directly
> from the file systems. Can we take it all the way and just pass the
> mapping_metadata_bhs to those functions?
I was looking into that. Passing these to sync_mapping_buffers(),
invalidate_inode_buffers() is easy. Passing to mark_buffer_dirty_inode() is
relatively tedious but doable. Where it gets difficult are calls like
bforget() and most importantly try_to_free_buffers() on bdev mapping where
you currently have no way to get to the mmb struct... We do have
b_assoc_map pointer in buffer_head which we could switch to point to mmb
instead but IO error handling on bhs needs to get to the mapping from bh so
we'd then have to add address_space pointer to mmb for these uses. All in
all it's doable but I've decided it isn't really worth it.
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH 17/32] fs: Move metadata bhs tracking to a separate struct
From: Christoph Hellwig @ 2026-03-04 13:38 UTC (permalink / raw)
To: Jan Kara
Cc: linux-fsdevel, Christian Brauner, Al Viro, linux-ext4, Ted Tso,
Tigran A. Aivazian, David Sterba, OGAWA Hirofumi, Muchun Song,
Oscar Salvador, David Hildenbrand, linux-mm, linux-aio,
Benjamin LaHaise
In-Reply-To: <20260303103406.4355-49-jack@suse.cz>
Maybe just call the structure buffer_head_list at it really just is a
totall generic list of buffer heads with a lock?
^ permalink raw reply
* Re: [PATCH 21/32] bdev: Drop pointless invalidate_mapping_buffers() call
From: Christoph Hellwig @ 2026-03-04 13:29 UTC (permalink / raw)
To: Jan Kara
Cc: Christoph Hellwig, linux-fsdevel, Christian Brauner, Al Viro,
linux-ext4, Ted Tso, Tigran A. Aivazian, David Sterba,
OGAWA Hirofumi, Muchun Song, Oscar Salvador, David Hildenbrand,
linux-mm, linux-aio, Benjamin LaHaise, Jens Axboe, linux-block
In-Reply-To: <n3anrkzfguzbn5sfwsom472cx4uzejyfemz3d5jm7c4rt3qr6z@ez2cgwzmbfyi>
On Wed, Mar 04, 2026 at 11:36:29AM +0100, Jan Kara wrote:
> On Tue 03-03-26 06:09:03, Christoph Hellwig wrote:
> > FYI, linux-block only got this patch which is totally messed up.
> > Please always send all patches to every list and person, otherwise
> > you fill peoples inboxes with unreviewable junk.
>
> Well, I've CCed on the whole series everybody who was non-trivially
> impacted. But there are couple of these trivial "remove effectively dead
> code" patches which stand on their own and a lot of people actually prefer
> to only get individual patches in such cases. So I don't plan on changing
> that but I guess I could have CCed linux-block on the whole series as
> buffer_heads are tangentially related to block layer anyway.
Ccing people or lists on just part of a series is always broken, don't
do that. If you think something is just a FYI only CC the list to cut
down the spam.
^ permalink raw reply
* Re: [PATCH 0/3] fix s_uuid and f_fsid consistency for cloned filesystems
From: Christoph Hellwig @ 2026-03-04 13:28 UTC (permalink / raw)
To: Anand Jain; +Cc: linux-btrfs, linux-ext4
In-Reply-To: <cover.1772095546.git.asj@kernel.org>
On Thu, Feb 26, 2026 at 10:23:32PM +0800, Anand Jain wrote:
> This series resolves the tradeoff by aligning btrfs and ext4 behaviour
> with XFS: f_fsid incorporates device identity (devt) to remain unique
> across clones, while s_uuid is preserved consistently matching the on-disk
> uuid.
While I like fixing this up, switching the f_fsid construction to a
different method might break things. Is there a way to only change
it for cloned file systems to reduce the surface of this change?
> Patches
> -------
> Patch 1/3: btrfs: fix f_fsid to include rootid and devt
> Patch 2/3: btrfs: fix s_uuid to be stable across mounts for cloned filesystems
> Patch 3/3: ext4: fix f_fsid to use devt instead of s_uuid
I don't really see that patch 3 in my inbox on linux-btrfs.
^ permalink raw reply
* Re: [PATCH 18/32] fs: Provide operation for fetching mapping_metadata_bhs
From: Christoph Hellwig @ 2026-03-04 13:19 UTC (permalink / raw)
To: Jan Kara
Cc: linux-fsdevel, Christian Brauner, Al Viro, linux-ext4, Ted Tso,
Tigran A. Aivazian, David Sterba, OGAWA Hirofumi, Muchun Song,
Oscar Salvador, David Hildenbrand, linux-mm, linux-aio,
Benjamin LaHaise
In-Reply-To: <20260303103406.4355-50-jack@suse.cz>
On Tue, Mar 03, 2026 at 11:34:07AM +0100, Jan Kara wrote:
> When we move mapping_metadata_bhs to fs-private part of an inode the
> generic code will need a way to get to this struct from general struct
> inode. Add inode operation for this similarly to operation for grabbing
> offset_ctx.
Do we even need this? With your previous cleanups almost all of the
places that need the buffers list are called more or less directly
from the file systems. Can we take it all the way and just pass the
mapping_metadata_bhs to those functions?
^ permalink raw reply
* Re: [PATCH 18/32] fs: Provide operation for fetching mapping_metadata_bhs
From: Christian Brauner @ 2026-03-04 12:48 UTC (permalink / raw)
To: Jan Kara
Cc: linux-fsdevel, Al Viro, linux-ext4, Ted Tso, Tigran A. Aivazian,
David Sterba, OGAWA Hirofumi, Muchun Song, Oscar Salvador,
David Hildenbrand, linux-mm, linux-aio, Benjamin LaHaise
In-Reply-To: <20260303103406.4355-50-jack@suse.cz>
On Tue, Mar 03, 2026 at 11:34:07AM +0100, Jan Kara wrote:
> When we move mapping_metadata_bhs to fs-private part of an inode the
> generic code will need a way to get to this struct from general struct
> inode. Add inode operation for this similarly to operation for grabbing
> offset_ctx.
>
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
Yeah, it's a good enough trade-off, I think.
^ permalink raw reply
* Re: [PATCH 0/8] mm: globalize rest_of_page() macro
From: Peter Zijlstra @ 2026-03-04 12:48 UTC (permalink / raw)
To: Sean Christopherson
Cc: Jens Axboe, Jakub Kicinski, Yury Norov, Andrew Morton,
David S. Miller, Michael S. Tsirkin, Theodore Ts'o, Albert Ou,
Alexander Duyck, Alexander Gordeev, Alexander Viro,
Alexandra Winter, Andreas Dilger, Andrew Lunn, Anna Schumaker,
Anton Yakovlev, Arnaldo Carvalho de Melo, Aswin Karuvally,
Borislav Petkov, Carlos Maiolino, Catalin Marinas, Chao Yu,
Christian Borntraeger, Christian Brauner, Claudio Imbrenda,
Dave Hansen, David Airlie, Dominique Martinet, Dongsheng Yang,
Eric Dumazet, Eric Van Hensbergen, Heiko Carstens, Herbert Xu,
Ingo Molnar, Jaegeuk Kim, Jani Nikula, Janosch Frank,
Jaroslav Kysela, Joonas Lahtinen, Latchesar Ionkov, Linus Walleij,
Madhavan Srinivasan, Mark Brown, Michael Ellerman, Miklos Szeredi,
Namhyung Kim, Palmer Dabbelt, Paolo Abeni, Paolo Bonzini,
Paul Walmsley, Rodrigo Vivi, Simona Vetter, Takashi Iwai,
Thomas Gleixner, Trond Myklebust, Tvrtko Ursulin, Vasily Gorbik,
Will Deacon, Yury Norov, Zheng Gu, linux-kernel, x86,
linux-arm-kernel, linuxppc-dev, linux-riscv, kvm, linux-s390,
linux-block, intel-gfx, dri-devel, dm-devel, netdev, linux-spi,
linux-ext4, linux-f2fs-devel, linux-fsdevel, linux-xfs, linux-nfs,
linux-crypto, linux-mm, linux-perf-users, v9fs, virtualization,
linux-sound
In-Reply-To: <aaedwFwXh9QXS3Ju@google.com>
On Tue, Mar 03, 2026 at 06:49:36PM -0800, Sean Christopherson wrote:
> On Tue, Mar 03, 2026, Jens Axboe wrote:
> > On 3/3/26 7:28 PM, Jakub Kicinski wrote:
> > > On Tue, 3 Mar 2026 20:27:08 -0500 Yury Norov wrote:
> > >> The net/9p networking driver has a handy macro to calculate the
> > >> amount of bytes from a given pointer to the end of page. Move it
> > >> to core/mm, and apply tree-wide. No functional changes intended.
> > >>
> > >> This series was originally introduced as a single patch #07/12 in:
> > >>
> > >> https://lore.kernel.org/all/20260219181407.290201-1-ynorov@nvidia.com/
> > >>
> > >> Split it for better granularity and submit separately.
> > >
> > > I don't get what the motivation is here. Another helper developers
> > > and readers of the code will need to know about just to replace
> > > obvious and easy to comprehend math.
> >
> > I fully agree, I had the same thought reading this.
>
> +1 from KVM-land.
Right, this. I hate these pointless helpers that obscure perfectly
sensible and obvious code -- and for me that includes things like
rounddown() and DIV_ROUND_UP().
It just makes the code harder to read.
^ permalink raw reply
* Re: [PATCH 0/32] fs: Move metadata bh tracking from address_space
From: Christian Brauner @ 2026-03-04 12:32 UTC (permalink / raw)
To: Jan Kara
Cc: linux-fsdevel, Al Viro, linux-ext4, Ted Tso, Tigran A. Aivazian,
David Sterba, OGAWA Hirofumi, Muchun Song, Oscar Salvador,
David Hildenbrand, linux-mm, linux-aio, Benjamin LaHaise
In-Reply-To: <20260303101717.27224-1-jack@suse.cz>
On Tue, Mar 03, 2026 at 11:33:49AM +0100, Jan Kara wrote:
> Hello,
>
> this patch series cleans up the mess that has accumulated over the years in
> metadata buffer_head tracking for inodes, moves the tracking into dedicated
> structure in filesystem-private part of the inode (so that we don't use
> private_list, private_data, and private_lock in struct address_space), and also
> moves couple other users of private_data and private_list so these are removed
> from struct address_space saving 3 longs in struct inode for 99% of inodes. I
Yes! I love it.
^ permalink raw reply
* Re: [PATCH 5/8] spi: use rest_of_page() macro where appropriate
From: Mark Brown @ 2026-03-04 11:10 UTC (permalink / raw)
To: Yury Norov
Cc: Andrew Morton, David S. Miller, Michael S. Tsirkin,
Theodore Ts'o, Albert Ou, Alexander Duyck, Alexander Gordeev,
Alexander Viro, Alexandra Winter, Andreas Dilger, Andrew Lunn,
Anna Schumaker, Anton Yakovlev, Arnaldo Carvalho de Melo,
Aswin Karuvally, Borislav Petkov, Carlos Maiolino,
Catalin Marinas, Chao Yu, Christian Borntraeger,
Christian Brauner, Claudio Imbrenda, Dave Hansen, David Airlie,
Dominique Martinet, Dongsheng Yang, Eric Dumazet,
Eric Van Hensbergen, Heiko Carstens, Herbert Xu, Ingo Molnar,
Jaegeuk Kim, Jakub Kicinski, Jani Nikula, Janosch Frank,
Jaroslav Kysela, Jens Axboe, Joonas Lahtinen, Latchesar Ionkov,
Linus Walleij, Madhavan Srinivasan, Michael Ellerman,
Miklos Szeredi, Namhyung Kim, Palmer Dabbelt, Paolo Abeni,
Paolo Bonzini, Paul Walmsley, Peter Zijlstra, Rodrigo Vivi,
Sean Christopherson, Simona Vetter, Takashi Iwai, Thomas Gleixner,
Trond Myklebust, Tvrtko Ursulin, Vasily Gorbik, Will Deacon,
Yury Norov, Zheng Gu, linux-kernel, x86, linux-arm-kernel,
linuxppc-dev, linux-riscv, kvm, linux-s390, linux-block,
intel-gfx, dri-devel, dm-devel, netdev, linux-spi, linux-ext4,
linux-f2fs-devel, linux-fsdevel, linux-xfs, linux-nfs,
linux-crypto, linux-mm, linux-perf-users, v9fs, virtualization,
linux-sound
In-Reply-To: <20260304012717.201797-6-ynorov@nvidia.com>
[-- Attachment #1: Type: text/plain, Size: 181 bytes --]
On Tue, Mar 03, 2026 at 08:27:13PM -0500, Yury Norov wrote:
> Switch SPI code to using the macro. No functional changes intended.
>
Acked-by: Mark Brown <broonie@kernel.org>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v4 2/2] jbd2: gracefully abort on transaction state corruptions
From: Jan Kara @ 2026-03-04 11:10 UTC (permalink / raw)
To: Milos Nikic; +Cc: jack, tytso, linux-ext4, linux-kernel
In-Reply-To: <20260303180157.53061-3-nikic.milos@gmail.com>
On Tue 03-03-26 10:01:57, Milos Nikic wrote:
> Auditing the jbd2 codebase reveals several legacy J_ASSERT calls
> that enforce internal state machine invariants (e.g., verifying
> jh->b_transaction or jh->b_next_transaction pointers).
>
> When these invariants are broken, the journal is in a corrupted
> state. However, triggering a fatal panic brings down the entire
> system for a localized filesystem error.
>
> This patch targets a specific class of these asserts: those
> residing inside functions that natively return integer error codes,
> booleans, or error pointers. It replaces the hard J_ASSERTs with
> WARN_ON_ONCE to capture the offending stack trace, safely drops
> any held locks, gracefully aborts the journal, and returns -EINVAL.
>
> This prevents a catastrophic kernel panic while ensuring the
> corrupted journal state is safely contained and upstream callers
> (like ext4 or ocfs2) can gracefully handle the aborted handle.
>
> Functions modified in fs/jbd2/transaction.c:
> - jbd2__journal_start()
> - do_get_write_access()
> - jbd2_journal_dirty_metadata()
> - jbd2_journal_forget()
> - jbd2_journal_try_to_free_buffers()
> - jbd2_journal_file_inode()
>
> Signed-off-by: Milos Nikic <nikic.milos@gmail.com>
Looks good. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/jbd2/transaction.c | 112 ++++++++++++++++++++++++++++++++----------
> 1 file changed, 86 insertions(+), 26 deletions(-)
>
> diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
> index 04d17a5f2a82..bae6c99d635c 100644
> --- a/fs/jbd2/transaction.c
> +++ b/fs/jbd2/transaction.c
> @@ -474,7 +474,8 @@ handle_t *jbd2__journal_start(journal_t *journal, int nblocks, int rsv_blocks,
> return ERR_PTR(-EROFS);
>
> if (handle) {
> - J_ASSERT(handle->h_transaction->t_journal == journal);
> + if (WARN_ON_ONCE(handle->h_transaction->t_journal != journal))
> + return ERR_PTR(-EINVAL);
> handle->h_ref++;
> return handle;
> }
> @@ -1036,7 +1037,13 @@ do_get_write_access(handle_t *handle, struct journal_head *jh,
> */
> if (!jh->b_transaction) {
> JBUFFER_TRACE(jh, "no transaction");
> - J_ASSERT_JH(jh, !jh->b_next_transaction);
> + if (WARN_ON_ONCE(jh->b_next_transaction)) {
> + spin_unlock(&jh->b_state_lock);
> + unlock_buffer(bh);
> + error = -EINVAL;
> + jbd2_journal_abort(journal, error);
> + goto out;
> + }
> JBUFFER_TRACE(jh, "file as BJ_Reserved");
> /*
> * Make sure all stores to jh (b_modified, b_frozen_data) are
> @@ -1069,13 +1076,27 @@ do_get_write_access(handle_t *handle, struct journal_head *jh,
> */
> if (jh->b_frozen_data) {
> JBUFFER_TRACE(jh, "has frozen data");
> - J_ASSERT_JH(jh, jh->b_next_transaction == NULL);
> + if (WARN_ON_ONCE(jh->b_next_transaction)) {
> + spin_unlock(&jh->b_state_lock);
> + error = -EINVAL;
> + jbd2_journal_abort(journal, error);
> + goto out;
> + }
> goto attach_next;
> }
>
> JBUFFER_TRACE(jh, "owned by older transaction");
> - J_ASSERT_JH(jh, jh->b_next_transaction == NULL);
> - J_ASSERT_JH(jh, jh->b_transaction == journal->j_committing_transaction);
> + if (WARN_ON_ONCE(jh->b_next_transaction ||
> + jh->b_transaction !=
> + journal->j_committing_transaction)) {
> + pr_err("JBD2: %s: assertion failure: b_next_transaction=%p b_transaction=%p j_committing_transaction=%p\n",
> + journal->j_devname, jh->b_next_transaction,
> + jh->b_transaction, journal->j_committing_transaction);
> + spin_unlock(&jh->b_state_lock);
> + error = -EINVAL;
> + jbd2_journal_abort(journal, error);
> + goto out;
> + }
>
> /*
> * There is one case we have to be very careful about. If the
> @@ -1496,7 +1517,7 @@ void jbd2_buffer_abort_trigger(struct journal_head *jh,
> int jbd2_journal_dirty_metadata(handle_t *handle, struct buffer_head *bh)
> {
> transaction_t *transaction = handle->h_transaction;
> - journal_t *journal;
> + journal_t *journal = transaction->t_journal;
> struct journal_head *jh;
> int ret = 0;
>
> @@ -1520,8 +1541,14 @@ int jbd2_journal_dirty_metadata(handle_t *handle, struct buffer_head *bh)
> if (data_race(jh->b_transaction != transaction &&
> jh->b_next_transaction != transaction)) {
> spin_lock(&jh->b_state_lock);
> - J_ASSERT_JH(jh, jh->b_transaction == transaction ||
> - jh->b_next_transaction == transaction);
> + if (WARN_ON_ONCE(jh->b_transaction != transaction &&
> + jh->b_next_transaction != transaction)) {
> + pr_err("JBD2: %s: assertion failure: b_transaction=%p transaction=%p b_next_transaction=%p\n",
> + journal->j_devname, jh->b_transaction,
> + transaction, jh->b_next_transaction);
> + ret = -EINVAL;
> + goto out_unlock_bh;
> + }
> spin_unlock(&jh->b_state_lock);
> }
> if (data_race(jh->b_modified == 1)) {
> @@ -1531,13 +1558,15 @@ int jbd2_journal_dirty_metadata(handle_t *handle, struct buffer_head *bh)
> spin_lock(&jh->b_state_lock);
> if (jh->b_transaction == transaction &&
> jh->b_jlist != BJ_Metadata)
> - pr_err("JBD2: assertion failure: h_type=%u "
> - "h_line_no=%u block_no=%llu jlist=%u\n",
> + pr_err("JBD2: assertion failure: h_type=%u h_line_no=%u block_no=%llu jlist=%u\n",
> handle->h_type, handle->h_line_no,
> (unsigned long long) bh->b_blocknr,
> jh->b_jlist);
> - J_ASSERT_JH(jh, jh->b_transaction != transaction ||
> - jh->b_jlist == BJ_Metadata);
> + if (WARN_ON_ONCE(jh->b_transaction == transaction &&
> + jh->b_jlist != BJ_Metadata)) {
> + ret = -EINVAL;
> + goto out_unlock_bh;
> + }
> spin_unlock(&jh->b_state_lock);
> }
> goto out;
> @@ -1557,8 +1586,6 @@ int jbd2_journal_dirty_metadata(handle_t *handle, struct buffer_head *bh)
> goto out_unlock_bh;
> }
>
> - journal = transaction->t_journal;
> -
> if (jh->b_modified == 0) {
> /*
> * This buffer's got modified and becoming part
> @@ -1636,7 +1663,10 @@ int jbd2_journal_dirty_metadata(handle_t *handle, struct buffer_head *bh)
> }
>
> /* That test should have eliminated the following case: */
> - J_ASSERT_JH(jh, jh->b_frozen_data == NULL);
> + if (WARN_ON_ONCE(jh->b_frozen_data)) {
> + ret = -EINVAL;
> + goto out_unlock_bh;
> + }
>
> JBUFFER_TRACE(jh, "file as BJ_Metadata");
> spin_lock(&journal->j_list_lock);
> @@ -1675,6 +1705,7 @@ int jbd2_journal_forget(handle_t *handle, struct buffer_head *bh)
> int err = 0;
> int was_modified = 0;
> int wait_for_writeback = 0;
> + int abort_journal = 0;
>
> if (is_handle_aborted(handle))
> return -EROFS;
> @@ -1708,7 +1739,11 @@ int jbd2_journal_forget(handle_t *handle, struct buffer_head *bh)
> jh->b_modified = 0;
>
> if (jh->b_transaction == transaction) {
> - J_ASSERT_JH(jh, !jh->b_frozen_data);
> + if (WARN_ON_ONCE(jh->b_frozen_data)) {
> + err = -EINVAL;
> + abort_journal = 1;
> + goto drop;
> + }
>
> /* If we are forgetting a buffer which is already part
> * of this transaction, then we can just drop it from
> @@ -1747,8 +1782,11 @@ int jbd2_journal_forget(handle_t *handle, struct buffer_head *bh)
> }
> spin_unlock(&journal->j_list_lock);
> } else if (jh->b_transaction) {
> - J_ASSERT_JH(jh, (jh->b_transaction ==
> - journal->j_committing_transaction));
> + if (WARN_ON_ONCE(jh->b_transaction != journal->j_committing_transaction)) {
> + err = -EINVAL;
> + abort_journal = 1;
> + goto drop;
> + }
> /* However, if the buffer is still owned by a prior
> * (committing) transaction, we can't drop it yet... */
> JBUFFER_TRACE(jh, "belongs to older transaction");
> @@ -1766,7 +1804,11 @@ int jbd2_journal_forget(handle_t *handle, struct buffer_head *bh)
> jh->b_next_transaction = transaction;
> spin_unlock(&journal->j_list_lock);
> } else {
> - J_ASSERT(jh->b_next_transaction == transaction);
> + if (WARN_ON_ONCE(jh->b_next_transaction != transaction)) {
> + err = -EINVAL;
> + abort_journal = 1;
> + goto drop;
> + }
>
> /*
> * only drop a reference if this transaction modified
> @@ -1812,6 +1854,8 @@ int jbd2_journal_forget(handle_t *handle, struct buffer_head *bh)
> drop:
> __brelse(bh);
> spin_unlock(&jh->b_state_lock);
> + if (abort_journal)
> + jbd2_journal_abort(journal, err);
> if (wait_for_writeback)
> wait_on_buffer(bh);
> jbd2_journal_put_journal_head(jh);
> @@ -2136,7 +2180,8 @@ bool jbd2_journal_try_to_free_buffers(journal_t *journal, struct folio *folio)
> struct buffer_head *bh;
> bool ret = false;
>
> - J_ASSERT(folio_test_locked(folio));
> + if (WARN_ON_ONCE(!folio_test_locked(folio)))
> + return false;
>
> head = folio_buffers(folio);
> bh = head;
> @@ -2651,6 +2696,8 @@ static int jbd2_journal_file_inode(handle_t *handle, struct jbd2_inode *jinode,
> {
> transaction_t *transaction = handle->h_transaction;
> journal_t *journal;
> + int err = 0;
> + int abort_transaction = 0;
>
> if (is_handle_aborted(handle))
> return -EROFS;
> @@ -2685,20 +2732,33 @@ static int jbd2_journal_file_inode(handle_t *handle, struct jbd2_inode *jinode,
> /* On some different transaction's list - should be
> * the committing one */
> if (jinode->i_transaction) {
> - J_ASSERT(jinode->i_next_transaction == NULL);
> - J_ASSERT(jinode->i_transaction ==
> - journal->j_committing_transaction);
> + if (WARN_ON_ONCE(jinode->i_next_transaction ||
> + jinode->i_transaction !=
> + journal->j_committing_transaction)) {
> + pr_err("JBD2: %s: assertion failure: i_next_transaction=%p i_transaction=%p j_committing_transaction=%p\n",
> + journal->j_devname, jinode->i_next_transaction,
> + jinode->i_transaction,
> + journal->j_committing_transaction);
> + err = -EINVAL;
> + abort_transaction = 1;
> + goto done;
> + }
> jinode->i_next_transaction = transaction;
> goto done;
> }
> /* Not on any transaction list... */
> - J_ASSERT(!jinode->i_next_transaction);
> + if (WARN_ON_ONCE(jinode->i_next_transaction)) {
> + err = -EINVAL;
> + abort_transaction = 1;
> + goto done;
> + }
> jinode->i_transaction = transaction;
> list_add(&jinode->i_list, &transaction->t_inode_list);
> done:
> spin_unlock(&journal->j_list_lock);
> -
> - return 0;
> + if (abort_transaction)
> + jbd2_journal_abort(journal, err);
> + return err;
> }
>
> int jbd2_journal_inode_ranged_write(handle_t *handle,
> --
> 2.53.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH 11/32] gfs2: Don't zero i_private_data
From: Jan Kara @ 2026-03-04 10:39 UTC (permalink / raw)
To: Andreas Gruenbacher
Cc: Jan Kara, linux-fsdevel, Christian Brauner, Al Viro, linux-ext4,
Ted Tso, Tigran A. Aivazian, David Sterba, OGAWA Hirofumi,
Muchun Song, Oscar Salvador, David Hildenbrand, linux-mm,
linux-aio, Benjamin LaHaise, gfs2
In-Reply-To: <CAHc6FU61tUwnFf4pXWun_nLnL2jyUYHLKAN7C1hanbKk0GTZMA@mail.gmail.com>
On Tue 03-03-26 13:32:31, Andreas Gruenbacher wrote:
> Jan,
>
> On Tue, Mar 3, 2026 at 11:34 AM Jan Kara <jack@suse.cz> wrote:
> > The zeroing is the only use within gfs2 so it is pointless.
>
> "Remove the explicit zeroing of mapping->i_private_data since this
> field is no longer used."
>
> Reviewed-by: Andreas Gruenbacher <agruenba@redhat.com>
Thanks for review. I've updated the changelog.
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH 21/32] bdev: Drop pointless invalidate_mapping_buffers() call
From: Jan Kara @ 2026-03-04 10:36 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jan Kara, linux-fsdevel, Christian Brauner, Al Viro, linux-ext4,
Ted Tso, Tigran A. Aivazian, David Sterba, OGAWA Hirofumi,
Muchun Song, Oscar Salvador, David Hildenbrand, linux-mm,
linux-aio, Benjamin LaHaise, Jens Axboe, linux-block
In-Reply-To: <aabrf4YhPJ2X7n9q@infradead.org>
On Tue 03-03-26 06:09:03, Christoph Hellwig wrote:
> FYI, linux-block only got this patch which is totally messed up.
> Please always send all patches to every list and person, otherwise
> you fill peoples inboxes with unreviewable junk.
Well, I've CCed on the whole series everybody who was non-trivially
impacted. But there are couple of these trivial "remove effectively dead
code" patches which stand on their own and a lot of people actually prefer
to only get individual patches in such cases. So I don't plan on changing
that but I guess I could have CCed linux-block on the whole series as
buffer_heads are tangentially related to block layer anyway.
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH 21/32] bdev: Drop pointless invalidate_mapping_buffers() call
From: Jan Kara @ 2026-03-04 10:30 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jan Kara, linux-fsdevel, Christian Brauner, Al Viro, linux-ext4,
Ted Tso, Tigran A. Aivazian, David Sterba, OGAWA Hirofumi,
Muchun Song, Oscar Salvador, David Hildenbrand, linux-mm,
linux-aio, Benjamin LaHaise, Jens Axboe, linux-block
In-Reply-To: <aabqTRvKIWo2mHz1@infradead.org>
On Tue 03-03-26 06:03:57, Christoph Hellwig wrote:
> > diff --git a/block/bdev.c b/block/bdev.c
> > index ed022f8c48c7..ad1660b6b324 100644
> > --- a/block/bdev.c
> > +++ b/block/bdev.c
> > @@ -420,7 +420,6 @@ static void init_once(void *data)
> > static void bdev_evict_inode(struct inode *inode)
> > {
> > truncate_inode_pages_final(&inode->i_data);
> > - invalidate_inode_buffers(inode); /* is it needed here? */
> > clear_inode(inode);
> > }
>
> With this, bdev_evict_inode can go away as it is equivalent to the
> default action when no ->evict_inode is provided.
Good point. I'll remove bdev_evict_inode(). Thanks!
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH -next v3] ext4: test if inode's all dirty pages are submitted to disk
From: Jan Kara @ 2026-03-04 10:19 UTC (permalink / raw)
To: Ye Bin; +Cc: tytso, adilger.kernel, linux-ext4, jack
In-Reply-To: <20260303012242.3206465-1-yebin@huaweicloud.com>
On Tue 03-03-26 09:22:42, Ye Bin wrote:
> From: Ye Bin <yebin10@huawei.com>
>
> The commit aa373cf55099 ("writeback: stop background/kupdate works from
> livelocking other works") introduced an issue where unmounting a filesystem
> in a multi-logical-partition scenario could lead to batch file data loss.
> This problem was not fixed until the commit d92109891f21 ("fs/writeback:
> bail out if there is no more inodes for IO and queued once"). It took
> considerable time to identify the root cause. Additionally, in actual
> production environments, we frequently encountered file data loss after
> normal system reboots. Therefore, we are adding a check in the inode
> release flow to verify whether all dirty pages have been flushed to disk,
> in order to determine whether the data loss is caused by a logic issue in
> the filesystem code.
>
> Signed-off-by: Ye Bin <yebin10@huawei.com>
OK, I guess the warning is better than the stacktrace. BTW, what did
trigger the false positive warnings this time (I didn't really look at
syzbot reproducers)? Anyway, feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/ext4/inode.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 396dc3a5d16b..d4d65593bce2 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -184,6 +184,14 @@ void ext4_evict_inode(struct inode *inode)
> if (EXT4_I(inode)->i_flags & EXT4_EA_INODE_FL)
> ext4_evict_ea_inode(inode);
> if (inode->i_nlink) {
> + /*
> + * If there's dirty page will lead to data loss, user
> + * could see stale data.
> + */
> + if (unlikely(!ext4_emergency_state(inode->i_sb) &&
> + mapping_tagged(&inode->i_data, PAGECACHE_TAG_DIRTY)))
> + ext4_warning_inode(inode, "data will be lost");
> +
> truncate_inode_pages_final(&inode->i_data);
>
> goto no_delete;
> --
> 2.34.1
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH v2 000/110] vfs: change inode->i_ino from unsigned long to u64
From: David Laight @ 2026-03-04 9:30 UTC (permalink / raw)
To: NeilBrown
Cc: Jeff Layton, linux-fsdevel, linux-kernel, linux-trace-kernel,
nvdimm, fsverity, linux-mm, netfs, linux-ext4, linux-f2fs-devel,
linux-nfs, linux-cifs, samba-technical, linux-nilfs, v9fs,
linux-afs, autofs, ceph-devel, codalist, ecryptfs, linux-mtd,
jfs-discussion, ntfs3, ocfs2-devel, devel, linux-unionfs,
apparmor, linux-security-module, linux-integrity, selinux,
amd-gfx, dri-devel, linux-media, linaro-mm-sig, netdev,
linux-perf-users, linux-fscrypt, linux-xfs, linux-hams, linux-x25,
audit, linux-bluetooth, linux-can, linux-sctp, bpf
In-Reply-To: <177260561903.7472.14075475865748618717@noble.neil.brown.name>
On Wed, 04 Mar 2026 17:26:59 +1100
NeilBrown <neilb@ownmail.net> wrote:
> On Tue, 03 Mar 2026, Jeff Layton wrote:
> > On Tue, 2026-03-03 at 10:55 +0000, David Howells wrote:
> > > Jeff Layton <jlayton@kernel.org> wrote:
> > >
> > > > This version splits the change up to be more bisectable. It first adds a
> > > > new kino_t typedef and a new "PRIino" macro to hold the width specifier
> > > > for format strings. The conversion is done, and then everything is
> > > > changed to remove the new macro and typedef.
> > >
> > > Why remove the typedef? It might be better to keep it.
> > >
> >
> > Why? After this change, internel kernel inodes will be u64's -- full
> > stop. I don't see what the macro or typedef will buy us at that point.
>
> Implicit documentation?
> ktime_t is (now) always s64, but we still keep the typedef;
>
> It would be cool if we could teach vsprintf to understand some new
> specifier to mean "kinode_t" or "ktime_t" etc. But that would trigger
> gcc warnings.
A more interesting one would be something that made gcc re-write the
format with the correct 'length modifier' for the parameter.
That would save a lot of effort!
David
>
> NeilBrown
>
^ permalink raw reply
* [PATCH] ext4: validate p_idx bounds in ext4_ext_correct_indexes
From: tejas bharambe @ 2026-03-04 7:32 UTC (permalink / raw)
To: tytso@mit.edu, adilger.kernel@dilger.ca
Cc: linux-ext4@vger.kernel.org, linux-kernel@vger.kernel.org
From 980406b391b29715589842a4ae4391a87afaaef9 Mon Sep 17 00:00:00 2001
From: Tejas Bharambe <tejas.bharambe@outlook.com>
Date: Tue, 3 Mar 2026 23:14:34 -0800
Subject: [PATCH] ext4: validate p_idx bounds in ext4_ext_correct_indexes
ext4_ext_correct_indexes() walks up the extent tree correcting
index entries when the first extent in a leaf is modified. Before
accessing path[k].p_idx->ei_block, there is no validation that
p_idx falls within the valid range of index entries for that
level.
If the on-disk extent header contains a corrupted or crafted
eh_entries value, p_idx can point past the end of the allocated
buffer, causing a slab-out-of-bounds read.
Fix this by validating path[k].p_idx against EXT_LAST_INDEX() at
both access sites: before the while loop and inside it. Return
-EFSCORRUPTED if the index pointer is out of range, consistent
with how other bounds violations are handled in the ext4 extent
tree code.
Reported-by: syzbot+04c4e65cab786a2e5b7e@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=04c4e65cab786a2e5b7e
Signed-off-by: Tejas Bharambe <tejas.bharambe@outlook.com>
---
fs/ext4/extents.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index ae3804f365..f204285c71 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -1736,6 +1736,13 @@ static int ext4_ext_correct_indexes(handle_t *handle, struct inode *inode,
err = ext4_ext_get_access(handle, inode, path + k);
if (err)
return err;
+ if (unlikely(path[k].p_idx > EXT_LAST_INDEX(path[k].p_hdr))) {
+ EXT4_ERROR_INODE(inode,
+ "path[%d].p_idx %p > EXT_LAST_INDEX %p",
+ k, path[k].p_idx,
+ EXT_LAST_INDEX(path[k].p_hdr));
+ return -EFSCORRUPTED;
+ }
path[k].p_idx->ei_block = border;
err = ext4_ext_dirty(handle, inode, path + k);
if (err)
@@ -1748,6 +1755,14 @@ static int ext4_ext_correct_indexes(handle_t *handle, struct inode *inode,
err = ext4_ext_get_access(handle, inode, path + k);
if (err)
goto clean;
+ if (unlikely(path[k].p_idx > EXT_LAST_INDEX(path[k].p_hdr))) {
+ EXT4_ERROR_INODE(inode,
+ "path[%d].p_idx %p > EXT_LAST_INDEX %p",
+ k, path[k].p_idx,
+ EXT_LAST_INDEX(path[k].p_hdr));
+ err = -EFSCORRUPTED;
+ goto clean;
+ }
path[k].p_idx->ei_block = border;
err = ext4_ext_dirty(handle, inode, path + k);
if (err)
--
2.53.0
^ permalink raw reply related
* Re: [PATCH v2 000/110] vfs: change inode->i_ino from unsigned long to u64
From: NeilBrown @ 2026-03-04 6:26 UTC (permalink / raw)
To: Jeff Layton
Cc: David Howells, Alexander Viro, Christian Brauner, Jan Kara,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, Dan Williams,
Matthew Wilcox, Eric Biggers, Theodore Y. Ts'o, Muchun Song,
Oscar Salvador, David Hildenbrand, Paulo Alcantara,
Andreas Dilger, Jan Kara, Jaegeuk Kim, Chao Yu, Trond Myklebust,
Anna Schumaker, Chuck Lever, Olga Kornievskaia, Dai Ngo,
Tom Talpey, Steve French, Ronnie Sahlberg, Shyam Prasad N,
Bharath SM, Alexander Aring, Ryusuke Konishi, Viacheslav Dubeyko,
Eric Van Hensbergen, Latchesar Ionkov, Dominique Martinet,
Christian Schoenebeck, David Sterba, Marc Dionne, Ian Kent,
Luis de Bethencourt, Salah Triki, Tigran A. Aivazian,
Ilya Dryomov, Alex Markuze, Jan Harkes, coda, Nicolas Pitre,
Tyler Hicks, Amir Goldstein, Christoph Hellwig,
John Paul Adrian Glaubitz, Yangtao Li, Mikulas Patocka,
David Woodhouse, Richard Weinberger, Dave Kleikamp,
Konstantin Komarov, Mark Fasheh, Joel Becker, Joseph Qi,
Mike Marshall, Martin Brandenburg, Miklos Szeredi, Anders Larsen,
Zhihao Cheng, Damien Le Moal, Naohiro Aota, Johannes Thumshirn,
John Johansen, Paul Moore, James Morris, Serge E. Hallyn,
Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Fan Wu,
Stephen Smalley, Ondrej Mosnacek, Casey Schaufler, Alex Deucher,
Christian König, David Airlie, Simona Vetter, Sumit Semwal,
Eric Dumazet, Kuniyuki Iwashima, Paolo Abeni, Willem de Bruijn,
David S. Miller, Jakub Kicinski, Simon Horman, Oleg Nesterov,
Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, James Clark, Darrick J. Wong,
Martin Schiller, Eric Paris, Joerg Reuter, Marcel Holtmann,
Johan Hedberg, Luiz Augusto von Dentz, Oliver Hartkopp,
Marc Kleine-Budde, David Ahern, Neal Cardwell, Steffen Klassert,
Herbert Xu, Remi Denis-Courmont, Marcelo Ricardo Leitner,
Xin Long, Magnus Karlsson, Maciej Fijalkowski, Stanislav Fomichev,
Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
John Fastabend, linux-fsdevel, linux-kernel, linux-trace-kernel,
nvdimm, fsverity, linux-mm, netfs, linux-ext4, linux-f2fs-devel,
linux-nfs, linux-cifs, samba-technical, linux-nilfs, v9fs,
linux-afs, autofs, ceph-devel, codalist, ecryptfs, linux-mtd,
jfs-discussion, ntfs3, ocfs2-devel, devel, linux-unionfs,
apparmor, linux-security-module, linux-integrity, selinux,
amd-gfx, dri-devel, linux-media, linaro-mm-sig, netdev,
linux-perf-users, linux-fscrypt, linux-xfs, linux-hams, linux-x25,
audit, linux-bluetooth, linux-can, linux-sctp, bpf
In-Reply-To: <1c28e34c7167acf4e20c3e201476504135aa44e8.camel@kernel.org>
On Tue, 03 Mar 2026, Jeff Layton wrote:
> On Tue, 2026-03-03 at 10:55 +0000, David Howells wrote:
> > Jeff Layton <jlayton@kernel.org> wrote:
> >
> > > This version splits the change up to be more bisectable. It first adds a
> > > new kino_t typedef and a new "PRIino" macro to hold the width specifier
> > > for format strings. The conversion is done, and then everything is
> > > changed to remove the new macro and typedef.
> >
> > Why remove the typedef? It might be better to keep it.
> >
>
> Why? After this change, internel kernel inodes will be u64's -- full
> stop. I don't see what the macro or typedef will buy us at that point.
Implicit documentation?
ktime_t is (now) always s64, but we still keep the typedef;
It would be cool if we could teach vsprintf to understand some new
specifier to mean "kinode_t" or "ktime_t" etc. But that would trigger
gcc warnings.
NeilBrown
^ permalink raw reply
* Re: [PATCH 0/8] mm: globalize rest_of_page() macro
From: Jens Axboe @ 2026-03-04 3:35 UTC (permalink / raw)
To: Yury Norov, Sean Christopherson
Cc: Jakub Kicinski, Andrew Morton, David S. Miller,
Michael S. Tsirkin, Theodore Ts'o, Albert Ou, Alexander Duyck,
Alexander Gordeev, Alexander Viro, Alexandra Winter,
Andreas Dilger, Andrew Lunn, Anna Schumaker, Anton Yakovlev,
Arnaldo Carvalho de Melo, Aswin Karuvally, Borislav Petkov,
Carlos Maiolino, Catalin Marinas, Chao Yu, Christian Borntraeger,
Christian Brauner, Claudio Imbrenda, Dave Hansen, David Airlie,
Dominique Martinet, Dongsheng Yang, Eric Dumazet,
Eric Van Hensbergen, Heiko Carstens, Herbert Xu, Ingo Molnar,
Jaegeuk Kim, Jani Nikula, Janosch Frank, Jaroslav Kysela,
Joonas Lahtinen, Latchesar Ionkov, Linus Walleij,
Madhavan Srinivasan, Mark Brown, Michael Ellerman, Miklos Szeredi,
Namhyung Kim, Palmer Dabbelt, Paolo Abeni, Paolo Bonzini,
Paul Walmsley, Peter Zijlstra, Rodrigo Vivi, Simona Vetter,
Takashi Iwai, Thomas Gleixner, Trond Myklebust, Tvrtko Ursulin,
Vasily Gorbik, Will Deacon, Yury Norov, Zheng Gu, linux-kernel,
x86, linux-arm-kernel, linuxppc-dev, linux-riscv, kvm, linux-s390,
linux-block, intel-gfx, dri-devel, dm-devel, netdev, linux-spi,
linux-ext4, linux-f2fs-devel, linux-fsdevel, linux-xfs, linux-nfs,
linux-crypto, linux-mm, linux-perf-users, v9fs, virtualization,
linux-sound
In-Reply-To: <aaen2pGs0UeiJqz1@yury>
On 3/3/26 8:32 PM, Yury Norov wrote:
> My motivation is that it helps to simplify constructions like this:
>
> - loff_t cmp_len = min(PAGE_SIZE - offset_in_page(srcoff),
> - PAGE_SIZE - offset_in_page(dstoff));
> + loff_t cmp_len = min(rest_of_page(srcoff), rest_of_page(dstoff));
>
> Or this:
>
> - if (folio_test_highmem(dst_folio) &&
> - chunk > PAGE_SIZE - offset_in_page(dst_off))
> - chunk = PAGE_SIZE - offset_in_page(dst_off);
> - if (folio_test_highmem(src_folio) &&
> - chunk > PAGE_SIZE - offset_in_page(src_off))
> - chunk = PAGE_SIZE - offset_in_page(src_off);
> + if (folio_test_highmem(dst_folio) && chunk > rest_of_page(dst_off))
> + chunk = rest_of_page(dst_off);
> + if (folio_test_highmem(src_folio) && chunk > rest_of_page(src_off))
> + chunk = rest_of_page(src_off);
>
> To a point where I don't have to use my brains to decode them. I agree
> it's an easy math. It's just too bulky to my (and 9p guys too) taste.
The thing is, now I have to go lookup what on earth rest_of_page() does,
whereas PAGE_SIZE - offset_in_page(page) is immediately obvious. It's a
classic case of "oh let's add this helper to simplify things" which
really just makes it worse, because now you have to jump to the
definition of rest_of_page().
IOW, just no.
--
Jens Axboe
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox