From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751500AbdLAIuP (ORCPT ); Fri, 1 Dec 2017 03:50:15 -0500 Received: from LGEAMRELO12.lge.com ([156.147.23.52]:33716 "EHLO lgeamrelo12.lge.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750994AbdLAIuN (ORCPT ); Fri, 1 Dec 2017 03:50:13 -0500 X-Original-SENDERIP: 156.147.1.127 X-Original-MAILFROM: hyc.lee@gmail.com X-Original-SENDERIP: 10.177.225.35 X-Original-MAILFROM: hyc.lee@gmail.com Message-ID: <5A2117C3.804@gmail.com> Date: Fri, 01 Dec 2017 17:50:11 +0900 From: Hyunchul Lee User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.3.0 MIME-Version: 1.0 To: Jaegeuk Kim , Chao Yu CC: linux-f2fs-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, kernel-team@lge.com, Jens Axboe , Hyunchul Lee Subject: Re: [PATCH 1/2] f2fs: pass down write hints to block layer for bufferd write References: <1511828607-624-1-git-send-email-hyc.lee@gmail.com> <20171201072820.GB86192@jaegeuk-macbookpro.roam.corp.google.com> In-Reply-To: <20171201072820.GB86192@jaegeuk-macbookpro.roam.corp.google.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Jaegeuk, On 12/01/2017 04:28 PM, Jaegeuk Kim wrote: > On 11/30, Chao Yu wrote: >> On 2017/11/28 8:23, Hyunchul Lee wrote: >>> From: Hyunchul Lee >>> >>> This implements which hint is passed down to block layer >>> for datas from the specific segment type. >>> >>> segment type hints >>> ------------ ----- >>> COLD_NODE & COLD_DATA WRITE_LIFE_EXTREME >>> WARM_DATA WRITE_LIFE_NONE >>> HOT_NODE & WARM_NODE WRITE_LIFE_LONG >>> HOT_DATA WRITE_LIFE_MEDIUM >>> META_DATA WRITE_LIFE_SHORT >> >> The correspondence looks good to me. > > Still, I don't fully get the point. Why do we have to assign WRITE_LIFE_NONE > to WARM_DATA? Why not using WRITE_LIFE_NOT_SET? > I think LIFE_NONE and LIFE_NOT_SET are the same. So I chose LIFE_NONE simply. Thanks. >>> >>> Many thanks to Chao Yu and Jaegeuk Kim for comments to >>> implement this patch. >>> >>> Signed-off-by: Hyunchul Lee >>> --- >>> fs/f2fs/data.c | 1 + >>> fs/f2fs/f2fs.h | 2 ++ >>> fs/f2fs/segment.c | 29 +++++++++++++++++++++++++++++ >>> fs/f2fs/super.c | 2 ++ >>> 4 files changed, 34 insertions(+) >>> >>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c >>> index a7ae418..0919a43 100644 >>> --- a/fs/f2fs/data.c >>> +++ b/fs/f2fs/data.c >>> @@ -437,6 +437,7 @@ int f2fs_submit_page_write(struct f2fs_io_info *fio) >>> } >>> io->bio = __bio_alloc(sbi, fio->new_blkaddr, >>> BIO_MAX_PAGES, false); >>> + io->bio->bi_write_hint = io->write_hint; >> >> Need to assign bi_write_hint for IPU path? >> >> - rewrite_data_page >> - f2fs_submit_page_bio >> >>> io->fio = *fio; >>> } >>> >>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h >>> index 7bcd148..be3cb0c 100644 >>> --- a/fs/f2fs/f2fs.h >>> +++ b/fs/f2fs/f2fs.h >>> @@ -969,6 +969,7 @@ struct f2fs_bio_info { >>> struct rw_semaphore io_rwsem; /* blocking op for bio */ >>> spinlock_t io_lock; /* serialize DATA/NODE IOs */ >>> struct list_head io_list; /* track fios */ >>> + enum rw_hint write_hint; >> >> Add missing comment? >> >>> }; >>> >>> #define FDEV(i) (sbi->devs[i]) >>> @@ -2674,6 +2675,7 @@ int lookup_journal_in_cursum(struct f2fs_journal *journal, int type, >>> int __init create_segment_manager_caches(void); >>> void destroy_segment_manager_caches(void); >>> int rw_hint_to_seg_type(enum rw_hint hint); >>> +enum rw_hint io_type_to_rw_hint(enum page_type page, enum temp_type temp); >>> >>> /* >>> * checkpoint.c >>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c >>> index c117e09..0570db7 100644 >>> --- a/fs/f2fs/segment.c >>> +++ b/fs/f2fs/segment.c >>> @@ -2446,6 +2446,35 @@ int rw_hint_to_seg_type(enum rw_hint hint) >>> } >>> } >>> >> >> It will be better to copy commit log here to declare correspondence >> more clearly. >> >> Thanks, >> >>> +enum rw_hint io_type_to_rw_hint(enum page_type page, enum temp_type temp) >>> +{ >>> + if (page == DATA) { >>> + switch (temp) { >>> + case WARM: >>> + return WRITE_LIFE_NONE; >>> + case COLD: >>> + return WRITE_LIFE_EXTREME; >>> + case HOT: >>> + return WRITE_LIFE_MEDIUM; >>> + default: >>> + return WRITE_LIFE_NOT_SET; >>> + } >>> + } else if (page == NODE) { >>> + switch (temp) { >>> + case WARM: >>> + case HOT: >>> + return WRITE_LIFE_LONG; >>> + case COLD: >>> + return WRITE_LIFE_EXTREME; >>> + default: >>> + return WRITE_LIFE_NOT_SET; >>> + } >>> + >>> + } else { >>> + return WRITE_LIFE_SHORT; >>> + } >>> +} >>> + >>> static int __get_segment_type_2(struct f2fs_io_info *fio) >>> { >>> if (fio->type == DATA) >>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c >>> index a6c5dd4..51c19a0 100644 >>> --- a/fs/f2fs/super.c >>> +++ b/fs/f2fs/super.c >>> @@ -2508,6 +2508,8 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent) >>> sbi->write_io[i][j].bio = NULL; >>> spin_lock_init(&sbi->write_io[i][j].io_lock); >>> INIT_LIST_HEAD(&sbi->write_io[i][j].io_list); >>> + sbi->write_io[i][j].write_hint = >>> + io_type_to_rw_hint(i, j); >>> } >>> } >>> >>> >