All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@zeniv.linux.org.uk>
To: Mateusz Guzik <mjguzik@gmail.com>
Cc: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>,
	Christian Brauner <brauner@kernel.org>, Jan Kara <jack@suse.cz>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] vfs: show filesystem name at dump_inode()
Date: Mon, 11 Aug 2025 21:38:15 +0100	[thread overview]
Message-ID: <20250811203815.GS222315@ZenIV> (raw)
In-Reply-To: <CAGudoHEowsc290kfSgCjDJfB+RKOv2gLYS6y4OxyjhjPW07vMQ@mail.gmail.com>

On Mon, Aug 11, 2025 at 09:45:52PM +0200, Mateusz Guzik wrote:

> Better printing is a TODO in part because the routine must not trip
> over arbitrarily bogus state, in this case notably that's unset
> ->i_sb.

That... is a strange state.  It means having never been passed to
inode_init_always().  How do you get to it?  I mean, if the argument
is not pointing to a struct inode instance, sure, but then NULL is
not the only possibility - we are talking about the valur of
arbitrary word of memory that might contain anything whatsoever.

If, OTOH, it is a genuine struct inode, it must be in a very strange
point in the lifecycle - somewhere in the middle of alloc_inode(),
definitely before its address gets returned to the caller...

> See mm/debug.c:dump_vmg for an example.

Not quite relevant here...

>  void dump_inode(struct inode *inode, const char *reason)
>  {
> -       pr_warn("%s encountered for inode %px", reason, inode);
> +       struct super_block *sb = inode->i_sb; /* will be careful deref later */
> +
> +       pr_warn("%s encountered for inode %px [fs %s]", reason, inode,
> sb ? sb->s_type->name : "NOT SET");

That's really misleading - this "NOT SET" is not a valid state; ->i_sb is
an assign-once member that gets set by constructor before the object is
returned and it's never modified afterwards.  In particular, it is never
cleared.

There is a weird debugging in generic_shutdown_super() that goes through
the inodes of dying superblock that had survived the fs shutdown
("Busy inodes after unmount" thing) and poisons their ->i_sb, but that's
VFS_PTR_POINSON, not NULL.

We literally never store NULL there.  Not even with kmem_cache_zalloc()...

  reply	other threads:[~2025-08-11 20:38 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-11  6:50 [PATCH] vfs: show filesystem name at dump_inode() Tetsuo Handa
2025-08-11 13:51 ` Christian Brauner
2025-08-12 15:22   ` Tetsuo Handa
2025-08-11 19:45 ` Mateusz Guzik
2025-08-11 20:38   ` Al Viro [this message]
2025-08-12  4:07     ` Mateusz Guzik

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=20250811203815.GS222315@ZenIV \
    --to=viro@zeniv.linux.org.uk \
    --cc=brauner@kernel.org \
    --cc=jack@suse.cz \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mjguzik@gmail.com \
    --cc=penguin-kernel@i-love.sakura.ne.jp \
    /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.