All of lore.kernel.org
 help / color / mirror / Atom feed
* [f2fs-dev] a fix and a bunch of cleanups
@ 2022-11-28  9:15 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
                   ` (15 more replies)
  0 siblings, 16 replies; 22+ messages in thread
From: Christoph Hellwig @ 2022-11-28  9:15 UTC (permalink / raw)
  To: Jaegeuk Kim, Chao Yu; +Cc: linux-f2fs-devel

Hi Jaegeuk and Chao,

the first patch in this series fixes a warning and subsequent hang when
testing zoned f2fs.  The other patches are misc cleanups for the I/O path.

Diffstat
 fs/f2fs/compress.c          |    2 
 fs/f2fs/data.c              |  544 ++++++++++++++++++++++----------------------
 fs/f2fs/extent_cache.c      |   19 -
 fs/f2fs/f2fs.h              |   24 -
 fs/f2fs/file.c              |   16 -
 fs/f2fs/gc.c                |    4 
 include/trace/events/f2fs.h |   11 
 7 files changed, 309 insertions(+), 311 deletions(-)


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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [f2fs-dev] [PATCH 01/15] f2fs: don't rely on F2FS_MAP_* in f2fs_iomap_begin
  2022-11-28  9:15 [f2fs-dev] a fix and a bunch of cleanups Christoph Hellwig
@ 2022-11-28  9:15 ` Christoph Hellwig
  2022-11-28  9:15 ` [f2fs-dev] [PATCH 02/15] f2fs: decouple F2FS_MAP_ from buffer head flags Christoph Hellwig
                   ` (14 subsequent siblings)
  15 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2022-11-28  9:15 UTC (permalink / raw)
  To: Jaegeuk Kim, Chao Yu; +Cc: linux-f2fs-devel

When testing with a mixed zoned / convention device combination, there
are regular but not 100% reproducible failures in xfstests generic/113
where the __is_valid_data_blkaddr assert hits due to finding a hole.

This seems to be because f2fs_map_blocks can set this flag on a hole
when it was found in the extent cache.

Rework f2fs_iomap_begin to just check the special block numbers directly.
This has the added benefits of the WARN_ON showing which invalid block
address we found, and being properly error out on delalloc blocks that
are confusingly called unwritten but not actually suitable for direct
I/O.

Fixes: 1517c1a7a445 ("f2fs: implement iomap operations")
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/f2fs/data.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index a71e818cd67b43..541d625d94be42 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -4134,20 +4134,24 @@ static int f2fs_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
 	 */
 	map.m_len = fscrypt_limit_io_blocks(inode, map.m_lblk, map.m_len);
 
-	if (map.m_flags & (F2FS_MAP_MAPPED | F2FS_MAP_UNWRITTEN)) {
-		iomap->length = blks_to_bytes(inode, map.m_len);
-		if (map.m_flags & F2FS_MAP_MAPPED) {
-			iomap->type = IOMAP_MAPPED;
-			iomap->flags |= IOMAP_F_MERGED;
-		} else {
-			iomap->type = IOMAP_UNWRITTEN;
-		}
-		if (WARN_ON_ONCE(!__is_valid_data_blkaddr(map.m_pblk)))
-			return -EINVAL;
+	/*
+	 * We should never see delalloc or compressed extents here based on
+	 * prior flushing and checks.
+	 */
+	if (WARN_ON_ONCE(map.m_pblk == NEW_ADDR))
+		return -EINVAL;
+	if (WARN_ON_ONCE(map.m_pblk == COMPRESS_ADDR))
+		return -EINVAL;
 
+	if (map.m_pblk != NULL_ADDR) {
+		iomap->length = blks_to_bytes(inode, map.m_len);
+		iomap->type = IOMAP_MAPPED;
+		iomap->flags |= IOMAP_F_MERGED;
 		iomap->bdev = map.m_bdev;
 		iomap->addr = blks_to_bytes(inode, map.m_pblk);
 	} else {
+		if (flags & IOMAP_WRITE)
+			return -ENOTBLK;
 		iomap->length = blks_to_bytes(inode, next_pgofs) -
 				iomap->offset;
 		iomap->type = IOMAP_HOLE;
-- 
2.30.2



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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [f2fs-dev] [PATCH 02/15] f2fs: decouple F2FS_MAP_ from buffer head flags
  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 ` Christoph Hellwig
  2022-11-28  9:15 ` [f2fs-dev] [PATCH 03/15] f2fs: rename F2FS_MAP_UNWRITTEN to F2FS_MAP_DELALLOC Christoph Hellwig
                   ` (13 subsequent siblings)
  15 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2022-11-28  9:15 UTC (permalink / raw)
  To: Jaegeuk Kim, Chao Yu; +Cc: linux-f2fs-devel

m_flags is never interchanged with the buffer_heads b_flags directly,
so use separate codepoints from that.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/f2fs/f2fs.h | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index e6355a5683b75c..db2dc3fa73162b 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -643,13 +643,11 @@ struct extent_tree {
 };
 
 /*
- * This structure is taken from ext4_map_blocks.
- *
- * Note that, however, f2fs uses NEW and MAPPED flags for f2fs_map_blocks().
+ * State of block returned by f2fs_map_blocks.
  */
-#define F2FS_MAP_NEW		(1 << BH_New)
-#define F2FS_MAP_MAPPED		(1 << BH_Mapped)
-#define F2FS_MAP_UNWRITTEN	(1 << BH_Unwritten)
+#define F2FS_MAP_NEW		(1U << 0)
+#define F2FS_MAP_MAPPED		(1U << 1)
+#define F2FS_MAP_UNWRITTEN	(1U << 2)
 #define F2FS_MAP_FLAGS		(F2FS_MAP_NEW | F2FS_MAP_MAPPED |\
 				F2FS_MAP_UNWRITTEN)
 
-- 
2.30.2



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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [f2fs-dev] [PATCH 03/15] f2fs: rename F2FS_MAP_UNWRITTEN to F2FS_MAP_DELALLOC
  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 ` Christoph Hellwig
  2022-11-28  9:15 ` [f2fs-dev] [PATCH 04/15] f2fs: split __submit_bio Christoph Hellwig
                   ` (12 subsequent siblings)
  15 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2022-11-28  9:15 UTC (permalink / raw)
  To: Jaegeuk Kim, Chao Yu; +Cc: linux-f2fs-devel

NEW_ADDR blocks are purely in-memory preallocated blocks, and thus
equivalent to what the core FS code calls delayed allocations, and not
unwritten extents which do have on-disk blocks allocated from which
reads always return zeroes until they are converted to written status.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/f2fs/data.c | 8 ++++----
 fs/f2fs/f2fs.h | 4 ++--
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 541d625d94be42..444865e0cb6397 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1638,9 +1638,9 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
 		bidx = f2fs_target_device_index(sbi, blkaddr);
 
 	if (map->m_len == 0) {
-		/* preallocated unwritten block should be mapped for fiemap. */
+		/* reserved delalloc block should be mapped for fiemap. */
 		if (blkaddr == NEW_ADDR)
-			map->m_flags |= F2FS_MAP_UNWRITTEN;
+			map->m_flags |= F2FS_MAP_DELALLOC;
 		map->m_flags |= F2FS_MAP_MAPPED;
 
 		map->m_pblk = blkaddr;
@@ -1962,7 +1962,7 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
 
 	compr_appended = false;
 	/* In a case of compressed cluster, append this to the last extent */
-	if (compr_cluster && ((map.m_flags & F2FS_MAP_UNWRITTEN) ||
+	if (compr_cluster && ((map.m_flags & F2FS_MAP_DELALLOC) ||
 			!(map.m_flags & F2FS_MAP_FLAGS))) {
 		compr_appended = true;
 		goto skip_fill;
@@ -2008,7 +2008,7 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
 				compr_cluster = false;
 				size += blks_to_bytes(inode, 1);
 			}
-		} else if (map.m_flags & F2FS_MAP_UNWRITTEN) {
+		} else if (map.m_flags & F2FS_MAP_DELALLOC) {
 			flags = FIEMAP_EXTENT_UNWRITTEN;
 		}
 
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index db2dc3fa73162b..709bfd1c7c409e 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -647,9 +647,9 @@ struct extent_tree {
  */
 #define F2FS_MAP_NEW		(1U << 0)
 #define F2FS_MAP_MAPPED		(1U << 1)
-#define F2FS_MAP_UNWRITTEN	(1U << 2)
+#define F2FS_MAP_DELALLOC	(1U << 2)
 #define F2FS_MAP_FLAGS		(F2FS_MAP_NEW | F2FS_MAP_MAPPED |\
-				F2FS_MAP_UNWRITTEN)
+				F2FS_MAP_DELALLOC)
 
 struct f2fs_map_blocks {
 	struct block_device *m_bdev;	/* for multi-device dio */
-- 
2.30.2



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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [f2fs-dev] [PATCH 04/15] f2fs: split __submit_bio
  2022-11-28  9:15 [f2fs-dev] a fix and a bunch of cleanups Christoph Hellwig
                   ` (2 preceding siblings ...)
  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 ` 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
                   ` (11 subsequent siblings)
  15 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2022-11-28  9:15 UTC (permalink / raw)
  To: Jaegeuk Kim, Chao Yu; +Cc: linux-f2fs-devel

Split __submit_bio into one function each for reads and writes, and a
helper for aligning writes.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/f2fs/compress.c |   2 +-
 fs/f2fs/data.c     | 111 +++++++++++++++++++++++----------------------
 fs/f2fs/f2fs.h     |   4 +-
 3 files changed, 61 insertions(+), 56 deletions(-)

diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index d315c2de136f26..355200b6dad774 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -1073,7 +1073,7 @@ static int prepare_compress_overwrite(struct compress_ctx *cc,
 		if (ret)
 			goto out;
 		if (bio)
-			f2fs_submit_bio(sbi, bio, DATA);
+			f2fs_submit_read_bio(sbi, bio, DATA);
 
 		ret = f2fs_init_compress_ctx(cc);
 		if (ret)
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 444865e0cb6397..404e1637e31072 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -492,65 +492,66 @@ static bool f2fs_crypt_mergeable_bio(struct bio *bio, const struct inode *inode,
 	return fscrypt_mergeable_bio(bio, inode, next_idx);
 }
 
-static inline void __submit_bio(struct f2fs_sb_info *sbi,
-				struct bio *bio, enum page_type type)
+void f2fs_submit_read_bio(struct f2fs_sb_info *sbi, struct bio *bio,
+				 enum page_type type)
 {
-	if (!is_read_io(bio_op(bio))) {
-		unsigned int start;
+	WARN_ON_ONCE(!is_read_io(bio_op(bio)));
+	trace_f2fs_submit_read_bio(sbi->sb, type, bio);
 
-		if (type != DATA && type != NODE)
-			goto submit_io;
+	iostat_update_submit_ctx(bio, type);
+	submit_bio(bio);
+}
 
-		if (f2fs_lfs_mode(sbi) && current->plug)
-			blk_finish_plug(current->plug);
+static void f2fs_align_write_bio(struct f2fs_sb_info *sbi, struct bio *bio)
+{
+	unsigned int start =
+		(bio->bi_iter.bi_size >> F2FS_BLKSIZE_BITS) % F2FS_IO_SIZE(sbi);
 
-		if (!F2FS_IO_ALIGNED(sbi))
-			goto submit_io;
+	if (start == 0)
+		return;
 
-		start = bio->bi_iter.bi_size >> F2FS_BLKSIZE_BITS;
-		start %= F2FS_IO_SIZE(sbi);
+	/* fill dummy pages */
+	for (; start < F2FS_IO_SIZE(sbi); start++) {
+		struct page *page =
+			mempool_alloc(sbi->write_io_dummy,
+				      GFP_NOIO | __GFP_NOFAIL);
+		f2fs_bug_on(sbi, !page);
 
-		if (start == 0)
-			goto submit_io;
+		lock_page(page);
 
-		/* fill dummy pages */
-		for (; start < F2FS_IO_SIZE(sbi); start++) {
-			struct page *page =
-				mempool_alloc(sbi->write_io_dummy,
-					      GFP_NOIO | __GFP_NOFAIL);
-			f2fs_bug_on(sbi, !page);
+		zero_user_segment(page, 0, PAGE_SIZE);
+		set_page_private_dummy(page);
 
-			lock_page(page);
+		if (bio_add_page(bio, page, PAGE_SIZE, 0) < PAGE_SIZE)
+			f2fs_bug_on(sbi, 1);
+	}
+}
 
-			zero_user_segment(page, 0, PAGE_SIZE);
-			set_page_private_dummy(page);
+static void f2fs_submit_write_bio(struct f2fs_sb_info *sbi, struct bio *bio,
+				  enum page_type type)
+{
+	WARN_ON_ONCE(is_read_io(bio_op(bio)));
+
+	if (type == DATA || type == NODE) {
+		if (f2fs_lfs_mode(sbi) && current->plug)
+			blk_finish_plug(current->plug);
 
-			if (bio_add_page(bio, page, PAGE_SIZE, 0) < PAGE_SIZE)
-				f2fs_bug_on(sbi, 1);
+		if (F2FS_IO_ALIGNED(sbi)) {
+			f2fs_align_write_bio(sbi, bio);
+			/*
+			 * In the NODE case, we lose next block address chain.
+			 * So, we need to do checkpoint in f2fs_sync_file.
+			 */
+			if (type == NODE)
+				set_sbi_flag(sbi, SBI_NEED_CP);
 		}
-		/*
-		 * In the NODE case, we lose next block address chain. So, we
-		 * need to do checkpoint in f2fs_sync_file.
-		 */
-		if (type == NODE)
-			set_sbi_flag(sbi, SBI_NEED_CP);
 	}
-submit_io:
-	if (is_read_io(bio_op(bio)))
-		trace_f2fs_submit_read_bio(sbi->sb, type, bio);
-	else
-		trace_f2fs_submit_write_bio(sbi->sb, type, bio);
 
+	trace_f2fs_submit_write_bio(sbi->sb, type, bio);
 	iostat_update_submit_ctx(bio, type);
 	submit_bio(bio);
 }
 
-void f2fs_submit_bio(struct f2fs_sb_info *sbi,
-				struct bio *bio, enum page_type type)
-{
-	__submit_bio(sbi, bio, type);
-}
-
 static void __submit_merged_bio(struct f2fs_bio_info *io)
 {
 	struct f2fs_io_info *fio = &io->fio;
@@ -558,12 +559,13 @@ static void __submit_merged_bio(struct f2fs_bio_info *io)
 	if (!io->bio)
 		return;
 
-	if (is_read_io(fio->op))
+	if (is_read_io(fio->op)) {
 		trace_f2fs_prepare_read_bio(io->sbi->sb, fio->type, io->bio);
-	else
+		f2fs_submit_read_bio(io->sbi, io->bio, fio->type);
+	} else {
 		trace_f2fs_prepare_write_bio(io->sbi->sb, fio->type, io->bio);
-
-	__submit_bio(io->sbi, io->bio, fio->type);
+		f2fs_submit_write_bio(io->sbi, io->bio, fio->type);
+	}
 	io->bio = NULL;
 }
 
@@ -731,7 +733,10 @@ int f2fs_submit_page_bio(struct f2fs_io_info *fio)
 	inc_page_count(fio->sbi, is_read_io(fio->op) ?
 			__read_io_type(page) : WB_DATA_TYPE(fio->page));
 
-	__submit_bio(fio->sbi, bio, fio->type);
+	if (is_read_io(bio_op(bio)))
+		f2fs_submit_read_bio(fio->sbi, bio, fio->type);
+	else
+		f2fs_submit_write_bio(fio->sbi, bio, fio->type);
 	return 0;
 }
 
@@ -833,7 +838,7 @@ static int add_ipu_page(struct f2fs_io_info *fio, struct bio **bio,
 
 			/* page can't be merged into bio; submit the bio */
 			del_bio_entry(be);
-			__submit_bio(sbi, *bio, DATA);
+			f2fs_submit_write_bio(sbi, *bio, DATA);
 			break;
 		}
 		f2fs_up_write(&io->bio_list_lock);
@@ -896,7 +901,7 @@ void f2fs_submit_merged_ipu_write(struct f2fs_sb_info *sbi,
 	}
 
 	if (found)
-		__submit_bio(sbi, target, DATA);
+		f2fs_submit_write_bio(sbi, target, DATA);
 	if (bio && *bio) {
 		bio_put(*bio);
 		*bio = NULL;
@@ -1092,7 +1097,7 @@ static int f2fs_submit_page_read(struct inode *inode, struct page *page,
 	ClearPageError(page);
 	inc_page_count(sbi, F2FS_RD_DATA);
 	f2fs_update_iostat(sbi, NULL, FS_DATA_READ_IO, F2FS_BLKSIZE);
-	__submit_bio(sbi, bio, DATA);
+	f2fs_submit_read_bio(sbi, bio, DATA);
 	return 0;
 }
 
@@ -2115,7 +2120,7 @@ static int f2fs_read_single_page(struct inode *inode, struct page *page,
 				       *last_block_in_bio, block_nr) ||
 		    !f2fs_crypt_mergeable_bio(bio, inode, page->index, NULL))) {
 submit_and_realloc:
-		__submit_bio(F2FS_I_SB(inode), bio, DATA);
+		f2fs_submit_read_bio(F2FS_I_SB(inode), bio, DATA);
 		bio = NULL;
 	}
 	if (bio == NULL) {
@@ -2263,7 +2268,7 @@ int f2fs_read_multi_pages(struct compress_ctx *cc, struct bio **bio_ret,
 					*last_block_in_bio, blkaddr) ||
 		    !f2fs_crypt_mergeable_bio(bio, inode, page->index, NULL))) {
 submit_and_realloc:
-			__submit_bio(sbi, bio, DATA);
+			f2fs_submit_read_bio(sbi, bio, DATA);
 			bio = NULL;
 		}
 
@@ -2427,7 +2432,7 @@ static int f2fs_mpage_readpages(struct inode *inode,
 #endif
 	}
 	if (bio)
-		__submit_bio(F2FS_I_SB(inode), bio, DATA);
+		f2fs_submit_read_bio(F2FS_I_SB(inode), bio, DATA);
 	return ret;
 }
 
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 709bfd1c7c409e..9825b4fb2aa27d 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -3771,8 +3771,8 @@ int __init f2fs_init_bioset(void);
 void f2fs_destroy_bioset(void);
 int f2fs_init_bio_entry_cache(void);
 void f2fs_destroy_bio_entry_cache(void);
-void f2fs_submit_bio(struct f2fs_sb_info *sbi,
-				struct bio *bio, enum page_type type);
+void f2fs_submit_read_bio(struct f2fs_sb_info *sbi, struct bio *bio,
+			  enum page_type type);
 int f2fs_init_write_merge_io(struct f2fs_sb_info *sbi);
 void f2fs_submit_merged_write(struct f2fs_sb_info *sbi, enum page_type type);
 void f2fs_submit_merged_write_cond(struct f2fs_sb_info *sbi,
-- 
2.30.2



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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [f2fs-dev] [PATCH 05/15] f2fs: fold f2fs_lookup_extent_tree into f2fs_lookup_extent_cache
  2022-11-28  9:15 [f2fs-dev] a fix and a bunch of cleanups Christoph Hellwig
                   ` (3 preceding siblings ...)
  2022-11-28  9:15 ` [f2fs-dev] [PATCH 04/15] f2fs: split __submit_bio Christoph Hellwig
@ 2022-11-28  9:15 ` Christoph Hellwig
  2022-11-28  9:15 ` [f2fs-dev] [PATCH 06/15] f2fs: add a f2fs_lookup_extent_cache_block helper Christoph Hellwig
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2022-11-28  9:15 UTC (permalink / raw)
  To: Jaegeuk Kim, Chao Yu; +Cc: linux-f2fs-devel

No need to keep these two helpers separate.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/f2fs/extent_cache.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
index 932c070173b976..538e4b79f83c9b 100644
--- a/fs/f2fs/extent_cache.c
+++ b/fs/f2fs/extent_cache.c
@@ -407,14 +407,17 @@ void f2fs_init_extent_tree(struct inode *inode, struct page *ipage)
 		set_inode_flag(inode, FI_NO_EXTENT);
 }
 
-static bool f2fs_lookup_extent_tree(struct inode *inode, pgoff_t pgofs,
-							struct extent_info *ei)
+bool f2fs_lookup_extent_cache(struct inode *inode, pgoff_t pgofs,
+		struct extent_info *ei)
 {
 	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
 	struct extent_tree *et = F2FS_I(inode)->extent_tree;
 	struct extent_node *en;
 	bool ret = false;
 
+	if (!f2fs_may_extent_tree(inode))
+		return false;
+
 	f2fs_bug_on(sbi, !et);
 
 	trace_f2fs_lookup_extent_tree_start(inode, pgofs);
@@ -850,15 +853,6 @@ void f2fs_destroy_extent_tree(struct inode *inode)
 	trace_f2fs_destroy_extent_tree(inode, node_cnt);
 }
 
-bool f2fs_lookup_extent_cache(struct inode *inode, pgoff_t pgofs,
-					struct extent_info *ei)
-{
-	if (!f2fs_may_extent_tree(inode))
-		return false;
-
-	return f2fs_lookup_extent_tree(inode, pgofs, ei);
-}
-
 void f2fs_update_extent_cache(struct dnode_of_data *dn)
 {
 	pgoff_t fofs;
-- 
2.30.2



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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [f2fs-dev] [PATCH 06/15] f2fs: add a f2fs_lookup_extent_cache_block helper
  2022-11-28  9:15 [f2fs-dev] a fix and a bunch of cleanups Christoph Hellwig
                   ` (4 preceding siblings ...)
  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 ` Christoph Hellwig
  2022-11-28  9:15 ` [f2fs-dev] [PATCH 07/15] f2fs: add a f2fs_get_block_locked helper Christoph Hellwig
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2022-11-28  9:15 UTC (permalink / raw)
  To: Jaegeuk Kim, Chao Yu; +Cc: linux-f2fs-devel

All but three callers of f2fs_lookup_extent_cache just want the block
address.  Add a small helper to simplify them.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/f2fs/data.c         | 29 +++++++----------------------
 fs/f2fs/extent_cache.c | 11 +++++++++++
 fs/f2fs/f2fs.h         |  2 ++
 fs/f2fs/gc.c           |  4 +---
 4 files changed, 21 insertions(+), 25 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 404e1637e31072..a294a589ba1a91 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1199,14 +1199,8 @@ int f2fs_reserve_block(struct dnode_of_data *dn, pgoff_t index)
 
 int f2fs_get_block(struct dnode_of_data *dn, pgoff_t index)
 {
-	struct extent_info ei = {0, };
-	struct inode *inode = dn->inode;
-
-	if (f2fs_lookup_extent_cache(inode, index, &ei)) {
-		dn->data_blkaddr = ei.blk + index - ei.fofs;
+	if (f2fs_lookup_extent_cache_block(dn->inode, index, &dn->data_blkaddr))
 		return 0;
-	}
-
 	return f2fs_reserve_block(dn, index);
 }
 
@@ -1216,15 +1210,13 @@ struct page *f2fs_get_read_data_page(struct inode *inode, pgoff_t index,
 	struct address_space *mapping = inode->i_mapping;
 	struct dnode_of_data dn;
 	struct page *page;
-	struct extent_info ei = {0, };
 	int err;
 
 	page = f2fs_grab_cache_page(mapping, index, for_write);
 	if (!page)
 		return ERR_PTR(-ENOMEM);
 
-	if (f2fs_lookup_extent_cache(inode, index, &ei)) {
-		dn.data_blkaddr = ei.blk + index - ei.fofs;
+	if (f2fs_lookup_extent_cache_block(inode, index, &dn.data_blkaddr)) {
 		if (!f2fs_is_valid_blkaddr(F2FS_I_SB(inode), dn.data_blkaddr,
 						DATA_GENERIC_ENHANCE_READ)) {
 			err = -EFSCORRUPTED;
@@ -2623,7 +2615,6 @@ int f2fs_do_write_data_page(struct f2fs_io_info *fio)
 	struct page *page = fio->page;
 	struct inode *inode = page->mapping->host;
 	struct dnode_of_data dn;
-	struct extent_info ei = {0, };
 	struct node_info ni;
 	bool ipu_force = false;
 	int err = 0;
@@ -2635,9 +2626,8 @@ int f2fs_do_write_data_page(struct f2fs_io_info *fio)
 		set_new_dnode(&dn, inode, NULL, NULL, 0);
 
 	if (need_inplace_update(fio) &&
-			f2fs_lookup_extent_cache(inode, page->index, &ei)) {
-		fio->old_blkaddr = ei.blk + page->index - ei.fofs;
-
+	    f2fs_lookup_extent_cache_block(inode, page->index,
+					   &fio->old_blkaddr)) {
 		if (!f2fs_is_valid_blkaddr(fio->sbi, fio->old_blkaddr,
 						DATA_GENERIC_ENHANCE)) {
 			f2fs_handle_error(fio->sbi,
@@ -3310,7 +3300,6 @@ static int prepare_write_begin(struct f2fs_sb_info *sbi,
 	struct dnode_of_data dn;
 	struct page *ipage;
 	bool locked = false;
-	struct extent_info ei = {0, };
 	int err = 0;
 	int flag;
 
@@ -3359,9 +3348,8 @@ static int prepare_write_begin(struct f2fs_sb_info *sbi,
 	} else if (locked) {
 		err = f2fs_get_block(&dn, index);
 	} else {
-		if (f2fs_lookup_extent_cache(inode, index, &ei)) {
-			dn.data_blkaddr = ei.blk + index - ei.fofs;
-		} else {
+		if (!f2fs_lookup_extent_cache_block(inode, index,
+				&dn.data_blkaddr)) {
 			/* hole case */
 			err = f2fs_get_dnode_of_data(&dn, index, LOOKUP_NODE);
 			if (err || dn.data_blkaddr == NULL_ADDR) {
@@ -3391,7 +3379,6 @@ static int __find_data_block(struct inode *inode, pgoff_t index,
 {
 	struct dnode_of_data dn;
 	struct page *ipage;
-	struct extent_info ei = {0, };
 	int err = 0;
 
 	ipage = f2fs_get_node_page(F2FS_I_SB(inode), inode->i_ino);
@@ -3400,9 +3387,7 @@ static int __find_data_block(struct inode *inode, pgoff_t index,
 
 	set_new_dnode(&dn, inode, ipage, ipage, 0);
 
-	if (f2fs_lookup_extent_cache(inode, index, &ei)) {
-		dn.data_blkaddr = ei.blk + index - ei.fofs;
-	} else {
+	if (!f2fs_lookup_extent_cache_block(inode, index, &dn.data_blkaddr)) {
 		/* hole case */
 		err = f2fs_get_dnode_of_data(&dn, index, LOOKUP_NODE);
 		if (err) {
diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
index 538e4b79f83c9b..992a3d6bd6e7c7 100644
--- a/fs/f2fs/extent_cache.c
+++ b/fs/f2fs/extent_cache.c
@@ -853,6 +853,17 @@ void f2fs_destroy_extent_tree(struct inode *inode)
 	trace_f2fs_destroy_extent_tree(inode, node_cnt);
 }
 
+bool f2fs_lookup_extent_cache_block(struct inode *inode, pgoff_t index,
+		block_t *blkaddr)
+{
+	struct extent_info ei = {};
+
+	if (!f2fs_lookup_extent_cache(inode, index, &ei))
+		return false;
+	*blkaddr = ei.blk + index - ei.fofs;
+	return true;
+}
+
 void f2fs_update_extent_cache(struct dnode_of_data *dn)
 {
 	pgoff_t fofs;
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 9825b4fb2aa27d..c0a1a987889167 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -4157,6 +4157,8 @@ unsigned int f2fs_destroy_extent_node(struct inode *inode);
 void f2fs_destroy_extent_tree(struct inode *inode);
 bool f2fs_lookup_extent_cache(struct inode *inode, pgoff_t pgofs,
 			struct extent_info *ei);
+bool f2fs_lookup_extent_cache_block(struct inode *inode, pgoff_t index,
+			block_t *blkaddr);
 void f2fs_update_extent_cache(struct dnode_of_data *dn);
 void f2fs_update_extent_cache_range(struct dnode_of_data *dn,
 			pgoff_t fofs, block_t blkaddr, unsigned int len);
diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index 4546e01b2ee082..3a82d763e074f6 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -1141,7 +1141,6 @@ static int ra_data_block(struct inode *inode, pgoff_t index)
 	struct address_space *mapping = inode->i_mapping;
 	struct dnode_of_data dn;
 	struct page *page;
-	struct extent_info ei = {0, 0, 0};
 	struct f2fs_io_info fio = {
 		.sbi = sbi,
 		.ino = inode->i_ino,
@@ -1159,8 +1158,7 @@ static int ra_data_block(struct inode *inode, pgoff_t index)
 	if (!page)
 		return -ENOMEM;
 
-	if (f2fs_lookup_extent_cache(inode, index, &ei)) {
-		dn.data_blkaddr = ei.blk + index - ei.fofs;
+	if (f2fs_lookup_extent_cache_block(inode, index, &dn.data_blkaddr)) {
 		if (unlikely(!f2fs_is_valid_blkaddr(sbi, dn.data_blkaddr,
 						DATA_GENERIC_ENHANCE_READ))) {
 			err = -EFSCORRUPTED;
-- 
2.30.2



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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [f2fs-dev] [PATCH 07/15] f2fs: add a f2fs_get_block_locked helper
  2022-11-28  9:15 [f2fs-dev] a fix and a bunch of cleanups Christoph Hellwig
                   ` (5 preceding siblings ...)
  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 ` Christoph Hellwig
  2022-11-28  9:15 ` [f2fs-dev] [PATCH 08/15] f2fs: f2fs_do_map_lock Christoph Hellwig
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2022-11-28  9:15 UTC (permalink / raw)
  To: Jaegeuk Kim, Chao Yu; +Cc: linux-f2fs-devel

This allows to keep the f2fs_do_map_lock based locking scheme
private to data.c.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/f2fs/data.c | 16 ++++++++++++++--
 fs/f2fs/f2fs.h |  3 +--
 fs/f2fs/file.c |  4 +---
 3 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index a294a589ba1a91..6df80a30df263c 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1197,7 +1197,7 @@ int f2fs_reserve_block(struct dnode_of_data *dn, pgoff_t index)
 	return err;
 }
 
-int f2fs_get_block(struct dnode_of_data *dn, pgoff_t index)
+static int f2fs_get_block(struct dnode_of_data *dn, pgoff_t index)
 {
 	if (f2fs_lookup_extent_cache_block(dn->inode, index, &dn->data_blkaddr))
 		return 0;
@@ -1427,7 +1427,7 @@ static int __allocate_data_block(struct dnode_of_data *dn, int seg_type)
 	return 0;
 }
 
-void f2fs_do_map_lock(struct f2fs_sb_info *sbi, int flag, bool lock)
+static void f2fs_do_map_lock(struct f2fs_sb_info *sbi, int flag, bool lock)
 {
 	if (flag == F2FS_GET_BLOCK_PRE_AIO) {
 		if (lock)
@@ -1442,6 +1442,18 @@ void f2fs_do_map_lock(struct f2fs_sb_info *sbi, int flag, bool lock)
 	}
 }
 
+int f2fs_get_block_locked(struct dnode_of_data *dn, pgoff_t index)
+{
+	struct f2fs_sb_info *sbi = F2FS_I_SB(dn->inode);
+	int err;
+
+	f2fs_do_map_lock(sbi, F2FS_GET_BLOCK_PRE_AIO, true);
+	err = f2fs_get_block(dn, index);
+	f2fs_do_map_lock(sbi, F2FS_GET_BLOCK_PRE_AIO, false);
+
+	return err;
+}
+
 /*
  * f2fs_map_blocks() tries to find or build mapping relationship which
  * maps continuous logical blocks to physical blocks, and return such
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index c0a1a987889167..a3789dab0aade9 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -3791,7 +3791,7 @@ void f2fs_set_data_blkaddr(struct dnode_of_data *dn);
 void f2fs_update_data_blkaddr(struct dnode_of_data *dn, block_t blkaddr);
 int f2fs_reserve_new_blocks(struct dnode_of_data *dn, blkcnt_t count);
 int f2fs_reserve_new_block(struct dnode_of_data *dn);
-int f2fs_get_block(struct dnode_of_data *dn, pgoff_t index);
+int f2fs_get_block_locked(struct dnode_of_data *dn, pgoff_t index);
 int f2fs_reserve_block(struct dnode_of_data *dn, pgoff_t index);
 struct page *f2fs_get_read_data_page(struct inode *inode, pgoff_t index,
 			blk_opf_t op_flags, bool for_write);
@@ -3801,7 +3801,6 @@ 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);
-void f2fs_do_map_lock(struct f2fs_sb_info *sbi, int flag, bool lock);
 int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
 			int create, int flag);
 int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 82cda12582272a..cbeb7bd880046e 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -113,10 +113,8 @@ static vm_fault_t f2fs_vm_page_mkwrite(struct vm_fault *vmf)
 
 	if (need_alloc) {
 		/* block allocation */
-		f2fs_do_map_lock(sbi, F2FS_GET_BLOCK_PRE_AIO, true);
 		set_new_dnode(&dn, inode, NULL, NULL, 0);
-		err = f2fs_get_block(&dn, page->index);
-		f2fs_do_map_lock(sbi, F2FS_GET_BLOCK_PRE_AIO, false);
+		err = f2fs_get_block_locked(&dn, page->index);
 	}
 
 #ifdef CONFIG_F2FS_FS_COMPRESSION
-- 
2.30.2



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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [f2fs-dev] [PATCH 08/15] f2fs: f2fs_do_map_lock
  2022-11-28  9:15 [f2fs-dev] a fix and a bunch of cleanups Christoph Hellwig
                   ` (6 preceding siblings ...)
  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 ` Christoph Hellwig
  2022-11-28  9:15 ` [f2fs-dev] [PATCH 09/15] f2fs: reflow prepare_write_begin Christoph Hellwig
                   ` (7 subsequent siblings)
  15 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2022-11-28  9:15 UTC (permalink / raw)
  To: Jaegeuk Kim, Chao Yu; +Cc: linux-f2fs-devel

Split f2fs_do_map_lock into a lock and unlock helper to make the code
using it easier to read.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/f2fs/data.c | 46 +++++++++++++++++++++++-----------------------
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 6df80a30df263c..b7c8e3e0113aff 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1427,19 +1427,20 @@ static int __allocate_data_block(struct dnode_of_data *dn, int seg_type)
 	return 0;
 }
 
-static void f2fs_do_map_lock(struct f2fs_sb_info *sbi, int flag, bool lock)
+static void f2fs_map_lock(struct f2fs_sb_info *sbi, int flag)
 {
-	if (flag == F2FS_GET_BLOCK_PRE_AIO) {
-		if (lock)
-			f2fs_down_read(&sbi->node_change);
-		else
-			f2fs_up_read(&sbi->node_change);
-	} else {
-		if (lock)
-			f2fs_lock_op(sbi);
-		else
-			f2fs_unlock_op(sbi);
-	}
+	if (flag == F2FS_GET_BLOCK_PRE_AIO)
+		f2fs_down_read(&sbi->node_change);
+	else
+		f2fs_lock_op(sbi);
+}
+
+static void f2fs_map_unlock(struct f2fs_sb_info *sbi, int flag)
+{
+	if (flag == F2FS_GET_BLOCK_PRE_AIO)
+		f2fs_up_read(&sbi->node_change);
+	else
+		f2fs_unlock_op(sbi);
 }
 
 int f2fs_get_block_locked(struct dnode_of_data *dn, pgoff_t index)
@@ -1447,9 +1448,9 @@ int f2fs_get_block_locked(struct dnode_of_data *dn, pgoff_t index)
 	struct f2fs_sb_info *sbi = F2FS_I_SB(dn->inode);
 	int err;
 
-	f2fs_do_map_lock(sbi, F2FS_GET_BLOCK_PRE_AIO, true);
+	f2fs_map_lock(sbi, F2FS_GET_BLOCK_PRE_AIO);
 	err = f2fs_get_block(dn, index);
-	f2fs_do_map_lock(sbi, F2FS_GET_BLOCK_PRE_AIO, false);
+	f2fs_map_unlock(sbi, F2FS_GET_BLOCK_PRE_AIO);
 
 	return err;
 }
@@ -1524,7 +1525,7 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
 
 next_dnode:
 	if (map->m_may_create)
-		f2fs_do_map_lock(sbi, flag, true);
+		f2fs_map_lock(sbi, flag);
 
 	/* When reading holes, we need its node page */
 	set_new_dnode(&dn, inode, NULL, NULL, 0);
@@ -1708,7 +1709,7 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
 	f2fs_put_dnode(&dn);
 
 	if (map->m_may_create) {
-		f2fs_do_map_lock(sbi, flag, false);
+		f2fs_map_unlock(sbi, flag);
 		f2fs_balance_fs(sbi, dn.node_changed);
 	}
 	goto next_dnode;
@@ -1754,7 +1755,7 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
 	f2fs_put_dnode(&dn);
 unlock_out:
 	if (map->m_may_create) {
-		f2fs_do_map_lock(sbi, flag, false);
+		f2fs_map_unlock(sbi, flag);
 		f2fs_balance_fs(sbi, dn.node_changed);
 	}
 out:
@@ -3330,7 +3331,7 @@ static int prepare_write_begin(struct f2fs_sb_info *sbi,
 
 	if (f2fs_has_inline_data(inode) ||
 			(pos & PAGE_MASK) >= i_size_read(inode)) {
-		f2fs_do_map_lock(sbi, flag, true);
+		f2fs_map_lock(sbi, flag);
 		locked = true;
 	}
 
@@ -3366,8 +3367,7 @@ static int prepare_write_begin(struct f2fs_sb_info *sbi,
 			err = f2fs_get_dnode_of_data(&dn, index, LOOKUP_NODE);
 			if (err || dn.data_blkaddr == NULL_ADDR) {
 				f2fs_put_dnode(&dn);
-				f2fs_do_map_lock(sbi, F2FS_GET_BLOCK_PRE_AIO,
-								true);
+				f2fs_map_lock(sbi, F2FS_GET_BLOCK_PRE_AIO);
 				WARN_ON(flag != F2FS_GET_BLOCK_PRE_AIO);
 				locked = true;
 				goto restart;
@@ -3382,7 +3382,7 @@ static int prepare_write_begin(struct f2fs_sb_info *sbi,
 	f2fs_put_dnode(&dn);
 unlock_out:
 	if (locked)
-		f2fs_do_map_lock(sbi, flag, false);
+		f2fs_map_unlock(sbi, flag);
 	return err;
 }
 
@@ -3420,7 +3420,7 @@ static int __reserve_data_block(struct inode *inode, pgoff_t index,
 	struct page *ipage;
 	int err = 0;
 
-	f2fs_do_map_lock(sbi, F2FS_GET_BLOCK_PRE_AIO, true);
+	f2fs_map_lock(sbi, F2FS_GET_BLOCK_PRE_AIO);
 
 	ipage = f2fs_get_node_page(sbi, inode->i_ino);
 	if (IS_ERR(ipage)) {
@@ -3436,7 +3436,7 @@ static int __reserve_data_block(struct inode *inode, pgoff_t index,
 	f2fs_put_dnode(&dn);
 
 unlock_out:
-	f2fs_do_map_lock(sbi, F2FS_GET_BLOCK_PRE_AIO, false);
+	f2fs_map_unlock(sbi, F2FS_GET_BLOCK_PRE_AIO);
 	return err;
 }
 
-- 
2.30.2



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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [f2fs-dev] [PATCH 09/15] f2fs: reflow prepare_write_begin
  2022-11-28  9:15 [f2fs-dev] a fix and a bunch of cleanups Christoph Hellwig
                   ` (7 preceding siblings ...)
  2022-11-28  9:15 ` [f2fs-dev] [PATCH 08/15] f2fs: f2fs_do_map_lock Christoph Hellwig
@ 2022-11-28  9:15 ` Christoph Hellwig
  2022-11-28  9:15 ` [f2fs-dev] [PATCH 10/15] f2fs: simplify __allocate_data_block Christoph Hellwig
                   ` (6 subsequent siblings)
  15 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2022-11-28  9:15 UTC (permalink / raw)
  To: Jaegeuk Kim, Chao Yu; +Cc: linux-f2fs-devel

Reflow prepare_write_begin so that it reads more straight forward,
and so that there is one place that does an extent cache lookup
instead of three, two of which are hidden in f2fs_get_block calls.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/f2fs/data.c | 57 +++++++++++++++++++++++++-------------------------
 1 file changed, 28 insertions(+), 29 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index b7c8e3e0113aff..fc5207859912ce 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -3313,8 +3313,8 @@ static int prepare_write_begin(struct f2fs_sb_info *sbi,
 	struct dnode_of_data dn;
 	struct page *ipage;
 	bool locked = false;
+	int flag = F2FS_GET_BLOCK_PRE_AIO;
 	int err = 0;
-	int flag;
 
 	/*
 	 * If a whole page is being written and we already preallocated all the
@@ -3324,13 +3324,12 @@ static int prepare_write_begin(struct f2fs_sb_info *sbi,
 		return 0;
 
 	/* f2fs_lock_op avoids race between write CP and convert_inline_page */
-	if (f2fs_has_inline_data(inode) && pos + len > MAX_INLINE_DATA(inode))
-		flag = F2FS_GET_BLOCK_DEFAULT;
-	else
-		flag = F2FS_GET_BLOCK_PRE_AIO;
-
-	if (f2fs_has_inline_data(inode) ||
-			(pos & PAGE_MASK) >= i_size_read(inode)) {
+	if (f2fs_has_inline_data(inode)) {
+		if (pos + len > MAX_INLINE_DATA(inode))
+			flag = F2FS_GET_BLOCK_DEFAULT;
+		f2fs_map_lock(sbi, flag);
+		locked = true;
+	} else if ((pos & PAGE_MASK) >= i_size_read(inode)) {
 		f2fs_map_lock(sbi, flag);
 		locked = true;
 	}
@@ -3351,34 +3350,34 @@ static int prepare_write_begin(struct f2fs_sb_info *sbi,
 			set_inode_flag(inode, FI_DATA_EXIST);
 			if (inode->i_nlink)
 				set_page_private_inline(ipage);
-		} else {
-			err = f2fs_convert_inline_page(&dn, page);
-			if (err)
-				goto out;
-			if (dn.data_blkaddr == NULL_ADDR)
-				err = f2fs_get_block(&dn, index);
+			goto out;
 		}
-	} else if (locked) {
-		err = f2fs_get_block(&dn, index);
-	} else {
-		if (!f2fs_lookup_extent_cache_block(inode, index,
-				&dn.data_blkaddr)) {
+		err = f2fs_convert_inline_page(&dn, page);
+		if (err || dn.data_blkaddr != NULL_ADDR)
+			goto out;
+	}
+
+	if (!f2fs_lookup_extent_cache_block(inode, index, &dn.data_blkaddr)) {
+		if (!locked) {
 			/* hole case */
 			err = f2fs_get_dnode_of_data(&dn, index, LOOKUP_NODE);
-			if (err || dn.data_blkaddr == NULL_ADDR) {
-				f2fs_put_dnode(&dn);
-				f2fs_map_lock(sbi, F2FS_GET_BLOCK_PRE_AIO);
-				WARN_ON(flag != F2FS_GET_BLOCK_PRE_AIO);
-				locked = true;
-				goto restart;
-			}
+			if (!err && dn.data_blkaddr != NULL_ADDR)
+				goto out;
+			f2fs_put_dnode(&dn);
+			f2fs_map_lock(sbi, F2FS_GET_BLOCK_PRE_AIO);
+			WARN_ON(flag != F2FS_GET_BLOCK_PRE_AIO);
+			locked = true;
+			goto restart;
 		}
+		err = f2fs_reserve_block(&dn, index);
 	}
 
-	/* convert_inline_page can make node_changed */
-	*blk_addr = dn.data_blkaddr;
-	*node_changed = dn.node_changed;
 out:
+	if (!err) {
+		/* convert_inline_page can make node_changed */
+		*blk_addr = dn.data_blkaddr;
+		*node_changed = dn.node_changed;
+	}
 	f2fs_put_dnode(&dn);
 unlock_out:
 	if (locked)
-- 
2.30.2



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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [f2fs-dev] [PATCH 10/15] f2fs: simplify __allocate_data_block
  2022-11-28  9:15 [f2fs-dev] a fix and a bunch of cleanups Christoph Hellwig
                   ` (8 preceding siblings ...)
  2022-11-28  9:15 ` [f2fs-dev] [PATCH 09/15] f2fs: reflow prepare_write_begin Christoph Hellwig
@ 2022-11-28  9:15 ` Christoph Hellwig
  2022-11-28  9:15 ` [f2fs-dev] [PATCH 11/15] f2fs: remove f2fs_get_block Christoph Hellwig
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2022-11-28  9:15 UTC (permalink / raw)
  To: Jaegeuk Kim, Chao Yu; +Cc: linux-f2fs-devel

Just use a simple if block for the conditional call to
inc_valid_block_count.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/f2fs/data.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index fc5207859912ce..87c17602a3fdd4 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1407,13 +1407,12 @@ static int __allocate_data_block(struct dnode_of_data *dn, int seg_type)
 		return err;
 
 	dn->data_blkaddr = f2fs_data_blkaddr(dn);
-	if (dn->data_blkaddr != NULL_ADDR)
-		goto alloc;
-
-	if (unlikely((err = inc_valid_block_count(sbi, dn->inode, &count))))
-		return err;
+	if (dn->data_blkaddr == NULL_ADDR) {
+		err = inc_valid_block_count(sbi, dn->inode, &count);
+		if (unlikely(err))
+			return err;
+	}
 
-alloc:
 	set_summary(&sum, dn->nid, dn->ofs_in_node, ni.version);
 	old_blkaddr = dn->data_blkaddr;
 	f2fs_allocate_data_block(sbi, NULL, old_blkaddr, &dn->data_blkaddr,
-- 
2.30.2



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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [f2fs-dev] [PATCH 11/15] f2fs: remove f2fs_get_block
  2022-11-28  9:15 [f2fs-dev] a fix and a bunch of cleanups Christoph Hellwig
                   ` (9 preceding siblings ...)
  2022-11-28  9:15 ` [f2fs-dev] [PATCH 10/15] f2fs: simplify __allocate_data_block Christoph Hellwig
@ 2022-11-28  9:15 ` Christoph Hellwig
  2022-11-28  9:15 ` [f2fs-dev] [PATCH 12/15] f2fs: remove the create argument to f2fs_map_blocks Christoph Hellwig
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2022-11-28  9:15 UTC (permalink / raw)
  To: Jaegeuk Kim, Chao Yu; +Cc: linux-f2fs-devel

Fold f2fs_get_block into the two remaining callers to simplify the
call chain a bit.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/f2fs/data.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 87c17602a3fdd4..2ae8fcf7cf49f4 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1197,13 +1197,6 @@ int f2fs_reserve_block(struct dnode_of_data *dn, pgoff_t index)
 	return err;
 }
 
-static int f2fs_get_block(struct dnode_of_data *dn, pgoff_t index)
-{
-	if (f2fs_lookup_extent_cache_block(dn->inode, index, &dn->data_blkaddr))
-		return 0;
-	return f2fs_reserve_block(dn, index);
-}
-
 struct page *f2fs_get_read_data_page(struct inode *inode, pgoff_t index,
 				     blk_opf_t op_flags, bool for_write)
 {
@@ -1445,10 +1438,12 @@ static void f2fs_map_unlock(struct f2fs_sb_info *sbi, int flag)
 int f2fs_get_block_locked(struct dnode_of_data *dn, pgoff_t index)
 {
 	struct f2fs_sb_info *sbi = F2FS_I_SB(dn->inode);
-	int err;
+	int err = 0;
 
 	f2fs_map_lock(sbi, F2FS_GET_BLOCK_PRE_AIO);
-	err = f2fs_get_block(dn, index);
+	if (!f2fs_lookup_extent_cache_block(dn->inode, index,
+					    &dn->data_blkaddr))
+		err = f2fs_reserve_block(dn, index);
 	f2fs_map_unlock(sbi, F2FS_GET_BLOCK_PRE_AIO);
 
 	return err;
@@ -3427,7 +3422,8 @@ static int __reserve_data_block(struct inode *inode, pgoff_t index,
 	}
 	set_new_dnode(&dn, inode, ipage, ipage, 0);
 
-	err = f2fs_get_block(&dn, index);
+	if (!f2fs_lookup_extent_cache_block(inode, index, &dn.data_blkaddr))
+		err = f2fs_reserve_block(&dn, index);
 
 	*blk_addr = dn.data_blkaddr;
 	*node_changed = dn.node_changed;
-- 
2.30.2



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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [f2fs-dev] [PATCH 12/15] f2fs: remove the create argument to f2fs_map_blocks
  2022-11-28  9:15 [f2fs-dev] a fix and a bunch of cleanups Christoph Hellwig
                   ` (10 preceding siblings ...)
  2022-11-28  9:15 ` [f2fs-dev] [PATCH 11/15] f2fs: remove f2fs_get_block Christoph Hellwig
@ 2022-11-28  9:15 ` Christoph Hellwig
  2022-12-13  1:22   ` Jaegeuk Kim
  2022-11-28  9:15 ` [f2fs-dev] [PATCH 13/15] f2fs: factor a f2f_map_blocks_cached helper Christoph Hellwig
                   ` (3 subsequent siblings)
  15 siblings, 1 reply; 22+ messages in thread
From: Christoph Hellwig @ 2022-11-28  9:15 UTC (permalink / raw)
  To: Jaegeuk Kim, Chao Yu; +Cc: linux-f2fs-devel

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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [f2fs-dev] [PATCH 13/15] f2fs: factor a f2f_map_blocks_cached helper
  2022-11-28  9:15 [f2fs-dev] a fix and a bunch of cleanups Christoph Hellwig
                   ` (11 preceding siblings ...)
  2022-11-28  9:15 ` [f2fs-dev] [PATCH 12/15] f2fs: remove the create argument to f2fs_map_blocks Christoph Hellwig
@ 2022-11-28  9:15 ` Christoph Hellwig
  2022-11-28  9:15 ` [f2fs-dev] [PATCH 14/15] f2fs: factor out a f2fs_map_no_dnode Christoph Hellwig
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2022-11-28  9:15 UTC (permalink / raw)
  To: Jaegeuk Kim, Chao Yu; +Cc: linux-f2fs-devel

Add a helper to deal with everything needed to return a f2fs_map_blocks
structure based on a lookup in the extent cache.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/f2fs/data.c | 62 +++++++++++++++++++++++++++++++-------------------
 1 file changed, 38 insertions(+), 24 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 730b58ba97c0ae..f6124cedd121a2 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1449,6 +1449,41 @@ int f2fs_get_block_locked(struct dnode_of_data *dn, pgoff_t index)
 	return err;
 }
 
+static bool f2fs_map_blocks_cached(struct inode *inode,
+		struct f2fs_map_blocks *map, int flag)
+{
+	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
+	unsigned int maxblocks = map->m_len;
+	pgoff_t pgoff = (pgoff_t)map->m_lblk;
+	struct extent_info ei = {};
+
+	if (!f2fs_lookup_extent_cache(inode, pgoff, &ei))
+		return false;
+
+	map->m_pblk = ei.blk + pgoff - ei.fofs;
+	map->m_len = min((pgoff_t)maxblocks, ei.fofs + ei.len - pgoff);
+	map->m_flags = F2FS_MAP_MAPPED;
+	if (map->m_next_extent)
+		*map->m_next_extent = pgoff + map->m_len;
+
+	/* for hardware encryption, but to avoid potential issue in future */
+	if (flag == F2FS_GET_BLOCK_DIO)
+		f2fs_wait_on_block_writeback_range(inode,
+					map->m_pblk, map->m_len);
+
+	if (f2fs_allow_multi_device_dio(sbi, flag)) {
+		int bidx = f2fs_target_device_index(sbi, map->m_pblk);
+		struct f2fs_dev_info *dev = &sbi->devs[bidx];
+
+		map->m_bdev = dev->bdev;
+		map->m_pblk -= dev->start_blk;
+		map->m_len = min(map->m_len, dev->end_blk + 1 - map->m_pblk);
+	} else {
+		map->m_bdev = inode->i_sb->s_bdev;
+	}
+	return true;
+}
+
 /*
  * f2fs_map_blocks() tries to find or build mapping relationship which
  * maps continuous logical blocks to physical blocks, and return such
@@ -1464,7 +1499,6 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
 	int err = 0, ofs = 1;
 	unsigned int ofs_in_node, last_ofs_in_node;
 	blkcnt_t prealloc;
-	struct extent_info ei = {0, };
 	block_t blkaddr;
 	unsigned int start_pgofs;
 	int bidx = 0;
@@ -1472,6 +1506,9 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
 	if (!maxblocks)
 		return 0;
 
+	if (!map->m_may_create && f2fs_map_blocks_cached(inode, map, flag))
+		goto out;
+
 	map->m_bdev = inode->i_sb->s_bdev;
 	map->m_multidev_dio =
 		f2fs_allow_multi_device_dio(F2FS_I_SB(inode), flag);
@@ -1483,29 +1520,6 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
 	pgofs =	(pgoff_t)map->m_lblk;
 	end = pgofs + maxblocks;
 
-	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;
-		if (map->m_next_extent)
-			*map->m_next_extent = pgofs + map->m_len;
-
-		/* for hardware encryption, but to avoid potential issue in future */
-		if (flag == F2FS_GET_BLOCK_DIO)
-			f2fs_wait_on_block_writeback_range(inode,
-						map->m_pblk, map->m_len);
-
-		if (map->m_multidev_dio) {
-			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);
-		}
-		goto out;
-	}
-
 next_dnode:
 	if (map->m_may_create)
 		f2fs_map_lock(sbi, flag);
-- 
2.30.2



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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [f2fs-dev] [PATCH 14/15] f2fs: factor out a f2fs_map_no_dnode
  2022-11-28  9:15 [f2fs-dev] a fix and a bunch of cleanups Christoph Hellwig
                   ` (12 preceding siblings ...)
  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 ` 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
  15 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2022-11-28  9:15 UTC (permalink / raw)
  To: Jaegeuk Kim, Chao Yu; +Cc: linux-f2fs-devel

Factor out a helper to return a hole when no dnode was found.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/f2fs/data.c | 47 ++++++++++++++++++++++++-----------------------
 1 file changed, 24 insertions(+), 23 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index f6124cedd121a2..3c802ce397de52 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1449,6 +1449,28 @@ int f2fs_get_block_locked(struct dnode_of_data *dn, pgoff_t index)
 	return err;
 }
 
+static int f2fs_map_no_dnode(struct inode *inode,
+		struct f2fs_map_blocks *map, struct dnode_of_data *dn,
+		pgoff_t pgoff)
+{
+	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
+
+	/*
+	 * There is one exceptional case that read_node_page() may return
+	 * -ENOENT due to filesystem has been shutdown or cp_error, return
+	 * -EIO in that case.
+	 */
+	if (map->m_may_create &&
+	    (is_sbi_flag_set(sbi, SBI_IS_SHUTDOWN) || f2fs_cp_error(sbi)))
+		return -EIO;
+
+	if (map->m_next_pgofs)
+		*map->m_next_pgofs = f2fs_get_next_page_offset(dn, pgoff);
+	if (map->m_next_extent)
+		*map->m_next_extent = f2fs_get_next_page_offset(dn, pgoff);
+	return 0;
+}
+
 static bool f2fs_map_blocks_cached(struct inode *inode,
 		struct f2fs_map_blocks *map, int flag)
 {
@@ -1530,29 +1552,8 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
 	if (err) {
 		if (flag == F2FS_GET_BLOCK_BMAP)
 			map->m_pblk = 0;
-
-		if (err == -ENOENT) {
-			/*
-			 * There is one exceptional case that read_node_page()
-			 * may return -ENOENT due to filesystem has been
-			 * shutdown or cp_error, so force to convert error
-			 * number to EIO for such case.
-			 */
-			if (map->m_may_create &&
-				(is_sbi_flag_set(sbi, SBI_IS_SHUTDOWN) ||
-				f2fs_cp_error(sbi))) {
-				err = -EIO;
-				goto unlock_out;
-			}
-
-			err = 0;
-			if (map->m_next_pgofs)
-				*map->m_next_pgofs =
-					f2fs_get_next_page_offset(&dn, pgofs);
-			if (map->m_next_extent)
-				*map->m_next_extent =
-					f2fs_get_next_page_offset(&dn, pgofs);
-		}
+		if (err == -ENOENT)
+			err = f2fs_map_no_dnode(inode, map, &dn, pgofs);
 		goto unlock_out;
 	}
 
-- 
2.30.2



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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [f2fs-dev] [PATCH 15/15] f2fs: refactor the hole reporting and allocation logic in f2fs_map_blocks
  2022-11-28  9:15 [f2fs-dev] a fix and a bunch of cleanups Christoph Hellwig
                   ` (13 preceding siblings ...)
  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 ` Christoph Hellwig
  2023-01-03 17:21 ` [f2fs-dev] a fix and a bunch of cleanups Jaegeuk Kim
  15 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2022-11-28  9:15 UTC (permalink / raw)
  To: Jaegeuk Kim, Chao Yu; +Cc: linux-f2fs-devel

Add a is_hole local variable to figure out if the block number might need
allocation, and untangle to logic to report the hole or fill it with a
block allocation.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/f2fs/data.c | 113 ++++++++++++++++++++++++-------------------------
 1 file changed, 56 insertions(+), 57 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 3c802ce397de52..32e6823e1e9b1a 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1524,6 +1524,7 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
 	block_t blkaddr;
 	unsigned int start_pgofs;
 	int bidx = 0;
+	bool is_hole;
 
 	if (!maxblocks)
 		return 0;
@@ -1564,78 +1565,76 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
 
 next_block:
 	blkaddr = f2fs_data_blkaddr(&dn);
-
-	if (__is_valid_data_blkaddr(blkaddr) &&
-		!f2fs_is_valid_blkaddr(sbi, blkaddr, DATA_GENERIC_ENHANCE)) {
+	is_hole = !__is_valid_data_blkaddr(blkaddr);
+	if (!is_hole &&
+	    !f2fs_is_valid_blkaddr(sbi, blkaddr, DATA_GENERIC_ENHANCE)) {
 		err = -EFSCORRUPTED;
 		f2fs_handle_error(sbi, ERROR_INVALID_BLKADDR);
 		goto sync_out;
 	}
 
-	if (__is_valid_data_blkaddr(blkaddr)) {
-		/* use out-place-update for driect IO under LFS mode */
-		if (f2fs_lfs_mode(sbi) && flag == F2FS_GET_BLOCK_DIO &&
-							map->m_may_create) {
+	/* use out-place-update for direct IO under LFS mode */
+	if (map->m_may_create &&
+	    (is_hole || (f2fs_lfs_mode(sbi) && flag == F2FS_GET_BLOCK_DIO))) {
+		if (unlikely(f2fs_cp_error(sbi))) {
+			err = -EIO;
+			goto sync_out;
+		}
+
+		switch (flag) {
+		case F2FS_GET_BLOCK_PRE_AIO:
+			if (blkaddr == NULL_ADDR) {
+				prealloc++;
+				last_ofs_in_node = dn.ofs_in_node;
+			}
+			break;
+		case F2FS_GET_BLOCK_PRE_DIO:
+		case F2FS_GET_BLOCK_DIO:
 			err = __allocate_data_block(&dn, map->m_seg_type);
 			if (err)
 				goto sync_out;
-			blkaddr = dn.data_blkaddr;
+			if (flag == F2FS_GET_BLOCK_PRE_DIO)
+				file_need_truncate(inode);
 			set_inode_flag(inode, FI_APPEND_WRITE);
+			break;
+		default:
+			WARN_ON_ONCE(1);
+			err = -EIO;
+			goto sync_out;
 		}
-	} else {
-		if (map->m_may_create) {
-			if (unlikely(f2fs_cp_error(sbi))) {
-				err = -EIO;
-				goto sync_out;
-			}
-			if (flag == F2FS_GET_BLOCK_PRE_AIO) {
-				if (blkaddr == NULL_ADDR) {
-					prealloc++;
-					last_ofs_in_node = dn.ofs_in_node;
-				}
-			} else {
-				WARN_ON(flag != F2FS_GET_BLOCK_PRE_DIO &&
-					flag != F2FS_GET_BLOCK_DIO);
-				err = __allocate_data_block(&dn,
-							map->m_seg_type);
-				if (!err) {
-					if (flag == F2FS_GET_BLOCK_PRE_DIO)
-						file_need_truncate(inode);
-					set_inode_flag(inode, FI_APPEND_WRITE);
-				}
-			}
-			if (err)
-				goto sync_out;
+
+		blkaddr = dn.data_blkaddr;
+	    	if (is_hole)
 			map->m_flags |= F2FS_MAP_NEW;
-			blkaddr = dn.data_blkaddr;
-		} else {
-			if (f2fs_compressed_file(inode) &&
-					f2fs_sanity_check_cluster(&dn) &&
-					(flag != F2FS_GET_BLOCK_FIEMAP ||
-					IS_ENABLED(CONFIG_F2FS_CHECK_FS))) {
-				err = -EFSCORRUPTED;
-				f2fs_handle_error(sbi,
-						ERROR_CORRUPTED_CLUSTER);
-				goto sync_out;
-			}
-			if (flag == F2FS_GET_BLOCK_BMAP) {
-				map->m_pblk = 0;
-				goto sync_out;
-			}
-			if (flag == F2FS_GET_BLOCK_PRECACHE)
-				goto sync_out;
-			if (flag == F2FS_GET_BLOCK_FIEMAP &&
-						blkaddr == NULL_ADDR) {
-				if (map->m_next_pgofs)
-					*map->m_next_pgofs = pgofs + 1;
-				goto sync_out;
-			}
-			if (flag != F2FS_GET_BLOCK_FIEMAP) {
-				/* for defragment case */
+	} else if (is_hole) {
+		if (f2fs_compressed_file(inode) &&
+		    f2fs_sanity_check_cluster(&dn) &&
+		    (flag != F2FS_GET_BLOCK_FIEMAP ||
+		     IS_ENABLED(CONFIG_F2FS_CHECK_FS))) {
+			err = -EFSCORRUPTED;
+			f2fs_handle_error(sbi,
+					ERROR_CORRUPTED_CLUSTER);
+			goto sync_out;
+		}
+
+		switch (flag) {
+		case F2FS_GET_BLOCK_PRECACHE:
+			goto sync_out;
+		case F2FS_GET_BLOCK_BMAP:
+			map->m_pblk = 0;
+			goto sync_out;
+		case F2FS_GET_BLOCK_FIEMAP:
+			if (blkaddr == NULL_ADDR) {
 				if (map->m_next_pgofs)
 					*map->m_next_pgofs = pgofs + 1;
 				goto sync_out;
 			}
+			break;
+		default:
+			/* for defragment case */
+			if (map->m_next_pgofs)
+				*map->m_next_pgofs = pgofs + 1;
+			goto sync_out;
 		}
 	}
 
-- 
2.30.2



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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [f2fs-dev] [PATCH 12/15] f2fs: remove the create argument to f2fs_map_blocks
  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
  2022-12-13  5:57     ` Christoph Hellwig
  0 siblings, 1 reply; 22+ messages in thread
From: Jaegeuk Kim @ 2022-12-13  1:22 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-f2fs-devel

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [f2fs-dev] [PATCH 12/15] f2fs: remove the create argument to f2fs_map_blocks
  2022-12-13  1:22   ` Jaegeuk Kim
@ 2022-12-13  5:57     ` Christoph Hellwig
  2022-12-15  1:02       ` Jaegeuk Kim
  0 siblings, 1 reply; 22+ messages in thread
From: Christoph Hellwig @ 2022-12-13  5:57 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: Christoph Hellwig, linux-f2fs-devel

On Mon, Dec 12, 2022 at 05:22:41PM -0800, Jaegeuk Kim wrote:
> >  	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)

With "this condition" I guess you mean the:

		if (f2fs_lfs_mode(sbi) && flag == F2FS_GET_BLOCK_DIO &&
							map->m_may_create)
			goto next_dnode;

?

Now that the !create check above is replaced with !map->m_may_create
above, it is dead code, as the map->m_may_create part of the conditions
must be false.


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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [f2fs-dev] [PATCH 12/15] f2fs: remove the create argument to f2fs_map_blocks
  2022-12-13  5:57     ` Christoph Hellwig
@ 2022-12-15  1:02       ` Jaegeuk Kim
  0 siblings, 0 replies; 22+ messages in thread
From: Jaegeuk Kim @ 2022-12-15  1:02 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-f2fs-devel

On 12/13, Christoph Hellwig wrote:
> On Mon, Dec 12, 2022 at 05:22:41PM -0800, Jaegeuk Kim wrote:
> > >  	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)
> 
> With "this condition" I guess you mean the:
> 
> 		if (f2fs_lfs_mode(sbi) && flag == F2FS_GET_BLOCK_DIO &&
> 							map->m_may_create)
> 			goto next_dnode;
> 
> ?
> 
> Now that the !create check above is replaced with !map->m_may_create
> above, it is dead code, as the map->m_may_create part of the conditions
> must be false.

Ah, I missed it. Thanks.


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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [f2fs-dev] a fix and a bunch of cleanups
  2022-11-28  9:15 [f2fs-dev] a fix and a bunch of cleanups Christoph Hellwig
                   ` (14 preceding siblings ...)
  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 ` Jaegeuk Kim
  2023-01-06  6:54   ` Chao Yu
  2023-01-11  5:58   ` Christoph Hellwig
  15 siblings, 2 replies; 22+ messages in thread
From: Jaegeuk Kim @ 2023-01-03 17:21 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-f2fs-devel

Hi Christoph,

I applied the patch set with minor modification to address merge conflict.
Could you please take a look?

https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git/log/?h=dev-test&qt=author&q=Christoph+Hellwig

Thanks,

On 11/28, Christoph Hellwig wrote:
> Hi Jaegeuk and Chao,
> 
> the first patch in this series fixes a warning and subsequent hang when
> testing zoned f2fs.  The other patches are misc cleanups for the I/O path.
> 
> Diffstat
>  fs/f2fs/compress.c          |    2 
>  fs/f2fs/data.c              |  544 ++++++++++++++++++++++----------------------
>  fs/f2fs/extent_cache.c      |   19 -
>  fs/f2fs/f2fs.h              |   24 -
>  fs/f2fs/file.c              |   16 -
>  fs/f2fs/gc.c                |    4 
>  include/trace/events/f2fs.h |   11 
>  7 files changed, 309 insertions(+), 311 deletions(-)


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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [f2fs-dev] a fix and a bunch of cleanups
  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
  1 sibling, 0 replies; 22+ messages in thread
From: Chao Yu @ 2023-01-06  6:54 UTC (permalink / raw)
  To: Jaegeuk Kim, Christoph Hellwig; +Cc: linux-f2fs-devel

On 2023/1/4 1:21, Jaegeuk Kim wrote:
> Hi Christoph,
> 
> I applied the patch set with minor modification to address merge conflict.
> Could you please take a look?
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git/log/?h=dev-test&qt=author&q=Christoph+Hellwig

Looks good to me, Jaegeuk, please help to add my rvb tag for all patches in
this set. :)

Reviewed-by: Chao Yu <chao@kernel.org>

Thanks,

> 
> Thanks,
> 
> On 11/28, Christoph Hellwig wrote:
>> Hi Jaegeuk and Chao,
>>
>> the first patch in this series fixes a warning and subsequent hang when
>> testing zoned f2fs.  The other patches are misc cleanups for the I/O path.
>>
>> Diffstat
>>   fs/f2fs/compress.c          |    2
>>   fs/f2fs/data.c              |  544 ++++++++++++++++++++++----------------------
>>   fs/f2fs/extent_cache.c      |   19 -
>>   fs/f2fs/f2fs.h              |   24 -
>>   fs/f2fs/file.c              |   16 -
>>   fs/f2fs/gc.c                |    4
>>   include/trace/events/f2fs.h |   11
>>   7 files changed, 309 insertions(+), 311 deletions(-)


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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [f2fs-dev] a fix and a bunch of cleanups
  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
  1 sibling, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2023-01-11  5:58 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: Christoph Hellwig, linux-f2fs-devel

On Tue, Jan 03, 2023 at 09:21:37AM -0800, Jaegeuk Kim wrote:
> Hi Christoph,
> 
> I applied the patch set with minor modification to address merge conflict.
> Could you please take a look?

From a quick look this looks good.  I have a nother big batch of patches
that I'll rebase, and as part of that I'll take another deeper look.


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

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2023-01-11  5:59 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.