[f2fs-dev] [PATCH] f2fs: optimize the way of traversing free_nid_bitmap
We call scan_free_nid_bits only when there isn't many
free nids left, it means that marked bits in free_nid_bitmap
are supposed to be few, use find_next_bit_le is more
efficient in such case.
According to my tests, use find_next_bit_le instead of
test_bit_le will cut down the traversal time to one
third of its original.
Signed-off-by: Fan li <fanofcode...@samsung.com>
---
fs/f2fs/node.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index fef5c68..d234c6e 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -1955,6 +1955,7 @@ static void scan_free_nid_bits(struct f2fs_sb_info *sbi)
struct curseg_info *curseg = CURSEG_I(sbi, CURSEG_HOT_DATA);
struct f2fs_journal *journal = curseg->journal;
unsigned int i, idx;
+ nid_t nid;
down_read(_i->nat_tree_lock);
@@ -1964,10 +1965,10 @@ static void scan_free_nid_bits(struct f2fs_sb_info *sbi)
if (!nm_i->free_nid_count[i])
continue;
for (idx = 0; idx < NAT_ENTRY_PER_BLOCK; idx++) {
- nid_t nid;
-
- if (!test_bit_le(idx, nm_i->free_nid_bitmap[i]))
- continue;
+ idx = find_next_bit_le(nm_i->free_nid_bitmap[i],
+ NAT_ENTRY_PER_BLOCK, idx);
+ if (idx >= NAT_ENTRY_PER_BLOCK)
+ break;
nid = i * NAT_ENTRY_PER_BLOCK + idx;
add_free_nid(sbi, nid, true);
@@ -1980,7 +1981,6 @@ static void scan_free_nid_bits(struct f2fs_sb_info *sbi)
down_read(>journal_rwsem);
for (i = 0; i < nats_in_cursum(journal); i++) {
block_t addr;
- nid_t nid;
addr = le32_to_cpu(nat_in_journal(journal, i).block_addr);
nid = le32_to_cpu(nid_in_journal(journal, i));
--
2.7.4
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] f2fs: keep scanning until enough free nids are acquired
> -Original Message- > From: Jaegeuk Kim [mailto:jaeg...@kernel.org] > Sent: Tuesday, November 07, 2017 11:41 AM > To: Fan Li > Cc: 'Chao Yu'; 'Chao Yu'; linux-ker...@vger.kernel.org; > linux-f2fs-devel@lists.sourceforge.net > Subject: Re: [f2fs-dev] [PATCH] f2fs: keep scanning until enough free nids > are acquired > > Hi, > > I merged this patch after fixing some broken format. Could you please check > your email configuration? > > Thanks, > Sorry to bother you with so trivial problem, mail configuration is fine, but I use a wrong way to copy the text this time, won't happen again. > On 11/07, Fan Li wrote: > > In current version, after scan_free_nid_bits, the scan is over if > > nid_cnt[FREE_NID] != 0. > > In most cases, there are still free nids in the free list during the > > scan, and scan_free_nid_bits usually can't increase nid_cnt[FREE_NID]. > > It causes that __build_free_nids is called many times without solving > > the shortage of the free nids. This patch fixes that. > > > > Signed-off-by: Fan li <fanofcode...@samsung.com> > > --- > > fs/f2fs/node.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c index 3d0d1be..5cef118 > > 100644 > > --- a/fs/f2fs/node.c > > +++ b/fs/f2fs/node.c > > @@ -2010,7 +2010,7 @@ static void __build_free_nids(struct f2fs_sb_info > > *sbi, bool sync, bool mount) > > /* try to find free nids in free_nid_bitmap */ > > scan_free_nid_bits(sbi); > > > > - if (nm_i->nid_cnt[FREE_NID]) > > + if (nm_i->nid_cnt[FREE_NID] >= NAT_ENTRY_PER_BLOCK) > > return; > > } > > > > -- > > 2.7.4 -- Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH] f2fs: keep scanning until enough free nids are acquired
In current version, after scan_free_nid_bits, the scan is over if nid_cnt[FREE_NID] != 0. In most cases, there are still free nids in the free list during the scan, and scan_free_nid_bits usually can't increase nid_cnt[FREE_NID]. It causes that __build_free_nids is called many times without solving the shortage of the free nids. This patch fixes that. Signed-off-by: Fan li <fanofcode...@samsung.com> --- fs/f2fs/node.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c index 3d0d1be..5cef118 100644 --- a/fs/f2fs/node.c +++ b/fs/f2fs/node.c @@ -2010,7 +2010,7 @@ static void __build_free_nids(struct f2fs_sb_info *sbi, bool sync, bool mount) /* try to find free nids in free_nid_bitmap */ scan_free_nid_bits(sbi); - if (nm_i->nid_cnt[FREE_NID]) + if (nm_i->nid_cnt[FREE_NID] >= NAT_ENTRY_PER_BLOCK) return; } -- 2.7.4 -- Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH RESEND] f2fs: modify the procedure of scan free nid
> -Original Message-
> From: Chao Yu [mailto:c...@kernel.org]
> Sent: Friday, November 03, 2017 9:16 PM
> To: Fan Li; 'Chao Yu'; 'Jaegeuk Kim'
> Cc: linux-ker...@vger.kernel.org; linux-f2fs-devel@lists.sourceforge.net
> Subject: Re: [f2fs-dev] [PATCH RESEND] f2fs: modify the procedure of scan
> free nid
>
> On 2017/11/3 18:29, Fan Li wrote:
> >
> >
> >> -Original Message-
> >> From: Chao Yu [mailto:yuch...@huawei.com]
> >> Sent: Friday, November 03, 2017 4:54 PM
> >> To: Fan Li; 'Chao Yu'; 'Jaegeuk Kim'
> >> Cc: linux-ker...@vger.kernel.org;
> >> linux-f2fs-devel@lists.sourceforge.net
> >> Subject: Re: [f2fs-dev] [PATCH RESEND] f2fs: modify the procedure of
> >> scan free nid
> >>
> >> On 2017/11/3 15:31, Fan Li wrote:
> >>> In current version, we preserve 8 pages of nat blocks as free nids,
> >>> we build bitmaps for it and use them to allocate nids until its
> >>> number drops below NAT_ENTRY_PER_BLOCK.
> >>>
> >>> After that, we have a problem, scan_free_nid_bits will scan the same
> >>> 8 pages trying to find more free nids, but in most cases the free
> >>> nids in these bitmaps are already in free list, scan them won't get
> >>> us any new nids.
> >>> Further more, after scan_free_nid_bits, the scan is over if
> >>> nid_cnt[FREE_NID] != 0.
> >>> It causes that we scan the same pages over and over again, and no
> >>> new free nids are found until nid_cnt[FREE_NID]==0. While the
> >>> scanned pages increase, the problem grows worse.
> >>>
> >>> This patch mark the range where new free nids could exist and keep
> >>> scan for free nids until nid_cnt[FREE_NID] >= NAT_ENTRY_PER_BLOCK.
> >>> The new vairable first_scan_block marks the start of the range, it's
> >>> initialized with NEW_ADDR, which means all free nids before
> >>> next_scan_nid are already in free list; and use next_scan_nid as the
> >>> end of the range since all free nids which are scanned in
> >>> scan_free_nid_bits must be smaller next_scan_nid.
> >>
> >> Think over again, IMO, we can add an variable for stating total count
> >> of free nids in bitamp, if there is no free nid, just
> > skipping scanning all
> >> existed bitmap.
> >>
> >> And if there is only few free nid scattered in bitmap, the cost will
> >> be limited because we will skip scanning
> > nm_i::free_nid_bitmap if
> >> nm_i::free_nid_count is zero. Once we find one free nid, let's skip out.
> >>
> >> Since there shouldn't be very heavy overhead for CPU during traveling
> >> nm_i::nat_block_bitmap, I expect below change could be more simple for
> >> maintaining and being with the same effect.
> >>
> >> How do you think?
> >>
> >
> > I think if you need this to work, check total_bitmap_free_nid may not be
> > sufficient enough.
> > The problem this patch presents is that even all the free nids are
> > already in the free list, we still scan all the pages.
> > The scan proceeds once free nid count is below NAT_ENTRY_PER_BLOCK.
> > So in most cases, there are still free nids in the bitmap during the
> > scan, and current codes will check every one of them to see if they are
> > actually in free list.
> > If only check total_bitmap_free_nid == 0 won't take this overhead away.
>
> Oh, you could see that, we have added free_nid_count in each NAT block's
> free_nid_bitmap bitmap, before scan the bitmap, we will
make
> sure there is at least one free nid.
>
> scan_free_nid_bits()
> for (i = 0; i < nm_i->nat_blocks; i++) {
> if (!test_bit_le(i, nm_i->nat_block_bitmap))
> continue;
> if (!nm_i->free_nid_count[i])
> continue;
Do you mean free_nid_count here?
I thought free_nid_count only represents how many nats are marked free in
bitmap of one block.
To my understanding, even a nat is already in the free list, it will still have
a bit marked as free in
free_nid_bitmap and a count in free_nid_count.
That means if free_nid_count != 0, and there are marked bits in the bitmap, the
free nats in this
block could still be all in the free list.
The purpose of scan is to find new nats and add them to free list, go through
the nats which are
already in the free list isn't what we want.
And in xfstest, under most cases scan_free_nid_bits runs, all free nats are
indeed in the free list.
> for (idx = 0; idx < NAT_ENTRY_PER_BLOCK; i
Re: [f2fs-dev] [PATCH RESEND] f2fs: modify the procedure of scan free nid
> -Original Message-
> From: Chao Yu [mailto:yuch...@huawei.com]
> Sent: Friday, November 03, 2017 4:54 PM
> To: Fan Li; 'Chao Yu'; 'Jaegeuk Kim'
> Cc: linux-ker...@vger.kernel.org; linux-f2fs-devel@lists.sourceforge.net
> Subject: Re: [f2fs-dev] [PATCH RESEND] f2fs: modify the procedure of scan
> free nid
>
> On 2017/11/3 15:31, Fan Li wrote:
> > In current version, we preserve 8 pages of nat blocks as free nids, we
> > build bitmaps for it and use them to allocate nids until its number
> > drops below NAT_ENTRY_PER_BLOCK.
> >
> > After that, we have a problem, scan_free_nid_bits will scan the same
> > 8 pages trying to find more free nids, but in most cases the free nids
> > in these bitmaps are already in free list, scan them won't get us any
> > new nids.
> > Further more, after scan_free_nid_bits, the scan is over if
> > nid_cnt[FREE_NID] != 0.
> > It causes that we scan the same pages over and over again, and no new
> > free nids are found until nid_cnt[FREE_NID]==0. While the scanned
> > pages increase, the problem grows worse.
> >
> > This patch mark the range where new free nids could exist and keep
> > scan for free nids until nid_cnt[FREE_NID] >= NAT_ENTRY_PER_BLOCK.
> > The new vairable first_scan_block marks the start of the range, it's
> > initialized with NEW_ADDR, which means all free nids before
> > next_scan_nid are already in free list; and use next_scan_nid as the
> > end of the range since all free nids which are scanned in
> > scan_free_nid_bits must be smaller next_scan_nid.
>
> Think over again, IMO, we can add an variable for stating total count of free
> nids in bitamp, if there is no free nid, just
skipping scanning all
> existed bitmap.
>
> And if there is only few free nid scattered in bitmap, the cost will be
> limited because we will skip scanning
nm_i::free_nid_bitmap if
> nm_i::free_nid_count is zero. Once we find one free nid, let's skip out.
>
> Since there shouldn't be very heavy overhead for CPU during traveling
> nm_i::nat_block_bitmap, I expect below change could be more
> simple for maintaining and being with the same effect.
>
> How do you think?
>
I think if you need this to work, check total_bitmap_free_nid may not be
sufficient enough.
The problem this patch presents is that even all the free nids are already in
the free list,
we still scan all the pages.
The scan proceeds once free nid count is below NAT_ENTRY_PER_BLOCK.
So in most cases, there are still free nids in the bitmap during the scan, and
current codes will check every one of them to see if they are actually in free
list.
If only check total_bitmap_free_nid == 0 won't take this overhead away.
I considered a lot of ways to fix this problem before I submit this patch,
One of my idea is quite similar to yours, but I use
"if (total_bitmap_free_nid == nm_i->nid_cnt[FREE_NID])" to decide whether
skip or not.
If you insist, I can submit this simpler one instead, but some follow upgrade
would be unavailable, for example, use smaller granularity for tracking
last-scanned-position that we talked about.
I know sometimes I can be obsessed with the performance, I usually
choose the faster way over simpler ones. If you think it's too much,
please tell me, I'm sure we can find some middle ground.
Thank you
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index cb3f10bc8723..238d95e89dec
> 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -729,6 +729,7 @@ struct f2fs_nm_info {
> unsigned char (*free_nid_bitmap)[NAT_ENTRY_BITMAP_SIZE];
> unsigned char *nat_block_bitmap;
> unsigned short *free_nid_count; /* free nid count of NAT block */
> + unsigned int total_bitmap_free_nid; /* total free nid count in
> bitmap */
>
> /* for checkpoint */
> char *nat_bitmap; /* NAT bitmap pointer */
> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c index fef5c68886b1..e4861908a396
> 100644
> --- a/fs/f2fs/node.c
> +++ b/fs/f2fs/node.c
> @@ -1911,10 +1911,13 @@ static void update_free_nid_bitmap(struct
> f2fs_sb_info *sbi, nid_t nid,
> else
> __clear_bit_le(nid_ofs, nm_i->free_nid_bitmap[nat_ofs]);
>
> - if (set)
> + if (set) {
> nm_i->free_nid_count[nat_ofs]++;
> - else if (!build)
> + nm_i->total_bitmap_free_nid++;
> + } else if (!build) {
> nm_i->free_nid_count[nat_ofs]--;
> + nm_i->total_bitmap_free_nid--;
> + }
> }
>
> static void scan_nat_page(struct f2fs_sb_info *sbi, @@ -1958,6 +1961,9 @@
> static void scan_free_nid_bits(struct f2fs_sb_info
*sbi)
>
> down_read(_i->nat_tree_lock);
>
> +
[f2fs-dev] [PATCH RESEND] f2fs: modify the procedure of scan free nid
In current version, we preserve 8 pages of nat blocks as free nids,
we build bitmaps for it and use them to allocate nids until its number
drops below NAT_ENTRY_PER_BLOCK.
After that, we have a problem, scan_free_nid_bits will scan the same
8 pages trying to find more free nids, but in most cases the free nids
in these bitmaps are already in free list, scan them won't get us any
new nids.
Further more, after scan_free_nid_bits, the scan is over if
nid_cnt[FREE_NID] != 0.
It causes that we scan the same pages over and over again, and no new
free nids are found until nid_cnt[FREE_NID]==0. While the scanned pages
increase, the problem grows worse.
This patch mark the range where new free nids could exist and keep scan
for free nids until nid_cnt[FREE_NID] >= NAT_ENTRY_PER_BLOCK.
The new vairable first_scan_block marks the start of the range, it's
initialized with NEW_ADDR, which means all free nids before next_scan_nid
are already in free list;
and use next_scan_nid as the end of the range since all free nids which
are scanned in scan_free_nid_bits must be smaller next_scan_nid.
Signed-off-by: Fan li <fanofcode...@samsung.com>
---
fs/f2fs/f2fs.h | 1 +
fs/f2fs/node.c | 42 +++---
2 files changed, 36 insertions(+), 7 deletions(-)
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index e0ef31c..ae1cf91 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -705,6 +705,7 @@ struct f2fs_nm_info {
nid_t max_nid; /* maximum possible node ids */
nid_t available_nids; /* # of available node ids */
nid_t next_scan_nid;/* the next nid to be scanned */
+ block_t first_scan_block; /* the first NAT block to be scanned */
unsigned int ram_thresh;/* control the memory footprint */
unsigned int ra_nid_pages; /* # of nid pages to be readaheaded */
unsigned int dirty_nats_ratio; /* control dirty nats ratio threshold */
diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index 3d0d1be..f921e0c 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -1812,7 +1812,7 @@ static bool add_free_nid(struct f2fs_sb_info *sbi, nid_t
nid, bool build)
struct f2fs_nm_info *nm_i = NM_I(sbi);
struct free_nid *i, *e;
struct nat_entry *ne;
- int err = -EINVAL;
+ int need_free = 1;
bool ret = false;
/* 0 nid should not be used */
@@ -1863,13 +1863,25 @@ static bool add_free_nid(struct f2fs_sb_info *sbi,
nid_t nid, bool build)
}
}
ret = true;
- err = __insert_free_nid(sbi, i, FREE_NID);
+ need_free = __insert_free_nid(sbi, i, FREE_NID);
err_out:
spin_unlock(_i->nid_list_lock);
radix_tree_preload_end();
err:
- if (err)
+ if (need_free)
kmem_cache_free(free_nid_slab, i);
+ /*
+* For nid that should be free but not in the free
+* structure, update the scan range in hope of adding
+* it in the next scan.
+*/
+ if (!ret || need_free < 0) {
+ block_t tmp_block = NAT_BLOCK_OFFSET(nid);
+
+ if (tmp_block < nm_i->first_scan_block)
+ nm_i->first_scan_block = tmp_block;
+ }
+
return ret;
}
@@ -1950,10 +1962,17 @@ static void scan_free_nid_bits(struct f2fs_sb_info *sbi)
struct curseg_info *curseg = CURSEG_I(sbi, CURSEG_HOT_DATA);
struct f2fs_journal *journal = curseg->journal;
unsigned int i, idx;
+ unsigned int max_blocks = NAT_BLOCK_OFFSET(nm_i->next_scan_nid);
- down_read(_i->nat_tree_lock);
+ /* every free nid in blocks scanned previously is in the free list */
+ if (nm_i->first_scan_block == NEW_ADDR)
+ return;
- for (i = 0; i < nm_i->nat_blocks; i++) {
+ if (max_blocks == 0)
+ max_blocks = nm_i->nat_blocks;
+
+ down_read(_i->nat_tree_lock);
+ for (i = nm_i->first_scan_block; i < max_blocks; i++) {
if (!test_bit_le(i, nm_i->nat_block_bitmap))
continue;
if (!nm_i->free_nid_count[i])
@@ -1967,10 +1986,13 @@ static void scan_free_nid_bits(struct f2fs_sb_info *sbi)
nid = i * NAT_ENTRY_PER_BLOCK + idx;
add_free_nid(sbi, nid, true);
- if (nm_i->nid_cnt[FREE_NID] >= MAX_FREE_NIDS)
+ if (nm_i->nid_cnt[FREE_NID] >= MAX_FREE_NIDS) {
+ nm_i->first_scan_block = i;
goto out;
+ }
}
}
+ nm_i->first_scan_block = NEW_ADDR;
out:
down_read(>journal_rwsem);
for (i = 0; i < nats_in_cursum(journal); i++) {
@@ -2010,7 +2032,7 @@ static void __build_free_nids(struct f2fs_sb_info *sbi,
bool sync, bool mount)
/
[f2fs-dev] [PATCH] f2fs: save a multiplication for last_nid calculation
Use a slightly easier way to calculate last_nid. Signed-off-by: Fan li <fanofcode...@samsung.com> --- fs/f2fs/node.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c index 7834097..55ab330 100644 --- a/fs/f2fs/node.c +++ b/fs/f2fs/node.c @@ -2642,7 +2642,7 @@ static inline void load_free_nid_bitmap(struct f2fs_sb_info *sbi) __set_bit_le(i, nm_i->nat_block_bitmap); nid = i * NAT_ENTRY_PER_BLOCK; - last_nid = (i + 1) * NAT_ENTRY_PER_BLOCK; + last_nid = nid + NAT_ENTRY_PER_BLOCK; spin_lock(_I(sbi)->nid_list_lock); for (; nid < last_nid; nid++) -- 2.7.4 -- Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] f2fs: modify the procedure of scan free nid
> -Original Message-
> From: Chao Yu [mailto:c...@kernel.org]
> Sent: Wednesday, November 01, 2017 8:47 PM
> To: Fan Li; 'Jaegeuk Kim'
> Cc: linux-ker...@vger.kernel.org; linux-f2fs-devel@lists.sourceforge.net
> Subject: Re: [f2fs-dev] [PATCH] f2fs: modify the procedure of scan free nid
>
> On 2017/11/1 18:03, Fan Li wrote:
> >
> >
> >> -Original Message-
> >> From: Chao Yu [mailto:c...@kernel.org]
> >> Sent: Tuesday, October 31, 2017 10:32 PM
> >> To: Fan Li; 'Jaegeuk Kim'
> >> Cc: linux-ker...@vger.kernel.org;
> >> linux-f2fs-devel@lists.sourceforge.net
> >> Subject: Re: [f2fs-dev] [PATCH] f2fs: modify the procedure of scan
> >> free nid
> >>
> >> On 2017/10/31 21:37, Fan Li wrote:
> >>> In current version, we preserve 8 pages of nat blocks as free nids,
> >>> build bitmaps for it and use them to allocate nids until its number
> >>> drops below NAT_ENTRY_PER_BLOCK.
> >>>
> >>> After that, we have a problem, scan_free_nid_bits will scan the same
> >>> 8 pages trying to find more free nids, but in most cases the free
> >>> nids in these bitmaps are already in free list, scan them won't get
> >>> us any new nids.
> >>> Further more, after scan_free_nid_bits, the search is over if
> >>> nid_cnt[FREE_NID] != 0.
> >>> It causes that we scan the same pages over and over again, yet no
> >>> new free nids are found until nid_cnt[FREE_NID]==0.
> >>>
> >>> This patch mark the range where new free nids could exist and keep
> >>> scan for free nids until nid_cnt[FREE_NID] >= NAT_ENTRY_PER_BLOCK.
> >>> The new vairable first_scan_block marks the start of the range, it's
> >>> initialized with NEW_ADDR, which means all free nids before
> >>> next_scan_nid are already in free list; and use next_scan_nid as the
> >>> end of the range since all free nids which are scanned must be
> >>> smaller next_scan_nid.
> >>>
> >>>
> >>> Signed-off-by: Fan li <fanofcode...@samsung.com>
> >>> ---
> >>> fs/f2fs/f2fs.h | 1 +
> >>> fs/f2fs/node.c | 30 ++
> >>> 2 files changed, 27 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index e0ef31c..ae1cf91
> >>> 100644
> >>> --- a/fs/f2fs/f2fs.h
> >>> +++ b/fs/f2fs/f2fs.h
> >>> @@ -705,6 +705,7 @@ struct f2fs_nm_info {
> >>> nid_t max_nid; /* maximum possible node ids */
> >>> nid_t available_nids; /* # of available node ids */
> >>> nid_t next_scan_nid;/* the next nid to be scanned */
> >>> + block_t first_scan_block; /* the first NAT block to be scanned */
> >>
> >> As we are traveling bitmap, so how about using smaller granularity for
> >> tracking last-scanned-position. like:
> >>
> >> unsigned next_bitmap_pos; ?
> >>
> > Yes, I think it's a good idea, but original code scans nids by blocks,
> > if I change that, I need to change some other details too, and before that,
> > I want to make sure this idea of patch is right.
> > I also have some ideas about it, if that's OK, I tend to submit other
> > patches to implement them.
> >
> >>> unsigned int ram_thresh;/* control the memory footprint */
> >>> unsigned int ra_nid_pages; /* # of nid pages to be readaheaded */
> >>> unsigned int dirty_nats_ratio; /* control dirty nats ratio threshold */
> >>> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c index 3d0d1be..7834097
> >>> 100644
> >>> --- a/fs/f2fs/node.c
> >>> +++ b/fs/f2fs/node.c
> >>> @@ -1950,10 +1950,23 @@ static void scan_free_nid_bits(struct
> >>> f2fs_sb_info *sbi)
> >>> struct curseg_info *curseg = CURSEG_I(sbi, CURSEG_HOT_DATA);
> >>> struct f2fs_journal *journal = curseg->journal;
> >>> unsigned int i, idx;
> >>> + unsigned int max_blocks = NAT_BLOCK_OFFSET(nm_i->next_scan_nid);
> >>>
> >>> - down_read(_i->nat_tree_lock);
> >>> + /* every free nid in blocks scanned previously is in the free list */
> >>> + if (nm_i->first_scan_block == NEW_ADDR)
> >>
> >> How about using nm_i->max_nid as no more free nids in bitmap?
> >>
> > For now, I use the block as the unit of variable first_scan_block, for
> >
Re: [f2fs-dev] [PATCH] f2fs: modify the procedure of scan free nid
> -Original Message-
> From: Chao Yu [mailto:c...@kernel.org]
> Sent: Tuesday, October 31, 2017 10:32 PM
> To: Fan Li; 'Jaegeuk Kim'
> Cc: linux-ker...@vger.kernel.org; linux-f2fs-devel@lists.sourceforge.net
> Subject: Re: [f2fs-dev] [PATCH] f2fs: modify the procedure of scan free nid
>
> On 2017/10/31 21:37, Fan Li wrote:
> > In current version, we preserve 8 pages of nat blocks as free nids,
> > build bitmaps for it and use them to allocate nids until its number
> > drops below NAT_ENTRY_PER_BLOCK.
> >
> > After that, we have a problem, scan_free_nid_bits will scan the same
> > 8 pages trying to find more free nids, but in most cases the free nids
> > in these bitmaps are already in free list, scan them won't get us any
> > new nids.
> > Further more, after scan_free_nid_bits, the search is over if
> > nid_cnt[FREE_NID] != 0.
> > It causes that we scan the same pages over and over again, yet no new
> > free nids are found until nid_cnt[FREE_NID]==0.
> >
> > This patch mark the range where new free nids could exist and keep
> > scan for free nids until nid_cnt[FREE_NID] >= NAT_ENTRY_PER_BLOCK.
> > The new vairable first_scan_block marks the start of the range, it's
> > initialized with NEW_ADDR, which means all free nids before
> > next_scan_nid are already in free list; and use next_scan_nid as the
> > end of the range since all free nids which are scanned must be smaller
> > next_scan_nid.
> >
> >
> > Signed-off-by: Fan li <fanofcode...@samsung.com>
> > ---
> > fs/f2fs/f2fs.h | 1 +
> > fs/f2fs/node.c | 30 ++
> > 2 files changed, 27 insertions(+), 4 deletions(-)
> >
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index e0ef31c..ae1cf91
> > 100644
> > --- a/fs/f2fs/f2fs.h
> > +++ b/fs/f2fs/f2fs.h
> > @@ -705,6 +705,7 @@ struct f2fs_nm_info {
> > nid_t max_nid; /* maximum possible node ids */
> > nid_t available_nids; /* # of available node ids */
> > nid_t next_scan_nid;/* the next nid to be scanned */
> > + block_t first_scan_block; /* the first NAT block to be scanned */
>
> As we are traveling bitmap, so how about using smaller granularity for
> tracking last-scanned-position. like:
>
> unsigned next_bitmap_pos; ?
>
Yes, I think it's a good idea, but original code scans nids by blocks, if I
change that, I need to change some
other details too, and before that, I want to make sure this idea of patch is
right.
I also have some ideas about it, if that's OK, I tend to submit other patches
to implement them.
> > unsigned int ram_thresh;/* control the memory footprint */
> > unsigned int ra_nid_pages; /* # of nid pages to be readaheaded */
> > unsigned int dirty_nats_ratio; /* control dirty nats ratio threshold */
> > diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c index 3d0d1be..7834097
> > 100644
> > --- a/fs/f2fs/node.c
> > +++ b/fs/f2fs/node.c
> > @@ -1950,10 +1950,23 @@ static void scan_free_nid_bits(struct f2fs_sb_info
> > *sbi)
> > struct curseg_info *curseg = CURSEG_I(sbi, CURSEG_HOT_DATA);
> > struct f2fs_journal *journal = curseg->journal;
> > unsigned int i, idx;
> > + unsigned int max_blocks = NAT_BLOCK_OFFSET(nm_i->next_scan_nid);
> >
> > - down_read(_i->nat_tree_lock);
> > + /* every free nid in blocks scanned previously is in the free list */
> > + if (nm_i->first_scan_block == NEW_ADDR)
>
> How about using nm_i->max_nid as no more free nids in bitmap?
>
For now, I use the block as the unit of variable first_scan_block, for the same
reason above,
I tend to change it in another patch.
> > + return;
> >
> > - for (i = 0; i < nm_i->nat_blocks; i++) {
> > + /*
> > +* TODO: "next_scan_nid == 0" means after searching every nat block,
> > +* we still can't find enough free nids, there may not be any
> > +* more nid left to be found, we should stop at somewhere
> > +* instead of going through these all over again.
> > +*/
> > + if (max_blocks == 0)
> > + max_blocks = nm_i->nat_blocks;
> > +
> > + down_read(_i->nat_tree_lock);
> > + for (i = nm_i->first_scan_block; i < max_blocks; i++) {
>
> Free nids could be set free after nodes were truncated & checkpoint, if we
> start from first_scan_block, we will miss some free
nids.
>
This is the part I'm not sure. To my understanding, after nodes were truncated,
the nats will be cached as dirty nats,
the IS
[f2fs-dev] [PATCH] f2fs: modify the procedure of scan free nid
In current version, we preserve 8 pages of nat blocks as free nids,
build bitmaps for it and use them to allocate nids until its number
drops below NAT_ENTRY_PER_BLOCK.
After that, we have a problem, scan_free_nid_bits will scan the same
8 pages trying to find more free nids, but in most cases the free nids
in these bitmaps are already in free list, scan them won't get us any
new nids.
Further more, after scan_free_nid_bits, the search is over if
nid_cnt[FREE_NID] != 0.
It causes that we scan the same pages over and over again, yet no new
free nids are found until nid_cnt[FREE_NID]==0.
This patch mark the range where new free nids could exist and keep scan
for free nids until nid_cnt[FREE_NID] >= NAT_ENTRY_PER_BLOCK.
The new vairable first_scan_block marks the start of the range, it's
initialized with NEW_ADDR, which means all free nids before next_scan_nid
are already in free list;
and use next_scan_nid as the end of the range since all free nids which
are scanned must be smaller next_scan_nid.
Signed-off-by: Fan li <fanofcode...@samsung.com>
---
fs/f2fs/f2fs.h | 1 +
fs/f2fs/node.c | 30 ++
2 files changed, 27 insertions(+), 4 deletions(-)
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index e0ef31c..ae1cf91 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -705,6 +705,7 @@ struct f2fs_nm_info {
nid_t max_nid; /* maximum possible node ids */
nid_t available_nids; /* # of available node ids */
nid_t next_scan_nid;/* the next nid to be scanned */
+ block_t first_scan_block; /* the first NAT block to be scanned */
unsigned int ram_thresh;/* control the memory footprint */
unsigned int ra_nid_pages; /* # of nid pages to be readaheaded */
unsigned int dirty_nats_ratio; /* control dirty nats ratio threshold */
diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index 3d0d1be..7834097 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -1950,10 +1950,23 @@ static void scan_free_nid_bits(struct f2fs_sb_info *sbi)
struct curseg_info *curseg = CURSEG_I(sbi, CURSEG_HOT_DATA);
struct f2fs_journal *journal = curseg->journal;
unsigned int i, idx;
+ unsigned int max_blocks = NAT_BLOCK_OFFSET(nm_i->next_scan_nid);
- down_read(_i->nat_tree_lock);
+ /* every free nid in blocks scanned previously is in the free list */
+ if (nm_i->first_scan_block == NEW_ADDR)
+ return;
- for (i = 0; i < nm_i->nat_blocks; i++) {
+ /*
+* TODO: "next_scan_nid == 0" means after searching every nat block,
+* we still can't find enough free nids, there may not be any
+* more nid left to be found, we should stop at somewhere
+* instead of going through these all over again.
+*/
+ if (max_blocks == 0)
+ max_blocks = nm_i->nat_blocks;
+
+ down_read(_i->nat_tree_lock);
+ for (i = nm_i->first_scan_block; i < max_blocks; i++) {
if (!test_bit_le(i, nm_i->nat_block_bitmap))
continue;
if (!nm_i->free_nid_count[i])
@@ -1967,10 +1980,13 @@ static void scan_free_nid_bits(struct f2fs_sb_info *sbi)
nid = i * NAT_ENTRY_PER_BLOCK + idx;
add_free_nid(sbi, nid, true);
- if (nm_i->nid_cnt[FREE_NID] >= MAX_FREE_NIDS)
+ if (nm_i->nid_cnt[FREE_NID] >= MAX_FREE_NIDS) {
+ nm_i->first_scan_block = i;
goto out;
+ }
}
}
+ nm_i->first_scan_block = NEW_ADDR;
out:
down_read(>journal_rwsem);
for (i = 0; i < nats_in_cursum(journal); i++) {
@@ -2010,7 +2026,7 @@ static void __build_free_nids(struct f2fs_sb_info *sbi,
bool sync, bool mount)
/* try to find free nids in free_nid_bitmap */
scan_free_nid_bits(sbi);
- if (nm_i->nid_cnt[FREE_NID])
+ if (nm_i->nid_cnt[FREE_NID] >= NAT_ENTRY_PER_BLOCK)
return;
}
@@ -2163,6 +2179,7 @@ int try_to_free_nids(struct f2fs_sb_info *sbi, int
nr_shrink)
struct f2fs_nm_info *nm_i = NM_I(sbi);
struct free_nid *i, *next;
int nr = nr_shrink;
+ nid_t min_nid = nm_i->max_nid;
if (nm_i->nid_cnt[FREE_NID] <= MAX_FREE_NIDS)
return 0;
@@ -2176,11 +2193,15 @@ int try_to_free_nids(struct f2fs_sb_info *sbi, int
nr_shrink)
nm_i->nid_cnt[FREE_NID] <= MAX_FREE_NIDS)
break;
+ if (i->nid < min_nid)
+ min_nid = i->nid;
__remove_free_nid(sbi, i, FREE_NID);
kmem_cache_free(free_nid_slab, i);
Re: [f2fs-dev] [PATCH] f2fs: add a function to move nid
> -Original Message-
> From: Jaegeuk Kim [mailto:jaeg...@kernel.org]
> Sent: Monday, October 30, 2017 5:30 PM
> To: Chao Yu
> Cc: Fan Li; 'Chao Yu'; linux-ker...@vger.kernel.org;
> linux-f2fs-devel@lists.sourceforge.net
> Subject: Re: [f2fs-dev] [PATCH] f2fs: add a function to move nid
>
> On 10/28, Chao Yu wrote:
> > On 2017/10/28 19:03, Fan Li wrote:
> > > This patch add a new function to move nid from one state to another.
> > > Move operation is heavily used, by adding a new function for it we
> > > can cut down some branches from several flow.
> > >
> > >
> > > Signed-off-by: Fan li <fanofcode...@samsung.com>
> > > ---
> > > fs/f2fs/node.c | 51
> > > ++-
> > > 1 file changed, 30 insertions(+), 21 deletions(-)
> > >
> > > diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c index ac629d6..8116b50
> > > --- a/fs/f2fs/node.c
> > > +++ b/fs/f2fs/node.c
> > > @@ -1763,15 +1763,13 @@ static struct free_nid
> > > *__lookup_free_nid_list(struct f2fs_nm_info *nm_i, }
> > >
> > > static int __insert_free_nid(struct f2fs_sb_info *sbi,
> > > - struct free_nid *i, enum nid_state state, bool new)
> > > + struct free_nid *i, enum nid_state state)
> > > {
> > > struct f2fs_nm_info *nm_i = NM_I(sbi);
> > >
> > > - if (new) {
> > > - int err = radix_tree_insert(_i->free_nid_root, i->nid, i);
> > > - if (err)
> > > - return err;
> > > - }
> > > + int err = radix_tree_insert(_i->free_nid_root, i->nid, i);
> > > + if (err)
> > > + return err;
> > >
> > > f2fs_bug_on(sbi, state != i->state);
> > > nm_i->nid_cnt[state]++;
> > > @@ -1781,7 +1779,7 @@ static int __insert_free_nid(struct
> > > f2fs_sb_info *sbi, }
> > >
> > > static void __remove_free_nid(struct f2fs_sb_info *sbi,
> > > - struct free_nid *i, enum nid_state state, bool reuse)
> > > + struct free_nid *i, enum nid_state state)
> > > {
> > > struct f2fs_nm_info *nm_i = NM_I(sbi);
> > >
> > > @@ -1789,8 +1787,23 @@ static void __remove_free_nid(struct f2fs_sb_info
> > > *sbi,
> > > nm_i->nid_cnt[state]--;
> > > if (state == FREE_NID)
> > > list_del(>list);
> > > - if (!reuse)
> > > - radix_tree_delete(_i->free_nid_root, i->nid);
> > > + radix_tree_delete(_i->free_nid_root, i->nid); }
> > > +
> > > +static void __move_free_nid(struct f2fs_sb_info *sbi, struct free_nid *i,
> > > + enum nid_state org_state, enum nid_state dst_state) {
> > > + struct f2fs_nm_info *nm_i = NM_I(sbi);
> > > +
> > > + f2fs_bug_on(sbi, org_state != i->state);
> > > + i->state = dst_state;
> > > + nm_i->nid_cnt[org_state]--;
> > > + nm_i->nid_cnt[dst_state]++;
> > > +
> > > + if (org_state == FREE_NID)
> >
> > Welcome back to community, Fan. :)
> >
> > Minor, how about "if (dst_stat == PREALLOC_NID)" ?
>
> How about this?
>
> switch (dst_state) {
> case PREALLOC_NID:
> list_del(>list);
> break;
> case FREE_NID:
> list_add_tail(>list, _i->free_nid_list);
> break;
> default:
> BUG_ON(1);
> };
>
I had thought about it, the reason why I finally chose the current version is
because
" dst_state== PREALLOC_NID" implies that all states other than PREALLOC_NID
are in a list, so we need to call list_del.
Of course it's true now, but in the future if we add more states which aren't
in a list,
it would be harder for us to track this branch to modify it since it's not
related to
FREE_NID explicitly.
However the difference is trivial, if you think it's not a big deal, it's fine
by me.
> >
> > Otherwise this patch looks good to me, anyway, you can add:
> >
> > Reviewed-by: Chao Yu <yuch...@huawei.com>
> >
> > Thanks,
> >
> > > + list_del(>list);
> > > + else if (dst_state == FREE_NID)
> > > + list_add_tail(>list, _i->free_nid_list);
> > > }
> > >
> > > /* return if the nid is recognized as free */ @@ -1850,7 +1863,7 @@
> > > static bool add_free_nid(struct f2fs_sb_info *sbi, nid_t nid, bool build)
> > > }
> > >
[f2fs-dev] [PATCH] f2fs: optimize __update_nat_bits
Make three modification for __update_nat_bits:
1. Take the codes of dealing the nat with nid 0 out of the loop
Such nat only needs to be dealt with once at beginning.
2. Use " nat_index == 0" instead of " start_nid == 0" to decide if it's the
first nat block
It's better that we don't assume @start_nid is the first nid of the nat
block it's in.
3. Use " if (nat_blk->entries[i].block_addr != NULL_ADDR)" to explicitly
comfirm the value of block_addr
use constant to make sure the codes is right, even if the value of
NULL_ADDR changes.
Signed-off-by: Fan li <fanofcode...@samsung.com>
---
fs/f2fs/node.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index ac629d6..b97a031 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -2407,15 +2407,17 @@ static void __update_nat_bits(struct f2fs_sb_info *sbi,
nid_t start_nid,
unsigned int nat_index = start_nid / NAT_ENTRY_PER_BLOCK;
struct f2fs_nat_block *nat_blk = page_address(page);
int valid = 0;
- int i;
+ int i = 0;
if (!enabled_nat_bits(sbi, NULL))
return;
- for (i = 0; i < NAT_ENTRY_PER_BLOCK; i++) {
- if (start_nid == 0 && i == 0)
- valid++;
- if (nat_blk->entries[i].block_addr)
+ if (nat_index == 0) {
+ valid = 1;
+ i = 1;
+ }
+ for (; i < NAT_ENTRY_PER_BLOCK; i++) {
+ if (nat_blk->entries[i].block_addr != NULL_ADDR)
valid++;
}
if (valid == 0) {
--
2.7.4
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH] f2fs: add a function to move nid
This patch add a new function to move nid from one state to another.
Move operation is heavily used, by adding a new function for it
we can cut down some branches from several flow.
Signed-off-by: Fan li <fanofcode...@samsung.com>
---
fs/f2fs/node.c | 51 ++-
1 file changed, 30 insertions(+), 21 deletions(-)
diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index ac629d6..8116b50
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -1763,15 +1763,13 @@ static struct free_nid *__lookup_free_nid_list(struct
f2fs_nm_info *nm_i,
}
static int __insert_free_nid(struct f2fs_sb_info *sbi,
- struct free_nid *i, enum nid_state state, bool new)
+ struct free_nid *i, enum nid_state state)
{
struct f2fs_nm_info *nm_i = NM_I(sbi);
- if (new) {
- int err = radix_tree_insert(_i->free_nid_root, i->nid, i);
- if (err)
- return err;
- }
+ int err = radix_tree_insert(_i->free_nid_root, i->nid, i);
+ if (err)
+ return err;
f2fs_bug_on(sbi, state != i->state);
nm_i->nid_cnt[state]++;
@@ -1781,7 +1779,7 @@ static int __insert_free_nid(struct f2fs_sb_info *sbi,
}
static void __remove_free_nid(struct f2fs_sb_info *sbi,
- struct free_nid *i, enum nid_state state, bool reuse)
+ struct free_nid *i, enum nid_state state)
{
struct f2fs_nm_info *nm_i = NM_I(sbi);
@@ -1789,8 +1787,23 @@ static void __remove_free_nid(struct f2fs_sb_info *sbi,
nm_i->nid_cnt[state]--;
if (state == FREE_NID)
list_del(>list);
- if (!reuse)
- radix_tree_delete(_i->free_nid_root, i->nid);
+ radix_tree_delete(_i->free_nid_root, i->nid);
+}
+
+static void __move_free_nid(struct f2fs_sb_info *sbi, struct free_nid *i,
+ enum nid_state org_state, enum nid_state dst_state)
+{
+ struct f2fs_nm_info *nm_i = NM_I(sbi);
+
+ f2fs_bug_on(sbi, org_state != i->state);
+ i->state = dst_state;
+ nm_i->nid_cnt[org_state]--;
+ nm_i->nid_cnt[dst_state]++;
+
+ if (org_state == FREE_NID)
+ list_del(>list);
+ else if (dst_state == FREE_NID)
+ list_add_tail(>list, _i->free_nid_list);
}
/* return if the nid is recognized as free */
@@ -1850,7 +1863,7 @@ static bool add_free_nid(struct f2fs_sb_info *sbi, nid_t
nid, bool build)
}
}
ret = true;
- err = __insert_free_nid(sbi, i, FREE_NID, true);
+ err = __insert_free_nid(sbi, i, FREE_NID);
err_out:
spin_unlock(_i->nid_list_lock);
radix_tree_preload_end();
@@ -1869,7 +1882,7 @@ static void remove_free_nid(struct f2fs_sb_info *sbi,
nid_t nid)
spin_lock(_i->nid_list_lock);
i = __lookup_free_nid_list(nm_i, nid);
if (i && i->state == FREE_NID) {
- __remove_free_nid(sbi, i, FREE_NID, false);
+ __remove_free_nid(sbi, i, FREE_NID);
need_free = true;
}
spin_unlock(_i->nid_list_lock);
@@ -2080,9 +2093,7 @@ bool alloc_nid(struct f2fs_sb_info *sbi, nid_t *nid)
struct free_nid, list);
*nid = i->nid;
- __remove_free_nid(sbi, i, FREE_NID, true);
- i->state = PREALLOC_NID;
- __insert_free_nid(sbi, i, PREALLOC_NID, false);
+ __move_free_nid(sbi, i, FREE_NID, PREALLOC_NID);
nm_i->available_nids--;
update_free_nid_bitmap(sbi, *nid, false, false);
@@ -2108,7 +2119,7 @@ void alloc_nid_done(struct f2fs_sb_info *sbi, nid_t nid)
spin_lock(_i->nid_list_lock);
i = __lookup_free_nid_list(nm_i, nid);
f2fs_bug_on(sbi, !i);
- __remove_free_nid(sbi, i, PREALLOC_NID, false);
+ __remove_free_nid(sbi, i, PREALLOC_NID);
spin_unlock(_i->nid_list_lock);
kmem_cache_free(free_nid_slab, i);
@@ -2131,12 +2142,10 @@ void alloc_nid_failed(struct f2fs_sb_info *sbi, nid_t
nid)
f2fs_bug_on(sbi, !i);
if (!available_free_memory(sbi, FREE_NIDS)) {
- __remove_free_nid(sbi, i, PREALLOC_NID, false);
+ __remove_free_nid(sbi, i, PREALLOC_NID);
need_free = true;
} else {
- __remove_free_nid(sbi, i, PREALLOC_NID, true);
- i->state = FREE_NID;
- __insert_free_nid(sbi, i, FREE_NID, false);
+ __move_free_nid(sbi, i, PREALLOC_NID, FREE_NID);
}
nm_i->available_nids++;
@@ -2167,7 +2176,7 @@ int try_to_free_nids(struct f2fs_sb_info *sbi, int
nr_shrink)
nm_i->nid_cnt[FREE_NID] <= MAX_FREE_NIDS)
break;
- __remove_free_nid(
[f2fs-dev] [PATCH] f2fs: simplify the way of calulating next nat address
The index of segment which the next nat block is in has only one different bit than the current one, so to get the next nat address, we can simply alter that one bit. Signed-off-by: Fan Li <fanofcode...@samsung.com> --- fs/f2fs/node.h | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/fs/f2fs/node.h b/fs/f2fs/node.h index 558048e..da0b21b 100644 --- a/fs/f2fs/node.h +++ b/fs/f2fs/node.h @@ -224,10 +224,7 @@ static inline pgoff_t next_nat_addr(struct f2fs_sb_info *sbi, struct f2fs_nm_info *nm_i = NM_I(sbi); block_addr -= nm_i->nat_blkaddr; - if ((block_addr >> sbi->log_blocks_per_seg) % 2) - block_addr -= sbi->blocks_per_seg; - else - block_addr += sbi->blocks_per_seg; + block_addr ^= 1 << sbi->log_blocks_per_seg; return block_addr + nm_i->nat_blkaddr; } -- 2.7.4 -- Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] f2fs: adjust the way of calculating nat block
> -Original Message- > From: Jaegeuk Kim [mailto:jaeg...@kernel.org] > Sent: Thursday, March 09, 2017 5:56 AM > To: Fan Li > Cc: linux-f2fs-devel@lists.sourceforge.net > Subject: Re: [f2fs-dev] [PATCH] f2fs: adjust the way of calculating nat block > > On 03/08, Fan Li wrote: > > use a slightly simpler expression to calculate nat block with nid. > > > > Signed-off-by: Fan Li <fanofcode...@samsung.com> > > --- > > fs/f2fs/node.h | 4 +--- > > 1 file changed, 1 insertion(+), 3 deletions(-) > > > > diff --git a/fs/f2fs/node.h b/fs/f2fs/node.h index 2f9603f..f9fb5b8 > > 100644 > > --- a/fs/f2fs/node.h > > +++ b/fs/f2fs/node.h > > @@ -200,13 +200,11 @@ static inline pgoff_t current_nat_addr(struct > > f2fs_sb_info *sbi, nid_t start) > > struct f2fs_nm_info *nm_i = NM_I(sbi); > > pgoff_t block_off; > > pgoff_t block_addr; > > - int seg_off; > > > > block_off = NAT_BLOCK_OFFSET(start); > > - seg_off = block_off >> sbi->log_blocks_per_seg; > > > > block_addr = (pgoff_t)(nm_i->nat_blkaddr + > > - (seg_off << sbi->log_blocks_per_seg << 1) + > > + (block_off << 1) - > > NAK. This breaks the behavior. OK, I'm guessing by "breaks the behavior", you mean make the code hard to understand? I was concerned the same thing, but with the context, I think most people will be able to see right through it, and it saves some calculation, makes code as simple as possible, I just think maybe it's worthy. > > Thanks, > > > (block_off & (sbi->blocks_per_seg - 1))); > > > > if (f2fs_test_bit(block_off, nm_i->nat_bitmap)) > > -- > > 2.7.4 -- Announcing the Oxford Dictionaries API! The API offers world-renowned dictionary content that is easy and intuitive to access. Sign up for an account today to start using our lexical data to power your apps and projects. Get started today and enter our developer competition. http://sdm.link/oxford ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH] f2fs: adjust the way of calculating nat block
use a slightly simpler expression to calculate nat block with nid. Signed-off-by: Fan Li <fanofcode...@samsung.com> --- fs/f2fs/node.h | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/fs/f2fs/node.h b/fs/f2fs/node.h index 2f9603f..f9fb5b8 100644 --- a/fs/f2fs/node.h +++ b/fs/f2fs/node.h @@ -200,13 +200,11 @@ static inline pgoff_t current_nat_addr(struct f2fs_sb_info *sbi, nid_t start) struct f2fs_nm_info *nm_i = NM_I(sbi); pgoff_t block_off; pgoff_t block_addr; - int seg_off; block_off = NAT_BLOCK_OFFSET(start); - seg_off = block_off >> sbi->log_blocks_per_seg; block_addr = (pgoff_t)(nm_i->nat_blkaddr + - (seg_off << sbi->log_blocks_per_seg << 1) + + (block_off << 1) - (block_off & (sbi->blocks_per_seg - 1))); if (f2fs_test_bit(block_off, nm_i->nat_bitmap)) -- 2.7.4 -- Announcing the Oxford Dictionaries API! The API offers world-renowned dictionary content that is easy and intuitive to access. Sign up for an account today to start using our lexical data to power your apps and projects. Get started today and enter our developer competition. http://sdm.link/oxford ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH] f2fs: exclude special cases for f2fs_move_file_range
When src and dst is the same file, and the latter part of source region
overlaps with the former part of destination region, current implement
will overwrite data which hasn't been moved yet and truncate data in
overlapped region.
This patch return -EINVAL when such cases occur and return 0 when
source region and destination region is actually the same part of
the same file.
Signed-off-by: Fan li <fanofcode...@samsung.com>
---
fs/f2fs/file.c |7 +++
1 file changed, 7 insertions(+)
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index d5f60ad..116946b 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -2168,6 +2168,13 @@ static int f2fs_move_file_range(struct file *file_in,
loff_t pos_in,
if (f2fs_encrypted_inode(src) || f2fs_encrypted_inode(dst))
return -EOPNOTSUPP;
+ if (src == dst) {
+ if (pos_in == pos_out)
+ return 0;
+ if (pos_out > pos_in && pos_out < pos_in + len)
+ return -EINVAL;
+ }
+
inode_lock(src);
if (src != dst) {
if (!inode_trylock(dst)) {
--
1.7.9.5
--
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] f2fs: modify the readahead method in ra_node_page()
> -Original Message-
> From: Chao Yu [mailto:chao2...@samsung.com]
> Sent: Tuesday, March 01, 2016 5:53 PM
> To: 'Fan Li'; 'Jaegeuk Kim'
> Cc: linux-f2fs-devel@lists.sourceforge.net
> Subject: RE: [f2fs-dev] [PATCH] f2fs: modify the readahead method in
> ra_node_page()
>
> Hi
>
> > -----Original Message-
> > From: Fan Li [mailto:fanofcode...@samsung.com]
> > Sent: Monday, February 29, 2016 2:30 PM
> > To: 'Jaegeuk Kim'
> > Cc: linux-f2fs-devel@lists.sourceforge.net
> > Subject: [f2fs-dev] [PATCH] f2fs: modify the readahead method in
> > ra_node_page()
> >
> > ra_node_page() is used to read ahead one node page. Comparing to
> > regular read, it's faster because it doesn't wait for IO completion.
> > But if it is called twice for reading the same block, and the IO
> > request from the first call hasn't been completed before the second
> > call, the second call will have to wait until the read is over.
> >
> > Here use the code in __do_page_cache_readahead() to solve this problem.
> > It does nothing when someone else already puts the page in mapping.
> > The status of page should be assured by whoever puts it there.
> > This implement also prevents alteration of page reference count.
> >
> > Signed-off-by: Fan li <fanofcode...@samsung.com>
> > ---
> > fs/f2fs/node.c |9 -
> > 1 file changed, 4 insertions(+), 5 deletions(-)
> >
> > diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c index 511c0e7..6d8f107
> > 100644
> > --- a/fs/f2fs/node.c
> > +++ b/fs/f2fs/node.c
> > @@ -1080,12 +1080,11 @@ void ra_node_page(struct f2fs_sb_info *sbi, nid_t
> > nid)
> > return;
> > f2fs_bug_on(sbi, check_nid_range(sbi, nid));
> >
> > - apage = find_get_page(NODE_MAPPING(sbi), nid);
> > - if (apage && PageUptodate(apage)) {
> > - f2fs_put_page(apage, 0);
> > + rcu_read_lock();
> > + apage = radix_tree_lookup(_MAPPING(sbi)->page_tree, nid);
> > + rcu_read_unlock();
> > + if (apage)
> > return;
> > - }
> > - f2fs_put_page(apage, 0);
>
> How about use trylock_page to avoid contention here?
I thought about that, but after I saw the __do_page_cache_readahead(),
I think that's better.
they works almost the same, but it reduces the performance by
altering reference count and trying to lock the page.
The only difference between this two methods is how to deal a page
which is left unlock and not uptodate in mapping. I think only cause
of such page seems to be IO error, and it's too rare to call for optimization.
>
> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c index 26eb441..9cdb6f2 100644
> --- a/fs/f2fs/node.c
> +++ b/fs/f2fs/node.c
> @@ -1085,15 +1085,14 @@ void ra_node_page(struct f2fs_sb_info *sbi, nid_t nid)
> f2fs_bug_on(sbi, check_nid_range(sbi, nid));
>
> apage = find_get_page(NODE_MAPPING(sbi), nid);
> - if (apage && PageUptodate(apage)) {
> + if (!apage) {
> + apage = grab_cache_page(NODE_MAPPING(sbi), nid);
> + if (!apage)
> + return;
> + } else if (PageUptodate(apage) || !trylock_page(apage)) {
> f2fs_put_page(apage, 0);
> return;
> }
> - f2fs_put_page(apage, 0);
> -
> - apage = grab_cache_page(NODE_MAPPING(sbi), nid);
> - if (!apage)
> - return;
>
> err = read_node_page(apage, READA);
> f2fs_put_page(apage, err ? 1 : 0);
>
> Thanks,
>
> >
> > apage = grab_cache_page(NODE_MAPPING(sbi), nid);
> > if (!apage)
> > --
> > 1.7.9.5
> >
> >
> > --
> >
> > Site24x7 APM Insight: Get Deep Visibility into Application Performance
> > APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month
> > Monitor end-to-end web transactions and take corrective actions now
> > Troubleshoot faster and improve end-user experience. Signup Now!
> > http://pubads.g.doubleclick.net/gampad/clk?id=272487151=/4140
> > ___
> > Linux-f2fs-devel mailing list
> > Linux-f2fs-devel@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
--
Site24x7 APM Insight: Get Deep Visibility into Application Performance
APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month
Monitor end-to-end web transactions and take corrective actions now
Troubleshoot faster and improve end-user experience. Signup Now!
http://pubads.g.doubleclick.net/gampad/clk?id=272487151=/4140
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH v2] f2fs-tools: set segment_count in super block correctly
> -Original Message- > From: Junling Zheng [mailto:zhengjunl...@huawei.com] > Sent: Wednesday, February 24, 2016 4:34 PM > To: Fan Li; 'Jaegeuk Kim' > Cc: heyun...@huawei.com; linux-f2fs-devel@lists.sourceforge.net > Subject: Re: [f2fs-dev] [PATCH v2] f2fs-tools: set segment_count in super > block correctly > > Hi, Fan, > > On 2016/2/24 15:57, Fan Li wrote: > > > > > >> -Original Message- > >> From: Jaegeuk Kim [mailto:jaeg...@kernel.org] > >> Sent: Saturday, February 06, 2016 12:08 PM > >> To: Fan Li > >> Cc: 'Junling Zheng'; heyun...@huawei.com; > >> linux-f2fs-devel@lists.sourceforge.net > >> Subject: Re: [f2fs-dev] [PATCH v2] f2fs-tools: set segment_count in > >> super block correctly > >> > >> Hi Fan, > >> > >> Could you resubmit the final patch? > >> I'll remove the previous patch merged into dev branch. > > > > I think removing the patch would be sufficient enough. > > Sorry for the trouble. It's my first time to acquire the code of > > f2fs-tools, There seems to be a mistake. > > > > However, it seems that the problem you mentioned before still exists. > > Now, segment_count_main is aligned with zone size, but segment_count not, > which may cause: > > main_blkaddr + (segment_count_main << log_blocks_per_seg) != > segment0_blkaddr + (segment_count << log_blocks_per_seg) > > then, mount will fail. > > So, we still need to align segment_count with zone size:) According to new mkfs codes, segment_count has already been aligned with zone: set_sb(segment_count, (config.total_sectors * config.sector_size - zone_align_start_offset) / segment_size_bytes / config.segs_per_zone * config.segs_per_zone); It seems to be OK. What's your test parameters? > > Thanks, > > >> > >> Thanks, > >> > >> On Thu, Feb 04, 2016 at 05:33:55PM +0800, Fan Li wrote: > >>> > >>> > >>>> -Original Message- > >>>> From: Junling Zheng [mailto:zhengjunl...@huawei.com] > >>>> Sent: Thursday, February 04, 2016 10:52 AM > >>>> To: Fan Li; 'Jaegeuk Kim'; heyun...@huawei.com > >>>> Cc: linux-f2fs-devel@lists.sourceforge.net > >>>> Subject: Re: [f2fs-dev] [PATCH v2] f2fs-tools: set segment_count in > >>>> super block correctly > >>>> > >>>> On 2016/2/3 13:29, Fan Li wrote: > >>>>> Now f2fs will check statistics recorded in super block in > >>>>> sanity_check_area_boundary() during mount,if number of segments > >>>>> per section is greater than 1, and the disk space isn't aligned > >>>>> with section, > >>>> > >>>> Hi Fan, Hi Kim: > >>>> > >>>> I'm uncertain about which unit the disk space should be aligned with? > >>>> section or zone? > >>>> > >>>> It looks like commit "e9dfbbb"(mkfs.f2fs: introduce zone align for > >>>> main area) > >>>> > >>>> from Yunlei had changed the aligning unit from section to zone. > >>>> > >>>> So, should segment_count in superblock be aligned with zone rather than > >>>> section? > >>> > >>> I'm afraid that my codes of mkfs is too old, it doesn't contain this > >>> patch, and it still uses section size to align main area. > >>> > >>> Of course if main area should be aligned with zone, the following > >>> patch should be modified accordingly. > >>> > >>>> > >>>> Thanks, > >>>> > >>>> Junling > >>>> > >>>>> mount will fail due to following condition: > >>>>> > >>>>> main_blkaddr + (segment_count_main << log_blocks_per_seg) != > >>>>> segment0_blkaddr + (segment_count << log_blocks_per_seg) > >>>>> > >>>>> this is because when the length of main area isn't aligned with > >>>>> section, mkfs didn't add the number of excess segments to > >>>>> segment_count_main, but add it to segment_count. > >>>>> Here align segment_count with section size first to prevent such > >>>>> problem. > >>>>> > >>>>> Signed-off-by: Fan Li <fanofcode...@samsung.com> > >>>>> --- > >&
[f2fs-dev] [PATCH] f2fs: modify the readahead method in ra_node_page()
ra_node_page() is used to read ahead one node page. Comparing to regular
read, it's faster because it doesn't wait for IO completion.
But if it is called twice for reading the same block, and the IO request
from the first call hasn't been completed before the second call, the second
call will have to wait until the read is over.
Here use the code in __do_page_cache_readahead() to solve this problem.
It does nothing when someone else already puts the page in mapping. The
status of page should be assured by whoever puts it there.
This implement also prevents alteration of page reference count.
Signed-off-by: Fan li <fanofcode...@samsung.com>
---
fs/f2fs/node.c |9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index 511c0e7..6d8f107 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -1080,12 +1080,11 @@ void ra_node_page(struct f2fs_sb_info *sbi, nid_t nid)
return;
f2fs_bug_on(sbi, check_nid_range(sbi, nid));
- apage = find_get_page(NODE_MAPPING(sbi), nid);
- if (apage && PageUptodate(apage)) {
- f2fs_put_page(apage, 0);
+ rcu_read_lock();
+ apage = radix_tree_lookup(_MAPPING(sbi)->page_tree, nid);
+ rcu_read_unlock();
+ if (apage)
return;
- }
- f2fs_put_page(apage, 0);
apage = grab_cache_page(NODE_MAPPING(sbi), nid);
if (!apage)
--
1.7.9.5
--
Site24x7 APM Insight: Get Deep Visibility into Application Performance
APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month
Monitor end-to-end web transactions and take corrective actions now
Troubleshoot faster and improve end-user experience. Signup Now!
http://pubads.g.doubleclick.net/gampad/clk?id=272487151=/4140
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH v2] f2fs-tools: set segment_count in super block correctly
> -Original Message- > From: Jaegeuk Kim [mailto:jaeg...@kernel.org] > Sent: Saturday, February 06, 2016 12:08 PM > To: Fan Li > Cc: 'Junling Zheng'; heyun...@huawei.com; > linux-f2fs-devel@lists.sourceforge.net > Subject: Re: [f2fs-dev] [PATCH v2] f2fs-tools: set segment_count in super > block correctly > > Hi Fan, > > Could you resubmit the final patch? > I'll remove the previous patch merged into dev branch. I think removing the patch would be sufficient enough. Sorry for the trouble. It's my first time to acquire the code of f2fs-tools, There seems to be a mistake. > > Thanks, > > On Thu, Feb 04, 2016 at 05:33:55PM +0800, Fan Li wrote: > > > > > > > -Original Message- > > > From: Junling Zheng [mailto:zhengjunl...@huawei.com] > > > Sent: Thursday, February 04, 2016 10:52 AM > > > To: Fan Li; 'Jaegeuk Kim'; heyun...@huawei.com > > > Cc: linux-f2fs-devel@lists.sourceforge.net > > > Subject: Re: [f2fs-dev] [PATCH v2] f2fs-tools: set segment_count in > > > super block correctly > > > > > > On 2016/2/3 13:29, Fan Li wrote: > > > > Now f2fs will check statistics recorded in super block in > > > > sanity_check_area_boundary() during mount,if number of segments > > > > per section is greater than 1, and the disk space isn't aligned > > > > with section, > > > > > > Hi Fan, Hi Kim: > > > > > > I'm uncertain about which unit the disk space should be aligned with? > > > section or zone? > > > > > > It looks like commit "e9dfbbb"(mkfs.f2fs: introduce zone align for > > > main area) > > > > > > from Yunlei had changed the aligning unit from section to zone. > > > > > > So, should segment_count in superblock be aligned with zone rather than > > > section? > > > > I'm afraid that my codes of mkfs is too old, it doesn't contain this > > patch, and it still uses section size to align main area. > > > > Of course if main area should be aligned with zone, the following > > patch should be modified accordingly. > > > > > > > > Thanks, > > > > > > Junling > > > > > > > mount will fail due to following condition: > > > > > > > > main_blkaddr + (segment_count_main << log_blocks_per_seg) != > > > > segment0_blkaddr + (segment_count << log_blocks_per_seg) > > > > > > > > this is because when the length of main area isn't aligned with > > > > section, mkfs didn't add the number of excess segments to > > > > segment_count_main, but add it to segment_count. > > > > Here align segment_count with section size first to prevent such > > > > problem. > > > > > > > > Signed-off-by: Fan Li <fanofcode...@samsung.com> > > > > --- > > > > mkfs/f2fs_format.c |3 ++- > > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c index > > > > 66d7342..3366302 100644 > > > > --- a/mkfs/f2fs_format.c > > > > +++ b/mkfs/f2fs_format.c > > > > @@ -174,7 +174,8 @@ static int f2fs_prepare_super_block(void) > > > > } > > > > > > > > set_sb(segment_count, (config.total_sectors * > > > > config.sector_size - > > > > - zone_align_start_offset) / > > > > segment_size_bytes); > > > > + zone_align_start_offset) / > > > > segment_size_bytes / > > > > + config.segs_per_sec * > > > > + config.segs_per_sec); > > > > > > > > set_sb(segment0_blkaddr, zone_align_start_offset / > > > > blk_size_bytes); > > > > sb->cp_blkaddr = sb->segment0_blkaddr; > > > > > > > > > -- Site24x7 APM Insight: Get Deep Visibility into Application Performance APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month Monitor end-to-end web transactions and take corrective actions now Troubleshoot faster and improve end-user experience. Signup Now! http://pubads.g.doubleclick.net/gampad/clk?id=272487151=/4140 ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH v2] f2fs-tools: set segment_count in super block correctly
> -Original Message- > From: Junling Zheng [mailto:zhengjunl...@huawei.com] > Sent: Thursday, February 04, 2016 10:52 AM > To: Fan Li; 'Jaegeuk Kim'; heyun...@huawei.com > Cc: linux-f2fs-devel@lists.sourceforge.net > Subject: Re: [f2fs-dev] [PATCH v2] f2fs-tools: set segment_count in super > block correctly > > On 2016/2/3 13:29, Fan Li wrote: > > Now f2fs will check statistics recorded in super block in > > sanity_check_area_boundary() during mount,if number of segments per > > section is greater than 1, and the disk space isn't aligned with > > section, > > Hi Fan, Hi Kim: > > I'm uncertain about which unit the disk space should be aligned with? section > or zone? > > It looks like commit "e9dfbbb"(mkfs.f2fs: introduce zone align for main area) > > from Yunlei had changed the aligning unit from section to zone. > > So, should segment_count in superblock be aligned with zone rather than > section? I'm afraid that my codes of mkfs is too old, it doesn't contain this patch, and it still uses section size to align main area. Of course if main area should be aligned with zone, the following patch should be modified accordingly. > > Thanks, > > Junling > > > mount will fail due to following condition: > > > > main_blkaddr + (segment_count_main << log_blocks_per_seg) != > > segment0_blkaddr + (segment_count << log_blocks_per_seg) > > > > this is because when the length of main area isn't aligned with > > section, mkfs didn't add the number of excess segments to > > segment_count_main, but add it to segment_count. > > Here align segment_count with section size first to prevent such problem. > > > > Signed-off-by: Fan Li <fanofcode...@samsung.com> > > --- > > mkfs/f2fs_format.c |3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c index > > 66d7342..3366302 100644 > > --- a/mkfs/f2fs_format.c > > +++ b/mkfs/f2fs_format.c > > @@ -174,7 +174,8 @@ static int f2fs_prepare_super_block(void) > > } > > > > set_sb(segment_count, (config.total_sectors * config.sector_size - > > - zone_align_start_offset) / > > segment_size_bytes); > > + zone_align_start_offset) / > > segment_size_bytes / > > + config.segs_per_sec * > > + config.segs_per_sec); > > > > set_sb(segment0_blkaddr, zone_align_start_offset / blk_size_bytes); > > sb->cp_blkaddr = sb->segment0_blkaddr; > > > -- Site24x7 APM Insight: Get Deep Visibility into Application Performance APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month Monitor end-to-end web transactions and take corrective actions now Troubleshoot faster and improve end-user experience. Signup Now! http://pubads.g.doubleclick.net/gampad/clk?id=272487151=/4140 ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH] f2fs: avoid unnecessary search while finding victim in gc
variable nsearched in get_victim_by_default() indicates the number of
dirty segments we already checked. There are 2 problems about the way
it updates:
1. When p.ofs_unit is greater than 1, the victim we find consists
of multiple segments, possibly more than 1 dirty segment.
But nsearched always increases by 1.
2. If segments have been found but not been chosen, nsearched won't
increase. So even we have checked all dirty segments, nsearched
may still less than p.max_search.
All these problems could cause unnecessary search after all dirty
segments have already been checked.
Signed-off-by: Fan li <fanofcode...@samsung.com>
---
fs/f2fs/gc.c | 33 +
1 file changed, 25 insertions(+), 8 deletions(-)
diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index c958321..47ade35 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -245,6 +245,18 @@ static inline unsigned int get_gc_cost(struct f2fs_sb_info
*sbi,
return get_cb_cost(sbi, segno);
}
+static unsigned int count_bits(const unsigned long *addr,
+ unsigned int offset, unsigned int len)
+{
+ unsigned int end = offset + len, sum = 0;
+
+ while (offset < end) {
+ if (test_bit(offset++, addr))
+ ++sum;
+ }
+ return sum;
+}
+
/*
* This function is called from two paths.
* One is garbage collection and the other is SSR segment selection.
@@ -260,7 +272,7 @@ static int get_victim_by_default(struct f2fs_sb_info *sbi,
struct victim_sel_policy p;
unsigned int secno, max_cost;
unsigned int last_segment = MAIN_SEGS(sbi);
- int nsearched = 0;
+ unsigned int nsearched = 0;
mutex_lock(_i->seglist_lock);
@@ -295,26 +307,31 @@ static int get_victim_by_default(struct f2fs_sb_info *sbi,
}
p.offset = segno + p.ofs_unit;
- if (p.ofs_unit > 1)
+ if (p.ofs_unit > 1) {
p.offset -= segno % p.ofs_unit;
+ nsearched += count_bits(p.dirty_segmap,
+ p.offset - p.ofs_unit,
+ p.ofs_unit);
+ } else {
+ nsearched++;
+ }
+
secno = GET_SECNO(sbi, segno);
if (sec_usage_check(sbi, secno))
- continue;
+ goto next;
if (gc_type == BG_GC && test_bit(secno, dirty_i->victim_secmap))
- continue;
+ goto next;
cost = get_gc_cost(sbi, segno, );
if (p.min_cost > cost) {
p.min_segno = segno;
p.min_cost = cost;
- } else if (unlikely(cost == max_cost)) {
- continue;
}
-
- if (nsearched++ >= p.max_search) {
+next:
+ if (nsearched >= p.max_search) {
sbi->last_victim[p.gc_mode] = segno;
break;
}
--
1.7.9.5
--
Site24x7 APM Insight: Get Deep Visibility into Application Performance
APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month
Monitor end-to-end web transactions and take corrective actions now
Troubleshoot faster and improve end-user experience. Signup Now!
http://pubads.g.doubleclick.net/gampad/clk?id=267308311=/4140
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH v2] f2fs-tools: set segment_count in super block correctly
Now f2fs will check statistics recorded in super block in sanity_check_area_boundary() during mount,if number of segments per section is greater than 1, and the disk space isn't aligned with section, mount will fail due to following condition: main_blkaddr + (segment_count_main << log_blocks_per_seg) != segment0_blkaddr + (segment_count << log_blocks_per_seg) this is because when the length of main area isn't aligned with section, mkfs didn't add the number of excess segments to segment_count_main, but add it to segment_count. Here align segment_count with section size first to prevent such problem. Signed-off-by: Fan Li <fanofcode...@samsung.com> --- mkfs/f2fs_format.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c index 66d7342..3366302 100644 --- a/mkfs/f2fs_format.c +++ b/mkfs/f2fs_format.c @@ -174,7 +174,8 @@ static int f2fs_prepare_super_block(void) } set_sb(segment_count, (config.total_sectors * config.sector_size - - zone_align_start_offset) / segment_size_bytes); + zone_align_start_offset) / segment_size_bytes / + config.segs_per_sec * config.segs_per_sec); set_sb(segment0_blkaddr, zone_align_start_offset / blk_size_bytes); sb->cp_blkaddr = sb->segment0_blkaddr; -- 1.7.9.5 -- Site24x7 APM Insight: Get Deep Visibility into Application Performance APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month Monitor end-to-end web transactions and take corrective actions now Troubleshoot faster and improve end-user experience. Signup Now! http://pubads.g.doubleclick.net/gampad/clk?id=267308311=/4140 ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH] f2fs-tools: set segment_count in super block correctly
Now f2fs will check statistics recorded in super block in sanity_check_area_boundary() during mount,if number of segments per section is greater than 1, and disk space isn't aligned with section, mount will fail due to following condition: main_blkaddr + (segment_count_main << log_blocks_per_seg) != segment0_blkaddr + (segment_count << log_blocks_per_seg) this is because when the length of main area isn't aligned with section, mkfs didn't add the number of excess segments to segment_count_main, but add it to segment_count. Here align segment_count with section size to prevent such problem. Signed-off-by: Fan Li <fanofcode...@samsung.com> --- mkfs/f2fs_format.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c index 66d7342..aab2491 100644 --- a/mkfs/f2fs_format.c +++ b/mkfs/f2fs_format.c @@ -174,7 +174,8 @@ static int f2fs_prepare_super_block(void) } set_sb(segment_count, (config.total_sectors * config.sector_size - - zone_align_start_offset) / segment_size_bytes); + zone_align_start_offset) / segment_size_bytes/ + config.segs_per_sec*config.segs_per_sec); set_sb(segment0_blkaddr, zone_align_start_offset / blk_size_bytes); sb->cp_blkaddr = sb->segment0_blkaddr; -- 1.7.9.5 -- Site24x7 APM Insight: Get Deep Visibility into Application Performance APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month Monitor end-to-end web transactions and take corrective actions now Troubleshoot faster and improve end-user experience. Signup Now! http://pubads.g.doubleclick.net/gampad/clk?id=267308311=/4140 ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH] f2fs: optimize f2fs_write_cache_pages()
f2fs_write_cache_pages() writes cold/warm pages in different traverse to
make sure pages are written in continuous blocks.
There are three modifications in this patch:
1. If file is cold, all its pages are written in cold section. So don't
distinguish the type of pages in such case.
2. f2fs_write_cache_pages() always writes cold pages first and then
warm pages.
But it's rare that cold/warm pages both exist in mapping of the same
file because of the policy of gc. So if there are only one type of
pages, we only go through the mapping once.
3. If cycled is 0, the traverse will be separated into two parts if it
reachs the end of mapping. In that case if there are warm pages in
both parts, the second parts won't be handled correctly since step
is already set to 1 for dealing the first part. So move the assignment
of step after processing the second part.
I also found another two problems and describe them in comment
beginning with "FIXME". I tried to fix them, but my method is too complex,
so I mark it and hope someone else has better solution.
Signed-off-by: Fan li <fanofcode...@samsung.com>
---
fs/f2fs/data.c | 39 +--
1 file changed, 29 insertions(+), 10 deletions(-)
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index ac9e7c6..8dfdbf3 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1211,7 +1211,7 @@ static int f2fs_write_cache_pages(struct address_space
*mapping,
int ret = 0;
int done = 0;
struct pagevec pvec;
- int nr_pages;
+ int nr_pages, skipped_pages = 0;
pgoff_t uninitialized_var(writeback_index);
pgoff_t index;
pgoff_t end;/* Inclusive */
@@ -1220,8 +1220,12 @@ static int f2fs_write_cache_pages(struct address_space
*mapping,
int range_whole = 0;
int tag;
int step = 0;
+ int skip_type = -1; /* which type of pages needs to be skipped */
pagevec_init(, 0);
+ /* no need to skip any page when file is cold */
+ if (file_is_cold(mapping->host))
+ skip_type = 2;
next:
if (wbc->range_cyclic) {
writeback_index = mapping->writeback_index; /* prev offset */
@@ -1277,16 +1281,21 @@ continue_unlock:
goto continue_unlock;
}
- if (step == is_cold_data(page))
- goto continue_unlock;
-
if (PageWriteback(page)) {
- if (wbc->sync_mode != WB_SYNC_NONE)
+ if (wbc->sync_mode != WB_SYNC_NONE &&
+ skip_type != is_cold_data(page))
f2fs_wait_on_page_writeback(page, DATA);
else
goto continue_unlock;
}
+ if (skip_type == -1) {
+ skip_type = !is_cold_data(page);
+ } else if (unlikely(skip_type == is_cold_data(page))) {
+ skipped_pages++;
+ goto continue_unlock;
+ }
+
BUG_ON(PageWriteback(page));
if (!clear_page_dirty_for_io(page))
goto continue_unlock;
@@ -1313,17 +1322,27 @@ continue_unlock:
cond_resched();
}
- if (step < 1) {
- step++;
- goto next;
- }
-
if (!cycled && !done) {
cycled = 1;
index = 0;
end = writeback_index - 1;
goto retry;
}
+
+ /*
+* Retry when there are skipped pages.
+* FIXME: when done is 1, there may still be some
+* skipped pages, and will not be written.
+*/
+ if (!done && !step && skipped_pages) {
+ skip_type = !skip_type;
+ step = 1;
+ goto next;
+ }
+ /*
+* FIXME: if step is 1, index of the last written page may
+* not indicate the end of range we searched.
+*/
if (wbc->range_cyclic || (range_whole && wbc->nr_to_write > 0))
mapping->writeback_index = done_index;
--
1.7.9.5
--
Site24x7 APM Insight: Get Deep Visibility into Application Performance
APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month
Monitor end-to-end web transactions and take corrective actions now
Troubleshoot faster and improve end-user experience. Signup Now!
http://pubads.g.doubleclick.net/gampad/clk?id=267308311=/4140
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH 2/3] f2fs: support finding extents after isize
> -Original Message- > From: Chao Yu [mailto:chao2...@samsung.com] > Sent: Monday, January 04, 2016 6:00 PM > To: 'Fan Li'; 'Jaegeuk Kim' > Cc: linux-f2fs-devel@lists.sourceforge.net > Subject: RE: [f2fs-dev] [PATCH 2/3] f2fs: support finding extents after isize > > > -Original Message- > > From: Fan Li [mailto:fanofcode...@samsung.com] > > Sent: Monday, January 04, 2016 1:57 PM > > To: 'Chao Yu'; 'Jaegeuk Kim' > > Cc: linux-f2fs-devel@lists.sourceforge.net > > Subject: RE: [f2fs-dev] [PATCH 2/3] f2fs: support finding extents > > after isize > > > > > > > > > -Original Message- > > > From: Chao Yu [mailto:chao2...@samsung.com] > > > Sent: Thursday, December 31, 2015 2:34 PM > > > To: 'Fan Li'; 'Jaegeuk Kim' > > > Cc: linux-f2fs-devel@lists.sourceforge.net > > > Subject: RE: [f2fs-dev] [PATCH 2/3] f2fs: support finding extents > > > after isize > > > > > > > -Original Message- > > > > From: Fan Li [mailto:fanofcode...@samsung.com] > > > > Sent: Thursday, December 31, 2015 11:37 AM > > > > To: 'Chao Yu'; 'Jaegeuk Kim' > > > > Cc: linux-f2fs-devel@lists.sourceforge.net > > > > Subject: Re: [f2fs-dev] [PATCH 2/3] f2fs: support finding extents > > > > after isize > > > > > > > > > > > > > > > > > -Original Message- > > > > > From: Chao Yu [mailto:c...@kernel.org] > > > > > Sent: Wednesday, December 30, 2015 9:28 PM > > > > > To: Fan Li; 'Jaegeuk Kim' > > > > > Cc: linux-f2fs-devel@lists.sourceforge.net > > > > > Subject: Re: [f2fs-dev] [PATCH 2/3] f2fs: support finding > > > > > extents after isize > > > > > > > > > > Hi, > > > > > > > > > > On 12/30/15 5:17 PM, Fan Li wrote: > > > > > > f2fs allows preallocation beyond isize, but f2fs_fiemap only > > > > > > look up extents within isize. Therefore add this support. > > > > > > > > > > > > Note: It's possible that there are holes after isize, for > > > > > > example, fallocate multiple discontinuous extents after isize > > > > > > with FALLOC_FL_KEEP_SIZE set. Since I can tell no differences > > > > > > between EOF and holes from return of get_data_block, I'm afaid > > > > > > this patch can't support such scenarios. > > > > > > > > > > As you mentioned, preallocated block beyond isize can be > > > > > allocated in f2fs, and we are trying > > > > to support mapping extents across > > > > > whole data space of inode, so why we treat theses extents inside > > > > > i_size and outside i_size > > > > separately? IMO, instead using i_size, we > > > > > should use max blocks as boundary. > > > > > > > > > > Most important, this interface still can't support finding all > > > > > extents after i_size, which > > > > looks buggy for our user. > > > > > > > > Notice that this issue exists before my patch, by adding this > > > > patch, at least now it can support more scenarios such as > > > > fallocate a range right after isize. I'd say it's an improvement. > > > > > > Nope, what I'm talking about is *correctness* of our ->fiemap > > > interface, but you're trying > > to avoid it by saying "support more > > cases, > > > it's an improvement". That doesn't make any sense to me, since > > > correctness issue still not > > be fixed. > > > > I'm not sure what you mean by avoiding, I think the comment and reply > > I written has already stated the issue and limitation of this patch. > > What I mean here is it's better to stand in user's viewpoint, let the > interface return the correct result, since user only refer the manual > of interface, rather than comments in patch or reply in email. >From viewpoint of users, this patch makes the functionality of fiemap a >little closer to the manual, it supports one more scenario that manual says, why isn't it an improvement? Besides there are a lot of examples in kernel, that is written in comment and different from the manual, the old version of this function happens to be one of them, are you saying that if we don't solve this, we shouldn't support this function at all? Of course there are solutions at the end, as I said in reply, I have an idea about further improvement, bu
Re: [f2fs-dev] [PATCH 2/3] f2fs: support finding extents after isize
> -Original Message- > From: Jaegeuk Kim [mailto:jaeg...@kernel.org] > Sent: Monday, January 04, 2016 2:56 PM > To: Fan Li > Cc: 'Chao Yu'; linux-f2fs-devel@lists.sourceforge.net > Subject: Re: [f2fs-dev] [PATCH 2/3] f2fs: support finding extents after isize > > Hello, > > On Mon, Jan 04, 2016 at 01:57:27PM +0800, Fan Li wrote: > > > > > > > -Original Message- > > > From: Chao Yu [mailto:chao2...@samsung.com] > > > Sent: Thursday, December 31, 2015 2:34 PM > > > To: 'Fan Li'; 'Jaegeuk Kim' > > > Cc: linux-f2fs-devel@lists.sourceforge.net > > > Subject: RE: [f2fs-dev] [PATCH 2/3] f2fs: support finding extents > > > after isize > > > > > > > -Original Message- > > > > From: Fan Li [mailto:fanofcode...@samsung.com] > > > > Sent: Thursday, December 31, 2015 11:37 AM > > > > To: 'Chao Yu'; 'Jaegeuk Kim' > > > > Cc: linux-f2fs-devel@lists.sourceforge.net > > > > Subject: Re: [f2fs-dev] [PATCH 2/3] f2fs: support finding extents > > > > after isize > > > > > > > > > > > > > > > > > -Original Message- > > > > > From: Chao Yu [mailto:c...@kernel.org] > > > > > Sent: Wednesday, December 30, 2015 9:28 PM > > > > > To: Fan Li; 'Jaegeuk Kim' > > > > > Cc: linux-f2fs-devel@lists.sourceforge.net > > > > > Subject: Re: [f2fs-dev] [PATCH 2/3] f2fs: support finding > > > > > extents after isize > > > > > > > > > > Hi, > > > > > > > > > > On 12/30/15 5:17 PM, Fan Li wrote: > > > > > > f2fs allows preallocation beyond isize, but f2fs_fiemap only > > > > > > look up extents within isize. Therefore add this support. > > > > > > > > > > > > Note: It's possible that there are holes after isize, for > > > > > > example, fallocate multiple discontinuous extents after isize > > > > > > with FALLOC_FL_KEEP_SIZE set. Since I can tell no differences > > > > > > between EOF and holes from return of get_data_block, I'm afaid > > > > > > this patch can't support such scenarios. > > > > > > > > > > As you mentioned, preallocated block beyond isize can be > > > > > allocated in f2fs, and we are trying > > > > to support mapping extents across > > > > > whole data space of inode, so why we treat theses extents inside > > > > > i_size and outside i_size > > > > separately? IMO, instead using i_size, we > > > > > should use max blocks as boundary. > > > > > > > > > > Most important, this interface still can't support finding all > > > > > extents after i_size, which > > > > looks buggy for our user. > > > > > > > > Notice that this issue exists before my patch, by adding this > > > > patch, at least now it can support more scenarios such as > > > > fallocate a range right after isize. I'd say it's an improvement. > > > > > > Nope, what I'm talking about is *correctness* of our ->fiemap > > > interface, but you're trying to avoid it by saying "support more > > cases, > > > it's an improvement". That doesn't make any sense to me, since > > > correctness issue still not be fixed. > > > > I'm not sure what you mean by avoiding, I think the comment and reply > > I written has already stated the issue and limitation of this patch. > > Now there are two suggestions: > > 1. support one more scenario, and all old scenarios are dealt like > > before, but it still can't support discontinuous extent after isize. > > 2. support all scenarios, but sacrifice performance for lots of common > > scenarios by checking about 10^9 blocks. > > IMO, we can think about #2 whether there is an efficient way. > > How many cases does this incur? > One is fallocate with keeping i_size, ana other? > > How about adding FADVISE_OVER_ISIZE to represent inode has blocks beyond > i_size? > Then, we can set this flag in fallocate and reset it in f2fs_truncate. I have a similar idea that add an actual size which marks the end of last extent, so we can know if the current extent is the last one, even without searching for extents behind. But there is a problem I still can't figure out, after truncate an extent at the end of file beyond isize , how do I know where the new last extent ends or if there are still extents beyond isize? after all, the
Re: [f2fs-dev] [PATCH 2/3] f2fs: support finding extents after isize
> -Original Message- > From: Jaegeuk Kim [mailto:jaeg...@kernel.org] > Sent: Tuesday, January 05, 2016 7:14 AM > To: Fan Li > Cc: 'Chao Yu'; linux-f2fs-devel@lists.sourceforge.net > Subject: Re: [f2fs-dev] [PATCH 2/3] f2fs: support finding extents after isize > > Hi, > > On Mon, Jan 04, 2016 at 07:13:43PM +0800, Fan Li wrote: > > > > > > > -Original Message- > > > From: Jaegeuk Kim [mailto:jaeg...@kernel.org] > > > Sent: Monday, January 04, 2016 2:56 PM > > > To: Fan Li > > > Cc: 'Chao Yu'; linux-f2fs-devel@lists.sourceforge.net > > > Subject: Re: [f2fs-dev] [PATCH 2/3] f2fs: support finding extents > > > after isize > > > > > > Hello, > > > > > > On Mon, Jan 04, 2016 at 01:57:27PM +0800, Fan Li wrote: > > > > > > > > > > > > > -Original Message- > > > > > From: Chao Yu [mailto:chao2...@samsung.com] > > > > > Sent: Thursday, December 31, 2015 2:34 PM > > > > > To: 'Fan Li'; 'Jaegeuk Kim' > > > > > Cc: linux-f2fs-devel@lists.sourceforge.net > > > > > Subject: RE: [f2fs-dev] [PATCH 2/3] f2fs: support finding > > > > > extents after isize > > > > > > > > > > > -Original Message- > > > > > > From: Fan Li [mailto:fanofcode...@samsung.com] > > > > > > Sent: Thursday, December 31, 2015 11:37 AM > > > > > > To: 'Chao Yu'; 'Jaegeuk Kim' > > > > > > Cc: linux-f2fs-devel@lists.sourceforge.net > > > > > > Subject: Re: [f2fs-dev] [PATCH 2/3] f2fs: support finding > > > > > > extents after isize > > > > > > > > > > > > > > > > > > > > > > > > > -Original Message- > > > > > > > From: Chao Yu [mailto:c...@kernel.org] > > > > > > > Sent: Wednesday, December 30, 2015 9:28 PM > > > > > > > To: Fan Li; 'Jaegeuk Kim' > > > > > > > Cc: linux-f2fs-devel@lists.sourceforge.net > > > > > > > Subject: Re: [f2fs-dev] [PATCH 2/3] f2fs: support finding > > > > > > > extents after isize > > > > > > > > > > > > > > Hi, > > > > > > > > > > > > > > On 12/30/15 5:17 PM, Fan Li wrote: > > > > > > > > f2fs allows preallocation beyond isize, but f2fs_fiemap > > > > > > > > only look up extents within isize. Therefore add this support. > > > > > > > > > > > > > > > > Note: It's possible that there are holes after isize, for > > > > > > > > example, fallocate multiple discontinuous extents after > > > > > > > > isize with FALLOC_FL_KEEP_SIZE set. Since I can tell no > > > > > > > > differences between EOF and holes from return of > > > > > > > > get_data_block, I'm afaid this patch can't support such > > > > > > > > scenarios. > > > > > > > > > > > > > > As you mentioned, preallocated block beyond isize can be > > > > > > > allocated in f2fs, and we are trying > > > > > > to support mapping extents across > > > > > > > whole data space of inode, so why we treat theses extents > > > > > > > inside i_size and outside i_size > > > > > > separately? IMO, instead using i_size, we > > > > > > > should use max blocks as boundary. > > > > > > > > > > > > > > Most important, this interface still can't support finding > > > > > > > all extents after i_size, which > > > > > > looks buggy for our user. > > > > > > > > > > > > Notice that this issue exists before my patch, by adding this > > > > > > patch, at least now it can support more scenarios such as > > > > > > fallocate a range right after isize. I'd say it's an improvement. > > > > > > > > > > Nope, what I'm talking about is *correctness* of our ->fiemap > > > > > interface, but you're trying to avoid it by saying "support more > > > > cases, > > > > > it's an improvement". That doesn't make any sense to me, since > > > > > correctness issue still not be fixed. > > > > > > > > I'm not sure what you mean by avoiding, I think t
Re: [f2fs-dev] [PATCH 2/3] f2fs: support finding extents after isize
> -Original Message- > From: Jaegeuk Kim [mailto:jaeg...@kernel.org] > Sent: Tuesday, January 05, 2016 11:08 AM > To: Fan Li; ""@jaegeuk-2.local > Cc: 'Chao Yu'; linux-f2fs-devel@lists.sourceforge.net > Subject: Re: [f2fs-dev] [PATCH 2/3] f2fs: support finding extents after isize > > Hi, > > > > > > > > > > On 12/30/15 5:17 PM, Fan Li wrote: > > > > > > > > > > f2fs allows preallocation beyond isize, but > > > > > > > > > > f2fs_fiemap only look up extents within isize. Therefore > > > > > > > > > > add this support. > > > > > > > > > > > > > > > > > > > > Note: It's possible that there are holes after isize, > > > > > > > > > > for example, fallocate multiple discontinuous extents > > > > > > > > > > after isize with FALLOC_FL_KEEP_SIZE set. Since I can > > > > > > > > > > tell no differences between EOF and holes from return > > > > > > > > > > of get_data_block, I'm afaid this patch can't support such > > > > > > > > > > scenarios. > > > > > > > > > > > > > > > > > > As you mentioned, preallocated block beyond isize can be > > > > > > > > > allocated in f2fs, and we are trying > > > > > > > > to support mapping extents across > > > > > > > > > whole data space of inode, so why we treat theses > > > > > > > > > extents inside i_size and outside i_size > > > > > > > > separately? IMO, instead using i_size, we > > > > > > > > > should use max blocks as boundary. > > > > > > > > > > > > > > > > > > Most important, this interface still can't support > > > > > > > > > finding all extents after i_size, which > > > > > > > > looks buggy for our user. > > > > > > > > > > > > > > > > Notice that this issue exists before my patch, by adding > > > > > > > > this patch, at least now it can support more scenarios > > > > > > > > such as fallocate a range right after isize. I'd say it's an > > > > > > > > improvement. > > > > > > > > > > > > > > Nope, what I'm talking about is *correctness* of our > > > > > > > ->fiemap interface, but you're trying to avoid it by saying > > > > > > > "support more > > > > > > cases, > > > > > > > it's an improvement". That doesn't make any sense to me, since > > > > > > > correctness issue still not be fixed. > > > > > > > > > > > > I'm not sure what you mean by avoiding, I think the comment > > > > > > and reply I written has already stated the issue and limitation of > > > > > > this patch. > > > > > > Now there are two suggestions: > > > > > > 1. support one more scenario, and all old scenarios are dealt > > > > > > like before, but it still can't support discontinuous extent after > > > > > > isize. > > > > > > 2. support all scenarios, but sacrifice performance for lots of > > > > > > common scenarios by checking about 10^9 blocks. > > > > > > > > > > IMO, we can think about #2 whether there is an efficient way. > > > > > > > > > > How many cases does this incur? > > > > > One is fallocate with keeping i_size, ana other? > > > > > > > > > > How about adding FADVISE_OVER_ISIZE to represent inode has blocks > > > > > beyond i_size? > > > > > Then, we can set this flag in fallocate and reset it in f2fs_truncate. > > > > > > > > I have a similar idea that add an actual size which marks the end > > > > of last extent, so we can know if the current extent is the last one, > > > > even without searching for extents behind. > > > > > > Where do you want to store that size in disk? > > > > I'm still not very confident about this idea, so I didn't really think > > that through yet, inode may be a proper place. > > Or we just write a special function to find it, and call it only when > > fiemap is called and disk size isn't
Re: [f2fs-dev] [PATCH 2/3] f2fs: support finding extents after isize
> -Original Message- > From: Chao Yu [mailto:chao2...@samsung.com] > Sent: Thursday, December 31, 2015 2:34 PM > To: 'Fan Li'; 'Jaegeuk Kim' > Cc: linux-f2fs-devel@lists.sourceforge.net > Subject: RE: [f2fs-dev] [PATCH 2/3] f2fs: support finding extents after isize > > > -Original Message- > > From: Fan Li [mailto:fanofcode...@samsung.com] > > Sent: Thursday, December 31, 2015 11:37 AM > > To: 'Chao Yu'; 'Jaegeuk Kim' > > Cc: linux-f2fs-devel@lists.sourceforge.net > > Subject: Re: [f2fs-dev] [PATCH 2/3] f2fs: support finding extents > > after isize > > > > > > > > > -Original Message- > > > From: Chao Yu [mailto:c...@kernel.org] > > > Sent: Wednesday, December 30, 2015 9:28 PM > > > To: Fan Li; 'Jaegeuk Kim' > > > Cc: linux-f2fs-devel@lists.sourceforge.net > > > Subject: Re: [f2fs-dev] [PATCH 2/3] f2fs: support finding extents > > > after isize > > > > > > Hi, > > > > > > On 12/30/15 5:17 PM, Fan Li wrote: > > > > f2fs allows preallocation beyond isize, but f2fs_fiemap only look > > > > up extents within isize. Therefore add this support. > > > > > > > > Note: It's possible that there are holes after isize, for example, > > > > fallocate multiple discontinuous extents after isize with > > > > FALLOC_FL_KEEP_SIZE set. Since I can tell no differences between > > > > EOF and holes from return of get_data_block, I'm afaid this patch > > > > can't support such scenarios. > > > > > > As you mentioned, preallocated block beyond isize can be allocated > > > in f2fs, and we are trying > > to support mapping extents across > > > whole data space of inode, so why we treat theses extents inside > > > i_size and outside i_size > > separately? IMO, instead using i_size, we > > > should use max blocks as boundary. > > > > > > Most important, this interface still can't support finding all > > > extents after i_size, which > > looks buggy for our user. > > > > Notice that this issue exists before my patch, by adding this patch, > > at least now it can support more scenarios such as fallocate a range > > right after isize. I'd say it's an improvement. > > Nope, what I'm talking about is *correctness* of our ->fiemap interface, but > you're trying to avoid it by saying "support more cases, > it's an improvement". That doesn't make any sense to me, since correctness > issue still not be fixed. I'm not sure what you mean by avoiding, I think the comment and reply I written has already stated the issue and limitation of this patch. Now there are two suggestions: 1. support one more scenario, and all old scenarios are dealt like before, but it still can't support discontinuous extent after isize. 2. support all scenarios, but sacrifice performance for lots of common scenarios by checking about 10^9 blocks. I think we can all agree both ideas have their flaws. The only divergence is that I vote the first, and you the second. I think the most important thing is that it works fluently in most scenarios, and you think is that it works in every scenarios even it's very slow. I think my method is more pratical, but balance between performance and utility seems to be an Eternal problem. > > > > > use max blocks as boundary would get us the precise result, but it > > also means after we reach the EOF, we still need to look up every > > block between the EOF and sb-> s_maxbytes to make sure the EOF is > > true, that's about 4TB or 10^9 blocks. > > And it affects all scenarios where the search range covers the last > > extent in the file, not just extents beyond isize. I think this price > > is too high to pay. > > That's another performance issue. > > > > > I was hoping that I can make f2fs_map_block return an EOF to solve > > this problem some time later, or anyone have a better idea? > > At least we can seek valid dnode block like the way llseek use. > > In addition, for most cases, few of i_nid[5] in f2fs_inode will be NULL, we > could skip searching all dnode block in such non-allocated > indirect node, instead of searching dnode block f2fs_map_block one by one. > > Thanks, > > > > > > > > > Thanks, > > > > > > > > > > > > > > > Signed-off-by: Fan li <fanofcode...@samsung.com> > > > > --- > > > > fs/f2fs/data.c |7 +-- > > > > 1 file changed, 1 insertion(+), 6 deletions(-) > > > > > > > > diff
Re: [f2fs-dev] [PATCH 1/3] f2fs: avoid calling i_size_read when it's unnecessary
> -Original Message-
> From: Jaegeuk Kim [mailto:jaeg...@kernel.org]
> Sent: Friday, January 01, 2016 11:08 AM
> To: Fan Li
> Cc: linux-f2fs-devel@lists.sourceforge.net
> Subject: Re: [f2fs-dev] [PATCH 1/3] f2fs: avoid calling i_size_read when it's
> unnecessary
>
> Hi Fan,
>
> On Thu, Dec 31, 2015 at 10:53:32AM +0800, Fan Li wrote:
> >
> >
> > > -Original Message-
> > > From: Jaegeuk Kim [mailto:jaeg...@kernel.org]
> > > Sent: Thursday, December 31, 2015 2:28 AM
> > > To: Fan Li
> > > Cc: linux-f2fs-devel@lists.sourceforge.net
> > > Subject: Re: [f2fs-dev] [PATCH 1/3] f2fs: avoid calling i_size_read
> > > when it's unnecessary
> > >
> > > Hi Fan,
> > >
> > > On Wed, Dec 30, 2015 at 04:49:56PM +0800, Fan Li wrote:
> > > > i_size_read does more than reading a value, it's best that we use
> > > > it only when we need it.
> > > >
> > > > Signed-off-by: Fan li <fanofcode...@samsung.com>
> > > > ---
> > > > fs/f2fs/data.c |4 +++-
> > > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c index
> > > > d67c599..a9a4d89
> > > > 100644
> > > > --- a/fs/f2fs/data.c
> > > > +++ b/fs/f2fs/data.c
> > > > @@ -780,7 +780,7 @@ int f2fs_fiemap(struct inode *inode, struct
> > > > fiemap_extent_info *fieinfo, {
> > > > struct buffer_head map_bh;
> > > > sector_t start_blk, last_blk;
> > > > - loff_t isize = i_size_read(inode);
> > > > + loff_t isize;
> > > > u64 logical = 0, phys = 0, size = 0;
> > > > u32 flags = 0;
> > > > int ret = 0;
> > > > @@ -795,6 +795,8 @@ int f2fs_fiemap(struct inode *inode, struct
> > > > fiemap_extent_info *fieinfo,
> > > > return ret;
> > > > }
> > > >
> > > > + isize = i_size_read(inode);
> > >
> > > It seems that we need to get isize after grabbing i_mutex below in order
> > > to avoid data race.
> >
> > See if I got this right, isize should remain unchanged during the
> > entire procedure, so we need to add i_mutex upon it, and since there
> > is already a i_mutex, we can get inode->i_size directly instead of calling
> > i_read_size.
> > Is that right?
>
> Right, but I prefer to use i_size_read consistently.
> TOH, I'm not convincing why you're treating this as a so costly operation.
Yes, the difference between i_size_read and assignment is very small. I just
think it's best
to squeeze every bit of performance out of codes.
If the consistence is a more pressing issue, then i_size_read it is.
>
> Thanks,
>
> >
> > >
> > > Thanks,
> > >
> > > > +
> > > > mutex_lock(>i_mutex);
> > > > if (start >= isize)
> > > > goto out;
> > > > --
> > > > 1.7.9.5
> > > >
--
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH 1/3] f2fs: avoid calling i_size_read when it's unnecessary
> -Original Message-
> From: Jaegeuk Kim [mailto:jaeg...@kernel.org]
> Sent: Thursday, December 31, 2015 2:28 AM
> To: Fan Li
> Cc: linux-f2fs-devel@lists.sourceforge.net
> Subject: Re: [f2fs-dev] [PATCH 1/3] f2fs: avoid calling i_size_read when it's
> unnecessary
>
> Hi Fan,
>
> On Wed, Dec 30, 2015 at 04:49:56PM +0800, Fan Li wrote:
> > i_size_read does more than reading a value, it's best that we use it
> > only when we need it.
> >
> > Signed-off-by: Fan li <fanofcode...@samsung.com>
> > ---
> > fs/f2fs/data.c |4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c index d67c599..a9a4d89
> > 100644
> > --- a/fs/f2fs/data.c
> > +++ b/fs/f2fs/data.c
> > @@ -780,7 +780,7 @@ int f2fs_fiemap(struct inode *inode, struct
> > fiemap_extent_info *fieinfo, {
> > struct buffer_head map_bh;
> > sector_t start_blk, last_blk;
> > - loff_t isize = i_size_read(inode);
> > + loff_t isize;
> > u64 logical = 0, phys = 0, size = 0;
> > u32 flags = 0;
> > int ret = 0;
> > @@ -795,6 +795,8 @@ int f2fs_fiemap(struct inode *inode, struct
> > fiemap_extent_info *fieinfo,
> > return ret;
> > }
> >
> > + isize = i_size_read(inode);
>
> It seems that we need to get isize after grabbing i_mutex below in order to
> avoid data race.
See if I got this right, isize should remain unchanged during the entire
procedure, so we need
to add i_mutex upon it, and since there is already a i_mutex, we can get
inode->i_size
directly instead of calling i_read_size.
Is that right?
>
> Thanks,
>
> > +
> > mutex_lock(>i_mutex);
> > if (start >= isize)
> > goto out;
> > --
> > 1.7.9.5
> >
--
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH 2/3] f2fs: support finding extents after isize
> -Original Message- > From: Chao Yu [mailto:c...@kernel.org] > Sent: Wednesday, December 30, 2015 9:28 PM > To: Fan Li; 'Jaegeuk Kim' > Cc: linux-f2fs-devel@lists.sourceforge.net > Subject: Re: [f2fs-dev] [PATCH 2/3] f2fs: support finding extents after isize > > Hi, > > On 12/30/15 5:17 PM, Fan Li wrote: > > f2fs allows preallocation beyond isize, but f2fs_fiemap only look up > > extents within isize. Therefore add this support. > > > > Note: It's possible that there are holes after isize, for example, > > fallocate multiple discontinuous extents after isize with > > FALLOC_FL_KEEP_SIZE set. Since I can tell no differences between EOF > > and holes from return of get_data_block, I'm afaid this patch can't > > support such scenarios. > > As you mentioned, preallocated block beyond isize can be allocated in f2fs, > and we are trying to support mapping extents across > whole data space of inode, so why we treat theses extents inside i_size and > outside i_size separately? IMO, instead using i_size, we > should use max blocks as boundary. > > Most important, this interface still can't support finding all extents after > i_size, which looks buggy for our user. Notice that this issue exists before my patch, by adding this patch, at least now it can support more scenarios such as fallocate a range right after isize. I'd say it's an improvement. use max blocks as boundary would get us the precise result, but it also means after we reach the EOF, we still need to look up every block between the EOF and sb-> s_maxbytes to make sure the EOF is true, that's about 4TB or 10^9 blocks. And it affects all scenarios where the search range covers the last extent in the file, not just extents beyond isize. I think this price is too high to pay. I was hoping that I can make f2fs_map_block return an EOF to solve this problem some time later, or anyone have a better idea? > > Thanks, > > > > > > > Signed-off-by: Fan li <fanofcode...@samsung.com> > > --- > > fs/f2fs/data.c |7 +-- > > 1 file changed, 1 insertion(+), 6 deletions(-) > > > > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c index a9a4d89..f89cf07 > > 100644 > > --- a/fs/f2fs/data.c > > +++ b/fs/f2fs/data.c > > @@ -798,12 +798,6 @@ int f2fs_fiemap(struct inode *inode, struct > > fiemap_extent_info *fieinfo, > > isize = i_size_read(inode); > > > > mutex_lock(>i_mutex); > > - if (start >= isize) > > - goto out; > > - > > - if (start + len > isize) > > - len = isize - start; > > - > > if (logical_to_blk(inode, len) == 0) > > len = blk_to_logical(inode, 1); > > > > @@ -829,6 +823,7 @@ next: > > * punch holes beyond isize and keep size unchanged. > > */ > > flags |= FIEMAP_EXTENT_LAST; > > + last_blk = start_blk - 1; > > } > > > > if (size) > > -- ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH 1/3] f2fs: avoid calling i_size_read when it's unnecessary
i_size_read does more than reading a value, it's best that we use it
only when we need it.
Signed-off-by: Fan li <fanofcode...@samsung.com>
---
fs/f2fs/data.c |4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index d67c599..a9a4d89 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -780,7 +780,7 @@ int f2fs_fiemap(struct inode *inode, struct
fiemap_extent_info *fieinfo,
{
struct buffer_head map_bh;
sector_t start_blk, last_blk;
- loff_t isize = i_size_read(inode);
+ loff_t isize;
u64 logical = 0, phys = 0, size = 0;
u32 flags = 0;
int ret = 0;
@@ -795,6 +795,8 @@ int f2fs_fiemap(struct inode *inode, struct
fiemap_extent_info *fieinfo,
return ret;
}
+ isize = i_size_read(inode);
+
mutex_lock(>i_mutex);
if (start >= isize)
goto out;
--
1.7.9.5
--
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH] f2fs: fix bugs and simplify codes of f2fs_fiemap
fix bugs:
1. len could be updated incorrectly when start+len is beyond isize.
2. If there is a hole consisting of more than two blocks, it could
fail to add FIEMAP_EXTENT_LAST flag for the last extent.
3. If there is an extent beyond isize, when we search extents in a range
that ends at isize, it will also return the extent beyond isize,
which is outside the range.
Signed-off-by: Fan li <fanofcode...@samsung.com>
---
fs/f2fs/data.c | 80 +++-
1 file changed, 27 insertions(+), 53 deletions(-)
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 5c43b2d..d67c599 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -783,7 +783,6 @@ int f2fs_fiemap(struct inode *inode, struct
fiemap_extent_info *fieinfo,
loff_t isize = i_size_read(inode);
u64 logical = 0, phys = 0, size = 0;
u32 flags = 0;
- bool past_eof = false, whole_file = false;
int ret = 0;
ret = fiemap_check_flags(fieinfo, FIEMAP_FLAG_SYNC);
@@ -797,17 +796,18 @@ int f2fs_fiemap(struct inode *inode, struct
fiemap_extent_info *fieinfo,
}
mutex_lock(>i_mutex);
+ if (start >= isize)
+ goto out;
- if (len >= isize) {
- whole_file = true;
- len = isize;
- }
+ if (start + len > isize)
+ len = isize - start;
if (logical_to_blk(inode, len) == 0)
len = blk_to_logical(inode, 1);
start_blk = logical_to_blk(inode, start);
last_blk = logical_to_blk(inode, start + len - 1);
+
next:
memset(_bh, 0, sizeof(struct buffer_head));
map_bh.b_size = len;
@@ -819,59 +819,33 @@ next:
/* HOLE */
if (!buffer_mapped(_bh)) {
- start_blk++;
-
- if (!past_eof && blk_to_logical(inode, start_blk) >= isize)
- past_eof = 1;
-
- if (past_eof && size) {
- flags |= FIEMAP_EXTENT_LAST;
- ret = fiemap_fill_next_extent(fieinfo, logical,
- phys, size, flags);
- } else if (size) {
- ret = fiemap_fill_next_extent(fieinfo, logical,
- phys, size, flags);
- size = 0;
- }
+ /* Go through holes util pass the EOF */
+ if (blk_to_logical(inode, start_blk++) < isize)
+ goto prep_next;
+ /* Found a hole beyond isize means no more extents.
+* Note that the premise is that filesystems don't
+* punch holes beyond isize and keep size unchanged.
+*/
+ flags |= FIEMAP_EXTENT_LAST;
+ }
- /* if we have holes up to/past EOF then we're done */
- if (start_blk > last_blk || past_eof || ret)
- goto out;
- } else {
- if (start_blk > last_blk && !whole_file) {
- ret = fiemap_fill_next_extent(fieinfo, logical,
- phys, size, flags);
- goto out;
- }
+ if (size)
+ ret = fiemap_fill_next_extent(fieinfo, logical,
+ phys, size, flags);
- /*
-* if size != 0 then we know we already have an extent
-* to add, so add it.
-*/
- if (size) {
- ret = fiemap_fill_next_extent(fieinfo, logical,
- phys, size, flags);
- if (ret)
- goto out;
- }
+ if (start_blk > last_blk || ret)
+ goto out;
- logical = blk_to_logical(inode, start_blk);
- phys = blk_to_logical(inode, map_bh.b_blocknr);
- size = map_bh.b_size;
- flags = 0;
- if (buffer_unwritten(_bh))
- flags = FIEMAP_EXTENT_UNWRITTEN;
+ logical = blk_to_logical(inode, start_blk);
+ phys = blk_to_logical(inode, map_bh.b_blocknr);
+ size = map_bh.b_size;
+ flags = 0;
+ if (buffer_unwritten(_bh))
+ flags = FIEMAP_EXTENT_UNWRITTEN;
- start_blk += logical_to_blk(inode, size);
+ start_blk += logical_to_blk(inode, size);
- /*
-* If we are past the EOF, then we need to make sure as
-* soon as we find a hole that the last extent we found
-* is marked with FIEMAP_EXTENT_LAST
-*/
- if (!past_eof && logical + size >= isize)
- past_eof = true;
- }
+prep_next:
cond_resched();
if (fata
[f2fs-dev] [PATCH] f2fs: optimize the flow of f2fs_map_blocks
check map->m_len right after it changes to avoid excess call
to update dnode_of_data.
Signed-off-by: Fan li <fanofcode...@samsung.com>
---
fs/f2fs/data.c | 69 +---
1 file changed, 36 insertions(+), 33 deletions(-)
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 90a2ffe..8fa4560 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -573,6 +573,7 @@ int f2fs_map_blocks(struct inode *inode, struct
f2fs_map_blocks *map,
int err = 0, ofs = 1;
struct extent_info ei;
bool allocated = false;
+ block_t blkaddr;
map->m_len = 0;
map->m_flags = 0;
@@ -636,6 +637,9 @@ int f2fs_map_blocks(struct inode *inode, struct
f2fs_map_blocks *map,
pgofs++;
get_next:
+ if (map->m_len >= maxblocks)
+ goto sync_out;
+
if (dn.ofs_in_node >= end_offset) {
if (allocated)
sync_inode_page();
@@ -653,44 +657,43 @@ get_next:
end_offset = ADDRS_PER_PAGE(dn.node_page, F2FS_I(inode));
}
- if (maxblocks > map->m_len) {
- block_t blkaddr = datablock_addr(dn.node_page, dn.ofs_in_node);
+ blkaddr = datablock_addr(dn.node_page, dn.ofs_in_node);
- if (blkaddr == NEW_ADDR || blkaddr == NULL_ADDR) {
- if (create) {
- if (unlikely(f2fs_cp_error(sbi))) {
- err = -EIO;
- goto sync_out;
- }
- err = __allocate_data_block();
- if (err)
- goto sync_out;
- allocated = true;
- map->m_flags |= F2FS_MAP_NEW;
- blkaddr = dn.data_blkaddr;
- } else {
- /*
-* we only merge preallocated unwritten blocks
-* for fiemap.
-*/
- if (flag != F2FS_GET_BLOCK_FIEMAP ||
- blkaddr != NEW_ADDR)
- goto sync_out;
+ if (blkaddr == NEW_ADDR || blkaddr == NULL_ADDR) {
+ if (create) {
+ if (unlikely(f2fs_cp_error(sbi))) {
+ err = -EIO;
+ goto sync_out;
}
+ err = __allocate_data_block();
+ if (err)
+ goto sync_out;
+ allocated = true;
+ map->m_flags |= F2FS_MAP_NEW;
+ blkaddr = dn.data_blkaddr;
+ } else {
+ /*
+* we only merge preallocated unwritten blocks
+* for fiemap.
+*/
+ if (flag != F2FS_GET_BLOCK_FIEMAP ||
+ blkaddr != NEW_ADDR)
+ goto sync_out;
}
+ }
- /* Give more consecutive addresses for the readahead */
- if ((map->m_pblk != NEW_ADDR &&
- blkaddr == (map->m_pblk + ofs)) ||
- (map->m_pblk == NEW_ADDR &&
- blkaddr == NEW_ADDR)) {
- ofs++;
- dn.ofs_in_node++;
- pgofs++;
- map->m_len++;
- goto get_next;
- }
+ /* Give more consecutive addresses for the readahead */
+ if ((map->m_pblk != NEW_ADDR &&
+ blkaddr == (map->m_pblk + ofs)) ||
+ (map->m_pblk == NEW_ADDR &&
+ blkaddr == NEW_ADDR)) {
+ ofs++;
+ dn.ofs_in_node++;
+ pgofs++;
+ map->m_len++;
+ goto get_next;
}
+
sync_out:
if (allocated)
sync_inode_page();
--
1.7.9.5
--
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH] f2fs: write only the pages in range during defragment
@lend of filemap_write_and_wait_range is supposed to be a "offset in bytes where the range ends (inclusive)". Subtract 1 to avoid writing an extra page. Signed-off-by: Fan li <fanofcode...@samsung.com> --- fs/f2fs/file.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c index 294e715..5fac4f2 100644 --- a/fs/f2fs/file.c +++ b/fs/f2fs/file.c @@ -1686,7 +1686,7 @@ static int f2fs_defragment_range(struct f2fs_sb_info *sbi, /* writeback all dirty pages in the range */ err = filemap_write_and_wait_range(inode->i_mapping, range->start, - range->start + range->len); + range->start + range->len - 1); if (err) goto out; -- 1.7.9.5 -- ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH] f2fs: fix to update variable correctly when skip a unmapped block
map.m_len should be reduced after skip a block
Signed-off-by: Fan li <fanofcode...@samsung.com>
---
fs/f2fs/file.c |1 +
1 file changed, 1 insertion(+)
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 5fac4f2..9949d0f 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -1759,6 +1759,7 @@ do_map:
if (!(map.m_flags & F2FS_MAP_FLAGS)) {
map.m_lblk++;
+ map.m_len--;
continue;
}
--
1.7.9.5
--
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH] f2fs: optimize __find_rev_next_bit
1. Skip __reverse_ulong if the bitmap is empty.
2. Reduce branches and codes.
According to my test, the performance of this new version is 5% higher on
an empty bitmap of 64bytes, and remains about the same in the worst scenario.
Signed-off-by: Fan li <fanofcode...@samsung.com>
---
fs/f2fs/segment.c | 46 ++
1 file changed, 18 insertions(+), 28 deletions(-)
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index f77b325..efbf6b5 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -86,6 +86,7 @@ static inline unsigned long __reverse_ffs(unsigned long word)
/*
* __find_rev_next(_zero)_bit is copied from lib/find_next_bit.c because
* f2fs_set_bit makes MSB and LSB reversed in a byte.
+ * @size must be integral times of unsigned long.
* Example:
* MSB <--> LSB
* f2fs_set_bit(0, bitmap) => 1000
@@ -95,47 +96,36 @@ static unsigned long __find_rev_next_bit(const unsigned
long *addr,
unsigned long size, unsigned long offset)
{
const unsigned long *p = addr + BIT_WORD(offset);
- unsigned long result = offset & ~(BITS_PER_LONG - 1);
+ unsigned long result = size;
unsigned long tmp;
if (offset >= size)
return size;
- size -= result;
+ size -= (offset & ~(BITS_PER_LONG - 1));
offset %= BITS_PER_LONG;
- if (!offset)
- goto aligned;
- tmp = __reverse_ulong((unsigned char *)p);
- tmp &= ~0UL >> offset;
-
- if (size < BITS_PER_LONG)
- goto found_first;
- if (tmp)
- goto found_middle;
+ while (1) {
+ if (*p == 0)
+ goto pass;
- size -= BITS_PER_LONG;
- result += BITS_PER_LONG;
- p++;
-aligned:
- while (size & ~(BITS_PER_LONG-1)) {
tmp = __reverse_ulong((unsigned char *)p);
+
+ tmp &= ~0UL >> offset;
+ if (size < BITS_PER_LONG)
+ tmp &= (~0UL << (BITS_PER_LONG - size));
if (tmp)
- goto found_middle;
- result += BITS_PER_LONG;
+ goto found;
+pass:
+ if (size <= BITS_PER_LONG)
+ break;
size -= BITS_PER_LONG;
+ offset = 0;
p++;
}
- if (!size)
- return result;
-
- tmp = __reverse_ulong((unsigned char *)p);
-found_first:
- tmp &= (~0UL << (BITS_PER_LONG - size));
- if (!tmp) /* Are any bits set? */
- return result + size; /* Nope. */
-found_middle:
- return result + __reverse_ffs(tmp);
+ return result;
+found:
+ return result - size + __reverse_ffs(tmp);
}
static unsigned long __find_rev_next_zero_bit(const unsigned long *addr,
--
1.7.9.5
--
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] f2fs: refactor __find_rev_next_{zero}_bit
In bitmaps of f2fs, bytes are sorted in ascending order, but bits in a byte are
sorted
in descending order. It seems to be the reason why __reverse_ulong is needed.
May I ask why f2fs bitmap apply such order?
> -Original Message-
> From: Jaegeuk Kim [mailto:jaeg...@kernel.org]
> Sent: Friday, October 23, 2015 12:36 AM
> To: Chao Yu
> Cc: linux-fsde...@vger.kernel.org; linux-ker...@vger.kernel.org;
> linux-f2fs-devel@lists.sourceforge.net
> Subject: Re: [f2fs-dev] [PATCH] f2fs: refactor __find_rev_next_{zero}_bit
>
> Hi Chao,
>
> On Thu, Oct 22, 2015 at 07:24:01PM +0800, Chao Yu wrote:
> > Hi Jaegeuk,
> >
> > > -Original Message-
> > > From: Jaegeuk Kim [mailto:jaeg...@kernel.org]
> > > Sent: Thursday, October 22, 2015 6:30 AM
> > > To: linux-ker...@vger.kernel.org; linux-fsde...@vger.kernel.org;
> > > linux-f2fs-devel@lists.sourceforge.net
> > > Subject: Re: [f2fs-dev] [PATCH] f2fs: refactor
> > > __find_rev_next_{zero}_bit
> > >
> > > On Wed, Oct 21, 2015 at 02:16:43PM -0700, Jaegeuk Kim wrote:
> > > > This patch refactors __find_rev_next_{zero}_bit which was disabled
> > > > previously due to bugs.
> >
> > Actually I didn't know the history here, could you please explain
> > details about the bugs? then I can do the review.
>
> When I disabled the existing function by:
>
> commit e19ef527aa32f057710ec842fe656bffc263b0bb
> Author: Jaegeuk Kim
> Date: Mon May 18 11:45:15 2015 -0700
>
> f2fs: avoid buggy functions
>
> I found a mismatch of results between the existing __find_rev_next_bit and
> f2fs_test_bit work. I remember it happened on
> __find_rev_next_bit sometimes by xfstests which include trim_fs.
> (I didn't get an error from __find_rev_zero_bit though.)
>
> Since I have no time to investigate the bug in more detail at that time, I
> just disabled it. But, now I'm trying to refactor the
flow to fix
> this.
>
> Thanks,
>
> >
> > Thanks,
> >
> > > >
> > > > Signed-off-by: Jaegeuk Kim
> > > > ---
> > > > fs/f2fs/segment.c | 106
> > > > +-
> > > > 1 file changed, 49 insertions(+), 57 deletions(-)
> > > >
> > > > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c index
> > > > f37c212..50afc93 100644
> > > > --- a/fs/f2fs/segment.c
> > > > +++ b/fs/f2fs/segment.c
> > > > @@ -29,6 +29,21 @@ static struct kmem_cache *discard_entry_slab;
> > > > static struct kmem_cache *sit_entry_set_slab; static struct
> > > > kmem_cache *inmem_entry_slab;
> > > >
> > > > +static unsigned long __reverse_ulong(unsigned char *str) {
> > > > + unsigned long tmp = 0;
> > > > + int shift = 24, idx = 0;
> > > > +
> > > > +#if BITS_PER_LONG == 64
> > > > + shift = 56;
> > > > +#endif
> > > > + while (shift >= 0) {
> > > > + tmp |= (unsigned long)str[idx++] << shift;
> > > > + shift -= BITS_PER_BYTE;
> > > > + }
> > > > + return tmp;
> > > > +}
> > > > +
> > > > /*
> > > > * __reverse_ffs is copied from include/asm-generic/bitops/__ffs.h
> > > > since
> > > > * MSB and LSB are reversed in a byte by f2fs_set_bit.
> > > > @@ -38,27 +53,31 @@ static inline unsigned long __reverse_ffs(unsigned
> > > > long word)
> > > > int num = 0;
> > > >
> > > > #if BITS_PER_LONG == 64
> > > > - if ((word & 0x) == 0) {
> > > > + if ((word & 0x) == 0)
> > >
> > > I fixed the sparse warning by modifying:
> > > if ((word & 0xUL) == 0)
> > >
> > > Thanks,
> > >
> > > > num += 32;
> > > > + else
> > > > word >>= 32;
> > > > - }
> > > > #endif
> > > > - if ((word & 0x) == 0) {
> > > > + if ((word & 0x) == 0)
> > > > num += 16;
> > > > + else
> > > > word >>= 16;
> > > > - }
> > > > - if ((word & 0xff) == 0) {
> > > > +
> > > > + if ((word & 0xff00) == 0)
> > > > num += 8;
> > > > + else
> > > > word >>= 8;
> > > > - }
> > > > +
> > > > if ((word & 0xf0) == 0)
> > > > num += 4;
> > > > else
> > > > word >>= 4;
> > > > +
> > > > if ((word & 0xc) == 0)
> > > > num += 2;
> > > > else
> > > > word >>= 2;
> > > > +
> > > > if ((word & 0x2) == 0)
> > > > num += 1;
> > > > return num;
> > > > @@ -68,26 +87,16 @@ static inline unsigned long __reverse_ffs(unsigned
> > > > long word)
> > > > * __find_rev_next(_zero)_bit is copied from lib/find_next_bit.c
> > > > because
> > > > * f2fs_set_bit makes MSB and LSB reversed in a byte.
> > > > * Example:
> > > > - * LSB <--> MSB
> > > > - * f2fs_set_bit(0, bitmap) => 0001
> > > > - * f2fs_set_bit(7, bitmap) => 1000
> > > > + * MSB <--> LSB
> > > > + * f2fs_set_bit(0, bitmap) => 1000
[f2fs-dev] [PATCH 1/2] f2fs: drop largest extent by range
now we update extent by range, fofs may not be on the largest
extent if the new extent overlaps with it. so add a new function
to drop largest extent properly.
Signed-off-by: Fan li <fanofcode...@samsung.com>
---
fs/f2fs/extent_cache.c |9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
index 63068b7..2f013e2 100644
--- a/fs/f2fs/extent_cache.c
+++ b/fs/f2fs/extent_cache.c
@@ -155,11 +155,12 @@ static unsigned int __free_extent_tree(struct
f2fs_sb_info *sbi,
return count - et->count;
}
-static void __drop_largest_extent(struct inode *inode, pgoff_t fofs)
+static void __drop_largest_extent(struct inode *inode,
+ pgoff_t fofs, unsigned int len)
{
struct extent_info *largest = _I(inode)->extent_tree->largest;
- if (largest->fofs <= fofs && largest->fofs + largest->len > fofs)
+ if (fofs < largest->fofs + largest->len && fofs + len > largest->fofs)
largest->len = 0;
}
@@ -168,7 +169,7 @@ void f2fs_drop_largest_extent(struct inode *inode, pgoff_t
fofs)
if (!f2fs_may_extent_tree(inode))
return;
- __drop_largest_extent(inode, fofs);
+ __drop_largest_extent(inode, fofs, 1);
}
void f2fs_init_extent_tree(struct inode *inode, struct f2fs_extent *i_ext)
@@ -422,7 +423,7 @@ static unsigned int f2fs_update_extent_tree_range(struct
inode *inode,
dei.len = 0;
/* we do not guarantee that the largest extent is cached all the time */
- __drop_largest_extent(inode, fofs);
+ __drop_largest_extent(inode, fofs, len);
/* 1. lookup first extent node in range [fofs, fofs + len - 1] */
en = __lookup_extent_tree_ret(et, fofs, _en, _en,
--
1.7.9.5
--
Monitor Your Dynamic Infrastructure at Any Scale With Datadog!
Get real-time metrics from all of your servers, apps and tools
in one place.
SourceForge users - Click here to start your Free Trial of Datadog now!
http://pubads.g.doubleclick.net/gampad/clk?id=241902991=/4140
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH v2] f2fs: optimize code of f2fs_update_extent_tree_range
> -Original Message-
> From: Jaegeuk Kim [mailto:jaeg...@kernel.org]
> Sent: Wednesday, September 16, 2015 2:37 AM
> To: Fan Li
> Cc: linux-f2fs-devel@lists.sourceforge.net
> Subject: Re: [f2fs-dev] [PATCH v2] f2fs: optimize code of
> f2fs_update_extent_tree_range
>
> Hi Fan,
>
> On Thu, Sep 10, 2015 at 04:48:22PM +0800, Fan Li wrote:
> > Fix 3 potential problems:
> > 1. when largest extent needs to be invalidated, it will be reset in
> >__drop_largest_extent, which makes __is_extent_same after always
> >return false, and largest extent unchanged. Now we update it properly.
> >
> > 2. when extent is split and the latter part remains in tree, next_en
> >should be the latter part instead of next extent of original extent.
> >It will cause merge failure if there is in-place update, although
> >there is not, I think this fix will still makes codes less ambiguous.
> >
> > 3. now we update extent in range, fofs may not be on the largest
> > extent if the new extent overlaps with it. so add a new function
> > to drop largest extent properly.
> >
> > This patch also simpfies codes of invalidating extents, and optimizes
> > the procedues that split extent into two.
> >
> >
> > Signed-off-by: Fan li <fanofcode...@samsung.com>
> > ---
> > fs/f2fs/extent_cache.c | 161
> > +---
> > 1 file changed, 70 insertions(+), 91 deletions(-)
> >
> > diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c index
> > 997ac86..cbd1108 100644
> > --- a/fs/f2fs/extent_cache.c
> > +++ b/fs/f2fs/extent_cache.c
> > @@ -163,6 +163,15 @@ static void __drop_largest_extent(struct inode *inode,
> > pgoff_t fofs)
> > largest->len = 0;
> > }
> >
> > +static void __drop_largest_extent_range(struct inode *inode,
> > + pgoff_t fofs, unsigned int len)
> > {
> > + struct extent_info *largest = _I(inode)->extent_tree->largest;
> > +
> > + if (fofs < largest->fofs + largest->len && fofs + len > largest->fofs)
> > + largest->len = 0;
> > +}
> > +
> > void f2fs_drop_largest_extent(struct inode *inode, pgoff_t fofs) {
> > if (!f2fs_may_extent_tree(inode))
> > @@ -399,7 +408,7 @@ unsigned int f2fs_update_extent_tree_range(struct
> > inode *inode, {
> > struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
> > struct extent_tree *et = F2FS_I(inode)->extent_tree;
> > - struct extent_node *en = NULL, *en1 = NULL, *en2 = NULL, *en3 = NULL;
> > + struct extent_node *en = NULL, *en1 = NULL;
> > struct extent_node *prev_en = NULL, *next_en = NULL;
> > struct extent_info ei, dei, prev;
> > struct rb_node **insert_p = NULL, *insert_parent = NULL; @@ -419,8
> > +428,11 @@ unsigned int f2fs_update_extent_tree_range(struct inode *inode,
> > prev = et->largest;
> > dei.len = 0;
> >
> > - /* we do not guarantee that the largest extent is cached all the time */
> > - __drop_largest_extent(inode, fofs);
> > + /*
> > +* drop largest extent before lookup, in case it's already
> > +* been shrunk from extent tree
> > +*/
> > + __drop_largest_extent_range(inode, fofs, len);
>
> Could you write for the above fix as a sepearte patch?
Sure, I will write one.
>
> >
> > /* 1. lookup first extent node in range [fofs, fofs + len - 1] */
> > en = __lookup_extent_tree_ret(et, fofs, _en, _en, @@
> > -441,114 +453,81 @@ unsigned int f2fs_update_extent_tree_range(struct
> > inode *inode,
> >
> > /* 2. invlidate all extent nodes in range [fofs, fofs + len - 1] */
> > while (en) {
> > - struct rb_node *node;
> > + struct rb_node *node = NULL;
> > + int parts = 0; /* # of parts current extent split into */
> > + unsigned int org_end;
> >
> > if (pos >= end)
> > break;
> >
> > dei = en->ei;
> > - en1 = en2 = NULL;
> > + next_en = en1 = NULL;
> >
> > - node = rb_next(>rb_node);
> > + f2fs_bug_on(sbi, pos < dei.fofs || pos >= dei.fofs + dei.len);
> >
> > - /*
> > -* 2.1 there are four cases when we invalidate blkaddr in extent
> > -* node, |V: valid address, X: will be invalidated|
> > -*/
> > - /* case#1, invalidate
[f2fs-dev] [PATCH] f2fs: optimize code of f2fs_update_extent_tree_range
Fix two potential problems:
1. when largest extent needs to be invalidated, it will be reset in
__drop_largest_extent, which makes __is_extent_same after always
return false, and largest extent unchanged. Now we update it properly.
2. When extent is split and the latter part remains in tree, next_en
should be the latter part instead of next extent of original extent.
It will cause merge failure if there is in-place update, although
there is not, I think this fix will still makes codes less ambiguous.
This patch also simpfies codes of invalidating extents, and optimizes the
procedues that split extent into two.
Signed-off-by: Fan li <fanofcode...@samsung.com>
---
fs/f2fs/extent_cache.c | 150
+++-
1 file changed, 58 insertions(+), 92 deletions(-)
diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
index 997ac86..b95cfb3 100644
--- a/fs/f2fs/extent_cache.c
+++ b/fs/f2fs/extent_cache.c
@@ -399,7 +399,7 @@ unsigned int f2fs_update_extent_tree_range(struct inode
*inode,
{
struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
struct extent_tree *et = F2FS_I(inode)->extent_tree;
- struct extent_node *en = NULL, *en1 = NULL, *en2 = NULL, *en3 =
NULL;
+ struct extent_node *en = NULL, *en1 = NULL;
struct extent_node *prev_en = NULL, *next_en = NULL;
struct extent_info ei, dei, prev;
struct rb_node **insert_p = NULL, *insert_parent = NULL;
@@ -419,9 +419,6 @@ unsigned int f2fs_update_extent_tree_range(struct inode
*inode,
prev = et->largest;
dei.len = 0;
- /* we do not guarantee that the largest extent is cached all the
time */
- __drop_largest_extent(inode, fofs);
-
/* 1. lookup first extent node in range [fofs, fofs + len - 1] */
en = __lookup_extent_tree_ret(et, fofs, _en, _en,
_p, _parent);
@@ -441,114 +438,83 @@ unsigned int f2fs_update_extent_tree_range(struct
inode *inode,
/* 2. invlidate all extent nodes in range [fofs, fofs + len - 1] */
while (en) {
- struct rb_node *node;
+ struct rb_node *node = NULL;
+ int parts = 0; /* # of parts current extent split into */
+ unsigned int org_end;
if (pos >= end)
break;
dei = en->ei;
- en1 = en2 = NULL;
+ next_en = en1 = NULL;
- node = rb_next(>rb_node);
+ f2fs_bug_on(sbi, pos < dei.fofs || pos >= dei.fofs +
dei.len);
- /*
-* 2.1 there are four cases when we invalidate blkaddr in
extent
-* node, |V: valid address, X: will be invalidated|
-*/
- /* case#1, invalidate right part of extent node |VX|
*/
- if (pos > dei.fofs && end >= dei.fofs + dei.len) {
+ __drop_largest_extent(inode, pos);
+
+ if (pos > dei.fofs && pos - dei.fofs >= F2FS_MIN_EXTENT_LEN)
{
en->ei.len = pos - dei.fofs;
+ parts = 1;
+ }
- if (en->ei.len < F2FS_MIN_EXTENT_LEN) {
- __detach_extent_node(sbi, et, en);
- insert_p = NULL;
- insert_parent = NULL;
- goto update;
+ org_end = dei.fofs + dei.len;
+ if (end < org_end &&
+ org_end - end >= F2FS_MIN_EXTENT_LEN) {
+ if (parts) {
+ set_extent_info(, end,
+ end - dei.fofs + dei.blk,
+ org_end - end);
+ en1 = __insert_extent_tree(sbi, et, ,
+ NULL, NULL);
+ next_en = en1;
+ } else {
+ en->ei.fofs = end;
+ en->ei.blk += end - dei.fofs;
+ en->ei.len -= end - dei.fofs;
+ next_en = en;
}
-
- if (__is_extent_same(, >largest))
- et->largest = en->ei;
- goto next;
+ parts++;
}
- /* case#2, invalidate left part of extent node |XV|
*/
- if (pos <= dei.fofs && end < dei.fofs + dei.len) {
- en->ei.fofs = end;
- en->ei.blk += end - dei.fofs;
- en->ei.len -= end - dei.fofs;
-
- if (en->ei.len < F2FS_MIN_EXT
[f2fs-dev] [PATCH] f2fs: add new interfaces for extent tree
Add a lookup and a insertion interface for extent tree.
The new lookup return the insert position and the prev/next
extents closest to the offset we lookup when find no match.
The new insertion uses above parameters to improve performance.
There are three possible insertions after the lookup in
f2fs_update_extent_tree, two of them insert parts of removed extent
back to tree, since no merge happens during this process, new insertion
skips the merge check in this scanario; the another insertion inserts a
new extent to tree, new insertion uses prev/next extent and insert
position to insert this extent directly, and save the time of searching
down the tree.
As long as tree remains unchanged between lookup and insertion, this
would work fine. And the new lookup would be useful when add
multi-blocks extent support for insertion interface.
Signed-off-by: Fan li fanofcode...@samsung.com
---
fs/f2fs/extent_cache.c | 140
+---
1 file changed, 133 insertions(+), 7 deletions(-)
diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
index 5f78fc1..753adf4 100644
--- a/fs/f2fs/extent_cache.c
+++ b/fs/f2fs/extent_cache.c
@@ -302,6 +302,127 @@ out:
return ret;
}
+
+/*
+ * lookup extent at @fofs, if hit, return the extent
+ * if not, return NULL and
+ * @prev_ex: extent before fofs
+ * @next_ex: extent after fofs
+ * @insert_p: insert point for new extent at fofs
+ * in order to simpfy the insertion after.
+ * tree must stay unchanged between lookup and insertion.
+ */
+static struct extent_node *__lookup_extent_tree_ret(struct extent_tree *et,
+ unsigned int fofs, struct extent_node
**prev_ex,
+ struct extent_node **next_ex,
+ struct rb_node ***insert_p,
+ struct rb_node **insert_parent)
+{
+ struct rb_node **pnode = et-root.rb_node;
+ struct rb_node *parent = NULL, *tmp_node;
+ struct extent_node *en;
+
+ if (et-cached_en) {
+ struct extent_info *cei = et-cached_en-ei;
+
+ if (cei-fofs = fofs cei-fofs + cei-len fofs)
+ return et-cached_en;
+ }
+
+ while (*pnode) {
+ parent = *pnode;
+ en = rb_entry(*pnode, struct extent_node, rb_node);
+
+ if (fofs en-ei.fofs)
+ pnode = (*pnode)-rb_left;
+ else if (fofs = en-ei.fofs + en-ei.len)
+ pnode = (*pnode)-rb_right;
+ else
+ return en;
+ }
+
+ *insert_p = pnode;
+ *insert_parent = parent;
+
+ en = rb_entry(parent, struct extent_node, rb_node);
+ tmp_node = parent;
+ if (parent fofs en-ei.fofs)
+ tmp_node = rb_next(parent);
+ *next_ex = tmp_node ?
+ rb_entry(tmp_node, struct extent_node, rb_node) : NULL;
+
+ tmp_node = parent;
+ if (parent fofs en-ei.fofs)
+ tmp_node = rb_prev(parent);
+ *prev_ex = tmp_node ?
+ rb_entry(tmp_node, struct extent_node, rb_node) : NULL;
+
+ return NULL;
+}
+
+static struct extent_node *__insert_extent_tree_ret(struct f2fs_sb_info
*sbi,
+ struct extent_tree *et, struct extent_info
*ei,
+ struct extent_node **den,
+ struct extent_node *prev_ex,
+ struct extent_node *next_ex,
+ struct rb_node **insert_p,
+ struct rb_node *insert_parent)
+{
+ struct rb_node **p = et-root.rb_node;
+ struct rb_node *parent = NULL;
+ struct extent_node *en = NULL;
+ int merged = 0;
+
+ if (prev_ex __is_back_mergeable(ei, prev_ex-ei)) {
+ f2fs_bug_on(sbi, !den);
+ merged = 1;
+ prev_ex-ei.len += ei-len;
+ ei = prev_ex-ei;
+ en = prev_ex;
+ }
+ if (next_ex __is_front_mergeable(ei, next_ex-ei)) {
+ f2fs_bug_on(sbi, !den);
+ if (merged++) {
+ __detach_extent_node(sbi, et, prev_ex);
+ *den = prev_ex;
+ }
+ next_ex-ei.fofs = ei-fofs;
+ next_ex-ei.blk = ei-blk;
+ next_ex-ei.len += ei-len;
+ en = next_ex;
+ }
+ if (merged)
+ goto update_out;
+
+ if (insert_p insert_parent) {
+ parent = insert_parent;
+ p = insert_p;
+ goto do_insert;
+ }
+
+ while (*p) {
+ parent = *p;
+ en = rb_entry(parent, struct extent_node, rb_node);
+
+ if (ei-fofs en-ei.fofs)
+ p = (*p)-rb_left;
+ else if (ei-fofs = en-ei.fofs + en-ei.len)
+ p = (*p)-rb_right
Re: [f2fs-dev] [PATCH] f2fs: change the method of calculating the number summary blocks
Hi,
2013-10-28 (월), 08:54 +, 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...@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..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
--
Android is increasing in popularity, but the open development platform that
developers love is also attractive to malware creators. Download this white
paper to learn more about secure code signing practices that can help keep
Android apps secure.
http://pubads.g.doubleclick.net/gampad/clk?id=65839951iu=/4140/ostg.clktrk
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH] f2fs: change the method of calculating the number summary blocks
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.
Signed-off-by: Fan Li fanofcode...@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)/
+ 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
--
October Webinars: Code for Performance
Free Intel webinars can help you accelerate application performance.
Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most from
the latest Intel processors and coprocessors. See abstracts and register
http://pubads.g.doubleclick.net/gampad/clk?id=60135991iu=/4140/ostg.clktrk
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
