From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751844AbdLAH2X (ORCPT ); Fri, 1 Dec 2017 02:28:23 -0500 Received: from mail.kernel.org ([198.145.29.99]:46180 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751465AbdLAH2V (ORCPT ); Fri, 1 Dec 2017 02:28:21 -0500 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 173AF2199C Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=jaegeuk@kernel.org Date: Thu, 30 Nov 2017 23:28:20 -0800 From: Jaegeuk Kim To: Chao Yu Cc: Hyunchul Lee , 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 Message-ID: <20171201072820.GB86192@jaegeuk-macbookpro.roam.corp.google.com> References: <1511828607-624-1-git-send-email-hyc.lee@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.8.2 (2017-04-18) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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? > > > > 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); > > } > > } > > > > From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jaegeuk Kim Subject: Re: [PATCH 1/2] f2fs: pass down write hints to block layer for bufferd write Date: Thu, 30 Nov 2017 23:28:20 -0800 Message-ID: <20171201072820.GB86192@jaegeuk-macbookpro.roam.corp.google.com> References: <1511828607-624-1-git-send-email-hyc.lee@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from sfi-mx-2.v28.ch3.sourceforge.com ([172.29.28.192] helo=mx.sourceforge.net) by sfs-ml-3.v29.ch3.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.89) (envelope-from ) id 1eKfkJ-0001li-Q4 for linux-f2fs-devel@lists.sourceforge.net; Fri, 01 Dec 2017 07:28:27 +0000 Received: from mail.kernel.org ([198.145.29.99]) by sfi-mx-2.v28.ch3.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.89) id 1eKfkI-0002Cm-Nb for linux-f2fs-devel@lists.sourceforge.net; Fri, 01 Dec 2017 07:28:27 +0000 Content-Disposition: inline In-Reply-To: List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: linux-f2fs-devel-bounces@lists.sourceforge.net To: Chao Yu Cc: Jens Axboe , linux-kernel@vger.kernel.org, Hyunchul Lee , linux-f2fs-devel@lists.sourceforge.net, kernel-team@lge.com, linux-fsdevel@vger.kernel.org, Hyunchul Lee 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? > > > > 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); > > } > > } > > > > ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot