linux-ext4 - Re: [PATCH v2 1/5] ext4: convert i_fc

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: <CAD+ocbyCVc9EL-HdrNHgnHqGS1ymhC6U7eQXuQeJW228WvrHyw@mail.gmail.com>
Date:   Thu, 10 Mar 2022 20:02:33 -0800
From:   harshad shirwadkar <harshadshirwadkar@...il.com>
To:     Jan Kara <jack@...e.cz>
Cc:     Ext4 Developers List <linux-ext4@...r.kernel.org>,
        Ritesh Harjani <riteshh@...ux.ibm.com>,
        "Theodore Y. Ts'o" <tytso@....edu>
Subject: Re: [PATCH v2 1/5] ext4: convert i_fc_lock to spinlock

On Wed, 9 Mar 2022 at 02:10, Jan Kara <jack@...e.cz> wrote:
>
> On Tue 08-03-22 08:33:15, Harshad Shirwadkar wrote:
> > From: Harshad Shirwadkar <harshadshirwadkar@...il.com>
> >
> > Convert ext4_inode_info->i_fc_lock to spinlock to avoid sleeping
> > in invalid contexts.
> >
> > Signed-off-by: Harshad Shirwadkar <harshadshirwadkar@...il.com>
>
> One comment below...
>
> > @@ -972,9 +970,13 @@ static int ext4_fc_wait_inode_data_all(journal_t *journal)
> >
> >       spin_lock(&sbi->s_fc_lock);
> >       list_for_each_entry_safe(pos, n, &sbi->s_fc_q[FC_Q_MAIN], i_fc_list) {
> > +             spin_lock(&pos->i_fc_lock);
> >               if (!ext4_test_inode_state(&pos->vfs_inode,
> > -                                        EXT4_STATE_FC_COMMITTING))
> > +                                        EXT4_STATE_FC_COMMITTING)) {
> > +                     spin_unlock(&pos->i_fc_lock);
> >                       continue;
> > +             }
> > +             spin_unlock(&pos->i_fc_lock);
> >               spin_unlock(&sbi->s_fc_lock);
>
> Why do you add a lock here in a pure lock-conversion patch? Furthermore I
> don't think the lock is needed...
Oops sorry, this was an unintentional leftover from the first version,
I'll remove it in the next one, thanks!

- Harshad
>
>                                                                 Honza
> --
> Jan Kara <jack@...e.com>
> SUSE Labs, CR