[f2fs-dev] [PATCH v2] f2fs: fix to avoid race during access gc_thread pointer

2018-05-07 Thread Chao Yu 
Thread AThread B
- f2fs_remount
 - stop_gc_thread
- f2fs_sbi_store
   sbi->gc_thread = NULL;
  access sbi->gc_thread->gc_*

Previously, we allocate memory for sbi->gc_thread based on background
gc thread mount option, the memory can be released if we turn off
that mount option, but still there are several places access gc_thread
pointer without considering race condition, result in NULL point
dereference.

In order to fix this issue, use sb->s_umount to exclude those operations.

Signed-off-by: Chao Yu 
---
v2:
- fix to cover __struct_ptr() with sb->s_umount suggested by Jaegeuk.
 fs/f2fs/sysfs.c | 18 +-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
index 7432192ebe17..79f4e4ac8200 100644
--- a/fs/f2fs/sysfs.c
+++ b/fs/f2fs/sysfs.c
@@ -168,7 +168,7 @@ static ssize_t f2fs_sbi_show(struct f2fs_attr *a,
return snprintf(buf, PAGE_SIZE, "%u\n", *ui);
 }
 
-static ssize_t f2fs_sbi_store(struct f2fs_attr *a,
+static ssize_t __f2fs_sbi_store(struct f2fs_attr *a,
struct f2fs_sb_info *sbi,
const char *buf, size_t count)
 {
@@ -261,6 +261,22 @@ static ssize_t f2fs_sbi_store(struct f2fs_attr *a,
return count;
 }
 
+static ssize_t f2fs_sbi_store(struct f2fs_attr *a,
+   struct f2fs_sb_info *sbi,
+   const char *buf, size_t count)
+{
+   ssize_t ret;
+   bool gc_entry = (a->struct_type == GC_THREAD);
+
+   if (gc_entry)
+   down_read(>sb->s_umount);
+   ret = __f2fs_sbi_store(a, sbi, buf, count);
+   if (gc_entry)
+   up_read(>sb->s_umount);
+
+   return ret;
+}
+
 static ssize_t f2fs_attr_show(struct kobject *kobj,
struct attribute *attr, char *buf)
 {
-- 
2.17.0.391.g1f1cddd558b5


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH] f2fs: fix to wait IO writeback in __revoke_inmem_pages()

2018-05-07 Thread Jaegeuk Kim 
On 05/08, Chao Yu wrote:
> On 2018/5/8 4:46, Jaegeuk Kim wrote:
> > On 04/27, Chao Yu wrote:
> >> On 2018/4/27 0:36, Jaegeuk Kim wrote:
> >>> On 04/26, Chao Yu wrote:
>  On 2018/4/26 23:48, Jaegeuk Kim wrote:
> > On 04/26, Chao Yu wrote:
> >> Thread A   Thread B
> >> - f2fs_ioc_commit_atomic_write
> >>  - commit_inmem_pages
> >>   - f2fs_submit_merged_write_cond
> >>   : write data
> >>- write_checkpoint
> >> - do_checkpoint
> >> : commit all node within CP
> >> -> SPO
> >>   - f2fs_do_sync_file
> >>- file_write_and_wait_range
> >>: wait data writeback
> >>
> >> In above race condition, data/node can be flushed in reversed order 
> >> when
> >> coming a checkpoint before f2fs_do_sync_file, after SPOR, it results in
> >> atomic written data being corrupted.
> >
> > Wait, what is the problem here? Thread B could succeed checkpoint, 
> > there is
> > no problem. If it fails, there is no fsync mark where we can recover 
> > it, so
> 
>  Node is flushed by checkpoint before data, with reversed order, that's 
>  the problem.
> >>>
> >>> What do you mean? Data should be in disk, in order to proceed checkpoint.
> >>
> >> 1. thread A: commit_inmem_pages submit data into block layer, but haven't 
> >> waited
> >> it writeback.
> >> 2. thread A: commit_inmem_pages update related node.
> >> 3. thread B: do checkpoint, flush all nodes to disk
> > 
> > How about, in block_operations(),
> > 
> > down_read_trylock(_I(inode)->i_gc_rwsem[WRITE]);
> > if (fail)
> > wait_on_all_pages_writeback(F2FS_WB_DATA);
> > else
> > up_read(_I(inode)->i_gc_rwsem[WRITE]);
> 
> I sent one patch for that, could you check it?
> 
> Adding wait_on_all_pages_writeback in block_operations() can make checkpoint()
> wait pages writeback one more time, which break IO flow, so what's your 
> concern
> here?

Performance. And I can see wait_on_all_pages_writeback() waits only for
F2FS_WB_CP_DATA in checkpoint()?


> 
> Thanks,
> 
> > 
> > 
> >> 4. SPOR
> >>
> >> Then, atomic file becomes corrupted since nodes is flushed before data.
> >>
> >> Thanks,
> >>
> >>>
> 
>  Thanks,
> 
> > we can just ignore the last written data as nothing.
> >
> >>
> >> This patch adds f2fs_wait_on_page_writeback in __revoke_inmem_pages() 
> >> to
> >> keep data and node of atomic file being flushed orderly.
> >>
> >> Signed-off-by: Chao Yu 
> >> ---
> >>  fs/f2fs/file.c| 4 
> >>  fs/f2fs/segment.c | 3 +++
> >>  2 files changed, 7 insertions(+)
> >>
> >> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> >> index be7578774a47..a352804af244 100644
> >> --- a/fs/f2fs/file.c
> >> +++ b/fs/f2fs/file.c
> >> @@ -217,6 +217,9 @@ static int f2fs_do_sync_file(struct file *file, 
> >> loff_t start, loff_t end,
> >>  
> >>trace_f2fs_sync_file_enter(inode);
> >>  
> >> +  if (atomic)
> >> +  goto write_done;
> >> +
> >>/* if fdatasync is triggered, let's do in-place-update */
> >>if (datasync || get_dirty_pages(inode) <= 
> >> SM_I(sbi)->min_fsync_blocks)
> >>set_inode_flag(inode, FI_NEED_IPU);
> >> @@ -228,6 +231,7 @@ static int f2fs_do_sync_file(struct file *file, 
> >> loff_t start, loff_t end,
> >>return ret;
> >>}
> >>  
> >> +write_done:
> >>/* if the inode is dirty, let's recover all the time */
> >>if (!f2fs_skip_inode_update(inode, datasync)) {
> >>f2fs_write_inode(inode, NULL);
> >> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> >> index 584483426584..9ca3d0a43d93 100644
> >> --- a/fs/f2fs/segment.c
> >> +++ b/fs/f2fs/segment.c
> >> @@ -230,6 +230,8 @@ static int __revoke_inmem_pages(struct inode 
> >> *inode,
> >>  
> >>lock_page(page);
> >>  
> >> +  f2fs_wait_on_page_writeback(page, DATA, true);
> >> +
> >>if (recover) {
> >>struct dnode_of_data dn;
> >>struct node_info ni;
> >> @@ -415,6 +417,7 @@ static int __commit_inmem_pages(struct inode 
> >> *inode)
> >>/* drop all uncommitted pages */
> >>__revoke_inmem_pages(inode, >inmem_pages, true, 
> >> false);
> >>} else {
> >> +  /* wait all committed IOs writeback and release them 
> >> from list */
> >>__revoke_inmem_pages(inode, _list, false, false);
> >>}
> >>  
> >> -- 
> >> 2.15.0.55.gc2ece9dc4de6
> >>>
> >>> .
>

Re: [f2fs-dev] [PATCH v4] f2fs: fix to avoid race during access gc_thread pointer

2018-05-07 Thread Chao Yu 
On 2018/5/8 11:17, Jaegeuk Kim wrote:
> On 05/08, Chao Yu wrote:
>> On 2018/5/8 5:36, Jaegeuk Kim wrote:
>>> On 05/07, Chao Yu wrote:
 Thread A   Thread BThread C
 - f2fs_remount
  - stop_gc_thread
- f2fs_sbi_store
- issue_discard_thread
sbi->gc_thread = NULL;
  sbi->gc_thread->gc_wake = 1
  access 
 sbi->gc_thread->gc_urgent

 Previously, we allocate memory for sbi->gc_thread based on background
 gc thread mount option, the memory can be released if we turn off
 that mount option, but still there are several places access gc_thread
 pointer without considering race condition, result in NULL point
 dereference.

 In order to fix this issue, introduce gc_rwsem to exclude those operations.

 Signed-off-by: Chao Yu 
 ---
 v4:
 - use introduced sbi.gc_rwsem lock instead of sb.s_umount.
>>>
>>> We can use this first.
>>>
>>> >From e62e8d3ece6ee8a4aeac8ffd6161d25851f8b3f0 Mon Sep 17 00:00:00 2001
>>> From: Jaegeuk Kim 
>>> Date: Mon, 7 May 2018 14:22:40 -0700
>>> Subject: [PATCH] f2fs: introduce sbi->gc_mode to determine the policy
>>>
>>> This is to avoid sbi->gc_thread pointer access.
>>>
>>> Signed-off-by: Jaegeuk Kim 
>>> ---
>>>  fs/f2fs/f2fs.h|  8 
>>>  fs/f2fs/gc.c  | 28 
>>>  fs/f2fs/gc.h  |  2 --
>>>  fs/f2fs/segment.c |  4 ++--
>>>  fs/f2fs/sysfs.c   | 33 +
>>>  5 files changed, 47 insertions(+), 28 deletions(-)
>>>
>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>> index 80490a7991a7..779d8b26878c 100644
>>> --- a/fs/f2fs/f2fs.h
>>> +++ b/fs/f2fs/f2fs.h
>>> @@ -1065,6 +1065,13 @@ enum {
>>> MAX_TIME,
>>>  };
>>>  
>>> +enum {
>>> +   GC_NORMAL,
>>> +   GC_IDLE_CB,
>>> +   GC_IDLE_GREEDY,
>>> +   GC_URGENT,
>>> +};
>>> +
>>>  enum {
>>> WHINT_MODE_OFF, /* not pass down write hints */
>>> WHINT_MODE_USER,/* try to pass down hints given by users */
>>> @@ -1193,6 +1200,7 @@ struct f2fs_sb_info {
>>> struct mutex gc_mutex;  /* mutex for GC */
>>> struct f2fs_gc_kthread  *gc_thread; /* GC thread */
>>> unsigned int cur_victim_sec;/* current victim section num */
>>> +   unsigned int gc_mode;   /* current GC state */
>>>  
>>> /* threshold for gc trials on pinned files */
>>> u64 gc_pin_file_threshold;
>>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
>>> index 9bb2ddbbed1e..7ec8ea75dfde 100644
>>> --- a/fs/f2fs/gc.c
>>> +++ b/fs/f2fs/gc.c
>>> @@ -76,7 +76,7 @@ static int gc_thread_func(void *data)
>>>  * invalidated soon after by user update or deletion.
>>>  * So, I'd like to wait some time to collect dirty segments.
>>>  */
>>> -   if (gc_th->gc_urgent) {
>>> +   if (sbi->gc_mode == GC_URGENT) {
>>> wait_ms = gc_th->urgent_sleep_time;
>>> mutex_lock(>gc_mutex);
>>> goto do_gc;
>>> @@ -131,8 +131,6 @@ int start_gc_thread(struct f2fs_sb_info *sbi)
>>> gc_th->max_sleep_time = DEF_GC_THREAD_MAX_SLEEP_TIME;
>>> gc_th->no_gc_sleep_time = DEF_GC_THREAD_NOGC_SLEEP_TIME;
>>>  
>>> -   gc_th->gc_idle = 0;
>>> -   gc_th->gc_urgent = 0;
>>> gc_th->gc_wake= 0;
>>>  
>>> sbi->gc_thread = gc_th;
>>> @@ -158,21 +156,19 @@ void stop_gc_thread(struct f2fs_sb_info *sbi)
>>> sbi->gc_thread = NULL;
>>>  }
>>>  
>>> -static int select_gc_type(struct f2fs_gc_kthread *gc_th, int gc_type)
>>> +static int select_gc_type(struct f2fs_sb_info *sbi, int gc_type)
>>>  {
>>> int gc_mode = (gc_type == BG_GC) ? GC_CB : GC_GREEDY;
>>>  
>>> -   if (!gc_th)
>>> -   return gc_mode;
>>> -
>>> -   if (gc_th->gc_idle) {
>>> -   if (gc_th->gc_idle == 1)
>>> -   gc_mode = GC_CB;
>>> -   else if (gc_th->gc_idle == 2)
>>> -   gc_mode = GC_GREEDY;
>>> -   }
>>> -   if (gc_th->gc_urgent)
>>> +   switch (sbi->gc_mode) {
>>> +   case GC_IDLE_CB:
>>> +   gc_mode = GC_CB;
>>> +   break;
>>> +   case GC_IDLE_GREEDY:
>>> +   case GC_URGENT:
>>> gc_mode = GC_GREEDY;
>>> +   break;
>>> +   }
>>> return gc_mode;
>>>  }
>>>  
>>> @@ -187,7 +183,7 @@ static void select_policy(struct f2fs_sb_info *sbi, int 
>>> gc_type,
>>> p->max_search = dirty_i->nr_dirty[type];
>>> p->ofs_unit = 1;
>>> } else {
>>> -   p->gc_mode = select_gc_type(sbi->gc_thread, gc_type);
>>> +   p->gc_mode = select_gc_type(sbi, gc_type);
>>> p->dirty_segmap = dirty_i->dirty_segmap[DIRTY];
>>> p->max_search = dirty_i->nr_dirty[DIRTY];
>>> p->ofs_unit = sbi->segs_per_sec;
>>> @@

Re: [f2fs-dev] [PATCH v4] f2fs: fix to avoid race during access gc_thread pointer

2018-05-07 Thread Jaegeuk Kim 
On 05/08, Chao Yu wrote:
> On 2018/5/8 5:36, Jaegeuk Kim wrote:
> > On 05/07, Chao Yu wrote:
> >> Thread A   Thread BThread C
> >> - f2fs_remount
> >>  - stop_gc_thread
> >>- f2fs_sbi_store
> >>- issue_discard_thread
> >>sbi->gc_thread = NULL;
> >>  sbi->gc_thread->gc_wake = 1
> >>  access 
> >> sbi->gc_thread->gc_urgent
> >>
> >> Previously, we allocate memory for sbi->gc_thread based on background
> >> gc thread mount option, the memory can be released if we turn off
> >> that mount option, but still there are several places access gc_thread
> >> pointer without considering race condition, result in NULL point
> >> dereference.
> >>
> >> In order to fix this issue, introduce gc_rwsem to exclude those operations.
> >>
> >> Signed-off-by: Chao Yu 
> >> ---
> >> v4:
> >> - use introduced sbi.gc_rwsem lock instead of sb.s_umount.
> > 
> > We can use this first.
> > 
> >>From e62e8d3ece6ee8a4aeac8ffd6161d25851f8b3f0 Mon Sep 17 00:00:00 2001
> > From: Jaegeuk Kim 
> > Date: Mon, 7 May 2018 14:22:40 -0700
> > Subject: [PATCH] f2fs: introduce sbi->gc_mode to determine the policy
> > 
> > This is to avoid sbi->gc_thread pointer access.
> > 
> > Signed-off-by: Jaegeuk Kim 
> > ---
> >  fs/f2fs/f2fs.h|  8 
> >  fs/f2fs/gc.c  | 28 
> >  fs/f2fs/gc.h  |  2 --
> >  fs/f2fs/segment.c |  4 ++--
> >  fs/f2fs/sysfs.c   | 33 +
> >  5 files changed, 47 insertions(+), 28 deletions(-)
> > 
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > index 80490a7991a7..779d8b26878c 100644
> > --- a/fs/f2fs/f2fs.h
> > +++ b/fs/f2fs/f2fs.h
> > @@ -1065,6 +1065,13 @@ enum {
> > MAX_TIME,
> >  };
> >  
> > +enum {
> > +   GC_NORMAL,
> > +   GC_IDLE_CB,
> > +   GC_IDLE_GREEDY,
> > +   GC_URGENT,
> > +};
> > +
> >  enum {
> > WHINT_MODE_OFF, /* not pass down write hints */
> > WHINT_MODE_USER,/* try to pass down hints given by users */
> > @@ -1193,6 +1200,7 @@ struct f2fs_sb_info {
> > struct mutex gc_mutex;  /* mutex for GC */
> > struct f2fs_gc_kthread  *gc_thread; /* GC thread */
> > unsigned int cur_victim_sec;/* current victim section num */
> > +   unsigned int gc_mode;   /* current GC state */
> >  
> > /* threshold for gc trials on pinned files */
> > u64 gc_pin_file_threshold;
> > diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> > index 9bb2ddbbed1e..7ec8ea75dfde 100644
> > --- a/fs/f2fs/gc.c
> > +++ b/fs/f2fs/gc.c
> > @@ -76,7 +76,7 @@ static int gc_thread_func(void *data)
> >  * invalidated soon after by user update or deletion.
> >  * So, I'd like to wait some time to collect dirty segments.
> >  */
> > -   if (gc_th->gc_urgent) {
> > +   if (sbi->gc_mode == GC_URGENT) {
> > wait_ms = gc_th->urgent_sleep_time;
> > mutex_lock(>gc_mutex);
> > goto do_gc;
> > @@ -131,8 +131,6 @@ int start_gc_thread(struct f2fs_sb_info *sbi)
> > gc_th->max_sleep_time = DEF_GC_THREAD_MAX_SLEEP_TIME;
> > gc_th->no_gc_sleep_time = DEF_GC_THREAD_NOGC_SLEEP_TIME;
> >  
> > -   gc_th->gc_idle = 0;
> > -   gc_th->gc_urgent = 0;
> > gc_th->gc_wake= 0;
> >  
> > sbi->gc_thread = gc_th;
> > @@ -158,21 +156,19 @@ void stop_gc_thread(struct f2fs_sb_info *sbi)
> > sbi->gc_thread = NULL;
> >  }
> >  
> > -static int select_gc_type(struct f2fs_gc_kthread *gc_th, int gc_type)
> > +static int select_gc_type(struct f2fs_sb_info *sbi, int gc_type)
> >  {
> > int gc_mode = (gc_type == BG_GC) ? GC_CB : GC_GREEDY;
> >  
> > -   if (!gc_th)
> > -   return gc_mode;
> > -
> > -   if (gc_th->gc_idle) {
> > -   if (gc_th->gc_idle == 1)
> > -   gc_mode = GC_CB;
> > -   else if (gc_th->gc_idle == 2)
> > -   gc_mode = GC_GREEDY;
> > -   }
> > -   if (gc_th->gc_urgent)
> > +   switch (sbi->gc_mode) {
> > +   case GC_IDLE_CB:
> > +   gc_mode = GC_CB;
> > +   break;
> > +   case GC_IDLE_GREEDY:
> > +   case GC_URGENT:
> > gc_mode = GC_GREEDY;
> > +   break;
> > +   }
> > return gc_mode;
> >  }
> >  
> > @@ -187,7 +183,7 @@ static void select_policy(struct f2fs_sb_info *sbi, int 
> > gc_type,
> > p->max_search = dirty_i->nr_dirty[type];
> > p->ofs_unit = 1;
> > } else {
> > -   p->gc_mode = select_gc_type(sbi->gc_thread, gc_type);
> > +   p->gc_mode = select_gc_type(sbi, gc_type);
> > p->dirty_segmap = dirty_i->dirty_segmap[DIRTY];
> > p->max_search = dirty_i->nr_dirty[DIRTY];
> > p->ofs_unit = sbi->segs_per_sec;
> > @@ -195,7 +191,7 @@ static void

Re: [f2fs-dev] [PATCH] sload.f2fs: give correct file type

2018-05-07 Thread Jaegeuk Kim 
On 05/08, Junling Zheng wrote:
> Hi, Jaegeuk
> 
> My previous patch (sload.f2fs: fix the missing of bit mask for file type) had
> already fix this bug :)

I guessed it before, but this is also reasonable to me where de->file_type
is based on F2FS_FT_DIR.

> 
> Thanks,
> Junling
> 
> On 2018/5/8 2:58, Jaegeuk Kim wrote:
> > From: Lianjun Huang 
> > 
> > This fixes permission error due to wrong file type.
> > 
> > Signed-off-by: Lianjun Huang 
> > Signed-off-by: Jaegeuk Kim 
> > ---
> >  fsck/sload.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/fsck/sload.c b/fsck/sload.c
> > index 2842f2c..2fb13f2 100644
> > --- a/fsck/sload.c
> > +++ b/fsck/sload.c
> > @@ -106,7 +106,7 @@ static int set_perms_and_caps(struct dentry *de)
> >  
> > /* Permissions */
> > if (fs_config_func != NULL) {
> > -   fs_config_func(mnt_path, S_ISDIR(de->mode),
> > +   fs_config_func(mnt_path, de->file_type == F2FS_FT_DIR,
> > c.target_out_dir, , , ,
> > );
> > de->uid = uid & 0x;
> > 
> 

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH] f2fs: fix to wait IO writeback in __revoke_inmem_pages()

2018-05-07 Thread Chao Yu 
On 2018/5/8 4:46, Jaegeuk Kim wrote:
> On 04/27, Chao Yu wrote:
>> On 2018/4/27 0:36, Jaegeuk Kim wrote:
>>> On 04/26, Chao Yu wrote:
 On 2018/4/26 23:48, Jaegeuk Kim wrote:
> On 04/26, Chao Yu wrote:
>> Thread A Thread B
>> - f2fs_ioc_commit_atomic_write
>>  - commit_inmem_pages
>>   - f2fs_submit_merged_write_cond
>>   : write data
>>  - write_checkpoint
>>   - do_checkpoint
>>   : commit all node within CP
>>   -> SPO
>>   - f2fs_do_sync_file
>>- file_write_and_wait_range
>>: wait data writeback
>>
>> In above race condition, data/node can be flushed in reversed order when
>> coming a checkpoint before f2fs_do_sync_file, after SPOR, it results in
>> atomic written data being corrupted.
>
> Wait, what is the problem here? Thread B could succeed checkpoint, there 
> is
> no problem. If it fails, there is no fsync mark where we can recover it, 
> so

 Node is flushed by checkpoint before data, with reversed order, that's the 
 problem.
>>>
>>> What do you mean? Data should be in disk, in order to proceed checkpoint.
>>
>> 1. thread A: commit_inmem_pages submit data into block layer, but haven't 
>> waited
>> it writeback.
>> 2. thread A: commit_inmem_pages update related node.
>> 3. thread B: do checkpoint, flush all nodes to disk
> 
> How about, in block_operations(),
> 
>   down_read_trylock(_I(inode)->i_gc_rwsem[WRITE]);
>   if (fail)
>   wait_on_all_pages_writeback(F2FS_WB_DATA);
>   else
>   up_read(_I(inode)->i_gc_rwsem[WRITE]);

I sent one patch for that, could you check it?

Adding wait_on_all_pages_writeback in block_operations() can make checkpoint()
wait pages writeback one more time, which break IO flow, so what's your concern
here?

Thanks,

> 
> 
>> 4. SPOR
>>
>> Then, atomic file becomes corrupted since nodes is flushed before data.
>>
>> Thanks,
>>
>>>

 Thanks,

> we can just ignore the last written data as nothing.
>
>>
>> This patch adds f2fs_wait_on_page_writeback in __revoke_inmem_pages() to
>> keep data and node of atomic file being flushed orderly.
>>
>> Signed-off-by: Chao Yu 
>> ---
>>  fs/f2fs/file.c| 4 
>>  fs/f2fs/segment.c | 3 +++
>>  2 files changed, 7 insertions(+)
>>
>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>> index be7578774a47..a352804af244 100644
>> --- a/fs/f2fs/file.c
>> +++ b/fs/f2fs/file.c
>> @@ -217,6 +217,9 @@ static int f2fs_do_sync_file(struct file *file, 
>> loff_t start, loff_t end,
>>  
>>  trace_f2fs_sync_file_enter(inode);
>>  
>> +if (atomic)
>> +goto write_done;
>> +
>>  /* if fdatasync is triggered, let's do in-place-update */
>>  if (datasync || get_dirty_pages(inode) <= 
>> SM_I(sbi)->min_fsync_blocks)
>>  set_inode_flag(inode, FI_NEED_IPU);
>> @@ -228,6 +231,7 @@ static int f2fs_do_sync_file(struct file *file, 
>> loff_t start, loff_t end,
>>  return ret;
>>  }
>>  
>> +write_done:
>>  /* if the inode is dirty, let's recover all the time */
>>  if (!f2fs_skip_inode_update(inode, datasync)) {
>>  f2fs_write_inode(inode, NULL);
>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>> index 584483426584..9ca3d0a43d93 100644
>> --- a/fs/f2fs/segment.c
>> +++ b/fs/f2fs/segment.c
>> @@ -230,6 +230,8 @@ static int __revoke_inmem_pages(struct inode *inode,
>>  
>>  lock_page(page);
>>  
>> +f2fs_wait_on_page_writeback(page, DATA, true);
>> +
>>  if (recover) {
>>  struct dnode_of_data dn;
>>  struct node_info ni;
>> @@ -415,6 +417,7 @@ static int __commit_inmem_pages(struct inode *inode)
>>  /* drop all uncommitted pages */
>>  __revoke_inmem_pages(inode, >inmem_pages, true, 
>> false);
>>  } else {
>> +/* wait all committed IOs writeback and release them 
>> from list */
>>  __revoke_inmem_pages(inode, _list, false, false);
>>  }
>>  
>> -- 
>> 2.15.0.55.gc2ece9dc4de6
>>>
>>> .
>>>
> 
> .
> 


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[f2fs-dev] [PATCH] f2fs: fix to wait atomic pages writeback in block_operations()

2018-05-07 Thread Chao Yu 
1. thread A: commit_inmem_pages submit data into block layer, but
haven't waited it writeback.
2. thread A: commit_inmem_pages update related node.
3. thread B: do checkpoint, flush all nodes to disk.
4. SPOR

Then, atomic file becomes corrupted since nodes is flushed before data.

This patch fixes to try to wait all atomic pages writeback in
block_operations().

Signed-off-by: Chao Yu 
---
 fs/f2fs/checkpoint.c |  4 +++-
 fs/f2fs/data.c   |  2 ++
 fs/f2fs/f2fs.h   |  2 ++
 fs/f2fs/segment.c| 17 +
 4 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index 33d2da006789..d53d53f55c51 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -1067,6 +1067,8 @@ static int block_operations(struct f2fs_sb_info *sbi)
goto retry_flush_dents;
}
 
+   wait_inmem_pages_writeback(sbi);
+
/*
 * POR: we should ensure that there are no dirty node pages
 * until finishing nat/sit flush. inode->i_blocks can be updated.
@@ -1115,7 +1117,7 @@ static void unblock_operations(struct f2fs_sb_info *sbi)
f2fs_unlock_all(sbi);
 }
 
-static void wait_on_all_pages_writeback(struct f2fs_sb_info *sbi)
+void wait_on_all_pages_writeback(struct f2fs_sb_info *sbi)
 {
DEFINE_WAIT(wait);
 
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 5a979b5ee278..c181f58948c0 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -48,6 +48,8 @@ static bool __is_cp_guaranteed(struct page *page)
if (inode->i_ino == F2FS_META_INO(sbi) ||
inode->i_ino ==  F2FS_NODE_INO(sbi) ||
S_ISDIR(inode->i_mode) ||
+   (S_ISREG(inode->i_mode) &&
+   is_inode_flag_set(inode, FI_ATOMIC_FILE)) ||
is_cold_data(page))
return true;
return false;
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index bda9c3ce08ef..adfd512ae4a1 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -2839,6 +2839,7 @@ void destroy_node_manager_caches(void);
 bool need_SSR(struct f2fs_sb_info *sbi);
 void register_inmem_page(struct inode *inode, struct page *page);
 void drop_inmem_pages_all(struct f2fs_sb_info *sbi, bool gc_failure);
+void wait_inmem_pages_writeback(struct f2fs_sb_info *sbi);
 void drop_inmem_pages(struct inode *inode);
 void drop_inmem_page(struct inode *inode, struct page *page);
 int commit_inmem_pages(struct inode *inode);
@@ -2926,6 +2927,7 @@ int get_valid_checkpoint(struct f2fs_sb_info *sbi);
 void update_dirty_page(struct inode *inode, struct page *page);
 void remove_dirty_inode(struct inode *inode);
 int sync_dirty_inodes(struct f2fs_sb_info *sbi, enum inode_type type);
+void wait_on_all_pages_writeback(struct f2fs_sb_info *sbi);
 int write_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc);
 void init_ino_entry_info(struct f2fs_sb_info *sbi);
 int __init create_checkpoint_caches(void);
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 24b71d450374..e8a81cbd6808 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -305,6 +305,23 @@ void drop_inmem_pages_all(struct f2fs_sb_info *sbi, bool 
gc_failure)
goto next;
 }
 
+void wait_inmem_pages_writeback(struct f2fs_sb_info *sbi)
+{
+   struct list_head *head = >inode_list[ATOMIC_FILE];
+   struct f2fs_inode_info *fi;
+
+   spin_lock(>inode_lock[ATOMIC_FILE]);
+   list_for_each_entry(fi, head, inmem_ilist) {
+   if (!down_read_trylock(>i_gc_rwsem[WRITE])) {
+   spin_unlock(>inode_lock[ATOMIC_FILE]);
+   wait_on_all_pages_writeback(sbi);
+   return;
+   }
+   up_read(>i_gc_rwsem[WRITE]);
+   }
+   spin_unlock(>inode_lock[ATOMIC_FILE]);
+}
+
 void drop_inmem_pages(struct inode *inode)
 {
struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
-- 
2.17.0.391.g1f1cddd558b5


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH] sload.f2fs: give correct file type

2018-05-07 Thread Junling Zheng 
Hi, Jaegeuk

My previous patch (sload.f2fs: fix the missing of bit mask for file type) had
already fix this bug :)

Thanks,
Junling

On 2018/5/8 2:58, Jaegeuk Kim wrote:
> From: Lianjun Huang 
> 
> This fixes permission error due to wrong file type.
> 
> Signed-off-by: Lianjun Huang 
> Signed-off-by: Jaegeuk Kim 
> ---
>  fsck/sload.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fsck/sload.c b/fsck/sload.c
> index 2842f2c..2fb13f2 100644
> --- a/fsck/sload.c
> +++ b/fsck/sload.c
> @@ -106,7 +106,7 @@ static int set_perms_and_caps(struct dentry *de)
>  
>   /* Permissions */
>   if (fs_config_func != NULL) {
> - fs_config_func(mnt_path, S_ISDIR(de->mode),
> + fs_config_func(mnt_path, de->file_type == F2FS_FT_DIR,
>   c.target_out_dir, , , ,
>   );
>   de->uid = uid & 0x;
> 



--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[f2fs-dev] [PATCH v2] fscrypt: add Speck128/256 support

2018-05-07 Thread Eric Biggers via Linux-f2fs-devel 
fscrypt currently only supports AES encryption.  However, many low-end
mobile devices have older CPUs that don't have AES instructions, e.g.
the ARMv8 Cryptography Extensions.  Currently, user data on such devices
is not encrypted at rest because AES is too slow, even when the NEON
bit-sliced implementation of AES is used.  Unfortunately, it is
infeasible to encrypt these devices at all when AES is the only option.

Therefore, this patch updates fscrypt to support the Speck block cipher,
which was recently added to the crypto API.  The C implementation of
Speck is not especially fast, but Speck can be implemented very
efficiently with general-purpose vector instructions, e.g. ARM NEON.
For example, on an ARMv7 processor, we measured the NEON-accelerated
Speck128/256-XTS at 69 MB/s for both encryption and decryption, while
AES-256-XTS with the NEON bit-sliced implementation was only 22 MB/s
encryption and 19 MB/s decryption.

There are multiple variants of Speck.  This patch only adds support for
Speck128/256, which is the variant with a 128-bit block size and 256-bit
key size -- the same as AES-256.  This is believed to be the most secure
variant of Speck, and it's only about 6% slower than Speck128/128.
Speck64/128 would be at least 20% faster because it has 20% rounds, and
it can be even faster on CPUs that can't efficiently do the 64-bit
operations needed for Speck128.  However, Speck64's 64-bit block size is
not preferred security-wise.  ARM NEON also supports the needed 64-bit
operations even on 32-bit CPUs, resulting in Speck128 being fast enough
for our targeted use cases so far.

The chosen modes of operation are XTS for contents and CTS-CBC for
filenames.  These are the same modes of operation that fscrypt defaults
to for AES.  Note that as with the other fscrypt modes, Speck will not
be used unless userspace chooses to use it.  Nor are any of the existing
modes (which are all AES-based) being removed, of course.

We intentionally don't make CONFIG_FS_ENCRYPTION select
CONFIG_CRYPTO_SPECK, so people will have to enable Speck support
themselves if they need it.  This is because we shouldn't bloat the
FS_ENCRYPTION dependencies with every new cipher, especially ones that
aren't recommended for most users.  Moreover, CRYPTO_SPECK is just the
generic implementation, which won't be fast enough for many users; in
practice, they'll need to enable CRYPTO_SPECK_NEON to get acceptable
performance.

More details about our choice of Speck can be found in our patches that
added Speck to the crypto API, and the follow-on discussion threads.
We're planning a publication that explains the choice in more detail.
But briefly, we can't use ChaCha20 as we previously proposed, since it
would be insecure to use a stream cipher in this context, with potential
IV reuse during writes on f2fs and/or on wear-leveling flash storage.

We also evaluated many other lightweight and/or ARX-based block ciphers
such as Chaskey-LTS, RC5, LEA, CHAM, Threefish, RC6, NOEKEON, SPARX, and
XTEA.  However, all had disadvantages vs. Speck, such as insufficient
performance with NEON, much less published cryptanalysis, or an
insufficient security level.  Various design choices in Speck make it
perform better with NEON than competing ciphers while still having a
security margin similar to AES, and in the case of Speck128 also the
same available security levels.  Unfortunately, Speck does have some
political baggage attached -- it's an NSA designed cipher, and was
rejected from an ISO standard (though for context, as far as I know none
of the above-mentioned alternatives are ISO standards either).
Nevertheless, we believe it is a good solution to the problem from a
technical perspective.

Certain algorithms constructed from ChaCha or the ChaCha permutation,
such as MEM (Masked Even-Mansour) or HPolyC, may also meet our
performance requirements.  However, these are new constructions that
need more time to receive the cryptographic review and acceptance needed
to be confident in their security.  HPolyC hasn't been published yet,
and we are concerned that MEM makes stronger assumptions about the
underlying permutation than the ChaCha stream cipher does.  In contrast,
the XTS mode of operation is relatively well accepted, and Speck has
over 70 cryptanalysis papers.  Of course, these ChaCha-based algorithms
can still be added later if they become ready.

The best known attack on Speck128/256 is a differential cryptanalysis
attack on 25 of 34 rounds with 2^253 time complexity and 2^125 chosen
plaintexts, i.e. only marginally faster than brute force.  There is no
known attack on the full 34 rounds.

Signed-off-by: Eric Biggers 
---

Changed since v1:
- Improved commit message and documentation.

 Documentation/filesystems/fscrypt.rst | 10 ++
 fs/crypto/fscrypt_private.h   |  4 
 fs/crypto/keyinfo.c   |  2 ++
 include/uapi/linux/fs.h   |  2 ++
 4 files changed, 18 insertions(+)

Re: [f2fs-dev] [PATCH] libf2fs: quote non-printables when displaying "disk model"

2018-05-07 Thread Jaegeuk Kim 
On 05/07, Adam Borowski wrote:
> One offender I own has:
> STORAGE DEVICE  
> 1404x05xe3x07QGENEx00%x00x00x00x00x00x00x00x00x00x00x00x00x170x04
> 
> I limited it to pure ASCII only, but you might want to allow high-bit bytes,
> either validating UTF-8 or assuming that mojibake is not as bad as beep and
> zeroes as in the above example.  Or, cutting off at the first control
> character might be a good alternative too.
> 
> Signed-off-by: Adam Borowski 
> ---
>  lib/libf2fs.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/libf2fs.c b/lib/libf2fs.c
> index 5ef0214..8782afc 100644
> --- a/lib/libf2fs.c
> +++ b/lib/libf2fs.c
> @@ -884,8 +884,9 @@ int get_device_info(int i)
>  
>   MSG(0, "Info: [%s] Disk Model: ",
>   dev->path);
> - while (reply_buffer[i] != '`' && i < 80)
> - printf("%c", reply_buffer[i++]);
> + for (; reply_buffer[i] != '`' && i < 80; i++)
> + printf((reply_buffer[i] >= ' ' && 
> reply_buffer[i] < 127) ?

How about printing only valid characters here? In my virtualbox, this is
showing garbage characters.

Thanks,

> +"%c" : "x%02x", reply_buffer[i]);
>   printf("\n");
>   }
>  #endif
> -- 
> 2.17.0

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH v4] f2fs: fix to avoid race during access gc_thread pointer

2018-05-07 Thread Jaegeuk Kim 
On 05/07, Chao Yu wrote:
> Thread A  Thread BThread C
> - f2fs_remount
>  - stop_gc_thread
>   - f2fs_sbi_store
>   - issue_discard_thread
>sbi->gc_thread = NULL;
> sbi->gc_thread->gc_wake = 1
> access 
> sbi->gc_thread->gc_urgent
> 
> Previously, we allocate memory for sbi->gc_thread based on background
> gc thread mount option, the memory can be released if we turn off
> that mount option, but still there are several places access gc_thread
> pointer without considering race condition, result in NULL point
> dereference.
> 
> In order to fix this issue, introduce gc_rwsem to exclude those operations.
> 
> Signed-off-by: Chao Yu 
> ---
> v4:
> - use introduced sbi.gc_rwsem lock instead of sb.s_umount.

We can use this first.

>From e62e8d3ece6ee8a4aeac8ffd6161d25851f8b3f0 Mon Sep 17 00:00:00 2001
From: Jaegeuk Kim 
Date: Mon, 7 May 2018 14:22:40 -0700
Subject: [PATCH] f2fs: introduce sbi->gc_mode to determine the policy

This is to avoid sbi->gc_thread pointer access.

Signed-off-by: Jaegeuk Kim 
---
 fs/f2fs/f2fs.h|  8 
 fs/f2fs/gc.c  | 28 
 fs/f2fs/gc.h  |  2 --
 fs/f2fs/segment.c |  4 ++--
 fs/f2fs/sysfs.c   | 33 +
 5 files changed, 47 insertions(+), 28 deletions(-)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 80490a7991a7..779d8b26878c 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1065,6 +1065,13 @@ enum {
MAX_TIME,
 };
 
+enum {
+   GC_NORMAL,
+   GC_IDLE_CB,
+   GC_IDLE_GREEDY,
+   GC_URGENT,
+};
+
 enum {
WHINT_MODE_OFF, /* not pass down write hints */
WHINT_MODE_USER,/* try to pass down hints given by users */
@@ -1193,6 +1200,7 @@ struct f2fs_sb_info {
struct mutex gc_mutex;  /* mutex for GC */
struct f2fs_gc_kthread  *gc_thread; /* GC thread */
unsigned int cur_victim_sec;/* current victim section num */
+   unsigned int gc_mode;   /* current GC state */
 
/* threshold for gc trials on pinned files */
u64 gc_pin_file_threshold;
diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index 9bb2ddbbed1e..7ec8ea75dfde 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -76,7 +76,7 @@ static int gc_thread_func(void *data)
 * invalidated soon after by user update or deletion.
 * So, I'd like to wait some time to collect dirty segments.
 */
-   if (gc_th->gc_urgent) {
+   if (sbi->gc_mode == GC_URGENT) {
wait_ms = gc_th->urgent_sleep_time;
mutex_lock(>gc_mutex);
goto do_gc;
@@ -131,8 +131,6 @@ int start_gc_thread(struct f2fs_sb_info *sbi)
gc_th->max_sleep_time = DEF_GC_THREAD_MAX_SLEEP_TIME;
gc_th->no_gc_sleep_time = DEF_GC_THREAD_NOGC_SLEEP_TIME;
 
-   gc_th->gc_idle = 0;
-   gc_th->gc_urgent = 0;
gc_th->gc_wake= 0;
 
sbi->gc_thread = gc_th;
@@ -158,21 +156,19 @@ void stop_gc_thread(struct f2fs_sb_info *sbi)
sbi->gc_thread = NULL;
 }
 
-static int select_gc_type(struct f2fs_gc_kthread *gc_th, int gc_type)
+static int select_gc_type(struct f2fs_sb_info *sbi, int gc_type)
 {
int gc_mode = (gc_type == BG_GC) ? GC_CB : GC_GREEDY;
 
-   if (!gc_th)
-   return gc_mode;
-
-   if (gc_th->gc_idle) {
-   if (gc_th->gc_idle == 1)
-   gc_mode = GC_CB;
-   else if (gc_th->gc_idle == 2)
-   gc_mode = GC_GREEDY;
-   }
-   if (gc_th->gc_urgent)
+   switch (sbi->gc_mode) {
+   case GC_IDLE_CB:
+   gc_mode = GC_CB;
+   break;
+   case GC_IDLE_GREEDY:
+   case GC_URGENT:
gc_mode = GC_GREEDY;
+   break;
+   }
return gc_mode;
 }
 
@@ -187,7 +183,7 @@ static void select_policy(struct f2fs_sb_info *sbi, int 
gc_type,
p->max_search = dirty_i->nr_dirty[type];
p->ofs_unit = 1;
} else {
-   p->gc_mode = select_gc_type(sbi->gc_thread, gc_type);
+   p->gc_mode = select_gc_type(sbi, gc_type);
p->dirty_segmap = dirty_i->dirty_segmap[DIRTY];
p->max_search = dirty_i->nr_dirty[DIRTY];
p->ofs_unit = sbi->segs_per_sec;
@@ -195,7 +191,7 @@ static void select_policy(struct f2fs_sb_info *sbi, int 
gc_type,
 
/* we need to check every dirty segments in the FG_GC case */
if (gc_type != FG_GC &&
-   (sbi->gc_thread && !sbi->gc_thread->gc_urgent) &&
+   (sbi->gc_mode != GC_URGENT) &&
p->max_search >

Re: [f2fs-dev] [PATCH] f2fs: fix to wait IO writeback in __revoke_inmem_pages()

2018-05-07 Thread Jaegeuk Kim 
On 04/27, Chao Yu wrote:
> On 2018/4/27 0:36, Jaegeuk Kim wrote:
> > On 04/26, Chao Yu wrote:
> >> On 2018/4/26 23:48, Jaegeuk Kim wrote:
> >>> On 04/26, Chao Yu wrote:
>  Thread A Thread B
>  - f2fs_ioc_commit_atomic_write
>   - commit_inmem_pages
>    - f2fs_submit_merged_write_cond
>    : write data
>   - write_checkpoint
>    - do_checkpoint
>    : commit all node within CP
>    -> SPO
>    - f2fs_do_sync_file
> - file_write_and_wait_range
> : wait data writeback
> 
>  In above race condition, data/node can be flushed in reversed order when
>  coming a checkpoint before f2fs_do_sync_file, after SPOR, it results in
>  atomic written data being corrupted.
> >>>
> >>> Wait, what is the problem here? Thread B could succeed checkpoint, there 
> >>> is
> >>> no problem. If it fails, there is no fsync mark where we can recover it, 
> >>> so
> >>
> >> Node is flushed by checkpoint before data, with reversed order, that's the 
> >> problem.
> > 
> > What do you mean? Data should be in disk, in order to proceed checkpoint.
> 
> 1. thread A: commit_inmem_pages submit data into block layer, but haven't 
> waited
> it writeback.
> 2. thread A: commit_inmem_pages update related node.
> 3. thread B: do checkpoint, flush all nodes to disk

How about, in block_operations(),

down_read_trylock(_I(inode)->i_gc_rwsem[WRITE]);
if (fail)
wait_on_all_pages_writeback(F2FS_WB_DATA);
else
up_read(_I(inode)->i_gc_rwsem[WRITE]);


> 4. SPOR
> 
> Then, atomic file becomes corrupted since nodes is flushed before data.
> 
> Thanks,
> 
> > 
> >>
> >> Thanks,
> >>
> >>> we can just ignore the last written data as nothing.
> >>>
> 
>  This patch adds f2fs_wait_on_page_writeback in __revoke_inmem_pages() to
>  keep data and node of atomic file being flushed orderly.
> 
>  Signed-off-by: Chao Yu 
>  ---
>   fs/f2fs/file.c| 4 
>   fs/f2fs/segment.c | 3 +++
>   2 files changed, 7 insertions(+)
> 
>  diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>  index be7578774a47..a352804af244 100644
>  --- a/fs/f2fs/file.c
>  +++ b/fs/f2fs/file.c
>  @@ -217,6 +217,9 @@ static int f2fs_do_sync_file(struct file *file, 
>  loff_t start, loff_t end,
>   
>   trace_f2fs_sync_file_enter(inode);
>   
>  +if (atomic)
>  +goto write_done;
>  +
>   /* if fdatasync is triggered, let's do in-place-update */
>   if (datasync || get_dirty_pages(inode) <= 
>  SM_I(sbi)->min_fsync_blocks)
>   set_inode_flag(inode, FI_NEED_IPU);
>  @@ -228,6 +231,7 @@ static int f2fs_do_sync_file(struct file *file, 
>  loff_t start, loff_t end,
>   return ret;
>   }
>   
>  +write_done:
>   /* if the inode is dirty, let's recover all the time */
>   if (!f2fs_skip_inode_update(inode, datasync)) {
>   f2fs_write_inode(inode, NULL);
>  diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>  index 584483426584..9ca3d0a43d93 100644
>  --- a/fs/f2fs/segment.c
>  +++ b/fs/f2fs/segment.c
>  @@ -230,6 +230,8 @@ static int __revoke_inmem_pages(struct inode *inode,
>   
>   lock_page(page);
>   
>  +f2fs_wait_on_page_writeback(page, DATA, true);
>  +
>   if (recover) {
>   struct dnode_of_data dn;
>   struct node_info ni;
>  @@ -415,6 +417,7 @@ static int __commit_inmem_pages(struct inode *inode)
>   /* drop all uncommitted pages */
>   __revoke_inmem_pages(inode, >inmem_pages, true, 
>  false);
>   } else {
>  +/* wait all committed IOs writeback and release them 
>  from list */
>   __revoke_inmem_pages(inode, _list, false, false);
>   }
>   
>  -- 
>  2.15.0.55.gc2ece9dc4de6
> > 
> > .
> > 

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH] sload.f2fs: fix the missing of bit mask for file type

2018-05-07 Thread Jaegeuk Kim 
On 05/07, Junling Zheng wrote:
> Hi, Jaegeuk
> 
> Could you please merge this fix into external/f2fs-tools repo for AOSP?
> Since we found this bug while using sload feature in Android P :)

Sure. Will prepare soon.
Thanks,

> 
> Thanks,
> Junling
> 
> On 2018/5/3 19:25, Junling Zheng wrote:
> > Fix the missing of bit mask for the file type bit fields.
> > 
> > Signed-off-by: Junling Zheng 
> > Signed-off-by: Sheng Yong 
> > ---
> >  fsck/sload.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/fsck/sload.c b/fsck/sload.c
> > index 2842f2c..1b7a2d1 100644
> > --- a/fsck/sload.c
> > +++ b/fsck/sload.c
> > @@ -157,7 +157,7 @@ static void set_inode_metadata(struct dentry *de)
> >  
> > de->size = stat.st_size;
> > de->mode = stat.st_mode &
> > -   (S_ISUID|S_ISGID|S_ISVTX|S_IRWXU|S_IRWXG|S_IRWXO);
> > +   
> > (S_IFMT|S_ISUID|S_ISGID|S_ISVTX|S_IRWXU|S_IRWXG|S_IRWXO);
> > if (c.fixed_time == -1 && c.from_dir)
> > de->mtime = stat.st_mtime;
> > else
> > 
> 

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[f2fs-dev] [PATCH] sload.f2fs: give correct file type

2018-05-07 Thread Jaegeuk Kim 
From: Lianjun Huang 

This fixes permission error due to wrong file type.

Signed-off-by: Lianjun Huang 
Signed-off-by: Jaegeuk Kim 
---
 fsck/sload.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fsck/sload.c b/fsck/sload.c
index 2842f2c..2fb13f2 100644
--- a/fsck/sload.c
+++ b/fsck/sload.c
@@ -106,7 +106,7 @@ static int set_perms_and_caps(struct dentry *de)
 
/* Permissions */
if (fs_config_func != NULL) {
-   fs_config_func(mnt_path, S_ISDIR(de->mode),
+   fs_config_func(mnt_path, de->file_type == F2FS_FT_DIR,
c.target_out_dir, , , ,
);
de->uid = uid & 0x;
-- 
2.17.0.484.g0c8726318c-goog


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[f2fs-dev] [PATCH] fsck.f2fs: fix check order in -p1

2018-05-07 Thread Jaegeuk Kim 
Checking nat entries with nat_area_bitmap should be done before quota check,
since fsck_chk_quota_node() unsets quota inode numbers in nat_area_bitmap.
It causes for -p1 to conduct full scan.

Signed-off-by: Jaegeuk Kim 
---
 fsck/fsck.c | 26 +-
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/fsck/fsck.c b/fsck/fsck.c
index 91c8529..688f24b 100644
--- a/fsck/fsck.c
+++ b/fsck/fsck.c
@@ -1788,19 +1788,7 @@ int fsck_chk_meta(struct f2fs_sb_info *sbi)
if (fsck_chk_orphan_node(sbi))
return -EINVAL;
 
-   /* 5. check quota inode simply */
-   if (fsck_chk_quota_node(sbi))
-   return -EINVAL;
-
-   if (fsck->nat_valid_inode_cnt != le32_to_cpu(cp->valid_inode_count)) {
-   ASSERT_MSG("valid inode does not match: nat_valid_inode_cnt %u,"
-   " valid_inode_count %u",
-   fsck->nat_valid_inode_cnt,
-   le32_to_cpu(cp->valid_inode_count));
-   return -EINVAL;
-   }
-
-   /*check nat entry with sit_area_bitmap*/
+   /* 5. check nat entry -- must be done before quota check */
for (i = 0; i < fsck->nr_nat_entries; i++) {
u32 blk = le32_to_cpu(fsck->entries[i].block_addr);
nid_t ino = le32_to_cpu(fsck->entries[i].ino);
@@ -1840,6 +1828,18 @@ int fsck_chk_meta(struct f2fs_sb_info *sbi)
}
}
 
+   /* 6. check quota inode simply */
+   if (fsck_chk_quota_node(sbi))
+   return -EINVAL;
+
+   if (fsck->nat_valid_inode_cnt != le32_to_cpu(cp->valid_inode_count)) {
+   ASSERT_MSG("valid inode does not match: nat_valid_inode_cnt %u,"
+   " valid_inode_count %u",
+   fsck->nat_valid_inode_cnt,
+   le32_to_cpu(cp->valid_inode_count));
+   return -EINVAL;
+   }
+
return 0;
 }
 
-- 
2.17.0.484.g0c8726318c-goog


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[f2fs-dev] [PATCH v2 3/3] f2fs: avoid stucking GC due to atomic write

2018-05-07 Thread Chao Yu 
f2fs doesn't allow abuse on atomic write class interface, so except
limiting in-mem pages' total memory usage capacity, we need to limit
atomic-write usage as well when filesystem is seriously fragmented,
otherwise we may run into infinite loop during foreground GC because
target blocks in victim segment are belong to atomic opened file for
long time.

Now, we will detect failure due to atomic write in foreground GC, if
the count exceeds threshold, we will drop all atomic written data in
cache, by this, I expect it can keep our system running safely to
prevent Dos attack.

In addition, his patch adds to show GC skip information in debugfs,
now it just shows count of skipped caused by atomic write.

Signed-off-by: Chao Yu 
---
v2:
- rebase code.
 fs/f2fs/data.c|  2 +-
 fs/f2fs/debug.c   |  6 ++
 fs/f2fs/f2fs.h| 21 +++--
 fs/f2fs/file.c| 20 ++--
 fs/f2fs/gc.c  | 27 +++
 fs/f2fs/inode.c   |  6 --
 fs/f2fs/segment.c | 11 ++-
 fs/f2fs/segment.h |  2 ++
 8 files changed, 75 insertions(+), 20 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index f7365ce45450..e394b5486c91 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -2320,7 +2320,7 @@ static int f2fs_write_begin(struct file *file, struct 
address_space *mapping,
f2fs_put_page(page, 1);
f2fs_write_failed(mapping, pos + len);
if (drop_atomic)
-   drop_inmem_pages_all(sbi);
+   drop_inmem_pages_all(sbi, false);
return err;
 }
 
diff --git a/fs/f2fs/debug.c b/fs/f2fs/debug.c
index d92a01cb420c..8febd9160635 100644
--- a/fs/f2fs/debug.c
+++ b/fs/f2fs/debug.c
@@ -104,6 +104,8 @@ static void update_general_status(struct f2fs_sb_info *sbi)
si->avail_nids = NM_I(sbi)->available_nids;
si->alloc_nids = NM_I(sbi)->nid_cnt[PREALLOC_NID];
si->bg_gc = sbi->bg_gc;
+   si->skipped_atomic_files[BG_GC] = sbi->skipped_atomic_files[BG_GC];
+   si->skipped_atomic_files[FG_GC] = sbi->skipped_atomic_files[FG_GC];
si->util_free = (int)(free_user_blocks(sbi) >> sbi->log_blocks_per_seg)
* 100 / (int)(sbi->user_block_count >> sbi->log_blocks_per_seg)
/ 2;
@@ -342,6 +344,10 @@ static int stat_show(struct seq_file *s, void *v)
si->bg_data_blks);
seq_printf(s, "  - node blocks : %d (%d)\n", si->node_blks,
si->bg_node_blks);
+   seq_printf(s, "Skipped : atomic write %llu (%llu)\n",
+   si->skipped_atomic_files[BG_GC] +
+   si->skipped_atomic_files[FG_GC],
+   si->skipped_atomic_files[BG_GC]);
seq_puts(s, "\nExtent Cache:\n");
seq_printf(s, "  - Hit Count: L1-1:%llu L1-2:%llu L2:%llu\n",
si->hit_largest, si->hit_cached,
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 73a1a39889bc..3286127708f9 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -617,15 +617,20 @@ enum {
 
 #define DEF_DIR_LEVEL  0
 
+enum {
+   GC_FAILURE_PIN,
+   GC_FAILURE_ATOMIC,
+   MAX_GC_FAILURE
+};
+
 struct f2fs_inode_info {
struct inode vfs_inode; /* serve a vfs inode */
unsigned long i_flags;  /* keep an inode flags for ioctl */
unsigned char i_advise; /* use to give file attribute hints */
unsigned char i_dir_level;  /* use for dentry level for large dir */
-   union {
-   unsigned int i_current_depth;   /* only for directory depth */
-   unsigned short i_gc_failures;   /* only for regular file */
-   };
+   unsigned int i_current_depth;   /* only for directory depth */
+   /* for gc failure statistic */
+   unsigned int i_gc_failures[MAX_GC_FAILURE];
unsigned int i_pino;/* parent inode number */
umode_t i_acl_mode; /* keep file acl mode temporarily */
 
@@ -1194,6 +1199,8 @@ struct f2fs_sb_info {
struct rw_semaphore gc_rwsem;   /* rw semaphore for gc_thread */
struct f2fs_gc_kthread  *gc_thread; /* GC thread */
unsigned int cur_victim_sec;/* current victim section num */
+   /* for skip statistic */
+   unsigned long long skipped_atomic_files[2];
 
/* threshold for gc trials on pinned files */
u64 gc_pin_file_threshold;
@@ -2242,6 +2249,7 @@ enum {
FI_EXTRA_ATTR,  /* indicate file has extra attribute */
FI_PROJ_INHERIT,/* indicate file inherits projectid */
FI_PIN_FILE,/* indicate file should not be gced */
+   FI_ATOMIC_REVOKE_REQUEST,/* indicate atomic committed data has been 
dropped */
 };
 
 static inline void __mark_inode_dirty_flag(struct inode *inode,
@@ -2340,7 +2348,7 @@ static inline void f2fs_i_depth_write(struct inode 
*inode,

[f2fs-dev] [PATCH v2 2/3] f2fs: introduce GC_I for cleanup

2018-05-07 Thread Chao Yu 
Introduce GC_I to replace sbi->gc_thread for cleanup, no logic changes.

Signed-off-by: Chao Yu 
---
v2:
- rebase code.
 fs/f2fs/debug.c   |  2 +-
 fs/f2fs/f2fs.h|  5 +
 fs/f2fs/gc.c  | 14 +++---
 fs/f2fs/segment.c |  4 ++--
 fs/f2fs/super.c   |  4 ++--
 fs/f2fs/sysfs.c   |  8 
 6 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/fs/f2fs/debug.c b/fs/f2fs/debug.c
index a66107b5cfff..d92a01cb420c 100644
--- a/fs/f2fs/debug.c
+++ b/fs/f2fs/debug.c
@@ -221,7 +221,7 @@ static void update_mem_info(struct f2fs_sb_info *sbi)
si->cache_mem = 0;
 
/* build gc */
-   if (sbi->gc_thread)
+   if (GC_I(sbi))
si->cache_mem += sizeof(struct f2fs_gc_kthread);
 
/* build merge flush thread */
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index e238d0ea0be7..73a1a39889bc 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1412,6 +1412,11 @@ static inline struct sit_info *SIT_I(struct f2fs_sb_info 
*sbi)
return (struct sit_info *)(SM_I(sbi)->sit_info);
 }
 
+static inline struct f2fs_gc_kthread *GC_I(struct f2fs_sb_info *sbi)
+{
+   return (struct f2fs_gc_kthread *)(sbi->gc_thread);
+}
+
 static inline struct free_segmap_info *FREE_I(struct f2fs_sb_info *sbi)
 {
return (struct free_segmap_info *)(SM_I(sbi)->free_info);
diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index b74714be7be7..3c7914425b4e 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -26,8 +26,8 @@
 static int gc_thread_func(void *data)
 {
struct f2fs_sb_info *sbi = data;
-   struct f2fs_gc_kthread *gc_th = sbi->gc_thread;
-   wait_queue_head_t *wq = >gc_thread->gc_wait_queue_head;
+   struct f2fs_gc_kthread *gc_th = GC_I(sbi);
+   wait_queue_head_t *wq = _th->gc_wait_queue_head;
unsigned int wait_ms;
 
wait_ms = gc_th->min_sleep_time;
@@ -136,8 +136,8 @@ int start_gc_thread(struct f2fs_sb_info *sbi)
gc_th->gc_wake= 0;
 
sbi->gc_thread = gc_th;
-   init_waitqueue_head(>gc_thread->gc_wait_queue_head);
-   sbi->gc_thread->f2fs_gc_task = kthread_run(gc_thread_func, sbi,
+   init_waitqueue_head(_th->gc_wait_queue_head);
+   gc_th->f2fs_gc_task = kthread_run(gc_thread_func, sbi,
"f2fs_gc-%u:%u", MAJOR(dev), MINOR(dev));
if (IS_ERR(gc_th->f2fs_gc_task)) {
err = PTR_ERR(gc_th->f2fs_gc_task);
@@ -150,7 +150,7 @@ int start_gc_thread(struct f2fs_sb_info *sbi)
 
 void stop_gc_thread(struct f2fs_sb_info *sbi)
 {
-   struct f2fs_gc_kthread *gc_th = sbi->gc_thread;
+   struct f2fs_gc_kthread *gc_th = GC_I(sbi);
if (!gc_th)
return;
kthread_stop(gc_th->f2fs_gc_task);
@@ -188,7 +188,7 @@ static void select_policy(struct f2fs_sb_info *sbi, int 
gc_type,
p->ofs_unit = 1;
} else {
down_read(>gc_rwsem);
-   p->gc_mode = select_gc_type(sbi->gc_thread, gc_type);
+   p->gc_mode = select_gc_type(GC_I(sbi), gc_type);
up_read(>gc_rwsem);
p->dirty_segmap = dirty_i->dirty_segmap[DIRTY];
p->max_search = dirty_i->nr_dirty[DIRTY];
@@ -198,7 +198,7 @@ static void select_policy(struct f2fs_sb_info *sbi, int 
gc_type,
/* we need to check every dirty segments in the FG_GC case */
down_read(>gc_rwsem);
if (gc_type != FG_GC &&
-   (sbi->gc_thread && !sbi->gc_thread->gc_urgent) &&
+   (GC_I(sbi) && !GC_I(sbi)->gc_urgent) &&
p->max_search > sbi->max_victim_search)
p->max_search = sbi->max_victim_search;
up_read(>gc_rwsem);
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 33d146939048..c660efad7590 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -180,7 +180,7 @@ bool need_SSR(struct f2fs_sb_info *sbi)
return false;
 
down_read(>gc_rwsem);
-   if (sbi->gc_thread && sbi->gc_thread->gc_urgent)
+   if (GC_I(sbi) && GC_I(sbi)->gc_urgent)
gc_urgent = true;
up_read(>gc_rwsem);
 
@@ -1429,7 +1429,7 @@ static int issue_discard_thread(void *data)
dcc->discard_wake = 0;
 
down_read(>gc_rwsem);
-   if (sbi->gc_thread && sbi->gc_thread->gc_urgent)
+   if (GC_I(sbi) && GC_I(sbi)->gc_urgent)
init_discard_policy(, DPOLICY_FORCE, 1);
up_read(>gc_rwsem);
 
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 55ccc2eaaa2e..6bc0eb2084ff 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -1476,14 +1476,14 @@ static int f2fs_remount(struct super_block *sb, int 
*flags, char *data)
 */
if ((*flags & SB_RDONLY) || !test_opt(sbi, BG_GC)) {
down_write(>gc_rwsem);
-   if (sbi->gc_thread) {
+   if (GC_I(sbi)) {
stop_gc_thread(sbi);
need_restart_gc =

[f2fs-dev] [PATCH v2 1/3] f2fs: fix to initialize i_current_depth according to inode type

2018-05-07 Thread Chao Yu 
i_current_depth is used only for directory inode, but its space is
shared with i_gc_failures field used for regular inode, in order to
avoid affecting i_gc_failures' value, this patch fixes to initialize
the union's fields according to inode type.

Signed-off-by: Chao Yu 
---
v2:
- rebase code.
 fs/f2fs/inode.c | 12 +---
 fs/f2fs/namei.c |  3 +++
 fs/f2fs/super.c |  1 -
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
index 7f2fe4574c48..3a74a1cf3264 100644
--- a/fs/f2fs/inode.c
+++ b/fs/f2fs/inode.c
@@ -232,8 +232,10 @@ static int do_read_inode(struct inode *inode)
inode->i_ctime.tv_nsec = le32_to_cpu(ri->i_ctime_nsec);
inode->i_mtime.tv_nsec = le32_to_cpu(ri->i_mtime_nsec);
inode->i_generation = le32_to_cpu(ri->i_generation);
-
-   fi->i_current_depth = le32_to_cpu(ri->i_current_depth);
+   if (S_ISDIR(inode->i_mode))
+   fi->i_current_depth = le32_to_cpu(ri->i_current_depth);
+   else if (S_ISREG(inode->i_mode))
+   fi->i_gc_failures = le16_to_cpu(ri->i_gc_failures);
fi->i_xattr_nid = le32_to_cpu(ri->i_xattr_nid);
fi->i_flags = le32_to_cpu(ri->i_flags);
fi->flags = 0;
@@ -422,7 +424,11 @@ void update_inode(struct inode *inode, struct page 
*node_page)
ri->i_atime_nsec = cpu_to_le32(inode->i_atime.tv_nsec);
ri->i_ctime_nsec = cpu_to_le32(inode->i_ctime.tv_nsec);
ri->i_mtime_nsec = cpu_to_le32(inode->i_mtime.tv_nsec);
-   ri->i_current_depth = cpu_to_le32(F2FS_I(inode)->i_current_depth);
+   if (S_ISDIR(inode->i_mode))
+   ri->i_current_depth =
+   cpu_to_le32(F2FS_I(inode)->i_current_depth);
+   else if (S_ISREG(inode->i_mode))
+   ri->i_gc_failures = cpu_to_le16(F2FS_I(inode)->i_gc_failures);
ri->i_xattr_nid = cpu_to_le32(F2FS_I(inode)->i_xattr_nid);
ri->i_flags = cpu_to_le32(F2FS_I(inode)->i_flags);
ri->i_pino = cpu_to_le32(F2FS_I(inode)->i_pino);
diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
index fef6e3ab2135..bcfc4219b29e 100644
--- a/fs/f2fs/namei.c
+++ b/fs/f2fs/namei.c
@@ -54,6 +54,9 @@ static struct inode *f2fs_new_inode(struct inode *dir, 
umode_t mode)
F2FS_I(inode)->i_crtime = current_time(inode);
inode->i_generation = sbi->s_next_generation++;
 
+   if (S_ISDIR(inode->i_mode))
+   F2FS_I(inode)->i_current_depth = 1;
+
err = insert_inode_locked(inode);
if (err) {
err = -EINVAL;
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 294be9e92aee..55ccc2eaaa2e 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -826,7 +826,6 @@ static struct inode *f2fs_alloc_inode(struct super_block 
*sb)
 
/* Initialize f2fs-specific inode info */
atomic_set(>dirty_pages, 0);
-   fi->i_current_depth = 1;
init_rwsem(>i_sem);
INIT_LIST_HEAD(>dirty_list);
INIT_LIST_HEAD(>gdirty_list);
-- 
2.17.0.391.g1f1cddd558b5


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[f2fs-dev] [PATCH v4] f2fs: fix to avoid race during access gc_thread pointer

2018-05-07 Thread Chao Yu 
Thread AThread BThread C
- f2fs_remount
 - stop_gc_thread
- f2fs_sbi_store
- issue_discard_thread
   sbi->gc_thread = NULL;
  sbi->gc_thread->gc_wake = 1
  access 
sbi->gc_thread->gc_urgent

Previously, we allocate memory for sbi->gc_thread based on background
gc thread mount option, the memory can be released if we turn off
that mount option, but still there are several places access gc_thread
pointer without considering race condition, result in NULL point
dereference.

In order to fix this issue, introduce gc_rwsem to exclude those operations.

Signed-off-by: Chao Yu 
---
v4:
- use introduced sbi.gc_rwsem lock instead of sb.s_umount.
 fs/f2fs/f2fs.h|  1 +
 fs/f2fs/gc.c  |  4 
 fs/f2fs/segment.c | 11 ++-
 fs/f2fs/super.c   | 19 ++-
 fs/f2fs/sysfs.c   | 14 +++---
 5 files changed, 40 insertions(+), 9 deletions(-)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 80490a7991a7..e238d0ea0be7 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1191,6 +1191,7 @@ struct f2fs_sb_info {
 
/* for cleaning operations */
struct mutex gc_mutex;  /* mutex for GC */
+   struct rw_semaphore gc_rwsem;   /* rw semaphore for gc_thread */
struct f2fs_gc_kthread  *gc_thread; /* GC thread */
unsigned int cur_victim_sec;/* current victim section num */
 
diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index 9bb2ddbbed1e..b74714be7be7 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -187,17 +187,21 @@ static void select_policy(struct f2fs_sb_info *sbi, int 
gc_type,
p->max_search = dirty_i->nr_dirty[type];
p->ofs_unit = 1;
} else {
+   down_read(>gc_rwsem);
p->gc_mode = select_gc_type(sbi->gc_thread, gc_type);
+   up_read(>gc_rwsem);
p->dirty_segmap = dirty_i->dirty_segmap[DIRTY];
p->max_search = dirty_i->nr_dirty[DIRTY];
p->ofs_unit = sbi->segs_per_sec;
}
 
/* we need to check every dirty segments in the FG_GC case */
+   down_read(>gc_rwsem);
if (gc_type != FG_GC &&
(sbi->gc_thread && !sbi->gc_thread->gc_urgent) &&
p->max_search > sbi->max_victim_search)
p->max_search = sbi->max_victim_search;
+   up_read(>gc_rwsem);
 
/* let's select beginning hot/small space first in no_heap mode*/
if (test_opt(sbi, NOHEAP) &&
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 320cc1c57246..33d146939048 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -174,11 +174,18 @@ bool need_SSR(struct f2fs_sb_info *sbi)
int node_secs = get_blocktype_secs(sbi, F2FS_DIRTY_NODES);
int dent_secs = get_blocktype_secs(sbi, F2FS_DIRTY_DENTS);
int imeta_secs = get_blocktype_secs(sbi, F2FS_DIRTY_IMETA);
+   bool gc_urgent = false;
 
if (test_opt(sbi, LFS))
return false;
+
+   down_read(>gc_rwsem);
if (sbi->gc_thread && sbi->gc_thread->gc_urgent)
-   return true;
+   gc_urgent = true;
+   up_read(>gc_rwsem);
+
+   if (gc_urgent)
+   return false;
 
return free_sections(sbi) <= (node_secs + 2 * dent_secs + imeta_secs +
SM_I(sbi)->min_ssr_sections + reserved_sections(sbi));
@@ -1421,8 +1428,10 @@ static int issue_discard_thread(void *data)
if (dcc->discard_wake)
dcc->discard_wake = 0;
 
+   down_read(>gc_rwsem);
if (sbi->gc_thread && sbi->gc_thread->gc_urgent)
init_discard_policy(, DPOLICY_FORCE, 1);
+   up_read(>gc_rwsem);
 
sb_start_intwrite(sbi->sb);
 
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index c8e5fe5d71fe..294be9e92aee 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -1476,15 +1476,23 @@ static int f2fs_remount(struct super_block *sb, int 
*flags, char *data)
 * option. Also sync the filesystem.
 */
if ((*flags & SB_RDONLY) || !test_opt(sbi, BG_GC)) {
+   down_write(>gc_rwsem);
if (sbi->gc_thread) {
stop_gc_thread(sbi);
need_restart_gc = true;
}
-   } else if (!sbi->gc_thread) {
-   err = start_gc_thread(sbi);
-   if (err)
-   goto restore_opts;
-   need_stop_gc = true;
+   up_write(>gc_rwsem);
+   } else {
+   down_write(>gc_rwsem);
+   if (!sbi->gc_thread) {
+   err = start_gc_thread(sbi);
+   if (err) {
+

Re: [f2fs-dev] [RFC PATCH] f2fs-tools: introduce tune.f2fs

2018-05-07 Thread Chao Yu 
On 2018/5/5 3:06, Jaegeuk Kim wrote:
> On 05/02, Chao Yu wrote:
>> On 2018/4/28 10:49, Jaegeuk Kim wrote:
>>> On 04/27, Chao Yu wrote:
 On 2018/4/27 0:13, Jaegeuk Kim wrote:
> On 04/26, Junling Zheng wrote:
>> Ping...
>>
>> On 2018/4/23 15:32, Junling Zheng wrote:
>>> Introduce tune.f2fs tool to change the f2fs parameters.
>>> Currently this tool only supports adding or removing encrypt
>>> feature bit in superblock.
>
> What is the purpose of this empty tune.f2fs? How can we say we have this
> tool to users? You have to design what kind of things to support first.

 I checked very initial tune2fs.c, it only supports very few parameters 
 tuning
 functionality, but, can not say that is a bad start to introduce the misc 
 tool.

 +   fprintf (stderr, "Usage: %s [-c max-mounts-count] [-e 
 errors-behavior] "
 +"[-i interval[d|m]]\n"
 +"\t[-l] [-m reserved-blocks-percent] device\n", 
 program_name);
>>>
>>> I don't think We have to follow that.
>>>

 Maybe tuning 1. extension list, 2. multi device name later? just guess.
>>>
>>> First of all, does it make sense to unset feature bits? I don't think so.
>>
>> Some features can be turned off in a initial image? like encrypted, 
>> extra_attr,
>> checksum...?
> 
> Any reason to do thtat? And, we need to find all the files whether there is

Not very sure, just checked tune2fs, and found them in manual, guessing that
there came demand from user who want to disable some feature in a formatted
partition?

> any encrypted file.
> 
>>
>>>
>>> Agreed to your suggestion where:
>>> 1. feature set
>>>   - enable system quota
>>> 2. extension list
>>> 3. multi device name -- which may be really big trial
>>
>> So what's our plan now? fill those features into fsck when we need them?
> 
> No. If you want to add tune.f2fs, I'd like to see some demand, or very useful
> features like this. I just want to avoid introducing an (almost) empty tool.

Yeah, I see.

Maybe we can support below options except feature/extension_list/device_name.
-Q quota-optionsset,clear user/group/prjquota sysfile inode.
-U UUID set,clear uuid
-L volume-label set,clear volume name

Thanks,

> 
> Thanks,
> 
>>
>> Thanks,
>>
>>>

 Using fsck.f2fs to tune parameter, IMO, maybe a little confuse for user to
 understand the tool's functionality.

 Thanks,
>>>
>>> .
>>>
> 
> .
> 


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH] sload.f2fs: fix the missing of bit mask for file type

2018-05-07 Thread Junling Zheng 
Hi, Jaegeuk

Could you please merge this fix into external/f2fs-tools repo for AOSP?
Since we found this bug while using sload feature in Android P :)

Thanks,
Junling

On 2018/5/3 19:25, Junling Zheng wrote:
> Fix the missing of bit mask for the file type bit fields.
> 
> Signed-off-by: Junling Zheng 
> Signed-off-by: Sheng Yong 
> ---
>  fsck/sload.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fsck/sload.c b/fsck/sload.c
> index 2842f2c..1b7a2d1 100644
> --- a/fsck/sload.c
> +++ b/fsck/sload.c
> @@ -157,7 +157,7 @@ static void set_inode_metadata(struct dentry *de)
>  
>   de->size = stat.st_size;
>   de->mode = stat.st_mode &
> - (S_ISUID|S_ISGID|S_ISVTX|S_IRWXU|S_IRWXG|S_IRWXO);
> + 
> (S_IFMT|S_ISUID|S_ISGID|S_ISVTX|S_IRWXU|S_IRWXG|S_IRWXO);
>   if (c.fixed_time == -1 && c.from_dir)
>   de->mtime = stat.st_mtime;
>   else
> 



--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel