Received-SPF: pass (google.com: best guess record for domain of linux-ext4-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20190809034552.148629-1-harshadshirwadkar@gmail.com>
 <20190809034552.148629-9-harshadshirwadkar@gmail.com> <BC4620C8-1AC6-4543-BBE8-012067ED3CEA@dilger.ca>
In-Reply-To: <BC4620C8-1AC6-4543-BBE8-012067ED3CEA@dilger.ca>
From:   harshad shirwadkar <harshadshirwadkar@gmail.com>
Date:   Tue, 1 Oct 2019 00:51:19 -0700
Message-ID: <CAD+ocbyP2WzNyEa5Pgpk3ViahuSNUQ2bSB_OEBOze4DdBiHNsw@mail.gmail.com>
Subject: Re: [PATCH v2 08/12] ext4: track changed files for fast commit
To:     Andreas Dilger <adilger@dilger.ca>
Cc:     Ext4 Developers List <linux-ext4@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-ext4-owner@vger.kernel.org
Precedence: bulk

Thanks, done in V3.

On Fri, Aug 9, 2019 at 2:46 PM Andreas Dilger <adilger@dilger.ca> wrote:
>
> On Aug 8, 2019, at 9:45 PM, Harshad Shirwadkar <harshadshirwadkar@gmail.com> wrote:
> >
> > For fast commit, we need to remember all the files that have changed
> > since last fast commit / full commit. For changes that are fast commit
> > incompatible, we mark the file system fast commit incompatible. This
> > patch adds code to either remember files that have changed or to mark
> > ext4 as fast commit ineligible. We inspect every ext4_mark_inode_dirty
> > calls and decide whether that particular file change is fast
> > compatible or not.
> >
> > Signed-off-by: Harshad Shirwadkar <harshadshirwadkar@gmail.com>
>
> Some minor code style cleanups.
>
> > @@ -759,6 +761,8 @@ int ext4_write_inline_data_end(struct inode *inode, loff_t pos, unsigned len,
> >
> >       ext4_write_unlock_xattr(inode, &no_expand);
> >       brelse(iloc.bh);
> > +     ext4_fc_enqueue_inode(ext4_journal_current_handle(),
> > +                                        inode);
>
> (style) "inode" doesn't need to be split to a separate line
>
> >       mark_inode_dirty(inode);
> > out:
> >       return copied;
> > @@ -974,6 +978,8 @@ int ext4_da_write_inline_data_end(struct inode *inode, loff_t pos,
> >        * ordering of page lock and transaction start for journaling
> >        * filesystems.
> >        */
> > +     ext4_fc_enqueue_inode(ext4_journal_current_handle(),
> > +                                        inode);
>
> (style) "inode" doesn't need to be split to a separate line
>
> > @@ -5697,6 +5719,8 @@ int ext4_setattr(struct dentry *dentry, struct iattr *attr)
> >
> >       if (!error) {
> >               setattr_copy(inode, attr);
> > +             ext4_fc_enqueue_inode(ext4_journal_current_handle(),
> > +                                                inode);
>
> (style) "inode" doesn't need to be split to a separate line
>
> > diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> > index 0b833e9b61c1..c7bb52bdaf6e 100644
> > --- a/fs/ext4/super.c
> > +++ b/fs/ext4/super.c
> > @@ -1129,6 +1129,16 @@ static void ext4_destroy_inode(struct inode *inode)
> >                               true);
> >               dump_stack();
> >       }
> > +     if (!list_empty(&(EXT4_I(inode)->i_fc_list))) {
> > +#ifdef EXT4FS_DEBUG
> > +             if (EXT4_SB(inode->i_sb)->s_fc_eligible) {
> > +                     pr_warn("%s: INODE %ld in FC List with FC allowd",
> > +                             __func__, inode->i_ino);
>
> (style) this should use ext4fs_debug(), since pr_warn() is not really
> used in the ext4 code
>
> > +                     dump_stack();
> > +             }
> > +#endif
> > +             ext4_fc_del(inode);
> > +     }
> > }
>
>
> Cheers, Andreas
>
>
>
>
>