Hi Kim, > -----Original Message----- > From: Jaegeuk Kim [mailto:jaegeuk.kim@xxxxxxxxxxx] > Sent: Saturday, November 30, 2013 9:48 AM > Cc: linux-fsdevel@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux-f2fs-devel@xxxxxxxxxxxxxxxxxxxxx > Subject: [f2fs-dev] [PATCH] f2fs: remove the own bi_private allocation > > Previously f2fs allocates its own bi_private data structure all the time even > though we don't use it. But, can we remove this bi_private allocation? > > This patch removes such the additional bi_private allocation. > > 1. Retrieve f2fs_sb_info from its page->mapping->host->i_sb. > - This removes the usecases of bi_private in end_io. > > 2. Use bi_private only when we really need it. > - The bi_private is used only when the checkpoint procedure is conducted. > - When conducting the checkpoint, f2fs submits a META_FLUSH bio to wait its bio > completion. > - Since we have no dependancies to remove bi_private now, let's just use > bi_private pointer as the completion pointer. > > Signed-off-by: Jaegeuk Kim <jaegeuk.kim@xxxxxxxxxxx> > --- > fs/f2fs/segment.c | 43 ++++++++++++++++--------------------------- > fs/f2fs/segment.h | 7 ------- > 2 files changed, 16 insertions(+), 34 deletions(-) > > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c > index 0387863..0db4027 100644 > --- a/fs/f2fs/segment.c > +++ b/fs/f2fs/segment.c > @@ -791,7 +791,7 @@ static void f2fs_end_io_write(struct bio *bio, int err) > { > const int uptodate = test_bit(BIO_UPTODATE, &bio->bi_flags); > struct bio_vec *bvec = bio->bi_io_vec + bio->bi_vcnt - 1; > - struct bio_private *p = bio->bi_private; > + struct f2fs_sb_info *sbi = F2FS_SB(bvec->bv_page->mapping->host->i_sb); I'm not sure whether bvec->bv_page->mapping will be set to NULL in the flow where may not check WRITEBACK flag of page. Is it possible? > > do { > struct page *page = bvec->bv_page; > @@ -802,21 +802,21 @@ static void f2fs_end_io_write(struct bio *bio, int err) > SetPageError(page); > if (page->mapping) > set_bit(AS_EIO, &page->mapping->flags); > - set_ckpt_flags(p->sbi->ckpt, CP_ERROR_FLAG); > - p->sbi->sb->s_flags |= MS_RDONLY; > + > + set_ckpt_flags(sbi->ckpt, CP_ERROR_FLAG); > + sbi->sb->s_flags |= MS_RDONLY; > } > end_page_writeback(page); > - dec_page_count(p->sbi, F2FS_WRITEBACK); > + dec_page_count(sbi, F2FS_WRITEBACK); > } while (bvec >= bio->bi_io_vec); > > - if (p->is_sync) > - complete(p->wait); > + if (bio->bi_private) > + complete(bio->bi_private); > > - if (!get_pages(p->sbi, F2FS_WRITEBACK) && > - !list_empty(&p->sbi->cp_wait.task_list)) > - wake_up(&p->sbi->cp_wait); > + if (!get_pages(sbi, F2FS_WRITEBACK) && > + !list_empty(&sbi->cp_wait.task_list)) > + wake_up(&sbi->cp_wait); > > - kfree(p); > bio_put(bio); > } > > @@ -838,7 +838,6 @@ static void do_submit_bio(struct f2fs_sb_info *sbi, > int rw = sync ? WRITE_SYNC : WRITE; > enum page_type btype = PAGE_TYPE_OF_BIO(type); > struct f2fs_bio_info *io = &sbi->write_io[btype]; > - struct bio_private *p; > > if (!io->bio) > return; > @@ -851,18 +850,16 @@ static void do_submit_bio(struct f2fs_sb_info *sbi, > > trace_f2fs_submit_write_bio(sbi->sb, rw, btype, io->bio); > > - p = io->bio->bi_private; > - p->sbi = sbi; > - io->bio->bi_end_io = f2fs_end_io_write; > - > + /* > + * META_FLUSH is only from the checkpoint procedure, and we should wait > + * this metadata bio for FS consistency. > + */ > if (type == META_FLUSH) { > DECLARE_COMPLETION_ONSTACK(wait); > - p->is_sync = true; > - p->wait = &wait; > + io->bio->bi_private = &wait; > submit_bio(rw, io->bio); > wait_for_completion(&wait); > } else { > - p->is_sync = false; > submit_bio(rw, io->bio); > } > io->bio = NULL; > @@ -897,18 +894,10 @@ static void submit_write_page(struct f2fs_sb_info *sbi, struct page *page, > do_submit_bio(sbi, type, false); > alloc_new: > if (io->bio == NULL) { > - struct bio_private *priv; > -retry: > - priv = kmalloc(sizeof(struct bio_private), GFP_NOFS); > - if (!priv) { > - cond_resched(); > - goto retry; > - } > - > bio_blocks = MAX_BIO_BLOCKS(max_hw_blocks(sbi)); > io->bio = f2fs_bio_alloc(bdev, bio_blocks); > io->bio->bi_sector = SECTOR_FROM_BLOCK(sbi, blk_addr); > - io->bio->bi_private = priv; > + io->bio->bi_end_io = f2fs_end_io_write; > /* > * The end_io will be assigned at the sumbission phase. > * Until then, let bio_add_page() merge consecutive IOs as much > diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h > index 7fea2ee..26812fc 100644 > --- a/fs/f2fs/segment.h > +++ b/fs/f2fs/segment.h > @@ -92,13 +92,6 @@ > #define MAX_BIO_BLOCKS(max_hw_blocks) \ > (min((int)max_hw_blocks, BIO_MAX_PAGES)) > > -/* during checkpoint, bio_private is used to synchronize the last bio */ > -struct bio_private { > - struct f2fs_sb_info *sbi; > - bool is_sync; > - struct completion *wait; > -}; > - > /* > * indicate a block allocation direction: RIGHT and LEFT. > * RIGHT means allocating new sections towards the end of volume. > -- > 1.8.4.474.g128a96c > > > ------------------------------------------------------------------------------ > Rapidly troubleshoot problems before they affect your business. Most IT > organizations don't have a clear picture of how application performance > affects their revenue. With AppDynamics, you get 100% visibility into your > Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro! > http://pubads.g.doubleclick.net/gampad/clk?id=84349351&iu=/4140/ostg.clktrk > _______________________________________________ > Linux-f2fs-devel mailing list > Linux-f2fs-devel@xxxxxxxxxxxxxxxxxxxxx > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
