Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220308163319.1183625-1-harshads@google.com> <20220308163319.1183625-2-harshads@google.com>
 <20220309101057.3uuix2gvjinobt3i@quack3.lan>
In-Reply-To: <20220309101057.3uuix2gvjinobt3i@quack3.lan>
From:   harshad shirwadkar <harshadshirwadkar@gmail.com>
Date:   Thu, 10 Mar 2022 20:02:33 -0800
Message-ID: <CAD+ocbyCVc9EL-HdrNHgnHqGS1ymhC6U7eQXuQeJW228WvrHyw@mail.gmail.com>
Subject: Re: [PATCH v2 1/5] ext4: convert i_fc_lock to spinlock
To:     Jan Kara <jack@suse.cz>
Cc:     Ext4 Developers List <linux-ext4@vger.kernel.org>,
        Ritesh Harjani <riteshh@linux.ibm.com>,
        "Theodore Y. Ts'o" <tytso@mit.edu>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, 9 Mar 2022 at 02:10, Jan Kara <jack@suse.cz> wrote:
>
> On Tue 08-03-22 08:33:15, Harshad Shirwadkar wrote:
> > From: Harshad Shirwadkar <harshadshirwadkar@gmail.com>
> >
> > Convert ext4_inode_info->i_fc_lock to spinlock to avoid sleeping
> > in invalid contexts.
> >
> > Signed-off-by: Harshad Shirwadkar <harshadshirwadkar@gmail.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@suse.com>
> SUSE Labs, CR