On 11/21/2013 01:46 PM, Chao Yu wrote:
> Hi Gu,
>
>> -----Original Message-----
>> From: Gu Zheng [mailto:guz.f...@cn.fujitsu.com]
>> Sent: Wednesday, November 20, 2013 5:38 PM
>> To: Chao Yu
>> Cc: '???'; linux-fsde...@vger.kernel.org; linux-kernel@vger.kernel.org;
>> linux-f2fs-de...@lists.sourceforge.net; '谭姝'
>> Subject: Re: [f2fs-dev] [PATCH V2 2/2] f2fs: read contiguous sit entry pages
>> by merging for mount performance
>>
>> Hi Yu,
>> On 11/20/2013 01:37 PM, Chao Yu wrote:
>>
>>> Hi Gu,
>>>
>>>> -----Original Message-----
>>>> From: Gu Zheng [mailto:guz.f...@cn.fujitsu.com]
>>>> Sent: Monday, November 18, 2013 7:16 PM
>>>> To: Chao Yu
>>>> Cc: '???'; linux-fsde...@vger.kernel.org; linux-kernel@vger.kernel.org;
>>>> linux-f2fs-de...@lists.sourceforge.net; 谭
> 姝
>>>> Subject: Re: [f2fs-dev] [PATCH V2 2/2] f2fs: read contiguous sit entry
>>>> pages by merging for mount performance
>>>>
>>>> Hi Yu,
>>>> One more comment, please refer to inline.
>>>> On 11/16/2013 02:15 PM, Chao Yu wrote:
>>>>
>>>>> Previously we read sit entries page one by one, this method lost the
>>>>> chance of reading contiguous page together.
>>>>> So we read pages as contiguous as possible for better mount performance.
>>>>>
>>>>> v1-->v2:
>>>>> o merge judgements/use 'Continue' or 'Break' instead of 'Goto' as Gu
>>>>> Zheng suggested.
>>>>> o add mark_page_accessed () before release page to delay VM reclaiming
>>>>> them.
>>>>>
>>>>> Signed-off-by: Chao Yu <chao2...@samsung.com>
>>>>> ---
>>>>> fs/f2fs/segment.c | 108
>>>>> ++++++++++++++++++++++++++++++++++++++++-------------
>>>>> fs/f2fs/segment.h | 2 +
>>>>> 2 files changed, 84 insertions(+), 26 deletions(-)
>>>>>
>>>>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>>>>> index fa284d3..656fe40 100644
>>>>> --- a/fs/f2fs/segment.c
>>>>> +++ b/fs/f2fs/segment.c
>>>>> @@ -14,6 +14,7 @@
>>>>> #include <linux/blkdev.h>
>>>>> #include <linux/prefetch.h>
>>>>> #include <linux/vmalloc.h>
>>>>> +#include <linux/swap.h>
>>>>>
>>>>> #include "f2fs.h"
>>>>> #include "segment.h"
>>>>> @@ -1480,41 +1481,96 @@ static int build_curseg(struct f2fs_sb_info *sbi)
>>>>> return restore_curseg_summaries(sbi);
>>>>> }
>>>>>
>>>>> +static int ra_sit_pages(struct f2fs_sb_info *sbi, int start,
>>>>> + int nrpages, bool *is_order)
>>>>> +{
>>>>> + struct address_space *mapping = sbi->meta_inode->i_mapping;
>>>>> + struct sit_info *sit_i = SIT_I(sbi);
>>>>> + struct page *page;
>>>>> + block_t blk_addr;
>>>>> + int blkno = start, readcnt = 0;
>>>>> + int sit_blk_cnt = SIT_BLK_CNT(sbi);
>>>>> +
>>>>> + for (; blkno < start + nrpages && blkno < sit_blk_cnt; blkno++) {
>>>>> +
>>>>> + if ((!f2fs_test_bit(blkno, sit_i->sit_bitmap) ^ !*is_order)) {
>>>>> + *is_order = !*is_order;
>>>>> + break;
>>>>> + }
>>>>> +
>>>>> + blk_addr = sit_i->sit_base_addr + blkno;
>>>>> + if (*is_order)
>>>>> + blk_addr += sit_i->sit_blocks;
>>>>> +repeat:
>>>>> + page = grab_cache_page(mapping, blk_addr);
>>>>> + if (!page) {
>>>>> + cond_resched();
>>>>> + goto repeat;
>>>>> + }
>>>>> + if (PageUptodate(page)) {
>>>>> + mark_page_accessed(page);
>>>>> + f2fs_put_page(page, 1);
>>>>> + readcnt++;
>>>>> + continue;
>>>>> + }
>>>>> +
>>>>> + submit_read_page(sbi, page, blk_addr, READ_SYNC);
>>>>> +
>>>>> + mark_page_accessed(page);
>>>>> + f2fs_put_page(page, 0);
>>>>> + readcnt++;
>>>>> + }
>>>>> +
>>>>> + f2fs_submit_read_bio(sbi, READ_SYNC);
>>>>> + return readcnt;
>>>>> +}
>>>>> +
>>>>> static void build_sit_entries(struct f2fs_sb_info *sbi)
>>>>> {
>>>>> struct sit_info *sit_i = SIT_I(sbi);
>>>>> struct curseg_info *curseg = CURSEG_I(sbi, CURSEG_COLD_DATA);
>>>>> struct f2fs_summary_block *sum = curseg->sum_blk;
>>>>> - unsigned int start;
>>>>> -
>>>>> - for (start = 0; start < TOTAL_SEGS(sbi); start++) {
>>>>> - struct seg_entry *se = &sit_i->sentries[start];
>>>>> - struct f2fs_sit_block *sit_blk;
>>>>> - struct f2fs_sit_entry sit;
>>>>> - struct page *page;
>>>>> - int i;
>>>>> + bool is_order = f2fs_test_bit(0, sit_i->sit_bitmap) ? true : false;
>>>>> + int sit_blk_cnt = SIT_BLK_CNT(sbi);
>>>>> + unsigned int i, start, end;
>>>>> + unsigned int readed, start_blk = 0;
>>>>>
>>>>> - mutex_lock(&curseg->curseg_mutex);
>>>>> - for (i = 0; i < sits_in_cursum(sum); i++) {
>>>>> - if (le32_to_cpu(segno_in_journal(sum, i)) == start) {
>>>>> - sit = sit_in_journal(sum, i);
>>>>> - mutex_unlock(&curseg->curseg_mutex);
>>>>> - goto got_it;
>>>>> + do {
>>>>
>>>> How about using find_next_bit to get the suitable start_blk if the next blk
>>>> is not ordered here? And it also can simplify the logic of ra_sit_pages().
>>>
>>> That's a good idea.
>>> But I thought there maybe endianness problem between test_bit and
>>> f2fs_test_bit, so find_next_bit may get wrong result. Am I right?
>>
>> IMO, find_next_bit can do well with endianness issue internally, if
>> it's not so, that may be a weakness.
>
> Right, I mean that if we want to set first position in bitmap,
> we cloud use f2fs_set_bit(0, bitmap) or set_bit(7, bitmap).
> Two types of interface could not be used mixedly.
>
> So could we use {set,test,clear}_bit intead of f2fs_{set,test,clear}_bit?
We can convert f2fs_{set,test,clear}_bit to a wrapper of {set,test,clear}_bit.
Thanks,
Gu
>
>> On the other side, why not introduce a 'f2fs_find_next_bit' if it's
>> seriously needed?:)
>
> Agreed, that's a good point, they were really needed for
> uniform style and readability.
>
>>
>> Regards,
>> Gu
>>
>>>
>>> Thanks,
>>> Yu
>>>>
>>>> Thanks,
>>>> Gu
>>>>
>>>>> + readed = ra_sit_pages(sbi, start_blk, sit_blk_cnt, &is_order);
>>>>> +
>>>>> + start = start_blk * sit_i->sents_per_block;
>>>>> + end = (start_blk + readed) * sit_i->sents_per_block;
>>>>> +
>>>>> + for (; start < end && start < TOTAL_SEGS(sbi); start++) {
>>>>> + struct seg_entry *se = &sit_i->sentries[start];
>>>>> + struct f2fs_sit_block *sit_blk;
>>>>> + struct f2fs_sit_entry sit;
>>>>> + struct page *page;
>>>>> +
>>>>> + mutex_lock(&curseg->curseg_mutex);
>>>>> + for (i = 0; i < sits_in_cursum(sum); i++) {
>>>>> + if (le32_to_cpu(segno_in_journal(sum, i)) ==
>>>>> start) {
>>>>> + sit = sit_in_journal(sum, i);
>>>>> + mutex_unlock(&curseg->curseg_mutex);
>>>>> + goto got_it;
>>>>> + }
>>>>> }
>>>>> - }
>>>>> - mutex_unlock(&curseg->curseg_mutex);
>>>>> - page = get_current_sit_page(sbi, start);
>>>>> - sit_blk = (struct f2fs_sit_block *)page_address(page);
>>>>> - sit = sit_blk->entries[SIT_ENTRY_OFFSET(sit_i, start)];
>>>>> - f2fs_put_page(page, 1);
>>>>> + mutex_unlock(&curseg->curseg_mutex);
>>>>> +
>>>>> + page = get_current_sit_page(sbi, start);
>>>>> + sit_blk = (struct f2fs_sit_block *)page_address(page);
>>>>> + sit = sit_blk->entries[SIT_ENTRY_OFFSET(sit_i, start)];
>>>>> + f2fs_put_page(page, 1);
>>>>> got_it:
>>>>> - check_block_count(sbi, start, &sit);
>>>>> - seg_info_from_raw_sit(se, &sit);
>>>>> - if (sbi->segs_per_sec > 1) {
>>>>> - struct sec_entry *e = get_sec_entry(sbi, start);
>>>>> - e->valid_blocks += se->valid_blocks;
>>>>> + check_block_count(sbi, start, &sit);
>>>>> + seg_info_from_raw_sit(se, &sit);
>>>>> + if (sbi->segs_per_sec > 1) {
>>>>> + struct sec_entry *e = get_sec_entry(sbi, start);
>>>>> + e->valid_blocks += se->valid_blocks;
>>>>> + }
>>>>> }
>>>>> - }
>>>>> + start_blk += readed;
>>>>
>>>>
>>>>
>>>>
>>>>> + } while (start_blk < sit_blk_cnt);
>>>>> }
>>>>>
>>>>> static void init_free_segmap(struct f2fs_sb_info *sbi)
>>>>> diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
>>>>> index 269f690..ad5b9f1 100644
>>>>> --- a/fs/f2fs/segment.h
>>>>> +++ b/fs/f2fs/segment.h
>>>>> @@ -83,6 +83,8 @@
>>>>> (segno / SIT_ENTRY_PER_BLOCK)
>>>>> #define START_SEGNO(sit_i, segno) \
>>>>> (SIT_BLOCK_OFFSET(sit_i, segno) * SIT_ENTRY_PER_BLOCK)
>>>>> +#define SIT_BLK_CNT(sbi) \
>>>>> + ((TOTAL_SEGS(sbi) + SIT_ENTRY_PER_BLOCK - 1) / SIT_ENTRY_PER_BLOCK)
>>>>> #define f2fs_bitmap_size(nr) \
>>>>> (BITS_TO_LONGS(nr) * sizeof(unsigned long))
>>>>> #define TOTAL_SEGS(sbi) (SM_I(sbi)->main_segments)
>>>
>>>
>>>
>>
>>
>> =
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
