All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jaegeuk Kim <jaegeuk.kim@samsung.com>
To: fanofcode.li@samsung.com
Cc: "linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-f2fs-devel@lists.sourceforge.net"
	<linux-f2fs-devel@lists.sourceforge.net>
Subject: Re: [f2fs-dev] [PATCH] f2fs: change the method of calculating the number summary blocks
Date: Mon, 28 Oct 2013 19:34:36 +0900	[thread overview]
Message-ID: <1382956476.992.94.camel@kjgkr> (raw)
In-Reply-To: <21040538.71861382950468176.JavaMail.weblogic@epv6ml09>

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.

> Signed-off-by: Fan Li <fanofcode.li@samsung.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..ba2930f 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)/

	sum_in_page = (PAGE_CACHE_SIZE - 2 * SUM_JOURNAL_SIZE -
SUM_FOOTER_SIZE) / SUMMARY_SIZE;

Coding style.

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

ditto.

>  		return 2;
>  	return 3;
>  }
> --
> 1.7.9.5

-- 
Jaegeuk Kim
Samsung

--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: Jaegeuk Kim <jaegeuk.kim@samsung.com>
To: fanofcode.li@samsung.com
Cc: "linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-f2fs-devel@lists.sourceforge.net" 
	<linux-f2fs-devel@lists.sourceforge.net>
Subject: Re: [f2fs-dev] [PATCH] f2fs: change the method of calculating the number summary blocks
Date: Mon, 28 Oct 2013 19:34:36 +0900	[thread overview]
Message-ID: <1382956476.992.94.camel@kjgkr> (raw)
In-Reply-To: <21040538.71861382950468176.JavaMail.weblogic@epv6ml09>

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.

> Signed-off-by: Fan Li <fanofcode.li@samsung.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..ba2930f 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)/

	sum_in_page = (PAGE_CACHE_SIZE - 2 * SUM_JOURNAL_SIZE -
SUM_FOOTER_SIZE) / SUMMARY_SIZE;

Coding style.

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

ditto.

>  		return 2;
>  	return 3;
>  }
> --
> 1.7.9.5

-- 
Jaegeuk Kim
Samsung


  reply	other threads:[~2013-10-28 10:34 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-28  8:54 [f2fs-dev] [PATCH] f2fs: change the method of calculating the number summary blocks Fan Li
2013-10-28  8:54 ` Fan Li
2013-10-28 10:34 ` Jaegeuk Kim [this message]
2013-10-28 10:34   ` Jaegeuk Kim
2013-10-29  7:41   ` Fan Li

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=1382956476.992.94.camel@kjgkr \
    --to=jaegeuk.kim@samsung.com \
    --cc=fanofcode.li@samsung.com \
    --cc=linux-f2fs-devel@lists.sourceforge.net \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /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 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.