* [PATCH v3] fs: push list presence check into inode_io_list_del()
@ 2025-11-03 23:09 Mateusz Guzik
2025-11-04 12:07 ` Jan Kara
2025-11-05 11:33 ` Christian Brauner
0 siblings, 2 replies; 3+ messages in thread
From: Mateusz Guzik @ 2025-11-03 23:09 UTC (permalink / raw)
To: brauner; +Cc: viro, jack, linux-kernel, linux-fsdevel, Mateusz Guzik
For consistency with sb routines.
ext4 is the only consumer outside of evict(). Damage-controlling it is
outside of the scope of this cleanup.
Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
---
v3:
- address feedback by Jan: take care of ext4
if you don't like the specific comment added below I would appreciate if
you adjusted it yourself.
this patch replaces this guy: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git/commit/?h=vfs-6.19.inode&id=7ba2ca3d17bb69276de7c97587b1e1f3d989f389
the other patch in the previous series remains unchanged
fs/ext4/inode.c | 3 +--
fs/fs-writeback.c | 7 +++++++
fs/inode.c | 4 +---
3 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index b864e9645f85..bf978ece70b3 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -202,8 +202,7 @@ void ext4_evict_inode(struct inode *inode)
* the inode. Flush worker is ignoring it because of I_FREEING flag but
* we still need to remove the inode from the writeback lists.
*/
- if (!list_empty_careful(&inode->i_io_list))
- inode_io_list_del(inode);
+ inode_io_list_del(inode);
/*
* Protect us against freezing - iput() caller didn't have to have any
diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index f784d8b09b04..e2eed66aabf8 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -1349,6 +1349,13 @@ void inode_io_list_del(struct inode *inode)
{
struct bdi_writeback *wb;
+ /*
+ * FIXME: ext4 can call here from ext4_evict_inode() after evict() already
+ * unlinked the inode.
+ */
+ if (list_empty_careful(&inode->i_io_list))
+ return;
+
wb = inode_to_wb_and_lock_list(inode);
spin_lock(&inode->i_lock);
diff --git a/fs/inode.c b/fs/inode.c
index 0f3a56ea8f48..263da76ed4fc 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -815,9 +815,7 @@ static void evict(struct inode *inode)
BUG_ON(!(inode_state_read_once(inode) & I_FREEING));
BUG_ON(!list_empty(&inode->i_lru));
- if (!list_empty(&inode->i_io_list))
- inode_io_list_del(inode);
-
+ inode_io_list_del(inode);
inode_sb_list_del(inode);
spin_lock(&inode->i_lock);
--
2.34.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v3] fs: push list presence check into inode_io_list_del()
2025-11-03 23:09 [PATCH v3] fs: push list presence check into inode_io_list_del() Mateusz Guzik
@ 2025-11-04 12:07 ` Jan Kara
2025-11-05 11:33 ` Christian Brauner
1 sibling, 0 replies; 3+ messages in thread
From: Jan Kara @ 2025-11-04 12:07 UTC (permalink / raw)
To: Mateusz Guzik; +Cc: brauner, viro, jack, linux-kernel, linux-fsdevel
On Tue 04-11-25 00:09:11, Mateusz Guzik wrote:
> For consistency with sb routines.
>
> ext4 is the only consumer outside of evict(). Damage-controlling it is
> outside of the scope of this cleanup.
>
> Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
Thanks. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
One note below:
> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> index f784d8b09b04..e2eed66aabf8 100644
> --- a/fs/fs-writeback.c
> +++ b/fs/fs-writeback.c
> @@ -1349,6 +1349,13 @@ void inode_io_list_del(struct inode *inode)
> {
> struct bdi_writeback *wb;
>
> + /*
> + * FIXME: ext4 can call here from ext4_evict_inode() after evict() already
> + * unlinked the inode.
> + */
This is in fact due to a possible race between __mark_inode_dirty() called
from ending page writeback (which is perfectly legal for a filesystem to
do) and iput_final() + evict(). See bc12ac98ea2e ("ext4: silence the
warning when evicting inode with dioread_nolock") for details. So proper
solution should be in the generic code... E.g. checking emptiness of
i_io_list under i_lock should be enough to close the race but it's a bit
tricky to avoid the unnecessary lock roundtrip for clean inodes.
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v3] fs: push list presence check into inode_io_list_del()
2025-11-03 23:09 [PATCH v3] fs: push list presence check into inode_io_list_del() Mateusz Guzik
2025-11-04 12:07 ` Jan Kara
@ 2025-11-05 11:33 ` Christian Brauner
1 sibling, 0 replies; 3+ messages in thread
From: Christian Brauner @ 2025-11-05 11:33 UTC (permalink / raw)
To: Mateusz Guzik; +Cc: Christian Brauner, viro, jack, linux-kernel, linux-fsdevel
On Tue, 04 Nov 2025 00:09:11 +0100, Mateusz Guzik wrote:
> For consistency with sb routines.
>
> ext4 is the only consumer outside of evict(). Damage-controlling it is
> outside of the scope of this cleanup.
>
>
Applied to the vfs-6.19.inode branch of the vfs/vfs.git tree.
Patches in the vfs-6.19.inode branch should appear in linux-next soon.
Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.
It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.
Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.
tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs-6.19.inode
[1/1] fs: push list presence check into inode_io_list_del()
https://git.kernel.org/vfs/vfs/c/f6fe56e7a34e
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-11-05 11:34 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-03 23:09 [PATCH v3] fs: push list presence check into inode_io_list_del() Mateusz Guzik
2025-11-04 12:07 ` Jan Kara
2025-11-05 11:33 ` Christian Brauner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox