linux-ext4 - Re: [RFC PATCH v4 1/4] jbd2: introduce/export functions jbd2_journal_submit|finish_inode_data

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 for Android: free password hash cracker in your pocket

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

Date:   Wed, 30 Sep 2020 18:36:28 -0300
From:   Mauricio Faria de Oliveira <mfo@...onical.com>
To:     Andreas Dilger <adilger@...ger.ca>
Cc:     Jan Kara <jack@...e.cz>, linux-ext4@...r.kernel.org,
        dann frazier <dann.frazier@...onical.com>
Subject: Re: [RFC PATCH v4 1/4] jbd2: introduce/export functions jbd2_journal_submit|finish_inode_data_buffers()

On Mon, Sep 28, 2020 at 11:24 PM Andreas Dilger <adilger@...ger.ca> wrote:
>
> On Sep 28, 2020, at 1:41 PM, Mauricio Faria de Oliveira <mfo@...onical.com> wrote:
> >
> > Export functions that implement the current behavior done
> > for an inode in journal_submit|finish_inode_data_buffers().
> >
> > No functional change.
> >
> > Signed-off-by: Mauricio Faria de Oliveira <mfo@...onical.com>
> > Suggested-by: Jan Kara <jack@...e.cz>
> > Reviewed-by: Jan Kara <jack@...e.cz>
>
> A couple of minor cleanups below, but either way you could add:
>
> Reviewed-by: Andreas Dilger <adilger@...ger.ca>
>

Hey Andreas, thanks for reviewing!

These cleanups/style changes do look better -- applied to the two functions
in patch 1 (submit and finish), and another function in patch 4
(submit callback).

cheers,
Mauricio

> > +int jbd2_journal_finish_inode_data_buffers(struct jbd2_inode *jinode)
> > +{
> > +     struct address_space *mapping = jinode->i_vfs_inode->i_mapping;
> > +     loff_t dirty_start = jinode->i_dirty_start;
> > +     loff_t dirty_end = jinode->i_dirty_end;
> > +     int ret;
> > +
> > +     ret = filemap_fdatawait_range_keep_errors(mapping, dirty_start, dirty_end);
> > +     return ret;
> > +}
>
> (style) still prefer to wrap at 80 columns if possible.
> (style) there isn't any benefit to "dirty_start" and "dirty_end" as locals
> (style) there also isn't any benefit to "ret = ...; return ret"
>
> I thought it might be coded this way because the function is changed in a
> later patch in the series, but I couldn't find anything like that, so the
> shorter form is just as readable, IMHO:
>
> int jbd2_journal_finish_inode_data_buffers(struct jbd2_inode *jinode)
> {
>         struct address_space *mapping = jinode->i_vfs_inode->i_mapping;
>
>         return filemap_fdatawait_range_keep_errors(mapping,
>                                                    jinode->dirty_start,
>                                                    jinode->dirty_end);
> }
>
> Cheers, Andreas
>
>
>
>
>


-- 
Mauricio Faria de Oliveira