linux-f2fs-devel.lists.sourceforge.net archive mirror
 help / color / mirror / Atom feed
From: Jaegeuk Kim <jaegeuk@kernel.org>
To: Christoph Hellwig <hch@lst.de>
Cc: linux-f2fs-devel@lists.sourceforge.net
Subject: Re: [f2fs-dev] [PATCH 12/15] f2fs: remove the create argument to f2fs_map_blocks
Date: Mon, 12 Dec 2022 17:22:41 -0800	[thread overview]
Message-ID: <Y5fT4SQ9/vuZeWWB@google.com> (raw)
In-Reply-To: <20221128091523.1242584-13-hch@lst.de>

Hi Christoph,

On 11/28, Christoph Hellwig wrote:
> The create argument is always identicaly to map->m_may_create, so use
> that consistently.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/f2fs/data.c              | 32 ++++++++++----------------------
>  fs/f2fs/f2fs.h              |  3 +--
>  fs/f2fs/file.c              | 12 ++++++------
>  include/trace/events/f2fs.h | 11 ++++-------
>  4 files changed, 21 insertions(+), 37 deletions(-)
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 2ae8fcf7cf49f4..730b58ba97c0ae 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -1454,8 +1454,7 @@ int f2fs_get_block_locked(struct dnode_of_data *dn, pgoff_t index)
>   * maps continuous logical blocks to physical blocks, and return such
>   * info via f2fs_map_blocks structure.
>   */
> -int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
> -						int create, int flag)
> +int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>  {
>  	unsigned int maxblocks = map->m_len;
>  	struct dnode_of_data dn;
> @@ -1484,11 +1483,7 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
>  	pgofs =	(pgoff_t)map->m_lblk;
>  	end = pgofs + maxblocks;
>  
> -	if (!create && f2fs_lookup_extent_cache(inode, pgofs, &ei)) {
> -		if (f2fs_lfs_mode(sbi) && flag == F2FS_GET_BLOCK_DIO &&

Any reason to remove this condition?

Thanks,

> -							map->m_may_create)
> -			goto next_dnode;
> -
> +	if (!map->m_may_create && f2fs_lookup_extent_cache(inode, pgofs, &ei)) {
>  		map->m_pblk = ei.blk + pgofs - ei.fofs;
>  		map->m_len = min((pgoff_t)maxblocks, ei.fofs + ei.len - pgofs);
>  		map->m_flags = F2FS_MAP_MAPPED;
> @@ -1501,18 +1496,12 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
>  						map->m_pblk, map->m_len);
>  
>  		if (map->m_multidev_dio) {
> -			block_t blk_addr = map->m_pblk;
> -
>  			bidx = f2fs_target_device_index(sbi, map->m_pblk);
>  
>  			map->m_bdev = FDEV(bidx).bdev;
>  			map->m_pblk -= FDEV(bidx).start_blk;
>  			map->m_len = min(map->m_len,
>  				FDEV(bidx).end_blk + 1 - map->m_pblk);
> -
> -			if (map->m_may_create)
> -				f2fs_update_device_state(sbi, inode->i_ino,
> -							blk_addr, map->m_len);
>  		}
>  		goto out;
>  	}
> @@ -1579,7 +1568,7 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
>  			set_inode_flag(inode, FI_APPEND_WRITE);
>  		}
>  	} else {
> -		if (create) {
> +		if (map->m_may_create) {
>  			if (unlikely(f2fs_cp_error(sbi))) {
>  				err = -EIO;
>  				goto sync_out;
> @@ -1753,7 +1742,7 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
>  		f2fs_balance_fs(sbi, dn.node_changed);
>  	}
>  out:
> -	trace_f2fs_map_blocks(inode, map, create, flag, err);
> +	trace_f2fs_map_blocks(inode, map, flag, err);
>  	return err;
>  }
>  
> @@ -1775,7 +1764,7 @@ bool f2fs_overwrite_io(struct inode *inode, loff_t pos, size_t len)
>  
>  	while (map.m_lblk < last_lblk) {
>  		map.m_len = last_lblk - map.m_lblk;
> -		err = f2fs_map_blocks(inode, &map, 0, F2FS_GET_BLOCK_DEFAULT);
> +		err = f2fs_map_blocks(inode, &map, F2FS_GET_BLOCK_DEFAULT);
>  		if (err || map.m_len == 0)
>  			return false;
>  		map.m_lblk += map.m_len;
> @@ -1949,7 +1938,7 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>  		map.m_len = cluster_size - count_in_cluster;
>  	}
>  
> -	ret = f2fs_map_blocks(inode, &map, 0, F2FS_GET_BLOCK_FIEMAP);
> +	ret = f2fs_map_blocks(inode, &map, F2FS_GET_BLOCK_FIEMAP);
>  	if (ret)
>  		goto out;
>  
> @@ -2082,7 +2071,7 @@ static int f2fs_read_single_page(struct inode *inode, struct page *page,
>  	map->m_lblk = block_in_file;
>  	map->m_len = last_block - block_in_file;
>  
> -	ret = f2fs_map_blocks(inode, map, 0, F2FS_GET_BLOCK_DEFAULT);
> +	ret = f2fs_map_blocks(inode, map, F2FS_GET_BLOCK_DEFAULT);
>  	if (ret)
>  		goto out;
>  got_it:
> @@ -3779,7 +3768,7 @@ static sector_t f2fs_bmap(struct address_space *mapping, sector_t block)
>  		map.m_next_pgofs = NULL;
>  		map.m_seg_type = NO_CHECK_TYPE;
>  
> -		if (!f2fs_map_blocks(inode, &map, 0, F2FS_GET_BLOCK_BMAP))
> +		if (!f2fs_map_blocks(inode, &map, F2FS_GET_BLOCK_BMAP))
>  			blknr = map.m_pblk;
>  	}
>  out:
> @@ -3887,7 +3876,7 @@ static int check_swap_activate(struct swap_info_struct *sis,
>  		map.m_seg_type = NO_CHECK_TYPE;
>  		map.m_may_create = false;
>  
> -		ret = f2fs_map_blocks(inode, &map, 0, F2FS_GET_BLOCK_FIEMAP);
> +		ret = f2fs_map_blocks(inode, &map, F2FS_GET_BLOCK_FIEMAP);
>  		if (ret)
>  			goto out;
>  
> @@ -4116,8 +4105,7 @@ static int f2fs_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
>  	if (flags & IOMAP_WRITE)
>  		map.m_may_create = true;
>  
> -	err = f2fs_map_blocks(inode, &map, flags & IOMAP_WRITE,
> -			      F2FS_GET_BLOCK_DIO);
> +	err = f2fs_map_blocks(inode, &map, F2FS_GET_BLOCK_DIO);
>  	if (err)
>  		return err;
>  
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index a3789dab0aade9..2c49714ac176f3 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -3801,8 +3801,7 @@ struct page *f2fs_get_lock_data_page(struct inode *inode, pgoff_t index,
>  struct page *f2fs_get_new_data_page(struct inode *inode,
>  			struct page *ipage, pgoff_t index, bool new_i_size);
>  int f2fs_do_write_data_page(struct f2fs_io_info *fio);
> -int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
> -			int create, int flag);
> +int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag);
>  int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>  			u64 start, u64 len);
>  int f2fs_encrypt_one_page(struct f2fs_io_info *fio);
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index cbeb7bd880046e..cb3fce6eec6db9 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -1742,7 +1742,7 @@ static int expand_inode_data(struct inode *inode, loff_t offset,
>  		f2fs_unlock_op(sbi);
>  
>  		map.m_seg_type = CURSEG_COLD_DATA_PINNED;
> -		err = f2fs_map_blocks(inode, &map, 1, F2FS_GET_BLOCK_PRE_DIO);
> +		err = f2fs_map_blocks(inode, &map, F2FS_GET_BLOCK_PRE_DIO);
>  		file_dont_truncate(inode);
>  
>  		f2fs_up_write(&sbi->pin_sem);
> @@ -1755,7 +1755,7 @@ static int expand_inode_data(struct inode *inode, loff_t offset,
>  
>  		map.m_len = expanded;
>  	} else {
> -		err = f2fs_map_blocks(inode, &map, 1, F2FS_GET_BLOCK_PRE_AIO);
> +		err = f2fs_map_blocks(inode, &map, F2FS_GET_BLOCK_PRE_AIO);
>  		expanded = map.m_len;
>  	}
>  out_err:
> @@ -2588,7 +2588,7 @@ static int f2fs_defragment_range(struct f2fs_sb_info *sbi,
>  	 */
>  	while (map.m_lblk < pg_end) {
>  		map.m_len = pg_end - map.m_lblk;
> -		err = f2fs_map_blocks(inode, &map, 0, F2FS_GET_BLOCK_DEFAULT);
> +		err = f2fs_map_blocks(inode, &map, F2FS_GET_BLOCK_DEFAULT);
>  		if (err)
>  			goto out;
>  
> @@ -2635,7 +2635,7 @@ static int f2fs_defragment_range(struct f2fs_sb_info *sbi,
>  
>  do_map:
>  		map.m_len = pg_end - map.m_lblk;
> -		err = f2fs_map_blocks(inode, &map, 0, F2FS_GET_BLOCK_DEFAULT);
> +		err = f2fs_map_blocks(inode, &map, F2FS_GET_BLOCK_DEFAULT);
>  		if (err)
>  			goto clear_out;
>  
> @@ -3209,7 +3209,7 @@ int f2fs_precache_extents(struct inode *inode)
>  		map.m_len = end - map.m_lblk;
>  
>  		f2fs_down_write(&fi->i_gc_rwsem[WRITE]);
> -		err = f2fs_map_blocks(inode, &map, 0, F2FS_GET_BLOCK_PRECACHE);
> +		err = f2fs_map_blocks(inode, &map, F2FS_GET_BLOCK_PRECACHE);
>  		f2fs_up_write(&fi->i_gc_rwsem[WRITE]);
>  		if (err)
>  			return err;
> @@ -4446,7 +4446,7 @@ static int f2fs_preallocate_blocks(struct kiocb *iocb, struct iov_iter *iter,
>  		flag = F2FS_GET_BLOCK_PRE_AIO;
>  	}
>  
> -	ret = f2fs_map_blocks(inode, &map, 1, flag);
> +	ret = f2fs_map_blocks(inode, &map, flag);
>  	/* -ENOSPC|-EDQUOT are fine to report the number of allocated blocks. */
>  	if (ret < 0 && !((ret == -ENOSPC || ret == -EDQUOT) && map.m_len > 0))
>  		return ret;
> diff --git a/include/trace/events/f2fs.h b/include/trace/events/f2fs.h
> index c6b372401c2787..cbf7e0d1a22387 100644
> --- a/include/trace/events/f2fs.h
> +++ b/include/trace/events/f2fs.h
> @@ -562,10 +562,10 @@ TRACE_EVENT(f2fs_file_write_iter,
>  );
>  
>  TRACE_EVENT(f2fs_map_blocks,
> -	TP_PROTO(struct inode *inode, struct f2fs_map_blocks *map,
> -				int create, int flag, int ret),
> +	TP_PROTO(struct inode *inode, struct f2fs_map_blocks *map, int flag,
> +		 int ret),
>  
> -	TP_ARGS(inode, map, create, flag, ret),
> +	TP_ARGS(inode, map, flag, ret),
>  
>  	TP_STRUCT__entry(
>  		__field(dev_t,	dev)
> @@ -577,7 +577,6 @@ TRACE_EVENT(f2fs_map_blocks,
>  		__field(int,	m_seg_type)
>  		__field(bool,	m_may_create)
>  		__field(bool,	m_multidev_dio)
> -		__field(int,	create)
>  		__field(int,	flag)
>  		__field(int,	ret)
>  	),
> @@ -592,7 +591,6 @@ TRACE_EVENT(f2fs_map_blocks,
>  		__entry->m_seg_type	= map->m_seg_type;
>  		__entry->m_may_create	= map->m_may_create;
>  		__entry->m_multidev_dio	= map->m_multidev_dio;
> -		__entry->create		= create;
>  		__entry->flag		= flag;
>  		__entry->ret		= ret;
>  	),
> @@ -600,7 +598,7 @@ TRACE_EVENT(f2fs_map_blocks,
>  	TP_printk("dev = (%d,%d), ino = %lu, file offset = %llu, "
>  		"start blkaddr = 0x%llx, len = 0x%llx, flags = %u, "
>  		"seg_type = %d, may_create = %d, multidevice = %d, "
> -		"create = %d, flag = %d, err = %d",
> +		"flag = %d, err = %d",
>  		show_dev_ino(__entry),
>  		(unsigned long long)__entry->m_lblk,
>  		(unsigned long long)__entry->m_pblk,
> @@ -609,7 +607,6 @@ TRACE_EVENT(f2fs_map_blocks,
>  		__entry->m_seg_type,
>  		__entry->m_may_create,
>  		__entry->m_multidev_dio,
> -		__entry->create,
>  		__entry->flag,
>  		__entry->ret)
>  );
> -- 
> 2.30.2


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

  reply	other threads:[~2022-12-13  1:22 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-28  9:15 [f2fs-dev] a fix and a bunch of cleanups Christoph Hellwig
2022-11-28  9:15 ` [f2fs-dev] [PATCH 01/15] f2fs: don't rely on F2FS_MAP_* in f2fs_iomap_begin Christoph Hellwig
2022-11-28  9:15 ` [f2fs-dev] [PATCH 02/15] f2fs: decouple F2FS_MAP_ from buffer head flags Christoph Hellwig
2022-11-28  9:15 ` [f2fs-dev] [PATCH 03/15] f2fs: rename F2FS_MAP_UNWRITTEN to F2FS_MAP_DELALLOC Christoph Hellwig
2022-11-28  9:15 ` [f2fs-dev] [PATCH 04/15] f2fs: split __submit_bio Christoph Hellwig
2022-11-28  9:15 ` [f2fs-dev] [PATCH 05/15] f2fs: fold f2fs_lookup_extent_tree into f2fs_lookup_extent_cache Christoph Hellwig
2022-11-28  9:15 ` [f2fs-dev] [PATCH 06/15] f2fs: add a f2fs_lookup_extent_cache_block helper Christoph Hellwig
2022-11-28  9:15 ` [f2fs-dev] [PATCH 07/15] f2fs: add a f2fs_get_block_locked helper Christoph Hellwig
2022-11-28  9:15 ` [f2fs-dev] [PATCH 08/15] f2fs: f2fs_do_map_lock Christoph Hellwig
2022-11-28  9:15 ` [f2fs-dev] [PATCH 09/15] f2fs: reflow prepare_write_begin Christoph Hellwig
2022-11-28  9:15 ` [f2fs-dev] [PATCH 10/15] f2fs: simplify __allocate_data_block Christoph Hellwig
2022-11-28  9:15 ` [f2fs-dev] [PATCH 11/15] f2fs: remove f2fs_get_block Christoph Hellwig
2022-11-28  9:15 ` [f2fs-dev] [PATCH 12/15] f2fs: remove the create argument to f2fs_map_blocks Christoph Hellwig
2022-12-13  1:22   ` Jaegeuk Kim [this message]
2022-12-13  5:57     ` Christoph Hellwig
2022-12-15  1:02       ` Jaegeuk Kim
2022-11-28  9:15 ` [f2fs-dev] [PATCH 13/15] f2fs: factor a f2f_map_blocks_cached helper Christoph Hellwig
2022-11-28  9:15 ` [f2fs-dev] [PATCH 14/15] f2fs: factor out a f2fs_map_no_dnode Christoph Hellwig
2022-11-28  9:15 ` [f2fs-dev] [PATCH 15/15] f2fs: refactor the hole reporting and allocation logic in f2fs_map_blocks Christoph Hellwig
2023-01-03 17:21 ` [f2fs-dev] a fix and a bunch of cleanups Jaegeuk Kim
2023-01-06  6:54   ` Chao Yu
2023-01-11  5:58   ` Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y5fT4SQ9/vuZeWWB@google.com \
    --to=jaegeuk@kernel.org \
    --cc=hch@lst.de \
    --cc=linux-f2fs-devel@lists.sourceforge.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).