f2fs: allocate a bio for discarding when actually issuing it

PATCH In review Jaegeuk Kim 3 participants 5 messages Mar 8, 2017 – Mar 9, 2017

Subsystems (3)

VFS Block layer F2FS file system

lore

Share & tools

lore

Jaegeuk Kim

author maintainer

2017-03-08 02:33 UTC #0 lore raw

PATCH 1/1 f2fs: allocate a bio for discarding when actually issuing it

Let's allocate a bio when issuing discard commands later.

Signed-off-by Jaegeuk Kim <[email protected]>

---
 fs/f2fs/f2fs.h    |   4 +-
 fs/f2fs/segment.c | 113 ++++++++++++++++++++++++++++--------------------------
 2 files changed, 62 insertions(+), 55 deletions(-)

fs/f2fs/f2fs.h +3 -1

index a58c2e43bd2a..870bb4d9bc65 100644
@@ -197,10 +197,12 @@ enum {
 struct discard_cmd {
 	struct list_head list;		/* command list */
 	struct completion wait;		/* compleation */
+	struct block_device *bdev;	/* bdev */
 	block_t lstart;			/* logical start address */
+	block_t start;			/* actual start address in dev */
 	block_t len;			/* length */
-	struct bio *bio;		/* bio */
 	int state;			/* state */
+	int error;			/* bio error */
 };
 
 struct discard_cmd_control {

fs/f2fs/segment.c +59 ~~-54~~

177 lines not on this page · raw message

Signature · 9 lines

2.11.0


------------------------------------------------------------------------------
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

------------------------------------------------------------------------------
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

Jaegeuk Kim

author maintainer

2017-03-08 21:43 UTC #2 lore raw

Quoted 4 lines · Christoph Hellwig

Quoted 1 line · Jaegeuk Kim
Let's allocate a bio when issuing discard commands later.
Does this solve the issue with your queue stalls?

No, the patch just changes bio_alloc timings, but the stall happens when doing submit_bio.

Thanks,

Quoted 220 lines

Quoted 219 lines

Signed-off-by Jaegeuk Kim <[email protected]>

---
 fs/f2fs/f2fs.h    |   4 +-
 fs/f2fs/segment.c | 113 ++++++++++++++++++++++++++++--------------------------
 2 files changed, 62 insertions(+), 55 deletions(-)

fs/f2fs/f2fs.h +3 -1

index a58c2e43bd2a..870bb4d9bc65 100644
@@ -197,10 +197,12 @@ enum {
 struct discard_cmd {
 	struct list_head list;		/* command list */
 	struct completion wait;		/* compleation */
+	struct block_device *bdev;	/* bdev */
 	block_t lstart;			/* logical start address */
+	block_t start;			/* actual start address in dev */
 	block_t len;			/* length */
-	struct bio *bio;		/* bio */
 	int state;			/* state */
+	int error;			/* bio error */
 };
 
 struct discard_cmd_control {

fs/f2fs/segment.c +59 ~~-54~~

index 50c65cc4645a..d8f9e6c895cd 100644
@@ -636,7 +636,8 @@ static void locate_dirty_segment(struct f2fs_sb_info *sbi, unsigned int segno)
 }
 
 static void __add_discard_cmd(struct f2fs_sb_info *sbi,
-			struct bio *bio, block_t lstart, block_t len)
+		struct block_device *bdev, block_t lstart,
+		block_t start, block_t len)
 {
 	struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
 	struct list_head *cmd_list = &(dcc->discard_cmd_list);

@@ -644,9 +645,9 @@ static void __add_discard_cmd(struct f2fs_sb_info *sbi,
 
 	dc = f2fs_kmem_cache_alloc(discard_cmd_slab, GFP_NOFS);
 	INIT_LIST_HEAD(&dc->list);
-	dc->bio = bio;
-	bio->bi_private = dc;
+	dc->bdev = bdev;
 	dc->lstart = lstart;
+	dc->start = start;
 	dc->len = len;
 	dc->state = D_PREP;
 	init_completion(&dc->wait);

@@ -658,22 +659,66 @@ static void __add_discard_cmd(struct f2fs_sb_info *sbi,
 
 static void __remove_discard_cmd(struct f2fs_sb_info *sbi, struct discard_cmd *dc)
 {
-	int err = dc->bio->bi_error;
-
 	if (dc->state == D_DONE)
 		atomic_dec(&(SM_I(sbi)->dcc_info->submit_discard));
 
-	if (err == -EOPNOTSUPP)
-		err = 0;
+	if (dc->error == -EOPNOTSUPP)
+		dc->error = 0;
 
-	if (err)
+	if (dc->error)
 		f2fs_msg(sbi->sb, KERN_INFO,
-				"Issue discard failed, ret: %d", err);
-	bio_put(dc->bio);
+				"Issue discard failed, ret: %d", dc->error);
 	list_del(&dc->list);
 	kmem_cache_free(discard_cmd_slab, dc);
 }
 
+static void f2fs_submit_discard_endio(struct bio *bio)
+{
+	struct discard_cmd *dc = (struct discard_cmd *)bio->bi_private;
+
+	complete(&dc->wait);
+	dc->error = bio->bi_error;
+	dc->state = D_DONE;
+	bio_put(bio);
+}
+
+/* this function is copied from blkdev_issue_discard from block/blk-lib.c */
+static int __submit_discard_cmd(struct discard_cmd *dc)
+{
+	struct bio *bio = NULL;
+	int err;
+
+	err = __blkdev_issue_discard(dc->bdev,
+				SECTOR_FROM_BLOCK(dc->start),
+				SECTOR_FROM_BLOCK(dc->len),
+				GFP_NOFS, 0, &bio);
+	if (!err && bio) {
+		bio->bi_private = dc;
+		bio->bi_end_io = f2fs_submit_discard_endio;
+		bio->bi_opf |= REQ_SYNC;
+		submit_bio(bio);
+	}
+	dc->state = D_SUBMIT;
+	return err;
+}
+
+static int __queue_discard_cmd(struct f2fs_sb_info *sbi,
+		struct block_device *bdev, block_t blkstart, block_t blklen)
+{
+	block_t lblkstart = blkstart;
+
+	trace_f2fs_issue_discard(bdev, blkstart, blklen);
+
+	if (sbi->s_ndevs) {
+		int devi = f2fs_target_device_index(sbi, blkstart);
+
+		blkstart -= FDEV(devi).start_blk;
+	}
+	__add_discard_cmd(sbi, bdev, lblkstart, blkstart, blklen);
+	wake_up(&SM_I(sbi)->dcc_info->discard_wait_queue);
+	return 0;
+}
+
 /* This should be covered by global mutex, &sit_i->sentry_lock */
 void f2fs_wait_discard_bio(struct f2fs_sb_info *sbi, block_t blkaddr)
 {

@@ -690,8 +735,7 @@ void f2fs_wait_discard_bio(struct f2fs_sb_info *sbi, block_t blkaddr)
 
 		if (blkaddr == NULL_ADDR) {
 			if (dc->state == D_PREP) {
-				dc->state = D_SUBMIT;
-				submit_bio(dc->bio);
+				__submit_discard_cmd(dc);
 				atomic_inc(&dcc->submit_discard);
 			}
 			continue;

@@ -716,14 +760,6 @@ void f2fs_wait_discard_bio(struct f2fs_sb_info *sbi, block_t blkaddr)
 	mutex_unlock(&dcc->cmd_lock);
 }
 
-static void f2fs_submit_discard_endio(struct bio *bio)
-{
-	struct discard_cmd *dc = (struct discard_cmd *)bio->bi_private;
-
-	complete(&dc->wait);
-	dc->state = D_DONE;
-}
-
 static int issue_discard_thread(void *data)
 {
 	struct f2fs_sb_info *sbi = data;

@@ -742,8 +778,7 @@ static int issue_discard_thread(void *data)
 	mutex_lock(&dcc->cmd_lock);
 	list_for_each_entry_safe(dc, tmp, cmd_list, list) {
 		if (dc->state == D_PREP) {
-			dc->state = D_SUBMIT;
-			submit_bio(dc->bio);
+			__submit_discard_cmd(dc);
 			atomic_inc(&dcc->submit_discard);
 			if (iter++ > DISCARD_ISSUE_RATE)
 				break;

@@ -763,36 +798,6 @@ static int issue_discard_thread(void *data)
 	goto repeat;
 }
 
-
-/* this function is copied from blkdev_issue_discard from block/blk-lib.c */
-static int __f2fs_issue_discard_async(struct f2fs_sb_info *sbi,
-		struct block_device *bdev, block_t blkstart, block_t blklen)
-{
-	struct bio *bio = NULL;
-	block_t lblkstart = blkstart;
-	int err;
-
-	trace_f2fs_issue_discard(bdev, blkstart, blklen);
-
-	if (sbi->s_ndevs) {
-		int devi = f2fs_target_device_index(sbi, blkstart);
-
-		blkstart -= FDEV(devi).start_blk;
-	}
-	err = __blkdev_issue_discard(bdev,
-				SECTOR_FROM_BLOCK(blkstart),
-				SECTOR_FROM_BLOCK(blklen),
-				GFP_NOFS, 0, &bio);
-	if (!err && bio) {
-		bio->bi_end_io = f2fs_submit_discard_endio;
-		bio->bi_opf |= REQ_SYNC;
-
-		__add_discard_cmd(sbi, bio, lblkstart, blklen);
-		wake_up(&SM_I(sbi)->dcc_info->discard_wait_queue);
-	}
-	return err;
-}
-
 #ifdef CONFIG_BLK_DEV_ZONED
 static int __f2fs_issue_discard_zone(struct f2fs_sb_info *sbi,
 		struct block_device *bdev, block_t blkstart, block_t blklen)

@@ -816,7 +821,7 @@ static int __f2fs_issue_discard_zone(struct f2fs_sb_info *sbi,
 	case BLK_ZONE_TYPE_CONVENTIONAL:
 		if (!blk_queue_discard(bdev_get_queue(bdev)))
 			return 0;
-		return __f2fs_issue_discard_async(sbi, bdev, lblkstart, blklen);
+		return __queue_discard_cmd(sbi, bdev, lblkstart, blklen);
 	case BLK_ZONE_TYPE_SEQWRITE_REQ:
 	case BLK_ZONE_TYPE_SEQWRITE_PREF:
 		sector = SECTOR_FROM_BLOCK(blkstart);

@@ -848,7 +853,7 @@ static int __issue_discard_async(struct f2fs_sb_info *sbi,
 				bdev_zoned_model(bdev) != BLK_ZONED_NONE)
 		return __f2fs_issue_discard_zone(sbi, bdev, blkstart, blklen);
 #endif
-	return __f2fs_issue_discard_async(sbi, bdev, blkstart, blklen);
+	return __queue_discard_cmd(sbi, bdev, blkstart, blklen);
 }
 
 static int f2fs_issue_discard(struct f2fs_sb_info *sbi,

Signature · 1 line

2.11.0

---end quoted text---

Chao Yu

2017-03-09 06:36 UTC #3 lore raw

In reply to Jaegeuk Kim

Hi Jaegeuk,

Quoted 91 lines · Jaegeuk Kim

Let's allocate a bio when issuing discard commands later.

Signed-off-by Jaegeuk Kim <[email protected]>

---
 fs/f2fs/f2fs.h    |   4 +-
 fs/f2fs/segment.c | 113 ++++++++++++++++++++++++++++--------------------------
 2 files changed, 62 insertions(+), 55 deletions(-)

fs/f2fs/f2fs.h +3 -1

index a58c2e43bd2a..870bb4d9bc65 100644
@@ -197,10 +197,12 @@ enum {
 struct discard_cmd {
 	struct list_head list;		/* command list */
 	struct completion wait;		/* compleation */
+	struct block_device *bdev;	/* bdev */
 	block_t lstart;			/* logical start address */
+	block_t start;			/* actual start address in dev */
 	block_t len;			/* length */
-	struct bio *bio;		/* bio */
 	int state;			/* state */
+	int error;			/* bio error */
 };
 
 struct discard_cmd_control {

fs/f2fs/segment.c +23 ~~-10~~

index 50c65cc4645a..d8f9e6c895cd 100644
@@ -636,7 +636,8 @@ static void locate_dirty_segment(struct f2fs_sb_info *sbi, unsigned int segno)
 }
 
 static void __add_discard_cmd(struct f2fs_sb_info *sbi,
-			struct bio *bio, block_t lstart, block_t len)
+		struct block_device *bdev, block_t lstart,
+		block_t start, block_t len)
 {
 	struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
 	struct list_head *cmd_list = &(dcc->discard_cmd_list);

@@ -644,9 +645,9 @@ static void __add_discard_cmd(struct f2fs_sb_info *sbi,
 
 	dc = f2fs_kmem_cache_alloc(discard_cmd_slab, GFP_NOFS);
 	INIT_LIST_HEAD(&dc->list);
-	dc->bio = bio;
-	bio->bi_private = dc;
+	dc->bdev = bdev;
 	dc->lstart = lstart;
+	dc->start = start;
 	dc->len = len;
 	dc->state = D_PREP;
 	init_completion(&dc->wait);

@@ -658,22 +659,66 @@ static void __add_discard_cmd(struct f2fs_sb_info *sbi,
 
 static void __remove_discard_cmd(struct f2fs_sb_info *sbi, struct discard_cmd *dc)
 {
-	int err = dc->bio->bi_error;
-
 	if (dc->state == D_DONE)
 		atomic_dec(&(SM_I(sbi)->dcc_info->submit_discard));
 
-	if (err == -EOPNOTSUPP)
-		err = 0;
+	if (dc->error == -EOPNOTSUPP)
+		dc->error = 0;
 
-	if (err)
+	if (dc->error)
 		f2fs_msg(sbi->sb, KERN_INFO,
-				"Issue discard failed, ret: %d", err);
-	bio_put(dc->bio);
+				"Issue discard failed, ret: %d", dc->error);
 	list_del(&dc->list);
 	kmem_cache_free(discard_cmd_slab, dc);
 }
 
+static void f2fs_submit_discard_endio(struct bio *bio)
+{
+	struct discard_cmd *dc = (struct discard_cmd *)bio->bi_private;
+
+	complete(&dc->wait);
+	dc->error = bio->bi_error;
+	dc->state = D_DONE;
+	bio_put(bio);
+}
+
+/* this function is copied from blkdev_issue_discard from block/blk-lib.c */
+static int __submit_discard_cmd(struct discard_cmd *dc)
+{
+	struct bio *bio = NULL;
+	int err;

fs/f2fs/segment.c static void __add_discard_cmd(struct f2fs_sb_info *sbi, · line 699

Quoted 10 lines

fs/f2fs/segment.c +10 -0

+
+	err = __blkdev_issue_discard(dc->bdev,
+				SECTOR_FROM_BLOCK(dc->start),
+				SECTOR_FROM_BLOCK(dc->len),
+				GFP_NOFS, 0, &bio);
+	if (!err && bio) {
+		bio->bi_private = dc;
+		bio->bi_end_io = f2fs_submit_discard_endio;
+		bio->bi_opf |= REQ_SYNC;
+		submit_bio(bio);

Should set flag only if __blkdev_issue_discard is successful?

dc->state = D_SUBMIT;

And how about moving atomic_inc(&dcc->submit_discard); here?

Thanks,

Quoted 108 lines

fs/f2fs/segment.c +25 ~~-43~~

+	}
+	dc->state = D_SUBMIT;
+	return err;
+}
+
+static int __queue_discard_cmd(struct f2fs_sb_info *sbi,
+		struct block_device *bdev, block_t blkstart, block_t blklen)
+{
+	block_t lblkstart = blkstart;
+
+	trace_f2fs_issue_discard(bdev, blkstart, blklen);
+
+	if (sbi->s_ndevs) {
+		int devi = f2fs_target_device_index(sbi, blkstart);
+
+		blkstart -= FDEV(devi).start_blk;
+	}
+	__add_discard_cmd(sbi, bdev, lblkstart, blkstart, blklen);
+	wake_up(&SM_I(sbi)->dcc_info->discard_wait_queue);
+	return 0;
+}
+
 /* This should be covered by global mutex, &sit_i->sentry_lock */
 void f2fs_wait_discard_bio(struct f2fs_sb_info *sbi, block_t blkaddr)
 {

@@ -690,8 +735,7 @@ void f2fs_wait_discard_bio(struct f2fs_sb_info *sbi, block_t blkaddr)
 
 		if (blkaddr == NULL_ADDR) {
 			if (dc->state == D_PREP) {
-				dc->state = D_SUBMIT;
-				submit_bio(dc->bio);
+				__submit_discard_cmd(dc);
 				atomic_inc(&dcc->submit_discard);
 			}
 			continue;

@@ -716,14 +760,6 @@ void f2fs_wait_discard_bio(struct f2fs_sb_info *sbi, block_t blkaddr)
 	mutex_unlock(&dcc->cmd_lock);
 }
 
-static void f2fs_submit_discard_endio(struct bio *bio)
-{
-	struct discard_cmd *dc = (struct discard_cmd *)bio->bi_private;
-
-	complete(&dc->wait);
-	dc->state = D_DONE;
-}
-
 static int issue_discard_thread(void *data)
 {
 	struct f2fs_sb_info *sbi = data;

@@ -742,8 +778,7 @@ static int issue_discard_thread(void *data)
 	mutex_lock(&dcc->cmd_lock);
 	list_for_each_entry_safe(dc, tmp, cmd_list, list) {
 		if (dc->state == D_PREP) {
-			dc->state = D_SUBMIT;
-			submit_bio(dc->bio);
+			__submit_discard_cmd(dc);
 			atomic_inc(&dcc->submit_discard);
 			if (iter++ > DISCARD_ISSUE_RATE)
 				break;

@@ -763,36 +798,6 @@ static int issue_discard_thread(void *data)
 	goto repeat;
 }
 
-
-/* this function is copied from blkdev_issue_discard from block/blk-lib.c */
-static int __f2fs_issue_discard_async(struct f2fs_sb_info *sbi,
-		struct block_device *bdev, block_t blkstart, block_t blklen)
-{
-	struct bio *bio = NULL;
-	block_t lblkstart = blkstart;
-	int err;
-
-	trace_f2fs_issue_discard(bdev, blkstart, blklen);
-
-	if (sbi->s_ndevs) {
-		int devi = f2fs_target_device_index(sbi, blkstart);
-
-		blkstart -= FDEV(devi).start_blk;
-	}
-	err = __blkdev_issue_discard(bdev,
-				SECTOR_FROM_BLOCK(blkstart),
-				SECTOR_FROM_BLOCK(blklen),
-				GFP_NOFS, 0, &bio);
-	if (!err && bio) {
-		bio->bi_end_io = f2fs_submit_discard_endio;
-		bio->bi_opf |= REQ_SYNC;
-
-		__add_discard_cmd(sbi, bio, lblkstart, blklen);
-		wake_up(&SM_I(sbi)->dcc_info->discard_wait_queue);
-	}
-	return err;
-}
-
 #ifdef CONFIG_BLK_DEV_ZONED
 static int __f2fs_issue_discard_zone(struct f2fs_sb_info *sbi,
 		struct block_device *bdev, block_t blkstart, block_t blklen)

@@ -816,7 +821,7 @@ static int __f2fs_issue_discard_zone(struct f2fs_sb_info *sbi,
 	case BLK_ZONE_TYPE_CONVENTIONAL:
 		if (!blk_queue_discard(bdev_get_queue(bdev)))
 			return 0;
-		return __f2fs_issue_discard_async(sbi, bdev, lblkstart, blklen);
+		return __queue_discard_cmd(sbi, bdev, lblkstart, blklen);
 	case BLK_ZONE_TYPE_SEQWRITE_REQ:
 	case BLK_ZONE_TYPE_SEQWRITE_PREF:
 		sector = SECTOR_FROM_BLOCK(blkstart);

fs/f2fs/segment.c static int __issue_discard_async(struct f2fs_sb_info *sbi, · line 859

Quoted 8 lines

fs/f2fs/segment.c +1 -1

@@ -848,7 +853,7 @@ static int __issue_discard_async(struct f2fs_sb_info *sbi,
 				bdev_zoned_model(bdev) != BLK_ZONED_NONE)
 		return __f2fs_issue_discard_zone(sbi, bdev, blkstart, blklen);
 #endif
-	return __f2fs_issue_discard_async(sbi, bdev, blkstart, blklen);
+	return __queue_discard_cmd(sbi, bdev, blkstart, blklen);
 }
 
 static int f2fs_issue_discard(struct f2fs_sb_info *sbi,

Jaegeuk Kim

author maintainer

2017-03-09 22:49 UTC #4 lore raw

In reply to Jaegeuk Kim

Change log from v1:
o change error handling flows

Quoted 1 line

From 58bb056dfa207a0dc9b9432dcf78758a7784144a Mon Sep 17 00:00:00 2001

From: Jaegeuk Kim <[email protected]>
Date: Tue, 7 Mar 2017 18:02:02 -0800
Subject: [PATCH] f2fs: allocate a bio for discarding when actually issuing it

Let's allocate a bio when issuing discard commands later.

Signed-off-by Jaegeuk Kim <[email protected]>

---
 fs/f2fs/f2fs.h    |   4 +-
 fs/f2fs/segment.c | 136 +++++++++++++++++++++++++++++-------------------------
 2 files changed, 76 insertions(+), 64 deletions(-)

fs/f2fs/f2fs.h +3 -1

index a58c2e43bd2a..870bb4d9bc65 100644
@@ -197,10 +197,12 @@ enum {
 struct discard_cmd {
 	struct list_head list;		/* command list */
 	struct completion wait;		/* compleation */
+	struct block_device *bdev;	/* bdev */
 	block_t lstart;			/* logical start address */
+	block_t start;			/* actual start address in dev */
 	block_t len;			/* length */
-	struct bio *bio;		/* bio */
 	int state;			/* state */
+	int error;			/* bio error */
 };
 
 struct discard_cmd_control {

fs/f2fs/segment.c +73 ~~-63~~

203 lines not on this page · raw message

Signature · 1 line

2.11.0