linux-kernel - Re: [PATCH v2 1/2] fs: push list presence check into inode_io_list

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAGudoHHsV5oyOnKNuQNwQXTeS4ZQp2pTnbrQ3B9+_dvFkmA3bQ@mail.gmail.com>
Date: Thu, 30 Oct 2025 15:13:40 +0100
From: Mateusz Guzik <mjguzik@...il.com>
To: Jan Kara <jack@...e.cz>
Cc: brauner@...nel.org, viro@...iv.linux.org.uk, linux-kernel@...r.kernel.org, 
	linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH v2 1/2] fs: push list presence check into inode_io_list_del()

On Thu, Oct 30, 2025 at 2:54 PM Jan Kara <jack@...e.cz> wrote:
>
> On Wed 29-10-25 14:14:27, Mateusz Guzik wrote:
> > For consistency with sb routines.
> >
> > Signed-off-by: Mateusz Guzik <mjguzik@...il.com>
>
> Not sure if you've noticed but inode_io_list_del() is also called from
> ext4_evict_inode() (for annoying reasons but that's besides this thread).
> So there you have another list_empty() check to deduplicate. Plus ext4
> actually uses list_empty_careful() for the unlocked check which kind of
> makes sense because in theory these checks could race with someone removing
> the inode from writeback lists.
>

huh, gotta say this is pretty bad.

i'm going to the updates later for the sake of the clean up, but long
term if ext4 has to do this, the evict func should postpone
inode_io_list_del instead. there is related mess in interaction with
writeback as is though, so this is for some indeterminate point in the
future.

>                                                                 Honza
>
> > ---
> >
> > rebased
> >
> >  fs/fs-writeback.c | 3 +++
> >  fs/inode.c        | 4 +---
> >  2 files changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> > index f784d8b09b04..5dccbe5fb09d 100644
> > --- a/fs/fs-writeback.c
> > +++ b/fs/fs-writeback.c
> > @@ -1349,6 +1349,9 @@ void inode_io_list_del(struct inode *inode)
> >  {
> >       struct bdi_writeback *wb;
> >
> > +     if (list_empty(&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 1396f79b2551..b5c2efebaa18 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
> >
> --
> Jan Kara <jack@...e.com>
> SUSE Labs, CR