Re: [PATCH] f2fs: let fstrim issue discard commands in lower priority

2018-05-25 Thread Chao Yu 
On 2018/5/25 14:18, Jaegeuk Kim wrote:
> On 05/25, Chao Yu wrote:
>> Hi Jaegeuk,
>>
>> On 2018/5/25 13:10, Jaegeuk Kim wrote:
>>> The fstrim gathers huge number of large discard commands, and tries to issue
>>> without IO awareness, which results in long user-perceive IO latencies on
>>> READ, WRITE, and FLUSH in UFS. We've observed some of commands take several
>>> seconds due to long discard latency.
>>>
>>> This patch limits the maximum size to 2MB per candidate, and check IO 
>>> congestion
>>> when issuing them to disk.
>>>
>>> Signed-off-by: Jaegeuk Kim 
>>> ---
>>>  fs/f2fs/f2fs.h|   4 +-
>>>  fs/f2fs/segment.c | 123 +++---
>>>  2 files changed, 64 insertions(+), 63 deletions(-)
>>>
>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>> index 3bddf13794d9..75ae7fc86ae8 100644
>>> --- a/fs/f2fs/f2fs.h
>>> +++ b/fs/f2fs/f2fs.h
>>> @@ -178,6 +178,7 @@ enum {
>>>  
>>>  #define MAX_DISCARD_BLOCKS(sbi)BLKS_PER_SEC(sbi)
>>>  #define DEF_MAX_DISCARD_REQUEST8   /* issue 8 discards per 
>>> round */
>>> +#define DEF_MAX_DISCARD_LEN512 /* Max. 2MB per discard 
>>> */
>>>  #define DEF_MIN_DISCARD_ISSUE_TIME 50  /* 50 ms, if exists */
>>>  #define DEF_MID_DISCARD_ISSUE_TIME 500 /* 500 ms, if device busy */
>>>  #define DEF_MAX_DISCARD_ISSUE_TIME 6   /* 60 s, if no candidates */
>>> @@ -698,7 +699,8 @@ static inline void set_extent_info(struct extent_info 
>>> *ei, unsigned int fofs,
>>>  static inline bool __is_discard_mergeable(struct discard_info *back,
>>> struct discard_info *front)
>>>  {
>>> -   return back->lstart + back->len == front->lstart;
>>> +   return (back->lstart + back->len == front->lstart) &&
>>> +   (back->len + front->len < DEF_MAX_DISCARD_LEN);
>>>  }
>>>  
>>>  static inline bool __is_discard_back_mergeable(struct discard_info *cur,
>>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>>> index 843fc2e6d41c..ba996d4091bc 100644
>>> --- a/fs/f2fs/segment.c
>>> +++ b/fs/f2fs/segment.c
>>> @@ -1139,68 +1139,6 @@ static int __queue_discard_cmd(struct f2fs_sb_info 
>>> *sbi,
>>> return 0;
>>>  }
>>>  
>>> -static void __issue_discard_cmd_range(struct f2fs_sb_info *sbi,
>>> -   struct discard_policy *dpolicy,
>>> -   unsigned int start, unsigned int end)
>>> -{
>>> -   struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
>>> -   struct discard_cmd *prev_dc = NULL, *next_dc = NULL;
>>> -   struct rb_node **insert_p = NULL, *insert_parent = NULL;
>>> -   struct discard_cmd *dc;
>>> -   struct blk_plug plug;
>>> -   int issued;
>>> -
>>> -next:
>>> -   issued = 0;
>>> -
>>> -   mutex_lock(>cmd_lock);
>>> -   f2fs_bug_on(sbi, !__check_rb_tree_consistence(sbi, >root));
>>> -
>>> -   dc = (struct discard_cmd *)__lookup_rb_tree_ret(>root,
>>> -   NULL, start,
>>> -   (struct rb_entry **)_dc,
>>> -   (struct rb_entry **)_dc,
>>> -   _p, _parent, true);
>>> -   if (!dc)
>>> -   dc = next_dc;
>>> -
>>> -   blk_start_plug();
>>> -
>>> -   while (dc && dc->lstart <= end) {
>>> -   struct rb_node *node;
>>> -
>>> -   if (dc->len < dpolicy->granularity)
>>> -   goto skip;
>>> -
>>> -   if (dc->state != D_PREP) {
>>> -   list_move_tail(>list, >fstrim_list);
>>> -   goto skip;
>>> -   }
>>> -
>>> -   __submit_discard_cmd(sbi, dpolicy, dc);
>>> -
>>> -   if (++issued >= dpolicy->max_requests) {
>>> -   start = dc->lstart + dc->len;
>>> -
>>> -   blk_finish_plug();
>>> -   mutex_unlock(>cmd_lock);
>>> -
>>> -   schedule();
>>> -
>>> -   goto next;
>>> -   }
>>> -skip:
>>> -   node = rb_next(>rb_node);
>>> -   dc = rb_entry_safe(node, struct discard_cmd, rb_node);
>>> -
>>> -   if (fatal_signal_pending(current))
>>> -   break;
>>> -   }
>>> -
>>> -   blk_finish_plug();
>>> -   mutex_unlock(>cmd_lock);
>>> -}
>>> -
>>>  static int __issue_discard_cmd(struct f2fs_sb_info *sbi,
>>> struct discard_policy *dpolicy)
>>>  {
>>> @@ -2397,6 +2335,67 @@ bool exist_trim_candidates(struct f2fs_sb_info *sbi, 
>>> struct cp_control *cpc)
>>> return has_candidate;
>>>  }
>>>  
>>> +static void __issue_discard_cmd_range(struct f2fs_sb_info *sbi,
>>> +   struct discard_policy *dpolicy,
>>> +   unsigned int start, unsigned int end)
>>> +{
>>> +   struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
>>> +   struct discard_cmd *prev_dc = NULL, *next_dc = NULL;
>>> +   struct rb_node **insert_p = NULL, *insert_parent = NULL;
>>> +

Re: [PATCH] f2fs: let fstrim issue discard commands in lower priority

2018-05-25 Thread Chao Yu 
On 2018/5/25 14:18, Jaegeuk Kim wrote:
> On 05/25, Chao Yu wrote:
>> Hi Jaegeuk,
>>
>> On 2018/5/25 13:10, Jaegeuk Kim wrote:
>>> The fstrim gathers huge number of large discard commands, and tries to issue
>>> without IO awareness, which results in long user-perceive IO latencies on
>>> READ, WRITE, and FLUSH in UFS. We've observed some of commands take several
>>> seconds due to long discard latency.
>>>
>>> This patch limits the maximum size to 2MB per candidate, and check IO 
>>> congestion
>>> when issuing them to disk.
>>>
>>> Signed-off-by: Jaegeuk Kim 
>>> ---
>>>  fs/f2fs/f2fs.h|   4 +-
>>>  fs/f2fs/segment.c | 123 +++---
>>>  2 files changed, 64 insertions(+), 63 deletions(-)
>>>
>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>> index 3bddf13794d9..75ae7fc86ae8 100644
>>> --- a/fs/f2fs/f2fs.h
>>> +++ b/fs/f2fs/f2fs.h
>>> @@ -178,6 +178,7 @@ enum {
>>>  
>>>  #define MAX_DISCARD_BLOCKS(sbi)BLKS_PER_SEC(sbi)
>>>  #define DEF_MAX_DISCARD_REQUEST8   /* issue 8 discards per 
>>> round */
>>> +#define DEF_MAX_DISCARD_LEN512 /* Max. 2MB per discard 
>>> */
>>>  #define DEF_MIN_DISCARD_ISSUE_TIME 50  /* 50 ms, if exists */
>>>  #define DEF_MID_DISCARD_ISSUE_TIME 500 /* 500 ms, if device busy */
>>>  #define DEF_MAX_DISCARD_ISSUE_TIME 6   /* 60 s, if no candidates */
>>> @@ -698,7 +699,8 @@ static inline void set_extent_info(struct extent_info 
>>> *ei, unsigned int fofs,
>>>  static inline bool __is_discard_mergeable(struct discard_info *back,
>>> struct discard_info *front)
>>>  {
>>> -   return back->lstart + back->len == front->lstart;
>>> +   return (back->lstart + back->len == front->lstart) &&
>>> +   (back->len + front->len < DEF_MAX_DISCARD_LEN);
>>>  }
>>>  
>>>  static inline bool __is_discard_back_mergeable(struct discard_info *cur,
>>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>>> index 843fc2e6d41c..ba996d4091bc 100644
>>> --- a/fs/f2fs/segment.c
>>> +++ b/fs/f2fs/segment.c
>>> @@ -1139,68 +1139,6 @@ static int __queue_discard_cmd(struct f2fs_sb_info 
>>> *sbi,
>>> return 0;
>>>  }
>>>  
>>> -static void __issue_discard_cmd_range(struct f2fs_sb_info *sbi,
>>> -   struct discard_policy *dpolicy,
>>> -   unsigned int start, unsigned int end)
>>> -{
>>> -   struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
>>> -   struct discard_cmd *prev_dc = NULL, *next_dc = NULL;
>>> -   struct rb_node **insert_p = NULL, *insert_parent = NULL;
>>> -   struct discard_cmd *dc;
>>> -   struct blk_plug plug;
>>> -   int issued;
>>> -
>>> -next:
>>> -   issued = 0;
>>> -
>>> -   mutex_lock(>cmd_lock);
>>> -   f2fs_bug_on(sbi, !__check_rb_tree_consistence(sbi, >root));
>>> -
>>> -   dc = (struct discard_cmd *)__lookup_rb_tree_ret(>root,
>>> -   NULL, start,
>>> -   (struct rb_entry **)_dc,
>>> -   (struct rb_entry **)_dc,
>>> -   _p, _parent, true);
>>> -   if (!dc)
>>> -   dc = next_dc;
>>> -
>>> -   blk_start_plug();
>>> -
>>> -   while (dc && dc->lstart <= end) {
>>> -   struct rb_node *node;
>>> -
>>> -   if (dc->len < dpolicy->granularity)
>>> -   goto skip;
>>> -
>>> -   if (dc->state != D_PREP) {
>>> -   list_move_tail(>list, >fstrim_list);
>>> -   goto skip;
>>> -   }
>>> -
>>> -   __submit_discard_cmd(sbi, dpolicy, dc);
>>> -
>>> -   if (++issued >= dpolicy->max_requests) {
>>> -   start = dc->lstart + dc->len;
>>> -
>>> -   blk_finish_plug();
>>> -   mutex_unlock(>cmd_lock);
>>> -
>>> -   schedule();
>>> -
>>> -   goto next;
>>> -   }
>>> -skip:
>>> -   node = rb_next(>rb_node);
>>> -   dc = rb_entry_safe(node, struct discard_cmd, rb_node);
>>> -
>>> -   if (fatal_signal_pending(current))
>>> -   break;
>>> -   }
>>> -
>>> -   blk_finish_plug();
>>> -   mutex_unlock(>cmd_lock);
>>> -}
>>> -
>>>  static int __issue_discard_cmd(struct f2fs_sb_info *sbi,
>>> struct discard_policy *dpolicy)
>>>  {
>>> @@ -2397,6 +2335,67 @@ bool exist_trim_candidates(struct f2fs_sb_info *sbi, 
>>> struct cp_control *cpc)
>>> return has_candidate;
>>>  }
>>>  
>>> +static void __issue_discard_cmd_range(struct f2fs_sb_info *sbi,
>>> +   struct discard_policy *dpolicy,
>>> +   unsigned int start, unsigned int end)
>>> +{
>>> +   struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
>>> +   struct discard_cmd *prev_dc = NULL, *next_dc = NULL;
>>> +   struct rb_node **insert_p = NULL, *insert_parent = NULL;
>>> +   struct discard_cmd

Re: [PATCH] f2fs: let fstrim issue discard commands in lower priority

2018-05-25 Thread Jaegeuk Kim 
On 05/25, Chao Yu wrote:
> Hi Jaegeuk,
> 
> On 2018/5/25 13:10, Jaegeuk Kim wrote:
> > The fstrim gathers huge number of large discard commands, and tries to issue
> > without IO awareness, which results in long user-perceive IO latencies on
> > READ, WRITE, and FLUSH in UFS. We've observed some of commands take several
> > seconds due to long discard latency.
> > 
> > This patch limits the maximum size to 2MB per candidate, and check IO 
> > congestion
> > when issuing them to disk.
> > 
> > Signed-off-by: Jaegeuk Kim 
> > ---
> >  fs/f2fs/f2fs.h|   4 +-
> >  fs/f2fs/segment.c | 123 +++---
> >  2 files changed, 64 insertions(+), 63 deletions(-)
> > 
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > index 3bddf13794d9..75ae7fc86ae8 100644
> > --- a/fs/f2fs/f2fs.h
> > +++ b/fs/f2fs/f2fs.h
> > @@ -178,6 +178,7 @@ enum {
> >  
> >  #define MAX_DISCARD_BLOCKS(sbi)BLKS_PER_SEC(sbi)
> >  #define DEF_MAX_DISCARD_REQUEST8   /* issue 8 discards per 
> > round */
> > +#define DEF_MAX_DISCARD_LEN512 /* Max. 2MB per discard 
> > */
> >  #define DEF_MIN_DISCARD_ISSUE_TIME 50  /* 50 ms, if exists */
> >  #define DEF_MID_DISCARD_ISSUE_TIME 500 /* 500 ms, if device busy */
> >  #define DEF_MAX_DISCARD_ISSUE_TIME 6   /* 60 s, if no candidates */
> > @@ -698,7 +699,8 @@ static inline void set_extent_info(struct extent_info 
> > *ei, unsigned int fofs,
> >  static inline bool __is_discard_mergeable(struct discard_info *back,
> > struct discard_info *front)
> >  {
> > -   return back->lstart + back->len == front->lstart;
> > +   return (back->lstart + back->len == front->lstart) &&
> > +   (back->len + front->len < DEF_MAX_DISCARD_LEN);
> >  }
> >  
> >  static inline bool __is_discard_back_mergeable(struct discard_info *cur,
> > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> > index 843fc2e6d41c..ba996d4091bc 100644
> > --- a/fs/f2fs/segment.c
> > +++ b/fs/f2fs/segment.c
> > @@ -1139,68 +1139,6 @@ static int __queue_discard_cmd(struct f2fs_sb_info 
> > *sbi,
> > return 0;
> >  }
> >  
> > -static void __issue_discard_cmd_range(struct f2fs_sb_info *sbi,
> > -   struct discard_policy *dpolicy,
> > -   unsigned int start, unsigned int end)
> > -{
> > -   struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
> > -   struct discard_cmd *prev_dc = NULL, *next_dc = NULL;
> > -   struct rb_node **insert_p = NULL, *insert_parent = NULL;
> > -   struct discard_cmd *dc;
> > -   struct blk_plug plug;
> > -   int issued;
> > -
> > -next:
> > -   issued = 0;
> > -
> > -   mutex_lock(>cmd_lock);
> > -   f2fs_bug_on(sbi, !__check_rb_tree_consistence(sbi, >root));
> > -
> > -   dc = (struct discard_cmd *)__lookup_rb_tree_ret(>root,
> > -   NULL, start,
> > -   (struct rb_entry **)_dc,
> > -   (struct rb_entry **)_dc,
> > -   _p, _parent, true);
> > -   if (!dc)
> > -   dc = next_dc;
> > -
> > -   blk_start_plug();
> > -
> > -   while (dc && dc->lstart <= end) {
> > -   struct rb_node *node;
> > -
> > -   if (dc->len < dpolicy->granularity)
> > -   goto skip;
> > -
> > -   if (dc->state != D_PREP) {
> > -   list_move_tail(>list, >fstrim_list);
> > -   goto skip;
> > -   }
> > -
> > -   __submit_discard_cmd(sbi, dpolicy, dc);
> > -
> > -   if (++issued >= dpolicy->max_requests) {
> > -   start = dc->lstart + dc->len;
> > -
> > -   blk_finish_plug();
> > -   mutex_unlock(>cmd_lock);
> > -
> > -   schedule();
> > -
> > -   goto next;
> > -   }
> > -skip:
> > -   node = rb_next(>rb_node);
> > -   dc = rb_entry_safe(node, struct discard_cmd, rb_node);
> > -
> > -   if (fatal_signal_pending(current))
> > -   break;
> > -   }
> > -
> > -   blk_finish_plug();
> > -   mutex_unlock(>cmd_lock);
> > -}
> > -
> >  static int __issue_discard_cmd(struct f2fs_sb_info *sbi,
> > struct discard_policy *dpolicy)
> >  {
> > @@ -2397,6 +2335,67 @@ bool exist_trim_candidates(struct f2fs_sb_info *sbi, 
> > struct cp_control *cpc)
> > return has_candidate;
> >  }
> >  
> > +static void __issue_discard_cmd_range(struct f2fs_sb_info *sbi,
> > +   struct discard_policy *dpolicy,
> > +   unsigned int start, unsigned int end)
> > +{
> > +   struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
> > +   struct discard_cmd *prev_dc = NULL, *next_dc = NULL;
> > +   struct rb_node **insert_p = NULL, *insert_parent = NULL;
> > +   struct discard_cmd *dc;
> > +   struct

Re: [PATCH] f2fs: let fstrim issue discard commands in lower priority

2018-05-25 Thread Jaegeuk Kim 
On 05/25, Chao Yu wrote:
> Hi Jaegeuk,
> 
> On 2018/5/25 13:10, Jaegeuk Kim wrote:
> > The fstrim gathers huge number of large discard commands, and tries to issue
> > without IO awareness, which results in long user-perceive IO latencies on
> > READ, WRITE, and FLUSH in UFS. We've observed some of commands take several
> > seconds due to long discard latency.
> > 
> > This patch limits the maximum size to 2MB per candidate, and check IO 
> > congestion
> > when issuing them to disk.
> > 
> > Signed-off-by: Jaegeuk Kim 
> > ---
> >  fs/f2fs/f2fs.h|   4 +-
> >  fs/f2fs/segment.c | 123 +++---
> >  2 files changed, 64 insertions(+), 63 deletions(-)
> > 
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > index 3bddf13794d9..75ae7fc86ae8 100644
> > --- a/fs/f2fs/f2fs.h
> > +++ b/fs/f2fs/f2fs.h
> > @@ -178,6 +178,7 @@ enum {
> >  
> >  #define MAX_DISCARD_BLOCKS(sbi)BLKS_PER_SEC(sbi)
> >  #define DEF_MAX_DISCARD_REQUEST8   /* issue 8 discards per 
> > round */
> > +#define DEF_MAX_DISCARD_LEN512 /* Max. 2MB per discard 
> > */
> >  #define DEF_MIN_DISCARD_ISSUE_TIME 50  /* 50 ms, if exists */
> >  #define DEF_MID_DISCARD_ISSUE_TIME 500 /* 500 ms, if device busy */
> >  #define DEF_MAX_DISCARD_ISSUE_TIME 6   /* 60 s, if no candidates */
> > @@ -698,7 +699,8 @@ static inline void set_extent_info(struct extent_info 
> > *ei, unsigned int fofs,
> >  static inline bool __is_discard_mergeable(struct discard_info *back,
> > struct discard_info *front)
> >  {
> > -   return back->lstart + back->len == front->lstart;
> > +   return (back->lstart + back->len == front->lstart) &&
> > +   (back->len + front->len < DEF_MAX_DISCARD_LEN);
> >  }
> >  
> >  static inline bool __is_discard_back_mergeable(struct discard_info *cur,
> > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> > index 843fc2e6d41c..ba996d4091bc 100644
> > --- a/fs/f2fs/segment.c
> > +++ b/fs/f2fs/segment.c
> > @@ -1139,68 +1139,6 @@ static int __queue_discard_cmd(struct f2fs_sb_info 
> > *sbi,
> > return 0;
> >  }
> >  
> > -static void __issue_discard_cmd_range(struct f2fs_sb_info *sbi,
> > -   struct discard_policy *dpolicy,
> > -   unsigned int start, unsigned int end)
> > -{
> > -   struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
> > -   struct discard_cmd *prev_dc = NULL, *next_dc = NULL;
> > -   struct rb_node **insert_p = NULL, *insert_parent = NULL;
> > -   struct discard_cmd *dc;
> > -   struct blk_plug plug;
> > -   int issued;
> > -
> > -next:
> > -   issued = 0;
> > -
> > -   mutex_lock(>cmd_lock);
> > -   f2fs_bug_on(sbi, !__check_rb_tree_consistence(sbi, >root));
> > -
> > -   dc = (struct discard_cmd *)__lookup_rb_tree_ret(>root,
> > -   NULL, start,
> > -   (struct rb_entry **)_dc,
> > -   (struct rb_entry **)_dc,
> > -   _p, _parent, true);
> > -   if (!dc)
> > -   dc = next_dc;
> > -
> > -   blk_start_plug();
> > -
> > -   while (dc && dc->lstart <= end) {
> > -   struct rb_node *node;
> > -
> > -   if (dc->len < dpolicy->granularity)
> > -   goto skip;
> > -
> > -   if (dc->state != D_PREP) {
> > -   list_move_tail(>list, >fstrim_list);
> > -   goto skip;
> > -   }
> > -
> > -   __submit_discard_cmd(sbi, dpolicy, dc);
> > -
> > -   if (++issued >= dpolicy->max_requests) {
> > -   start = dc->lstart + dc->len;
> > -
> > -   blk_finish_plug();
> > -   mutex_unlock(>cmd_lock);
> > -
> > -   schedule();
> > -
> > -   goto next;
> > -   }
> > -skip:
> > -   node = rb_next(>rb_node);
> > -   dc = rb_entry_safe(node, struct discard_cmd, rb_node);
> > -
> > -   if (fatal_signal_pending(current))
> > -   break;
> > -   }
> > -
> > -   blk_finish_plug();
> > -   mutex_unlock(>cmd_lock);
> > -}
> > -
> >  static int __issue_discard_cmd(struct f2fs_sb_info *sbi,
> > struct discard_policy *dpolicy)
> >  {
> > @@ -2397,6 +2335,67 @@ bool exist_trim_candidates(struct f2fs_sb_info *sbi, 
> > struct cp_control *cpc)
> > return has_candidate;
> >  }
> >  
> > +static void __issue_discard_cmd_range(struct f2fs_sb_info *sbi,
> > +   struct discard_policy *dpolicy,
> > +   unsigned int start, unsigned int end)
> > +{
> > +   struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
> > +   struct discard_cmd *prev_dc = NULL, *next_dc = NULL;
> > +   struct rb_node **insert_p = NULL, *insert_parent = NULL;
> > +   struct discard_cmd *dc;
> > +   struct blk_plug plug;
> > +

Re: [PATCH] f2fs: let fstrim issue discard commands in lower priority

2018-05-25 Thread Chao Yu 
Hi Jaegeuk,

On 2018/5/25 13:10, Jaegeuk Kim wrote:
> The fstrim gathers huge number of large discard commands, and tries to issue
> without IO awareness, which results in long user-perceive IO latencies on
> READ, WRITE, and FLUSH in UFS. We've observed some of commands take several
> seconds due to long discard latency.
> 
> This patch limits the maximum size to 2MB per candidate, and check IO 
> congestion
> when issuing them to disk.
> 
> Signed-off-by: Jaegeuk Kim 
> ---
>  fs/f2fs/f2fs.h|   4 +-
>  fs/f2fs/segment.c | 123 +++---
>  2 files changed, 64 insertions(+), 63 deletions(-)
> 
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 3bddf13794d9..75ae7fc86ae8 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -178,6 +178,7 @@ enum {
>  
>  #define MAX_DISCARD_BLOCKS(sbi)  BLKS_PER_SEC(sbi)
>  #define DEF_MAX_DISCARD_REQUEST  8   /* issue 8 discards per 
> round */
> +#define DEF_MAX_DISCARD_LEN  512 /* Max. 2MB per discard */
>  #define DEF_MIN_DISCARD_ISSUE_TIME   50  /* 50 ms, if exists */
>  #define DEF_MID_DISCARD_ISSUE_TIME   500 /* 500 ms, if device busy */
>  #define DEF_MAX_DISCARD_ISSUE_TIME   6   /* 60 s, if no candidates */
> @@ -698,7 +699,8 @@ static inline void set_extent_info(struct extent_info 
> *ei, unsigned int fofs,
>  static inline bool __is_discard_mergeable(struct discard_info *back,
>   struct discard_info *front)
>  {
> - return back->lstart + back->len == front->lstart;
> + return (back->lstart + back->len == front->lstart) &&
> + (back->len + front->len < DEF_MAX_DISCARD_LEN);
>  }
>  
>  static inline bool __is_discard_back_mergeable(struct discard_info *cur,
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 843fc2e6d41c..ba996d4091bc 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -1139,68 +1139,6 @@ static int __queue_discard_cmd(struct f2fs_sb_info 
> *sbi,
>   return 0;
>  }
>  
> -static void __issue_discard_cmd_range(struct f2fs_sb_info *sbi,
> - struct discard_policy *dpolicy,
> - unsigned int start, unsigned int end)
> -{
> - struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
> - struct discard_cmd *prev_dc = NULL, *next_dc = NULL;
> - struct rb_node **insert_p = NULL, *insert_parent = NULL;
> - struct discard_cmd *dc;
> - struct blk_plug plug;
> - int issued;
> -
> -next:
> - issued = 0;
> -
> - mutex_lock(>cmd_lock);
> - f2fs_bug_on(sbi, !__check_rb_tree_consistence(sbi, >root));
> -
> - dc = (struct discard_cmd *)__lookup_rb_tree_ret(>root,
> - NULL, start,
> - (struct rb_entry **)_dc,
> - (struct rb_entry **)_dc,
> - _p, _parent, true);
> - if (!dc)
> - dc = next_dc;
> -
> - blk_start_plug();
> -
> - while (dc && dc->lstart <= end) {
> - struct rb_node *node;
> -
> - if (dc->len < dpolicy->granularity)
> - goto skip;
> -
> - if (dc->state != D_PREP) {
> - list_move_tail(>list, >fstrim_list);
> - goto skip;
> - }
> -
> - __submit_discard_cmd(sbi, dpolicy, dc);
> -
> - if (++issued >= dpolicy->max_requests) {
> - start = dc->lstart + dc->len;
> -
> - blk_finish_plug();
> - mutex_unlock(>cmd_lock);
> -
> - schedule();
> -
> - goto next;
> - }
> -skip:
> - node = rb_next(>rb_node);
> - dc = rb_entry_safe(node, struct discard_cmd, rb_node);
> -
> - if (fatal_signal_pending(current))
> - break;
> - }
> -
> - blk_finish_plug();
> - mutex_unlock(>cmd_lock);
> -}
> -
>  static int __issue_discard_cmd(struct f2fs_sb_info *sbi,
>   struct discard_policy *dpolicy)
>  {
> @@ -2397,6 +2335,67 @@ bool exist_trim_candidates(struct f2fs_sb_info *sbi, 
> struct cp_control *cpc)
>   return has_candidate;
>  }
>  
> +static void __issue_discard_cmd_range(struct f2fs_sb_info *sbi,
> + struct discard_policy *dpolicy,
> + unsigned int start, unsigned int end)
> +{
> + struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
> + struct discard_cmd *prev_dc = NULL, *next_dc = NULL;
> + struct rb_node **insert_p = NULL, *insert_parent = NULL;
> + struct discard_cmd *dc;
> + struct blk_plug plug;
> + int issued;

unsigned int cur = start;

> +
> +next:
> + issued = 0;
> +
> + mutex_lock(>cmd_lock);
> + f2fs_bug_on(sbi,

Re: [PATCH] f2fs: let fstrim issue discard commands in lower priority

2018-05-25 Thread Chao Yu 
Hi Jaegeuk,

On 2018/5/25 13:10, Jaegeuk Kim wrote:
> The fstrim gathers huge number of large discard commands, and tries to issue
> without IO awareness, which results in long user-perceive IO latencies on
> READ, WRITE, and FLUSH in UFS. We've observed some of commands take several
> seconds due to long discard latency.
> 
> This patch limits the maximum size to 2MB per candidate, and check IO 
> congestion
> when issuing them to disk.
> 
> Signed-off-by: Jaegeuk Kim 
> ---
>  fs/f2fs/f2fs.h|   4 +-
>  fs/f2fs/segment.c | 123 +++---
>  2 files changed, 64 insertions(+), 63 deletions(-)
> 
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 3bddf13794d9..75ae7fc86ae8 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -178,6 +178,7 @@ enum {
>  
>  #define MAX_DISCARD_BLOCKS(sbi)  BLKS_PER_SEC(sbi)
>  #define DEF_MAX_DISCARD_REQUEST  8   /* issue 8 discards per 
> round */
> +#define DEF_MAX_DISCARD_LEN  512 /* Max. 2MB per discard */
>  #define DEF_MIN_DISCARD_ISSUE_TIME   50  /* 50 ms, if exists */
>  #define DEF_MID_DISCARD_ISSUE_TIME   500 /* 500 ms, if device busy */
>  #define DEF_MAX_DISCARD_ISSUE_TIME   6   /* 60 s, if no candidates */
> @@ -698,7 +699,8 @@ static inline void set_extent_info(struct extent_info 
> *ei, unsigned int fofs,
>  static inline bool __is_discard_mergeable(struct discard_info *back,
>   struct discard_info *front)
>  {
> - return back->lstart + back->len == front->lstart;
> + return (back->lstart + back->len == front->lstart) &&
> + (back->len + front->len < DEF_MAX_DISCARD_LEN);
>  }
>  
>  static inline bool __is_discard_back_mergeable(struct discard_info *cur,
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 843fc2e6d41c..ba996d4091bc 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -1139,68 +1139,6 @@ static int __queue_discard_cmd(struct f2fs_sb_info 
> *sbi,
>   return 0;
>  }
>  
> -static void __issue_discard_cmd_range(struct f2fs_sb_info *sbi,
> - struct discard_policy *dpolicy,
> - unsigned int start, unsigned int end)
> -{
> - struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
> - struct discard_cmd *prev_dc = NULL, *next_dc = NULL;
> - struct rb_node **insert_p = NULL, *insert_parent = NULL;
> - struct discard_cmd *dc;
> - struct blk_plug plug;
> - int issued;
> -
> -next:
> - issued = 0;
> -
> - mutex_lock(>cmd_lock);
> - f2fs_bug_on(sbi, !__check_rb_tree_consistence(sbi, >root));
> -
> - dc = (struct discard_cmd *)__lookup_rb_tree_ret(>root,
> - NULL, start,
> - (struct rb_entry **)_dc,
> - (struct rb_entry **)_dc,
> - _p, _parent, true);
> - if (!dc)
> - dc = next_dc;
> -
> - blk_start_plug();
> -
> - while (dc && dc->lstart <= end) {
> - struct rb_node *node;
> -
> - if (dc->len < dpolicy->granularity)
> - goto skip;
> -
> - if (dc->state != D_PREP) {
> - list_move_tail(>list, >fstrim_list);
> - goto skip;
> - }
> -
> - __submit_discard_cmd(sbi, dpolicy, dc);
> -
> - if (++issued >= dpolicy->max_requests) {
> - start = dc->lstart + dc->len;
> -
> - blk_finish_plug();
> - mutex_unlock(>cmd_lock);
> -
> - schedule();
> -
> - goto next;
> - }
> -skip:
> - node = rb_next(>rb_node);
> - dc = rb_entry_safe(node, struct discard_cmd, rb_node);
> -
> - if (fatal_signal_pending(current))
> - break;
> - }
> -
> - blk_finish_plug();
> - mutex_unlock(>cmd_lock);
> -}
> -
>  static int __issue_discard_cmd(struct f2fs_sb_info *sbi,
>   struct discard_policy *dpolicy)
>  {
> @@ -2397,6 +2335,67 @@ bool exist_trim_candidates(struct f2fs_sb_info *sbi, 
> struct cp_control *cpc)
>   return has_candidate;
>  }
>  
> +static void __issue_discard_cmd_range(struct f2fs_sb_info *sbi,
> + struct discard_policy *dpolicy,
> + unsigned int start, unsigned int end)
> +{
> + struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
> + struct discard_cmd *prev_dc = NULL, *next_dc = NULL;
> + struct rb_node **insert_p = NULL, *insert_parent = NULL;
> + struct discard_cmd *dc;
> + struct blk_plug plug;
> + int issued;

unsigned int cur = start;

> +
> +next:
> + issued = 0;
> +
> + mutex_lock(>cmd_lock);
> + f2fs_bug_on(sbi, !__check_rb_tree_consistence(sbi,

[PATCH] f2fs: let fstrim issue discard commands in lower priority

2018-05-24 Thread Jaegeuk Kim 
The fstrim gathers huge number of large discard commands, and tries to issue
without IO awareness, which results in long user-perceive IO latencies on
READ, WRITE, and FLUSH in UFS. We've observed some of commands take several
seconds due to long discard latency.

This patch limits the maximum size to 2MB per candidate, and check IO congestion
when issuing them to disk.

Signed-off-by: Jaegeuk Kim 
---
 fs/f2fs/f2fs.h|   4 +-
 fs/f2fs/segment.c | 123 +++---
 2 files changed, 64 insertions(+), 63 deletions(-)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 3bddf13794d9..75ae7fc86ae8 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -178,6 +178,7 @@ enum {
 
 #define MAX_DISCARD_BLOCKS(sbi)BLKS_PER_SEC(sbi)
 #define DEF_MAX_DISCARD_REQUEST8   /* issue 8 discards per 
round */
+#define DEF_MAX_DISCARD_LEN512 /* Max. 2MB per discard */
 #define DEF_MIN_DISCARD_ISSUE_TIME 50  /* 50 ms, if exists */
 #define DEF_MID_DISCARD_ISSUE_TIME 500 /* 500 ms, if device busy */
 #define DEF_MAX_DISCARD_ISSUE_TIME 6   /* 60 s, if no candidates */
@@ -698,7 +699,8 @@ static inline void set_extent_info(struct extent_info *ei, 
unsigned int fofs,
 static inline bool __is_discard_mergeable(struct discard_info *back,
struct discard_info *front)
 {
-   return back->lstart + back->len == front->lstart;
+   return (back->lstart + back->len == front->lstart) &&
+   (back->len + front->len < DEF_MAX_DISCARD_LEN);
 }
 
 static inline bool __is_discard_back_mergeable(struct discard_info *cur,
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 843fc2e6d41c..ba996d4091bc 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -1139,68 +1139,6 @@ static int __queue_discard_cmd(struct f2fs_sb_info *sbi,
return 0;
 }
 
-static void __issue_discard_cmd_range(struct f2fs_sb_info *sbi,
-   struct discard_policy *dpolicy,
-   unsigned int start, unsigned int end)
-{
-   struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
-   struct discard_cmd *prev_dc = NULL, *next_dc = NULL;
-   struct rb_node **insert_p = NULL, *insert_parent = NULL;
-   struct discard_cmd *dc;
-   struct blk_plug plug;
-   int issued;
-
-next:
-   issued = 0;
-
-   mutex_lock(>cmd_lock);
-   f2fs_bug_on(sbi, !__check_rb_tree_consistence(sbi, >root));
-
-   dc = (struct discard_cmd *)__lookup_rb_tree_ret(>root,
-   NULL, start,
-   (struct rb_entry **)_dc,
-   (struct rb_entry **)_dc,
-   _p, _parent, true);
-   if (!dc)
-   dc = next_dc;
-
-   blk_start_plug();
-
-   while (dc && dc->lstart <= end) {
-   struct rb_node *node;
-
-   if (dc->len < dpolicy->granularity)
-   goto skip;
-
-   if (dc->state != D_PREP) {
-   list_move_tail(>list, >fstrim_list);
-   goto skip;
-   }
-
-   __submit_discard_cmd(sbi, dpolicy, dc);
-
-   if (++issued >= dpolicy->max_requests) {
-   start = dc->lstart + dc->len;
-
-   blk_finish_plug();
-   mutex_unlock(>cmd_lock);
-
-   schedule();
-
-   goto next;
-   }
-skip:
-   node = rb_next(>rb_node);
-   dc = rb_entry_safe(node, struct discard_cmd, rb_node);
-
-   if (fatal_signal_pending(current))
-   break;
-   }
-
-   blk_finish_plug();
-   mutex_unlock(>cmd_lock);
-}
-
 static int __issue_discard_cmd(struct f2fs_sb_info *sbi,
struct discard_policy *dpolicy)
 {
@@ -2397,6 +2335,67 @@ bool exist_trim_candidates(struct f2fs_sb_info *sbi, 
struct cp_control *cpc)
return has_candidate;
 }
 
+static void __issue_discard_cmd_range(struct f2fs_sb_info *sbi,
+   struct discard_policy *dpolicy,
+   unsigned int start, unsigned int end)
+{
+   struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
+   struct discard_cmd *prev_dc = NULL, *next_dc = NULL;
+   struct rb_node **insert_p = NULL, *insert_parent = NULL;
+   struct discard_cmd *dc;
+   struct blk_plug plug;
+   int issued;
+
+next:
+   issued = 0;
+
+   mutex_lock(>cmd_lock);
+   f2fs_bug_on(sbi, !__check_rb_tree_consistence(sbi, >root));
+
+   dc = (struct discard_cmd *)__lookup_rb_tree_ret(>root,
+   NULL, start,
+   (struct rb_entry **)_dc,
+

[PATCH] f2fs: let fstrim issue discard commands in lower priority

2018-05-24 Thread Jaegeuk Kim 
The fstrim gathers huge number of large discard commands, and tries to issue
without IO awareness, which results in long user-perceive IO latencies on
READ, WRITE, and FLUSH in UFS. We've observed some of commands take several
seconds due to long discard latency.

This patch limits the maximum size to 2MB per candidate, and check IO congestion
when issuing them to disk.

Signed-off-by: Jaegeuk Kim 
---
 fs/f2fs/f2fs.h|   4 +-
 fs/f2fs/segment.c | 123 +++---
 2 files changed, 64 insertions(+), 63 deletions(-)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 3bddf13794d9..75ae7fc86ae8 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -178,6 +178,7 @@ enum {
 
 #define MAX_DISCARD_BLOCKS(sbi)BLKS_PER_SEC(sbi)
 #define DEF_MAX_DISCARD_REQUEST8   /* issue 8 discards per 
round */
+#define DEF_MAX_DISCARD_LEN512 /* Max. 2MB per discard */
 #define DEF_MIN_DISCARD_ISSUE_TIME 50  /* 50 ms, if exists */
 #define DEF_MID_DISCARD_ISSUE_TIME 500 /* 500 ms, if device busy */
 #define DEF_MAX_DISCARD_ISSUE_TIME 6   /* 60 s, if no candidates */
@@ -698,7 +699,8 @@ static inline void set_extent_info(struct extent_info *ei, 
unsigned int fofs,
 static inline bool __is_discard_mergeable(struct discard_info *back,
struct discard_info *front)
 {
-   return back->lstart + back->len == front->lstart;
+   return (back->lstart + back->len == front->lstart) &&
+   (back->len + front->len < DEF_MAX_DISCARD_LEN);
 }
 
 static inline bool __is_discard_back_mergeable(struct discard_info *cur,
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 843fc2e6d41c..ba996d4091bc 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -1139,68 +1139,6 @@ static int __queue_discard_cmd(struct f2fs_sb_info *sbi,
return 0;
 }
 
-static void __issue_discard_cmd_range(struct f2fs_sb_info *sbi,
-   struct discard_policy *dpolicy,
-   unsigned int start, unsigned int end)
-{
-   struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
-   struct discard_cmd *prev_dc = NULL, *next_dc = NULL;
-   struct rb_node **insert_p = NULL, *insert_parent = NULL;
-   struct discard_cmd *dc;
-   struct blk_plug plug;
-   int issued;
-
-next:
-   issued = 0;
-
-   mutex_lock(>cmd_lock);
-   f2fs_bug_on(sbi, !__check_rb_tree_consistence(sbi, >root));
-
-   dc = (struct discard_cmd *)__lookup_rb_tree_ret(>root,
-   NULL, start,
-   (struct rb_entry **)_dc,
-   (struct rb_entry **)_dc,
-   _p, _parent, true);
-   if (!dc)
-   dc = next_dc;
-
-   blk_start_plug();
-
-   while (dc && dc->lstart <= end) {
-   struct rb_node *node;
-
-   if (dc->len < dpolicy->granularity)
-   goto skip;
-
-   if (dc->state != D_PREP) {
-   list_move_tail(>list, >fstrim_list);
-   goto skip;
-   }
-
-   __submit_discard_cmd(sbi, dpolicy, dc);
-
-   if (++issued >= dpolicy->max_requests) {
-   start = dc->lstart + dc->len;
-
-   blk_finish_plug();
-   mutex_unlock(>cmd_lock);
-
-   schedule();
-
-   goto next;
-   }
-skip:
-   node = rb_next(>rb_node);
-   dc = rb_entry_safe(node, struct discard_cmd, rb_node);
-
-   if (fatal_signal_pending(current))
-   break;
-   }
-
-   blk_finish_plug();
-   mutex_unlock(>cmd_lock);
-}
-
 static int __issue_discard_cmd(struct f2fs_sb_info *sbi,
struct discard_policy *dpolicy)
 {
@@ -2397,6 +2335,67 @@ bool exist_trim_candidates(struct f2fs_sb_info *sbi, 
struct cp_control *cpc)
return has_candidate;
 }
 
+static void __issue_discard_cmd_range(struct f2fs_sb_info *sbi,
+   struct discard_policy *dpolicy,
+   unsigned int start, unsigned int end)
+{
+   struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
+   struct discard_cmd *prev_dc = NULL, *next_dc = NULL;
+   struct rb_node **insert_p = NULL, *insert_parent = NULL;
+   struct discard_cmd *dc;
+   struct blk_plug plug;
+   int issued;
+
+next:
+   issued = 0;
+
+   mutex_lock(>cmd_lock);
+   f2fs_bug_on(sbi, !__check_rb_tree_consistence(sbi, >root));
+
+   dc = (struct discard_cmd *)__lookup_rb_tree_ret(>root,
+   NULL, start,
+   (struct rb_entry **)_dc,
+