linux-kernel - RE: [f2fs-dev] [PATCH] f2fs: change the method of calculating the number summary blocks

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

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

Message-id: <001401ced47a$61fe2fb0$25fa8f10$@samsung.com>
Date:	Tue, 29 Oct 2013 15:41:27 +0800
From:	Fan Li <fanofcode.li@...sung.com>
To:	jaegeuk.kim@...sung.com
Cc:	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-f2fs-devel@...ts.sourceforge.net
Subject: RE: [f2fs-dev] [PATCH] f2fs: change the method of calculating the
 number summary blocks

> Hi,
> 
> 2013-10-28 (월), 08:54 +0000, Fan Li:
> > "There is a HTML error in the previous email, so I send this one.If you
> already received this before, please ignore it.Sorry for the inconvenience"
> >
> > This patch change the method of calculating the number of summary blocks
> in function  npages_for_summary_flush.
> > npages_for_summary_flush uses (SUMMARY_SIZE + 1) as the size of a
> f2fs_summary while the actual size is just SUMMARY_SIZE.
> > As a result, occasionally the return value of npages_for_summary_flush will
> be bigger than the actual number by one, and the checkpoint won't be
> written contiguously  into disk.
> > Besides, struct f2fs_summary can't be split to two pages, so it could take
> more space than the sum of its size, the current version seems not to take it
> into account.
> >
> 
> Again. Please write a description not to exceed 80 columns.
> 

Sorry, didn't know. here's the short version:
npages_for_summary_flush uses (SUMMARY_SIZE + 1) as the size of a f2fs_summary 
while its actual size is  SUMMARY_SIZE. So the result sometimes is bigger than 
actual number by one, which causes checkpoint can't be written into disk contiguously,
and sometimes summary blocks can't be compacted like they should.
Besides, when writing summary blocks into pages, if remain space in a page isn't 
big enough for one f2fs_summary, it will be left unused, current code seems 
not to take it into account.

Signed-off-by: Fan Li <fanofcode.li@...sung.com>
---
 fs/f2fs/segment.c |   14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 487af61..e40cdc4 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -279,9 +279,8 @@ static void __add_sum_entry(struct f2fs_sb_info *sbi, int type,
  */
 int npages_for_summary_flush(struct f2fs_sb_info *sbi)
 {
-	int total_size_bytes = 0;
 	int valid_sum_count = 0;
-	int i, sum_space;
+	int i, sum_in_page;
 
 	for (i = CURSEG_HOT_DATA; i <= CURSEG_COLD_DATA; i++) {
 		if (sbi->ckpt->alloc_type[i] == SSR)
@@ -290,13 +289,12 @@ int npages_for_summary_flush(struct f2fs_sb_info *sbi)
 			valid_sum_count += curseg_blkoff(sbi, i);
 	}
 
-	total_size_bytes = valid_sum_count * (SUMMARY_SIZE + 1)
-			+ sizeof(struct nat_journal) + 2
-			+ sizeof(struct sit_journal) + 2;
-	sum_space = PAGE_CACHE_SIZE - SUM_FOOTER_SIZE;
-	if (total_size_bytes < sum_space)
+	sum_in_page = (PAGE_CACHE_SIZE - 2 * SUM_JOURNAL_SIZE -
+			SUM_FOOTER_SIZE) / SUMMARY_SIZE;
+	if (valid_sum_count <= sum_in_page)
 		return 1;
-	else if (total_size_bytes < 2 * sum_space)
+	else if ((valid_sum_count - sum_in_page) <=
+		(PAGE_CACHE_SIZE - SUM_FOOTER_SIZE) / SUMMARY_SIZE)
 		return 2;
 	return 3;
 }
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/