Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1586644 > unrolled thread

[PATCH 1/4] f2fs: avoid very large discard command

Started byJaegeuk Kim <jaegeuk@kernel.org>
First post2017-02-23 05:30 +0100
Last post2017-02-27 03:20 +0100
Articles 4 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH 1/4] f2fs: avoid very large discard command Jaegeuk Kim <jaegeuk@kernel.org> - 2017-02-23 05:30 +0100
    Re: [PATCH 1/4] f2fs: avoid very large discard command Christoph Hellwig <hch@infradead.org> - 2017-02-23 11:20 +0100
      Re: [PATCH 1/4] f2fs: avoid very large discard command Jaegeuk Kim <jaegeuk@kernel.org> - 2017-02-23 20:00 +0100
    Re: [f2fs-dev] [PATCH 1/4] f2fs: avoid very large discard command Chao Yu <yuchao0@huawei.com> - 2017-02-27 03:20 +0100

#1586644 — [PATCH 1/4] f2fs: avoid very large discard command

FromJaegeuk Kim <jaegeuk@kernel.org>
Date2017-02-23 05:30 +0100
Subject[PATCH 1/4] f2fs: avoid very large discard command
Message-ID<tdTct-4mi-5@gated-at.bofh.it>
This patch adds MAX_DISCARD_BLOCKS() to avoid issuing too much large single
discard command.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/f2fs.h    | 3 ++-
 fs/f2fs/segment.c | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index d076b94530bc..5f3fe97df055 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -133,7 +133,8 @@ enum {
 		(SM_I(sbi)->trim_sections * (sbi)->segs_per_sec)
 #define BATCHED_TRIM_BLOCKS(sbi)	\
 		(BATCHED_TRIM_SEGMENTS(sbi) << (sbi)->log_blocks_per_seg)
-
+#define MAX_DISCARD_BLOCKS(sbi)						\
+		((1 << (sbi)->log_blocks_per_seg) * (sbi)->segs_per_sec)
 #define DISCARD_ISSUE_RATE	8
 #define DEF_CP_INTERVAL			60	/* 60 secs */
 #define DEF_IDLE_INTERVAL		5	/* 5 secs */
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index fe434cd872b4..567019940e9b 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -886,7 +886,8 @@ static void __add_discard_entry(struct f2fs_sb_info *sbi,
 	if (!list_empty(head)) {
 		last = list_last_entry(head, struct discard_entry, list);
 		if (START_BLOCK(sbi, cpc->trim_start) + start ==
-						last->blkaddr + last->len) {
+				last->blkaddr + last->len &&
+				last->len < MAX_DISCARD_BLOCKS(sbi)) {
 			last->len += end - start;
 			goto done;
 		}
-- 
2.11.0

[toc] | [next] | [standalone]


#1586793

FromChristoph Hellwig <hch@infradead.org>
Date2017-02-23 11:20 +0100
Message-ID<tdYFc-8jU-11@gated-at.bofh.it>
In reply to#1586644
On Wed, Feb 22, 2017 at 08:28:47PM -0800, Jaegeuk Kim wrote:
> This patch adds MAX_DISCARD_BLOCKS() to avoid issuing too much large single
> discard command.

Needs an explanation in the code on why this number was chosen.
In doubt I suspect it should be a quirk in the driver for the device,
and not something decided by the fs.

> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
>  fs/f2fs/f2fs.h    | 3 ++-
>  fs/f2fs/segment.c | 3 ++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index d076b94530bc..5f3fe97df055 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -133,7 +133,8 @@ enum {
>  		(SM_I(sbi)->trim_sections * (sbi)->segs_per_sec)
>  #define BATCHED_TRIM_BLOCKS(sbi)	\
>  		(BATCHED_TRIM_SEGMENTS(sbi) << (sbi)->log_blocks_per_seg)
> -
> +#define MAX_DISCARD_BLOCKS(sbi)						\
> +		((1 << (sbi)->log_blocks_per_seg) * (sbi)->segs_per_sec)
>  #define DISCARD_ISSUE_RATE	8
>  #define DEF_CP_INTERVAL			60	/* 60 secs */
>  #define DEF_IDLE_INTERVAL		5	/* 5 secs */
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index fe434cd872b4..567019940e9b 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -886,7 +886,8 @@ static void __add_discard_entry(struct f2fs_sb_info *sbi,
>  	if (!list_empty(head)) {
>  		last = list_last_entry(head, struct discard_entry, list);
>  		if (START_BLOCK(sbi, cpc->trim_start) + start ==
> -						last->blkaddr + last->len) {
> +				last->blkaddr + last->len &&
> +				last->len < MAX_DISCARD_BLOCKS(sbi)) {
>  			last->len += end - start;
>  			goto done;
>  		}
> -- 
> 2.11.0
> 
---end quoted text---

[toc] | [prev] | [next] | [standalone]


#1587084

FromJaegeuk Kim <jaegeuk@kernel.org>
Date2017-02-23 20:00 +0100
Message-ID<te6Mp-58d-1@gated-at.bofh.it>
In reply to#1586793
On 02/23, Christoph Hellwig wrote:
> On Wed, Feb 22, 2017 at 08:28:47PM -0800, Jaegeuk Kim wrote:
> > This patch adds MAX_DISCARD_BLOCKS() to avoid issuing too much large single
> > discard command.
> 
> Needs an explanation in the code on why this number was chosen.

No need to add a trivial comment, since it's the section size which we can
easily translate it.

> In doubt I suspect it should be a quirk in the driver for the device,
> and not something decided by the fs.

The block allocator checks all the issued bios whether any of them contains
the newly allocated address or not. If one large discard command was issued,
it needs to wait for its completion, even if we want to allocate part of its
address space.

In addition, HM-SMR is required to issue discard/reset in a unit of section
size.

Thanks,

> 
> > 
> > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> > ---
> >  fs/f2fs/f2fs.h    | 3 ++-
> >  fs/f2fs/segment.c | 3 ++-
> >  2 files changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > index d076b94530bc..5f3fe97df055 100644
> > --- a/fs/f2fs/f2fs.h
> > +++ b/fs/f2fs/f2fs.h
> > @@ -133,7 +133,8 @@ enum {
> >  		(SM_I(sbi)->trim_sections * (sbi)->segs_per_sec)
> >  #define BATCHED_TRIM_BLOCKS(sbi)	\
> >  		(BATCHED_TRIM_SEGMENTS(sbi) << (sbi)->log_blocks_per_seg)
> > -
> > +#define MAX_DISCARD_BLOCKS(sbi)						\
> > +		((1 << (sbi)->log_blocks_per_seg) * (sbi)->segs_per_sec)
> >  #define DISCARD_ISSUE_RATE	8
> >  #define DEF_CP_INTERVAL			60	/* 60 secs */
> >  #define DEF_IDLE_INTERVAL		5	/* 5 secs */
> > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> > index fe434cd872b4..567019940e9b 100644
> > --- a/fs/f2fs/segment.c
> > +++ b/fs/f2fs/segment.c
> > @@ -886,7 +886,8 @@ static void __add_discard_entry(struct f2fs_sb_info *sbi,
> >  	if (!list_empty(head)) {
> >  		last = list_last_entry(head, struct discard_entry, list);
> >  		if (START_BLOCK(sbi, cpc->trim_start) + start ==
> > -						last->blkaddr + last->len) {
> > +				last->blkaddr + last->len &&
> > +				last->len < MAX_DISCARD_BLOCKS(sbi)) {
> >  			last->len += end - start;
> >  			goto done;
> >  		}
> > -- 
> > 2.11.0
> > 
> ---end quoted text---

[toc] | [prev] | [next] | [standalone]


#1588485 — Re: [f2fs-dev] [PATCH 1/4] f2fs: avoid very large discard command

FromChao Yu <yuchao0@huawei.com>
Date2017-02-27 03:20 +0100
SubjectRe: [f2fs-dev] [PATCH 1/4] f2fs: avoid very large discard command
Message-ID<tfj4S-79v-5@gated-at.bofh.it>
In reply to#1586644
On 2017/2/23 12:28, Jaegeuk Kim wrote:
> This patch adds MAX_DISCARD_BLOCKS() to avoid issuing too much large single
> discard command.
> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>

This patch set looks good to me, please add reviewed-by into all patches. :)

Reviewed-by: Chao Yu <yuchao0@huawei.com>

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web