Re: [RFC PATCH v4 1/4] jbd2: introduce/export functions jbd2_journal_submit|finish_inode_data_buffers()

[Andreas Dilger <adilger@dilger.ca>]

[-- Attachment #1: Type: text/plain, Size: 1561 bytes --]

On Sep 28, 2020, at 1:41 PM, Mauricio Faria de Oliveira <mfo@canonical.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@canonical.com>
> Suggested-by: Jan Kara <jack@suse.cz>
> Reviewed-by: Jan Kara <jack@suse.cz>

A couple of minor cleanups below, but either way you could add:

Reviewed-by: Andreas Dilger <adilger@dilger.ca>

> +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






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]