Re: [Ocfs2-devel] [PATCH 2/3] ocfs2: add ocfs2_overwrite_io function

2017-11-27 Thread Gang He
Hi Alex,


>>> 
> Hi Gang,
> 
> On 2017/11/28 13:33, Gang He wrote:
>> Hello Alex,
>> 
>> 
>
>>> Hi Gang,
>>>
>>> On 2017/11/27 17:46, Gang He wrote:
 Add ocfs2_overwrite_io function, which is used to judge if
 overwrite allocated blocks, otherwise, the write will bring extra
 block allocation overhead.

 Signed-off-by: Gang He 
 ---
  fs/ocfs2/extent_map.c | 67 
>>> +++
  fs/ocfs2/extent_map.h |  3 +++
  2 files changed, 70 insertions(+)

 diff --git a/fs/ocfs2/extent_map.c b/fs/ocfs2/extent_map.c
 index e4719e0..98bf325 100644
 --- a/fs/ocfs2/extent_map.c
 +++ b/fs/ocfs2/extent_map.c
 @@ -832,6 +832,73 @@ int ocfs2_fiemap(struct inode *inode, struct 
>>> fiemap_extent_info *fieinfo,
return ret;
  }
  
 +/* Is IO overwriting allocated blocks? */
 +int ocfs2_overwrite_io(struct inode *inode, u64 map_start, u64 map_len,
 + int wait)
 +{
 +  int ret = 0, is_last;
 +  u32 mapping_end, cpos;
 +  struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
 +  struct buffer_head *di_bh = NULL;
 +  struct ocfs2_extent_rec rec;
 +
 +  if (wait)
 +  ret = ocfs2_inode_lock(inode, &di_bh, 0);
 +  else
 +  ret = ocfs2_try_inode_lock(inode, &di_bh, 0);
 +  if (ret)
 +  goto out;
 +
 +  if (wait)
 +  down_read(&OCFS2_I(inode)->ip_alloc_sem);
 +  else {
 +  if (!down_read_trylock(&OCFS2_I(inode)->ip_alloc_sem)) {
 +  ret = -EAGAIN;
 +  goto out_unlock1;
 +  }
 +  }
 +
 +  if ((OCFS2_I(inode)->ip_dyn_features & OCFS2_INLINE_DATA_FL) &&
 + ((map_start + map_len) <= i_size_read(inode)))
 +  goto out_unlock2;
 +
 +  cpos = map_start >> osb->s_clustersize_bits;
 +  mapping_end = ocfs2_clusters_for_bytes(inode->i_sb,
 + map_start + map_len);
 +  is_last = 0;
 +  while (cpos < mapping_end && !is_last) {
 +  ret = ocfs2_get_clusters_nocache(inode, di_bh, cpos,
 +   NULL, &rec, &is_last);
 +  if (ret) {
 +  mlog_errno(ret);
 +  goto out_unlock2;
 +  }
 +
 +  if (rec.e_blkno == 0ULL)
 +  break;
>>> I think here the blocks is not overwrite, because the hold is found and the 
>>> blocks
>>> should be allocated.
>> If the rec.e_blkno == NULL, this means there is a hole.
>> The file hole means that these blocks are not allocated, it does not like 
> unwritten block.
>> The unwritten blocks means that these blocks are allocated, but still have 
> not been unwritten. 
>> 
> If we break the loop when we find the hold, out of this function we will 
> allocate the blocks in
> ocfs2_file_write_iter()->..->ocfs2_direct_IO->__blockdev_direct_IO->..->ocfs2_dio_wr_g
> et_block()
> ->ocfs2_write_begin_nolock. Does this violate the semantics of 'IOCB_NOWAIT';
Yes, then we need to check if this is a overwrite before doing direct-io.

> 
> BTW, should we consider the down_write() and ocfs2_inode_lock() in 
> ocfs2_dio_wr_get_block() when
> the flag 'IOCB_NOWAIT' is set;
I think that we should not consider that layer lock, otherwise, the code change 
will become more and more complex and big.
I also refer to ext4 file system code change for this 
feature(728fbc0e10b7f3ce2ee043b32e3453fd5201c055), they did not do any change 
in that layer.

Thanks
Gang

> 
 +
 +  if (rec.e_flags & OCFS2_EXT_REFCOUNTED)
 +  break;
 +
 +  cpos = le32_to_cpu(rec.e_cpos) +
 +  le16_to_cpu(rec.e_leaf_clusters);
 +  }
 +
 +  if (cpos < mapping_end)
 +  ret = 1;
 +
 +out_unlock2:
>>>
>>> I think the 'out_up_read' is more readable than the 'out_unlock2' .
>> Ok, I will use more readable tag here.
>>>
 +  brelse(di_bh);
 +
 +  up_read(&OCFS2_I(inode)->ip_alloc_sem);
 +
 +out_unlock1:
>>>
>>> We should release buffer head here.
>>>
 +  ocfs2_inode_unlock(inode, 0);
 +
 +out:
 +  return (ret ? 0 : 1);
 +}
 +
  int ocfs2_seek_data_hole_offset(struct file *file, loff_t *offset, int 
>>> whence)
  {
struct inode *inode = file->f_mapping->host;
 diff --git a/fs/ocfs2/extent_map.h b/fs/ocfs2/extent_map.h
 index 67ea57d..fd9e86a 100644
 --- a/fs/ocfs2/extent_map.h
 +++ b/fs/ocfs2/extent_map.h
 @@ -53,6 +53,9 @@ int ocfs2_extent_map_get_blocks(struct inode *inode, u64 
>>> v_blkno, u64 *p_blkno,
  int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
 u64 map_start, u64 map_len);
  
 +int ocfs2_overwrite_io(struct inode *inode, u64 map_start, u64 map_len,
 + int wait);
 +
  int o

Re: [Ocfs2-devel] [PATCH 2/3] ocfs2: add ocfs2_overwrite_io function

2017-11-27 Thread Gang He
Hello Joseph,


>>> 

> 
> On 17/11/28 11:35, Gang He wrote:
>> Hello Joseph,
>> 
>> 
>
>>> Hi Gang,
>>>
>>> On 17/11/27 17:46, Gang He wrote:
 Add ocfs2_overwrite_io function, which is used to judge if
 overwrite allocated blocks, otherwise, the write will bring extra
 block allocation overhead.

 Signed-off-by: Gang He 
 ---
  fs/ocfs2/extent_map.c | 67 
>>> +++
  fs/ocfs2/extent_map.h |  3 +++
  2 files changed, 70 insertions(+)

 diff --git a/fs/ocfs2/extent_map.c b/fs/ocfs2/extent_map.c
 index e4719e0..98bf325 100644
 --- a/fs/ocfs2/extent_map.c
 +++ b/fs/ocfs2/extent_map.c
 @@ -832,6 +832,73 @@ int ocfs2_fiemap(struct inode *inode, struct 
>>> fiemap_extent_info *fieinfo,
return ret;
  }
  
 +/* Is IO overwriting allocated blocks? */
 +int ocfs2_overwrite_io(struct inode *inode, u64 map_start, u64 map_len,
 + int wait)
 +{
 +  int ret = 0, is_last;
 +  u32 mapping_end, cpos;
 +  struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
 +  struct buffer_head *di_bh = NULL;
 +  struct ocfs2_extent_rec rec;
 +
 +  if (wait)
 +  ret = ocfs2_inode_lock(inode, &di_bh, 0);
 +  else
 +  ret = ocfs2_try_inode_lock(inode, &di_bh, 0);
 +  if (ret)
 +  goto out;
 +
 +  if (wait)
 +  down_read(&OCFS2_I(inode)->ip_alloc_sem);
 +  else {
 +  if (!down_read_trylock(&OCFS2_I(inode)->ip_alloc_sem)) {
 +  ret = -EAGAIN;
 +  goto out_unlock1;
 +  }
 +  }
 +
 +  if ((OCFS2_I(inode)->ip_dyn_features & OCFS2_INLINE_DATA_FL) &&
 + ((map_start + map_len) <= i_size_read(inode)))
 +  goto out_unlock2;
 +
 +  cpos = map_start >> osb->s_clustersize_bits;
 +  mapping_end = ocfs2_clusters_for_bytes(inode->i_sb,
 + map_start + map_len);
 +  is_last = 0;
 +  while (cpos < mapping_end && !is_last) {
 +  ret = ocfs2_get_clusters_nocache(inode, di_bh, cpos,
 +   NULL, &rec, &is_last);
 +  if (ret) {
 +  mlog_errno(ret);
 +  goto out_unlock2;
 +  }
 +
 +  if (rec.e_blkno == 0ULL)
 +  break;
 +
 +  if (rec.e_flags & OCFS2_EXT_REFCOUNTED)
 +  break;
 +
 +  cpos = le32_to_cpu(rec.e_cpos) +
 +  le16_to_cpu(rec.e_leaf_clusters);
 +  }
 +
 +  if (cpos < mapping_end)
 +  ret = 1;
 +
 +out_unlock2:
 +  brelse(di_bh);
 +
 +  up_read(&OCFS2_I(inode)->ip_alloc_sem);
 +
 +out_unlock1:
>>> Should brelse(di_bh) be here?
>> If the code jumps to out_unlock1 directly, the di_bh pointer should be NULL, 
> it is not necessary to release.
>> 
> Umm... No, once going out here, we have already taken inode lock. So
> di_bh should be released.
Sorry, you are right.

> 
>>>
 +  ocfs2_inode_unlock(inode, 0);
 +
 +out:
 +  return (ret ? 0 : 1);
>>> I don't think EAGAIN and other error code can be handled the same. We
>>> have to distinguish them.
>> Ok, I think we can add one line log to report the error in case the error is 
> not EAGAIN. 
>> 
> My point is, there is no need to try again in several cases, e.g. EROFS
> returned by ocfs2_get_clusters_nocache.
In this function ocfs2_overwrite_io() only can return True(1) or False(0), then 
I think we can only give a error print before return true/false.
It is not necessary to return another value, but should let the user know any 
possible error message.

Thanks
Gang 

> 
>>>
>>> Thanks,
>>> Joseph
>>>
 +}
 +
  int ocfs2_seek_data_hole_offset(struct file *file, loff_t *offset, int 
>>> whence)
  {
struct inode *inode = file->f_mapping->host;
 diff --git a/fs/ocfs2/extent_map.h b/fs/ocfs2/extent_map.h
 index 67ea57d..fd9e86a 100644
 --- a/fs/ocfs2/extent_map.h
 +++ b/fs/ocfs2/extent_map.h
 @@ -53,6 +53,9 @@ int ocfs2_extent_map_get_blocks(struct inode *inode, u64 
>>> v_blkno, u64 *p_blkno,
  int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
 u64 map_start, u64 map_len);
  
 +int ocfs2_overwrite_io(struct inode *inode, u64 map_start, u64 map_len,
 + int wait);
 +
  int ocfs2_seek_data_hole_offset(struct file *file, loff_t *offset, int 
>>> origin);
  
  int ocfs2_xattr_get_clusters(struct inode *inode, u32 v_cluster,

>> 

___
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel


Re: [Ocfs2-devel] [PATCH 2/3] ocfs2: add ocfs2_overwrite_io function

2017-11-27 Thread Changwei Ge
On 2017/11/28 13:44, Gang He wrote:
> Hi Changwei,
> 
> 

>> Hi,
>> Gang
>>
>> On 2017/11/27 17:48, Gang He wrote:
>>> Add ocfs2_overwrite_io function, which is used to judge if
>>> overwrite allocated blocks, otherwise, the write will bring extra
>>> block allocation overhead.
>>>
>>
>> Can you elaborate how this overhead is introduced?
>> Forgive me, I don't figure it.
> If the blocks have been allocated, we just write these block directly.
> If these blocks have not been allocated, that means we need to allocate these 
> block firstly before write,
> this allocation will bring the IO invoking be blocked, if the upper 
> application does not want take this kind of overhead,
> he can pass a nowait flag to avoid and return immediately with a -EAGAIN 
> error.

Thanks for your answer and contribution.
This makes sense.

Changwei

> 
> Thanks
> Gang
> 
>>
>> Thanks,
>> Changwei
>>
>>> Signed-off-by: Gang He 
>>> ---
>>>fs/ocfs2/extent_map.c | 67
>> +++
>>>fs/ocfs2/extent_map.h |  3 +++
>>>2 files changed, 70 insertions(+)
>>>
>>> diff --git a/fs/ocfs2/extent_map.c b/fs/ocfs2/extent_map.c
>>> index e4719e0..98bf325 100644
>>> --- a/fs/ocfs2/extent_map.c
>>> +++ b/fs/ocfs2/extent_map.c
>>> @@ -832,6 +832,73 @@ int ocfs2_fiemap(struct inode *inode, struct
>> fiemap_extent_info *fieinfo,
>>> return ret;
>>>}
>>>
>>> +/* Is IO overwriting allocated blocks? */
>>> +int ocfs2_overwrite_io(struct inode *inode, u64 map_start, u64 map_len,
>>> +  int wait)
>>> +{
>>> +   int ret = 0, is_last;
>>> +   u32 mapping_end, cpos;
>>> +   struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
>>> +   struct buffer_head *di_bh = NULL;
>>> +   struct ocfs2_extent_rec rec;
>>> +
>>> +   if (wait)
>>> +   ret = ocfs2_inode_lock(inode, &di_bh, 0);
>>> +   else
>>> +   ret = ocfs2_try_inode_lock(inode, &di_bh, 0);
>>> +   if (ret)
>>> +   goto out;
>>> +
>>> +   if (wait)
>>> +   down_read(&OCFS2_I(inode)->ip_alloc_sem);
>>> +   else {
>>> +   if (!down_read_trylock(&OCFS2_I(inode)->ip_alloc_sem)) {
>>> +   ret = -EAGAIN;
>>> +   goto out_unlock1;
>>> +   }
>>> +   }
>>> +
>>> +   if ((OCFS2_I(inode)->ip_dyn_features & OCFS2_INLINE_DATA_FL) &&
>>> +  ((map_start + map_len) <= i_size_read(inode)))
>>> +   goto out_unlock2;
>>> +
>>> +   cpos = map_start >> osb->s_clustersize_bits;
>>> +   mapping_end = ocfs2_clusters_for_bytes(inode->i_sb,
>>> +  map_start + map_len);
>>> +   is_last = 0;
>>> +   while (cpos < mapping_end && !is_last) {
>>> +   ret = ocfs2_get_clusters_nocache(inode, di_bh, cpos,
>>> +NULL, &rec, &is_last);
>>> +   if (ret) {
>>> +   mlog_errno(ret);
>>> +   goto out_unlock2;
>>> +   }
>>> +
>>> +   if (rec.e_blkno == 0ULL)
>>> +   break;
>>> +
>>> +   if (rec.e_flags & OCFS2_EXT_REFCOUNTED)
>>> +   break;
>>> +
>>> +   cpos = le32_to_cpu(rec.e_cpos) +
>>> +   le16_to_cpu(rec.e_leaf_clusters);
>>> +   }
>>> +
>>> +   if (cpos < mapping_end)
>>> +   ret = 1;
>>> +
>>> +out_unlock2:
>>> +   brelse(di_bh);
>>> +
>>> +   up_read(&OCFS2_I(inode)->ip_alloc_sem);
>>> +
>>> +out_unlock1:
>>> +   ocfs2_inode_unlock(inode, 0);
>>> +
>>> +out:
>>> +   return (ret ? 0 : 1);
>>> +}
>>> +
>>>int ocfs2_seek_data_hole_offset(struct file *file, loff_t *offset, int
>> whence)
>>>{
>>> struct inode *inode = file->f_mapping->host;
>>> diff --git a/fs/ocfs2/extent_map.h b/fs/ocfs2/extent_map.h
>>> index 67ea57d..fd9e86a 100644
>>> --- a/fs/ocfs2/extent_map.h
>>> +++ b/fs/ocfs2/extent_map.h
>>> @@ -53,6 +53,9 @@ int ocfs2_extent_map_get_blocks(struct inode *inode, u64
>> v_blkno, u64 *p_blkno,
>>>int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>>>  u64 map_start, u64 map_len);
>>>
>>> +int ocfs2_overwrite_io(struct inode *inode, u64 map_start, u64 map_len,
>>> +  int wait);
>>> +
>>>int ocfs2_seek_data_hole_offset(struct file *file, loff_t *offset, int
>> origin);
>>>
>>>int ocfs2_xattr_get_clusters(struct inode *inode, u32 v_cluster,
>>>
> 
> 


___
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel


Re: [Ocfs2-devel] [PATCH 2/3] ocfs2: add ocfs2_overwrite_io function

2017-11-27 Thread Joseph Qi


On 17/11/28 11:35, Gang He wrote:
> Hello Joseph,
> 
> 

>> Hi Gang,
>>
>> On 17/11/27 17:46, Gang He wrote:
>>> Add ocfs2_overwrite_io function, which is used to judge if
>>> overwrite allocated blocks, otherwise, the write will bring extra
>>> block allocation overhead.
>>>
>>> Signed-off-by: Gang He 
>>> ---
>>>  fs/ocfs2/extent_map.c | 67 
>> +++
>>>  fs/ocfs2/extent_map.h |  3 +++
>>>  2 files changed, 70 insertions(+)
>>>
>>> diff --git a/fs/ocfs2/extent_map.c b/fs/ocfs2/extent_map.c
>>> index e4719e0..98bf325 100644
>>> --- a/fs/ocfs2/extent_map.c
>>> +++ b/fs/ocfs2/extent_map.c
>>> @@ -832,6 +832,73 @@ int ocfs2_fiemap(struct inode *inode, struct 
>> fiemap_extent_info *fieinfo,
>>> return ret;
>>>  }
>>>  
>>> +/* Is IO overwriting allocated blocks? */
>>> +int ocfs2_overwrite_io(struct inode *inode, u64 map_start, u64 map_len,
>>> +  int wait)
>>> +{
>>> +   int ret = 0, is_last;
>>> +   u32 mapping_end, cpos;
>>> +   struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
>>> +   struct buffer_head *di_bh = NULL;
>>> +   struct ocfs2_extent_rec rec;
>>> +
>>> +   if (wait)
>>> +   ret = ocfs2_inode_lock(inode, &di_bh, 0);
>>> +   else
>>> +   ret = ocfs2_try_inode_lock(inode, &di_bh, 0);
>>> +   if (ret)
>>> +   goto out;
>>> +
>>> +   if (wait)
>>> +   down_read(&OCFS2_I(inode)->ip_alloc_sem);
>>> +   else {
>>> +   if (!down_read_trylock(&OCFS2_I(inode)->ip_alloc_sem)) {
>>> +   ret = -EAGAIN;
>>> +   goto out_unlock1;
>>> +   }
>>> +   }
>>> +
>>> +   if ((OCFS2_I(inode)->ip_dyn_features & OCFS2_INLINE_DATA_FL) &&
>>> +  ((map_start + map_len) <= i_size_read(inode)))
>>> +   goto out_unlock2;
>>> +
>>> +   cpos = map_start >> osb->s_clustersize_bits;
>>> +   mapping_end = ocfs2_clusters_for_bytes(inode->i_sb,
>>> +  map_start + map_len);
>>> +   is_last = 0;
>>> +   while (cpos < mapping_end && !is_last) {
>>> +   ret = ocfs2_get_clusters_nocache(inode, di_bh, cpos,
>>> +NULL, &rec, &is_last);
>>> +   if (ret) {
>>> +   mlog_errno(ret);
>>> +   goto out_unlock2;
>>> +   }
>>> +
>>> +   if (rec.e_blkno == 0ULL)
>>> +   break;
>>> +
>>> +   if (rec.e_flags & OCFS2_EXT_REFCOUNTED)
>>> +   break;
>>> +
>>> +   cpos = le32_to_cpu(rec.e_cpos) +
>>> +   le16_to_cpu(rec.e_leaf_clusters);
>>> +   }
>>> +
>>> +   if (cpos < mapping_end)
>>> +   ret = 1;
>>> +
>>> +out_unlock2:
>>> +   brelse(di_bh);
>>> +
>>> +   up_read(&OCFS2_I(inode)->ip_alloc_sem);
>>> +
>>> +out_unlock1:
>> Should brelse(di_bh) be here?
> If the code jumps to out_unlock1 directly, the di_bh pointer should be NULL, 
> it is not necessary to release.
> 
Umm... No, once going out here, we have already taken inode lock. So
di_bh should be released.

>>
>>> +   ocfs2_inode_unlock(inode, 0);
>>> +
>>> +out:
>>> +   return (ret ? 0 : 1);
>> I don't think EAGAIN and other error code can be handled the same. We
>> have to distinguish them.
> Ok, I think we can add one line log to report the error in case the error is 
> not EAGAIN. 
> 
My point is, there is no need to try again in several cases, e.g. EROFS
returned by ocfs2_get_clusters_nocache.

>>
>> Thanks,
>> Joseph
>>
>>> +}
>>> +
>>>  int ocfs2_seek_data_hole_offset(struct file *file, loff_t *offset, int 
>> whence)
>>>  {
>>> struct inode *inode = file->f_mapping->host;
>>> diff --git a/fs/ocfs2/extent_map.h b/fs/ocfs2/extent_map.h
>>> index 67ea57d..fd9e86a 100644
>>> --- a/fs/ocfs2/extent_map.h
>>> +++ b/fs/ocfs2/extent_map.h
>>> @@ -53,6 +53,9 @@ int ocfs2_extent_map_get_blocks(struct inode *inode, u64 
>> v_blkno, u64 *p_blkno,
>>>  int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>>>  u64 map_start, u64 map_len);
>>>  
>>> +int ocfs2_overwrite_io(struct inode *inode, u64 map_start, u64 map_len,
>>> +  int wait);
>>> +
>>>  int ocfs2_seek_data_hole_offset(struct file *file, loff_t *offset, int 
>> origin);
>>>  
>>>  int ocfs2_xattr_get_clusters(struct inode *inode, u32 v_cluster,
>>>
> 

___
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel


Re: [Ocfs2-devel] [PATCH 2/3] ocfs2: add ocfs2_overwrite_io function

2017-11-27 Thread alex chen
Hi Gang,

On 2017/11/28 13:33, Gang He wrote:
> Hello Alex,
> 
> 

>> Hi Gang,
>>
>> On 2017/11/27 17:46, Gang He wrote:
>>> Add ocfs2_overwrite_io function, which is used to judge if
>>> overwrite allocated blocks, otherwise, the write will bring extra
>>> block allocation overhead.
>>>
>>> Signed-off-by: Gang He 
>>> ---
>>>  fs/ocfs2/extent_map.c | 67 
>> +++
>>>  fs/ocfs2/extent_map.h |  3 +++
>>>  2 files changed, 70 insertions(+)
>>>
>>> diff --git a/fs/ocfs2/extent_map.c b/fs/ocfs2/extent_map.c
>>> index e4719e0..98bf325 100644
>>> --- a/fs/ocfs2/extent_map.c
>>> +++ b/fs/ocfs2/extent_map.c
>>> @@ -832,6 +832,73 @@ int ocfs2_fiemap(struct inode *inode, struct 
>> fiemap_extent_info *fieinfo,
>>> return ret;
>>>  }
>>>  
>>> +/* Is IO overwriting allocated blocks? */
>>> +int ocfs2_overwrite_io(struct inode *inode, u64 map_start, u64 map_len,
>>> +  int wait)
>>> +{
>>> +   int ret = 0, is_last;
>>> +   u32 mapping_end, cpos;
>>> +   struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
>>> +   struct buffer_head *di_bh = NULL;
>>> +   struct ocfs2_extent_rec rec;
>>> +
>>> +   if (wait)
>>> +   ret = ocfs2_inode_lock(inode, &di_bh, 0);
>>> +   else
>>> +   ret = ocfs2_try_inode_lock(inode, &di_bh, 0);
>>> +   if (ret)
>>> +   goto out;
>>> +
>>> +   if (wait)
>>> +   down_read(&OCFS2_I(inode)->ip_alloc_sem);
>>> +   else {
>>> +   if (!down_read_trylock(&OCFS2_I(inode)->ip_alloc_sem)) {
>>> +   ret = -EAGAIN;
>>> +   goto out_unlock1;
>>> +   }
>>> +   }
>>> +
>>> +   if ((OCFS2_I(inode)->ip_dyn_features & OCFS2_INLINE_DATA_FL) &&
>>> +  ((map_start + map_len) <= i_size_read(inode)))
>>> +   goto out_unlock2;
>>> +
>>> +   cpos = map_start >> osb->s_clustersize_bits;
>>> +   mapping_end = ocfs2_clusters_for_bytes(inode->i_sb,
>>> +  map_start + map_len);
>>> +   is_last = 0;
>>> +   while (cpos < mapping_end && !is_last) {
>>> +   ret = ocfs2_get_clusters_nocache(inode, di_bh, cpos,
>>> +NULL, &rec, &is_last);
>>> +   if (ret) {
>>> +   mlog_errno(ret);
>>> +   goto out_unlock2;
>>> +   }
>>> +
>>> +   if (rec.e_blkno == 0ULL)
>>> +   break;
>> I think here the blocks is not overwrite, because the hold is found and the 
>> blocks
>> should be allocated.
> If the rec.e_blkno == NULL, this means there is a hole.
> The file hole means that these blocks are not allocated, it does not like 
> unwritten block.
> The unwritten blocks means that these blocks are allocated, but still have 
> not been unwritten. 
> 
If we break the loop when we find the hold, out of this function we will 
allocate the blocks in
ocfs2_file_write_iter()->..->ocfs2_direct_IO->__blockdev_direct_IO->..->ocfs2_dio_wr_get_block()
->ocfs2_write_begin_nolock. Does this violate the semantics of 'IOCB_NOWAIT';

BTW, should we consider the down_write() and ocfs2_inode_lock() in 
ocfs2_dio_wr_get_block() when
the flag 'IOCB_NOWAIT' is set;

>>> +
>>> +   if (rec.e_flags & OCFS2_EXT_REFCOUNTED)
>>> +   break;
>>> +
>>> +   cpos = le32_to_cpu(rec.e_cpos) +
>>> +   le16_to_cpu(rec.e_leaf_clusters);
>>> +   }
>>> +
>>> +   if (cpos < mapping_end)
>>> +   ret = 1;
>>> +
>>> +out_unlock2:
>>
>> I think the 'out_up_read' is more readable than the 'out_unlock2' .
> Ok, I will use more readable tag here.
>>
>>> +   brelse(di_bh);
>>> +
>>> +   up_read(&OCFS2_I(inode)->ip_alloc_sem);
>>> +
>>> +out_unlock1:
>>
>> We should release buffer head here.
>>
>>> +   ocfs2_inode_unlock(inode, 0);
>>> +
>>> +out:
>>> +   return (ret ? 0 : 1);
>>> +}
>>> +
>>>  int ocfs2_seek_data_hole_offset(struct file *file, loff_t *offset, int 
>> whence)
>>>  {
>>> struct inode *inode = file->f_mapping->host;
>>> diff --git a/fs/ocfs2/extent_map.h b/fs/ocfs2/extent_map.h
>>> index 67ea57d..fd9e86a 100644
>>> --- a/fs/ocfs2/extent_map.h
>>> +++ b/fs/ocfs2/extent_map.h
>>> @@ -53,6 +53,9 @@ int ocfs2_extent_map_get_blocks(struct inode *inode, u64 
>> v_blkno, u64 *p_blkno,
>>>  int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>>>  u64 map_start, u64 map_len);
>>>  
>>> +int ocfs2_overwrite_io(struct inode *inode, u64 map_start, u64 map_len,
>>> +  int wait);
>>> +
>>>  int ocfs2_seek_data_hole_offset(struct file *file, loff_t *offset, int 
>> origin);
>>>  
>>>  int ocfs2_xattr_get_clusters(struct inode *inode, u32 v_cluster,
>>>
> 
> 
> .
> 


___
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel


Re: [Ocfs2-devel] [PATCH 3/3] ocfs2: nowait aio support

2017-11-27 Thread Gang He
Hello Alex,


>>> 
> Hi Gang,
> 
> On 2017/11/27 17:46, Gang He wrote:
>> Return EAGAIN if any of the following checks fail for direct I/O:
>> Can not get the related locks immediately,
>> Blocks are not allocated at the write location, it will trigger
>> block allocation and block IO operations.
>> 
>> Signed-off-by: Gang He 
>> ---
>>  fs/ocfs2/dir.c |  2 +-
>>  fs/ocfs2/dlmglue.c | 20 ++
>>  fs/ocfs2/dlmglue.h |  2 +-
>>  fs/ocfs2/file.c| 74 
>> +-
>>  fs/ocfs2/mmap.c|  2 +-
>>  fs/ocfs2/ocfs2_trace.h | 10 ---
>>  6 files changed, 79 insertions(+), 31 deletions(-)
>> 
>> diff --git a/fs/ocfs2/dir.c b/fs/ocfs2/dir.c
>> index febe631..ea50901 100644
>> --- a/fs/ocfs2/dir.c
>> +++ b/fs/ocfs2/dir.c
>> @@ -1957,7 +1957,7 @@ int ocfs2_readdir(struct file *file, struct 
>> dir_context 
> *ctx)
>>  
>>  trace_ocfs2_readdir((unsigned long long)OCFS2_I(inode)->ip_blkno);
>>  
>> -error = ocfs2_inode_lock_atime(inode, file->f_path.mnt, &lock_level);
>> +error = ocfs2_inode_lock_atime(inode, file->f_path.mnt, &lock_level, 1);
>>  if (lock_level && error >= 0) {
>>  /* We release EX lock which used to update atime
>>   * and get PR lock again to reduce contention
>> diff --git a/fs/ocfs2/dlmglue.c b/fs/ocfs2/dlmglue.c
>> index 5cfbd04..feb8dbe 100644
>> --- a/fs/ocfs2/dlmglue.c
>> +++ b/fs/ocfs2/dlmglue.c
>> @@ -2516,13 +2516,18 @@ int ocfs2_inode_lock_with_page(struct inode *inode,
>>  
>>  int ocfs2_inode_lock_atime(struct inode *inode,
>>struct vfsmount *vfsmnt,
>> -  int *level)
>> +  int *level, int wait)
>>  {
>>  int ret;
>>  
>> -ret = ocfs2_inode_lock(inode, NULL, 0);
>> +if (wait)
>> +ret = ocfs2_inode_lock(inode, NULL, 0);
>> +else
>> +ret = ocfs2_try_inode_lock(inode, NULL, 0);
>> +
>>  if (ret < 0) {
>> -mlog_errno(ret);
>> +if (ret != -EAGAIN)
>> +mlog_errno(ret);
>>  return ret;
>>  }
>>  
>> @@ -2534,9 +2539,14 @@ int ocfs2_inode_lock_atime(struct inode *inode,
>>  struct buffer_head *bh = NULL;
>>  
>>  ocfs2_inode_unlock(inode, 0);
>> -ret = ocfs2_inode_lock(inode, &bh, 1);
>> +if (wait)
>> +ret = ocfs2_inode_lock(inode, &bh, 1);
>> +else
>> +ret = ocfs2_try_inode_lock(inode, &bh, 1);
>> +
>>  if (ret < 0) {
>> -mlog_errno(ret);
>> +if (ret != -EAGAIN)
>> +mlog_errno(ret);
>>  return ret;
>>  }
>>  *level = 1;
>> diff --git a/fs/ocfs2/dlmglue.h b/fs/ocfs2/dlmglue.h
>> index 05910fc..c83dbb5 100644
>> --- a/fs/ocfs2/dlmglue.h
>> +++ b/fs/ocfs2/dlmglue.h
>> @@ -123,7 +123,7 @@ void ocfs2_refcount_lock_res_init(struct ocfs2_lock_res 
> *lockres,
>>  void ocfs2_open_unlock(struct inode *inode);
>>  int ocfs2_inode_lock_atime(struct inode *inode,
>>struct vfsmount *vfsmnt,
>> -  int *level);
>> +  int *level, int wait);
>>  int ocfs2_inode_lock_full_nested(struct inode *inode,
>>   struct buffer_head **ret_bh,
>>   int ex,
>> diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
>> index dc455d4..900f04e 100644
>> --- a/fs/ocfs2/file.c
>> +++ b/fs/ocfs2/file.c
>> @@ -140,6 +140,8 @@ static int ocfs2_file_open(struct inode *inode, struct 
> file *file)
>>  spin_unlock(&oi->ip_lock);
>>  }
>>  
>> +file->f_mode |= FMODE_NOWAIT;
>> +
>>  leave:
>>  return status;
>>  }
>> @@ -2132,8 +2134,7 @@ static int ocfs2_prepare_inode_for_refcount(struct 
> inode *inode,
>>  }
>>  
>>  static int ocfs2_prepare_inode_for_write(struct file *file,
>> - loff_t pos,
>> - size_t count)
>> + loff_t pos, size_t count, int wait)
>>  {
>>  int ret = 0, meta_level = 0;
>>  struct dentry *dentry = file->f_path.dentry;
>> @@ -2145,10 +2146,14 @@ static int ocfs2_prepare_inode_for_write(struct file 
> *file,
>>   * if we need to make modifications here.
>>   */
>>  for(;;) {
>> -ret = ocfs2_inode_lock(inode, NULL, meta_level);
>> +if (wait)
>> +ret = ocfs2_inode_lock(inode, NULL, meta_level);
>> +else
>> +ret = ocfs2_try_inode_lock(inode, NULL, meta_level);
>>  if (ret < 0) {
>>  meta_level = -1;
>> -mlog_errno(ret);
>> +if (ret != -EAGAIN)
>> +mlog_errno(ret);
>>  goto out;
>>  }
>>
> 
> We will lock inode again in 
> ocfs2_prepare_inode_for_write()->ocfs2_prepare

Re: [Ocfs2-devel] [PATCH 2/3] ocfs2: add ocfs2_overwrite_io function

2017-11-27 Thread Gang He
Hi Changwei,


>>> 
> Hi,
> Gang
> 
> On 2017/11/27 17:48, Gang He wrote:
>> Add ocfs2_overwrite_io function, which is used to judge if
>> overwrite allocated blocks, otherwise, the write will bring extra
>> block allocation overhead.
>> 
> 
> Can you elaborate how this overhead is introduced?
> Forgive me, I don't figure it.
If the blocks have been allocated, we just write these block directly.
If these blocks have not been allocated, that means we need to allocate these 
block firstly before write,
this allocation will bring the IO invoking be blocked, if the upper application 
does not want take this kind of overhead,
he can pass a nowait flag to avoid and return immediately with a -EAGAIN error.

Thanks
Gang

> 
> Thanks,
> Changwei
> 
>> Signed-off-by: Gang He 
>> ---
>>   fs/ocfs2/extent_map.c | 67 
> +++
>>   fs/ocfs2/extent_map.h |  3 +++
>>   2 files changed, 70 insertions(+)
>> 
>> diff --git a/fs/ocfs2/extent_map.c b/fs/ocfs2/extent_map.c
>> index e4719e0..98bf325 100644
>> --- a/fs/ocfs2/extent_map.c
>> +++ b/fs/ocfs2/extent_map.c
>> @@ -832,6 +832,73 @@ int ocfs2_fiemap(struct inode *inode, struct 
> fiemap_extent_info *fieinfo,
>>  return ret;
>>   }
>>   
>> +/* Is IO overwriting allocated blocks? */
>> +int ocfs2_overwrite_io(struct inode *inode, u64 map_start, u64 map_len,
>> +   int wait)
>> +{
>> +int ret = 0, is_last;
>> +u32 mapping_end, cpos;
>> +struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
>> +struct buffer_head *di_bh = NULL;
>> +struct ocfs2_extent_rec rec;
>> +
>> +if (wait)
>> +ret = ocfs2_inode_lock(inode, &di_bh, 0);
>> +else
>> +ret = ocfs2_try_inode_lock(inode, &di_bh, 0);
>> +if (ret)
>> +goto out;
>> +
>> +if (wait)
>> +down_read(&OCFS2_I(inode)->ip_alloc_sem);
>> +else {
>> +if (!down_read_trylock(&OCFS2_I(inode)->ip_alloc_sem)) {
>> +ret = -EAGAIN;
>> +goto out_unlock1;
>> +}
>> +}
>> +
>> +if ((OCFS2_I(inode)->ip_dyn_features & OCFS2_INLINE_DATA_FL) &&
>> +   ((map_start + map_len) <= i_size_read(inode)))
>> +goto out_unlock2;
>> +
>> +cpos = map_start >> osb->s_clustersize_bits;
>> +mapping_end = ocfs2_clusters_for_bytes(inode->i_sb,
>> +   map_start + map_len);
>> +is_last = 0;
>> +while (cpos < mapping_end && !is_last) {
>> +ret = ocfs2_get_clusters_nocache(inode, di_bh, cpos,
>> + NULL, &rec, &is_last);
>> +if (ret) {
>> +mlog_errno(ret);
>> +goto out_unlock2;
>> +}
>> +
>> +if (rec.e_blkno == 0ULL)
>> +break;
>> +
>> +if (rec.e_flags & OCFS2_EXT_REFCOUNTED)
>> +break;
>> +
>> +cpos = le32_to_cpu(rec.e_cpos) +
>> +le16_to_cpu(rec.e_leaf_clusters);
>> +}
>> +
>> +if (cpos < mapping_end)
>> +ret = 1;
>> +
>> +out_unlock2:
>> +brelse(di_bh);
>> +
>> +up_read(&OCFS2_I(inode)->ip_alloc_sem);
>> +
>> +out_unlock1:
>> +ocfs2_inode_unlock(inode, 0);
>> +
>> +out:
>> +return (ret ? 0 : 1);
>> +}
>> +
>>   int ocfs2_seek_data_hole_offset(struct file *file, loff_t *offset, int 
> whence)
>>   {
>>  struct inode *inode = file->f_mapping->host;
>> diff --git a/fs/ocfs2/extent_map.h b/fs/ocfs2/extent_map.h
>> index 67ea57d..fd9e86a 100644
>> --- a/fs/ocfs2/extent_map.h
>> +++ b/fs/ocfs2/extent_map.h
>> @@ -53,6 +53,9 @@ int ocfs2_extent_map_get_blocks(struct inode *inode, u64 
> v_blkno, u64 *p_blkno,
>>   int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>>   u64 map_start, u64 map_len);
>>   
>> +int ocfs2_overwrite_io(struct inode *inode, u64 map_start, u64 map_len,
>> +   int wait);
>> +
>>   int ocfs2_seek_data_hole_offset(struct file *file, loff_t *offset, int 
> origin);
>>   
>>   int ocfs2_xattr_get_clusters(struct inode *inode, u32 v_cluster,
>> 


___
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel


Re: [Ocfs2-devel] [PATCH 2/3] ocfs2: add ocfs2_overwrite_io function

2017-11-27 Thread Gang He
Hello Alex,


>>> 
> Hi Gang,
> 
> On 2017/11/27 17:46, Gang He wrote:
>> Add ocfs2_overwrite_io function, which is used to judge if
>> overwrite allocated blocks, otherwise, the write will bring extra
>> block allocation overhead.
>> 
>> Signed-off-by: Gang He 
>> ---
>>  fs/ocfs2/extent_map.c | 67 
> +++
>>  fs/ocfs2/extent_map.h |  3 +++
>>  2 files changed, 70 insertions(+)
>> 
>> diff --git a/fs/ocfs2/extent_map.c b/fs/ocfs2/extent_map.c
>> index e4719e0..98bf325 100644
>> --- a/fs/ocfs2/extent_map.c
>> +++ b/fs/ocfs2/extent_map.c
>> @@ -832,6 +832,73 @@ int ocfs2_fiemap(struct inode *inode, struct 
> fiemap_extent_info *fieinfo,
>>  return ret;
>>  }
>>  
>> +/* Is IO overwriting allocated blocks? */
>> +int ocfs2_overwrite_io(struct inode *inode, u64 map_start, u64 map_len,
>> +   int wait)
>> +{
>> +int ret = 0, is_last;
>> +u32 mapping_end, cpos;
>> +struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
>> +struct buffer_head *di_bh = NULL;
>> +struct ocfs2_extent_rec rec;
>> +
>> +if (wait)
>> +ret = ocfs2_inode_lock(inode, &di_bh, 0);
>> +else
>> +ret = ocfs2_try_inode_lock(inode, &di_bh, 0);
>> +if (ret)
>> +goto out;
>> +
>> +if (wait)
>> +down_read(&OCFS2_I(inode)->ip_alloc_sem);
>> +else {
>> +if (!down_read_trylock(&OCFS2_I(inode)->ip_alloc_sem)) {
>> +ret = -EAGAIN;
>> +goto out_unlock1;
>> +}
>> +}
>> +
>> +if ((OCFS2_I(inode)->ip_dyn_features & OCFS2_INLINE_DATA_FL) &&
>> +   ((map_start + map_len) <= i_size_read(inode)))
>> +goto out_unlock2;
>> +
>> +cpos = map_start >> osb->s_clustersize_bits;
>> +mapping_end = ocfs2_clusters_for_bytes(inode->i_sb,
>> +   map_start + map_len);
>> +is_last = 0;
>> +while (cpos < mapping_end && !is_last) {
>> +ret = ocfs2_get_clusters_nocache(inode, di_bh, cpos,
>> + NULL, &rec, &is_last);
>> +if (ret) {
>> +mlog_errno(ret);
>> +goto out_unlock2;
>> +}
>> +
>> +if (rec.e_blkno == 0ULL)
>> +break;
> I think here the blocks is not overwrite, because the hold is found and the 
> blocks
> should be allocated.
If the rec.e_blkno == NULL, this means there is a hole.
The file hole means that these blocks are not allocated, it does not like 
unwritten block.
The unwritten blocks means that these blocks are allocated, but still have not 
been unwritten. 

>> +
>> +if (rec.e_flags & OCFS2_EXT_REFCOUNTED)
>> +break;
>> +
>> +cpos = le32_to_cpu(rec.e_cpos) +
>> +le16_to_cpu(rec.e_leaf_clusters);
>> +}
>> +
>> +if (cpos < mapping_end)
>> +ret = 1;
>> +
>> +out_unlock2:
> 
> I think the 'out_up_read' is more readable than the 'out_unlock2' .
Ok, I will use more readable tag here.
> 
>> +brelse(di_bh);
>> +
>> +up_read(&OCFS2_I(inode)->ip_alloc_sem);
>> +
>> +out_unlock1:
> 
> We should release buffer head here.
> 
>> +ocfs2_inode_unlock(inode, 0);
>> +
>> +out:
>> +return (ret ? 0 : 1);
>> +}
>> +
>>  int ocfs2_seek_data_hole_offset(struct file *file, loff_t *offset, int 
> whence)
>>  {
>>  struct inode *inode = file->f_mapping->host;
>> diff --git a/fs/ocfs2/extent_map.h b/fs/ocfs2/extent_map.h
>> index 67ea57d..fd9e86a 100644
>> --- a/fs/ocfs2/extent_map.h
>> +++ b/fs/ocfs2/extent_map.h
>> @@ -53,6 +53,9 @@ int ocfs2_extent_map_get_blocks(struct inode *inode, u64 
> v_blkno, u64 *p_blkno,
>>  int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>>   u64 map_start, u64 map_len);
>>  
>> +int ocfs2_overwrite_io(struct inode *inode, u64 map_start, u64 map_len,
>> +   int wait);
>> +
>>  int ocfs2_seek_data_hole_offset(struct file *file, loff_t *offset, int 
> origin);
>>  
>>  int ocfs2_xattr_get_clusters(struct inode *inode, u32 v_cluster,
>> 


___
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel


Re: [Ocfs2-devel] [PATCH 1/3] ocfs2: add ocfs2_try_rw_lock and ocfs2_try_inode_lock

2017-11-27 Thread Gang He
Hello Changwei,


>>> 
> Hi Gang,
> 
> On 2017/11/27 17:48, Gang He wrote:
>> Add ocfs2_try_rw_lock and ocfs2_try_inode_lock functions, which
>> will be used in non-block IO scenarios.
>> 
>> Signed-off-by: Gang He 
>> ---
>>   fs/ocfs2/dlmglue.c | 22 ++
>>   fs/ocfs2/dlmglue.h |  4 
>>   2 files changed, 26 insertions(+)
>> 
>> diff --git a/fs/ocfs2/dlmglue.c b/fs/ocfs2/dlmglue.c
>> index 4689940..5cfbd04 100644
>> --- a/fs/ocfs2/dlmglue.c
>> +++ b/fs/ocfs2/dlmglue.c
>> @@ -1742,6 +1742,28 @@ int ocfs2_rw_lock(struct inode *inode, int write)
>>  return status;
>>   }
>>   
>> +int ocfs2_try_rw_lock(struct inode *inode, int write)
>> +{
>> +int status, level;
>> +struct ocfs2_lock_res *lockres;
>> +struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
>> +
>> +mlog(0, "inode %llu try to take %s RW lock\n",
>> + (unsigned long long)OCFS2_I(inode)->ip_blkno,
>> + write ? "EXMODE" : "PRMODE");
>> +
>> +if (ocfs2_mount_local(osb))
>> +return 0;
>> +
>> +lockres = &OCFS2_I(inode)->ip_rw_lockres;
>> +
>> +level = write ? DLM_LOCK_EX : DLM_LOCK_PR;
>> +
>> +status = ocfs2_cluster_lock(OCFS2_SB(inode->i_sb), lockres, level,
>> +DLM_LKF_NOQUEUE, 0);
>> +return status;
>> +}
> 
> The newly added function ocfs2_try_rw_lock almost has the same logic 
> with ocfs2_rw_lock.Is it possible to combine them into an unique one?
> That will be more elegant.
I prefer to keep ocfs2_try_rw_lock() separately, since there has been the 
similar function/code here (e.g. ocfs2_try_open_lock).
second, adding a new ocfs2_try_rw_lock() function can avoid impact the existing 
code.

> 
> Moreover, can you elaborate further why we need a *NOQUEUE* lock for 
> supporting non-block aio?
Non-block IO means that the invoking should return with -EAGAIN instead of 
being blocked to wait for certain resource (e.g. lock, block allocation, etc.).

> 
> Why can't we wait for a while to grant a lock request? Is this necessary?
Non-block IO is a way for the upper application to submit IO, if the invoking 
will be blocked, the invoking will failed with -EAGAIN, 
then, the upper application will submit this IO with the normal (block mode) 
way in a delayed thread, this IO mode will benefit some database application.

> 
> Thanks,
> Changwei
> 
>> +
>>   void ocfs2_rw_unlock(struct inode *inode, int write)
>>   {
>>  int level = write ? DLM_LOCK_EX : DLM_LOCK_PR;
>> diff --git a/fs/ocfs2/dlmglue.h b/fs/ocfs2/dlmglue.h
>> index a7fc18b..05910fc 100644
>> --- a/fs/ocfs2/dlmglue.h
>> +++ b/fs/ocfs2/dlmglue.h
>> @@ -116,6 +116,7 @@ void ocfs2_refcount_lock_res_init(struct ocfs2_lock_res 
> *lockres,
>>   int ocfs2_create_new_inode_locks(struct inode *inode);
>>   int ocfs2_drop_inode_locks(struct inode *inode);
>>   int ocfs2_rw_lock(struct inode *inode, int write);
>> +int ocfs2_try_rw_lock(struct inode *inode, int write);
>>   void ocfs2_rw_unlock(struct inode *inode, int write);
>>   int ocfs2_open_lock(struct inode *inode);
>>   int ocfs2_try_open_lock(struct inode *inode, int write);
>> @@ -140,6 +141,9 @@ int ocfs2_inode_lock_with_page(struct inode *inode,
>>   /* 99% of the time we don't want to supply any additional flags --
>>* those are for very specific cases only. */
>>   #define ocfs2_inode_lock(i, b, e) ocfs2_inode_lock_full_nested(i, b, e, 0, 
> OI_LS_NORMAL)
>> +#define ocfs2_try_inode_lock(i, b, e)\
>> +ocfs2_inode_lock_full_nested(i, b, e, OCFS2_META_LOCK_NOQUEUE,\
>> +OI_LS_NORMAL)
>>   void ocfs2_inode_unlock(struct inode *inode,
>> int ex);
>>   int ocfs2_super_lock(struct ocfs2_super *osb,
>> 


___
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel


Re: [Ocfs2-devel] [PATCH 2/3] ocfs2: add ocfs2_overwrite_io function

2017-11-27 Thread Gang He



>>> 
> On 2017/11/28 9:52, piaojun wrote:
>> Hi Gang,
>> 
>> If ocfs2_overwrite_io is only called in 'nowait' scenarios, I wonder if
>> we can discard 'int wait' just as ext4 does:
>> 
>> static bool ext4_overwrite_io(struct inode *inode, loff_t pos, loff_t len);
> 
> Yes, Jun has a point.
> It seems that ocfs2_overwrite_io is only involved in non-blocking aio 
> and no other code spot is calling ocfs2_overwrite_io with wait=1 passed.
Ok, I will do this change.

> 
>> 
>> thans,
>> Jun
>> 
>> On 2017/11/27 17:46, Gang He wrote:
>>> Add ocfs2_overwrite_io function, which is used to judge if
>>> overwrite allocated blocks, otherwise, the write will bring extra
>>> block allocation overhead.
>>>
>>> Signed-off-by: Gang He 
>>> ---
>>>   fs/ocfs2/extent_map.c | 67 
> +++
>>>   fs/ocfs2/extent_map.h |  3 +++
>>>   2 files changed, 70 insertions(+)
>>>
>>> diff --git a/fs/ocfs2/extent_map.c b/fs/ocfs2/extent_map.c
>>> index e4719e0..98bf325 100644
>>> --- a/fs/ocfs2/extent_map.c
>>> +++ b/fs/ocfs2/extent_map.c
>>> @@ -832,6 +832,73 @@ int ocfs2_fiemap(struct inode *inode, struct 
> fiemap_extent_info *fieinfo,
>>> return ret;
>>>   }
>>>   
>>> +/* Is IO overwriting allocated blocks? */
>>> +int ocfs2_overwrite_io(struct inode *inode, u64 map_start, u64 map_len,
>>> +  int wait)
>>> +{
>>> +   int ret = 0, is_last;
>>> +   u32 mapping_end, cpos;
>>> +   struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
>>> +   struct buffer_head *di_bh = NULL;
>>> +   struct ocfs2_extent_rec rec;
>>> +
>>> +   if (wait)
>>> +   ret = ocfs2_inode_lock(inode, &di_bh, 0);
>>> +   else
>>> +   ret = ocfs2_try_inode_lock(inode, &di_bh, 0);
>>> +   if (ret)
>>> +   goto out;
>>> +
>>> +   if (wait)
>>> +   down_read(&OCFS2_I(inode)->ip_alloc_sem);
>>> +   else {
>>> +   if (!down_read_trylock(&OCFS2_I(inode)->ip_alloc_sem)) {
>>> +   ret = -EAGAIN;
> Here is a little strange, it seems that you don't care much about how 
> this function fails. Why evaluate _ret_ to  -EAGAIN here and ignore it 
> later?
> 
> Thanks,
> Changwei
> 
>>> +   goto out_unlock1;
>>> +   }
>>> +   }
>>> +
>>> +   if ((OCFS2_I(inode)->ip_dyn_features & OCFS2_INLINE_DATA_FL) &&
>>> +  ((map_start + map_len) <= i_size_read(inode)))
>>> +   goto out_unlock2;
>>> +
>>> +   cpos = map_start >> osb->s_clustersize_bits;
>>> +   mapping_end = ocfs2_clusters_for_bytes(inode->i_sb,
>>> +  map_start + map_len);
>>> +   is_last = 0;
>>> +   while (cpos < mapping_end && !is_last) {
>>> +   ret = ocfs2_get_clusters_nocache(inode, di_bh, cpos,
>>> +NULL, &rec, &is_last);
>>> +   if (ret) {
>>> +   mlog_errno(ret);
>>> +   goto out_unlock2;
>>> +   }
>>> +
>>> +   if (rec.e_blkno == 0ULL)
>>> +   break;
>>> +
>>> +   if (rec.e_flags & OCFS2_EXT_REFCOUNTED)
>>> +   break;
>>> +
>>> +   cpos = le32_to_cpu(rec.e_cpos) +
>>> +   le16_to_cpu(rec.e_leaf_clusters);
>>> +   }
>>> +
>>> +   if (cpos < mapping_end)
>>> +   ret = 1;
>>> +
>>> +out_unlock2:
>>> +   brelse(di_bh);
>>> +
>>> +   up_read(&OCFS2_I(inode)->ip_alloc_sem);
>>> +
>>> +out_unlock1:
>>> +   ocfs2_inode_unlock(inode, 0);
>>> +
>>> +out:
>>> +   return (ret ? 0 : 1);
>>> +}
>>> +
>>>   int ocfs2_seek_data_hole_offset(struct file *file, loff_t *offset, int 
> whence)
>>>   {
>>> struct inode *inode = file->f_mapping->host;
>>> diff --git a/fs/ocfs2/extent_map.h b/fs/ocfs2/extent_map.h
>>> index 67ea57d..fd9e86a 100644
>>> --- a/fs/ocfs2/extent_map.h
>>> +++ b/fs/ocfs2/extent_map.h
>>> @@ -53,6 +53,9 @@ int ocfs2_extent_map_get_blocks(struct inode *inode, u64 
> v_blkno, u64 *p_blkno,
>>>   int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>>>  u64 map_start, u64 map_len);
>>>   
>>> +int ocfs2_overwrite_io(struct inode *inode, u64 map_start, u64 map_len,
>>> +  int wait);
>>> +
>>>   int ocfs2_seek_data_hole_offset(struct file *file, loff_t *offset, int 
> origin);
>>>   
>>>   int ocfs2_xattr_get_clusters(struct inode *inode, u32 v_cluster,
>>>
>> 
>> ___
>> Ocfs2-devel mailing list
>> Ocfs2-devel@oss.oracle.com 
>> https://oss.oracle.com/mailman/listinfo/ocfs2-devel 
>> 

___
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel


Re: [Ocfs2-devel] [PATCH 2/3] ocfs2: add ocfs2_overwrite_io function

2017-11-27 Thread Gang He
Hi Jun,


>>> 
> Hi Gang,
> 
> If ocfs2_overwrite_io is only called in 'nowait' scenarios, I wonder if
> we can discard 'int wait' just as ext4 does:
> 
> static bool ext4_overwrite_io(struct inode *inode, loff_t pos, loff_t len);
Ok, it looks most people prefer to get rid of "wait" parameter.

Thanks
Gang

> 
> thans,
> Jun
> 
> On 2017/11/27 17:46, Gang He wrote:
>> Add ocfs2_overwrite_io function, which is used to judge if
>> overwrite allocated blocks, otherwise, the write will bring extra
>> block allocation overhead.
>> 
>> Signed-off-by: Gang He 
>> ---
>>  fs/ocfs2/extent_map.c | 67 
> +++
>>  fs/ocfs2/extent_map.h |  3 +++
>>  2 files changed, 70 insertions(+)
>> 
>> diff --git a/fs/ocfs2/extent_map.c b/fs/ocfs2/extent_map.c
>> index e4719e0..98bf325 100644
>> --- a/fs/ocfs2/extent_map.c
>> +++ b/fs/ocfs2/extent_map.c
>> @@ -832,6 +832,73 @@ int ocfs2_fiemap(struct inode *inode, struct 
> fiemap_extent_info *fieinfo,
>>  return ret;
>>  }
>>  
>> +/* Is IO overwriting allocated blocks? */
>> +int ocfs2_overwrite_io(struct inode *inode, u64 map_start, u64 map_len,
>> +   int wait)
>> +{
>> +int ret = 0, is_last;
>> +u32 mapping_end, cpos;
>> +struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
>> +struct buffer_head *di_bh = NULL;
>> +struct ocfs2_extent_rec rec;
>> +
>> +if (wait)
>> +ret = ocfs2_inode_lock(inode, &di_bh, 0);
>> +else
>> +ret = ocfs2_try_inode_lock(inode, &di_bh, 0);
>> +if (ret)
>> +goto out;
>> +
>> +if (wait)
>> +down_read(&OCFS2_I(inode)->ip_alloc_sem);
>> +else {
>> +if (!down_read_trylock(&OCFS2_I(inode)->ip_alloc_sem)) {
>> +ret = -EAGAIN;
>> +goto out_unlock1;
>> +}
>> +}
>> +
>> +if ((OCFS2_I(inode)->ip_dyn_features & OCFS2_INLINE_DATA_FL) &&
>> +   ((map_start + map_len) <= i_size_read(inode)))
>> +goto out_unlock2;
>> +
>> +cpos = map_start >> osb->s_clustersize_bits;
>> +mapping_end = ocfs2_clusters_for_bytes(inode->i_sb,
>> +   map_start + map_len);
>> +is_last = 0;
>> +while (cpos < mapping_end && !is_last) {
>> +ret = ocfs2_get_clusters_nocache(inode, di_bh, cpos,
>> + NULL, &rec, &is_last);
>> +if (ret) {
>> +mlog_errno(ret);
>> +goto out_unlock2;
>> +}
>> +
>> +if (rec.e_blkno == 0ULL)
>> +break;
>> +
>> +if (rec.e_flags & OCFS2_EXT_REFCOUNTED)
>> +break;
>> +
>> +cpos = le32_to_cpu(rec.e_cpos) +
>> +le16_to_cpu(rec.e_leaf_clusters);
>> +}
>> +
>> +if (cpos < mapping_end)
>> +ret = 1;
>> +
>> +out_unlock2:
>> +brelse(di_bh);
>> +
>> +up_read(&OCFS2_I(inode)->ip_alloc_sem);
>> +
>> +out_unlock1:
>> +ocfs2_inode_unlock(inode, 0);
>> +
>> +out:
>> +return (ret ? 0 : 1);
>> +}
>> +
>>  int ocfs2_seek_data_hole_offset(struct file *file, loff_t *offset, int 
> whence)
>>  {
>>  struct inode *inode = file->f_mapping->host;
>> diff --git a/fs/ocfs2/extent_map.h b/fs/ocfs2/extent_map.h
>> index 67ea57d..fd9e86a 100644
>> --- a/fs/ocfs2/extent_map.h
>> +++ b/fs/ocfs2/extent_map.h
>> @@ -53,6 +53,9 @@ int ocfs2_extent_map_get_blocks(struct inode *inode, u64 
> v_blkno, u64 *p_blkno,
>>  int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>>   u64 map_start, u64 map_len);
>>  
>> +int ocfs2_overwrite_io(struct inode *inode, u64 map_start, u64 map_len,
>> +   int wait);
>> +
>>  int ocfs2_seek_data_hole_offset(struct file *file, loff_t *offset, int 
> origin);
>>  
>>  int ocfs2_xattr_get_clusters(struct inode *inode, u32 v_cluster,
>> 


___
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel


Re: [Ocfs2-devel] [PATCH 1/3] ocfs2: add ocfs2_try_rw_lock and ocfs2_try_inode_lock

2017-11-27 Thread Gang He
Hello Jun,


>>> 
> Hi Gang,
> 
> On 2017/11/27 17:46, Gang He wrote:
>> Add ocfs2_try_rw_lock and ocfs2_try_inode_lock functions, which
>> will be used in non-block IO scenarios.
>> 
>> Signed-off-by: Gang He 
>> ---
>>  fs/ocfs2/dlmglue.c | 22 ++
>>  fs/ocfs2/dlmglue.h |  4 
>>  2 files changed, 26 insertions(+)
>> 
>> diff --git a/fs/ocfs2/dlmglue.c b/fs/ocfs2/dlmglue.c
>> index 4689940..5cfbd04 100644
>> --- a/fs/ocfs2/dlmglue.c
>> +++ b/fs/ocfs2/dlmglue.c
>> @@ -1742,6 +1742,28 @@ int ocfs2_rw_lock(struct inode *inode, int write)
>>  return status;
>>  }
>>  
>> +int ocfs2_try_rw_lock(struct inode *inode, int write)
>> +{
>> +int status, level;
>> +struct ocfs2_lock_res *lockres;
>> +struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
>> +
>> +mlog(0, "inode %llu try to take %s RW lock\n",
>> + (unsigned long long)OCFS2_I(inode)->ip_blkno,
>> + write ? "EXMODE" : "PRMODE");
>> +
>> +if (ocfs2_mount_local(osb))
>> +return 0;
>> +
>> +lockres = &OCFS2_I(inode)->ip_rw_lockres;
>> +
>> +level = write ? DLM_LOCK_EX : DLM_LOCK_PR;
>> +
>> +status = ocfs2_cluster_lock(OCFS2_SB(inode->i_sb), lockres, level,
>> +DLM_LKF_NOQUEUE, 0);
> 
> we'd better use 'osb' instead of 'OCFS2_SB(inode->i_sb)'.
Ok, I did this change in next version.

> 
>> +return status;
>> +}
>> +
>>  void ocfs2_rw_unlock(struct inode *inode, int write)
>>  {
>>  int level = write ? DLM_LOCK_EX : DLM_LOCK_PR;
>> diff --git a/fs/ocfs2/dlmglue.h b/fs/ocfs2/dlmglue.h
>> index a7fc18b..05910fc 100644
>> --- a/fs/ocfs2/dlmglue.h
>> +++ b/fs/ocfs2/dlmglue.h
>> @@ -116,6 +116,7 @@ void ocfs2_refcount_lock_res_init(struct ocfs2_lock_res 
> *lockres,
>>  int ocfs2_create_new_inode_locks(struct inode *inode);
>>  int ocfs2_drop_inode_locks(struct inode *inode);
>>  int ocfs2_rw_lock(struct inode *inode, int write);
>> +int ocfs2_try_rw_lock(struct inode *inode, int write);
>>  void ocfs2_rw_unlock(struct inode *inode, int write);
>>  int ocfs2_open_lock(struct inode *inode);
>>  int ocfs2_try_open_lock(struct inode *inode, int write);
>> @@ -140,6 +141,9 @@ int ocfs2_inode_lock_with_page(struct inode *inode,
>>  /* 99% of the time we don't want to supply any additional flags --
>>   * those are for very specific cases only. */
>>  #define ocfs2_inode_lock(i, b, e) ocfs2_inode_lock_full_nested(i, b, e, 0, 
> OI_LS_NORMAL)
>> +#define ocfs2_try_inode_lock(i, b, e)\
>> +ocfs2_inode_lock_full_nested(i, b, e, OCFS2_META_LOCK_NOQUEUE,\
>> +OI_LS_NORMAL)
>>  void ocfs2_inode_unlock(struct inode *inode,
>> int ex);
>>  int ocfs2_super_lock(struct ocfs2_super *osb,
>> 


___
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel


Re: [Ocfs2-devel] [PATCH 2/3] ocfs2: add ocfs2_overwrite_io function

2017-11-27 Thread Gang He
Hello Joseph,


>>> 
> Hi Gang,
> 
> On 17/11/27 17:46, Gang He wrote:
>> Add ocfs2_overwrite_io function, which is used to judge if
>> overwrite allocated blocks, otherwise, the write will bring extra
>> block allocation overhead.
>> 
>> Signed-off-by: Gang He 
>> ---
>>  fs/ocfs2/extent_map.c | 67 
> +++
>>  fs/ocfs2/extent_map.h |  3 +++
>>  2 files changed, 70 insertions(+)
>> 
>> diff --git a/fs/ocfs2/extent_map.c b/fs/ocfs2/extent_map.c
>> index e4719e0..98bf325 100644
>> --- a/fs/ocfs2/extent_map.c
>> +++ b/fs/ocfs2/extent_map.c
>> @@ -832,6 +832,73 @@ int ocfs2_fiemap(struct inode *inode, struct 
> fiemap_extent_info *fieinfo,
>>  return ret;
>>  }
>>  
>> +/* Is IO overwriting allocated blocks? */
>> +int ocfs2_overwrite_io(struct inode *inode, u64 map_start, u64 map_len,
>> +   int wait)
>> +{
>> +int ret = 0, is_last;
>> +u32 mapping_end, cpos;
>> +struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
>> +struct buffer_head *di_bh = NULL;
>> +struct ocfs2_extent_rec rec;
>> +
>> +if (wait)
>> +ret = ocfs2_inode_lock(inode, &di_bh, 0);
>> +else
>> +ret = ocfs2_try_inode_lock(inode, &di_bh, 0);
>> +if (ret)
>> +goto out;
>> +
>> +if (wait)
>> +down_read(&OCFS2_I(inode)->ip_alloc_sem);
>> +else {
>> +if (!down_read_trylock(&OCFS2_I(inode)->ip_alloc_sem)) {
>> +ret = -EAGAIN;
>> +goto out_unlock1;
>> +}
>> +}
>> +
>> +if ((OCFS2_I(inode)->ip_dyn_features & OCFS2_INLINE_DATA_FL) &&
>> +   ((map_start + map_len) <= i_size_read(inode)))
>> +goto out_unlock2;
>> +
>> +cpos = map_start >> osb->s_clustersize_bits;
>> +mapping_end = ocfs2_clusters_for_bytes(inode->i_sb,
>> +   map_start + map_len);
>> +is_last = 0;
>> +while (cpos < mapping_end && !is_last) {
>> +ret = ocfs2_get_clusters_nocache(inode, di_bh, cpos,
>> + NULL, &rec, &is_last);
>> +if (ret) {
>> +mlog_errno(ret);
>> +goto out_unlock2;
>> +}
>> +
>> +if (rec.e_blkno == 0ULL)
>> +break;
>> +
>> +if (rec.e_flags & OCFS2_EXT_REFCOUNTED)
>> +break;
>> +
>> +cpos = le32_to_cpu(rec.e_cpos) +
>> +le16_to_cpu(rec.e_leaf_clusters);
>> +}
>> +
>> +if (cpos < mapping_end)
>> +ret = 1;
>> +
>> +out_unlock2:
>> +brelse(di_bh);
>> +
>> +up_read(&OCFS2_I(inode)->ip_alloc_sem);
>> +
>> +out_unlock1:
> Should brelse(di_bh) be here?
If the code jumps to out_unlock1 directly, the di_bh pointer should be NULL, it 
is not necessary to release.

> 
>> +ocfs2_inode_unlock(inode, 0);
>> +
>> +out:
>> +return (ret ? 0 : 1);
> I don't think EAGAIN and other error code can be handled the same. We
> have to distinguish them.
Ok, I think we can add one line log to report the error in case the error is 
not EAGAIN. 

> 
> Thanks,
> Joseph
> 
>> +}
>> +
>>  int ocfs2_seek_data_hole_offset(struct file *file, loff_t *offset, int 
> whence)
>>  {
>>  struct inode *inode = file->f_mapping->host;
>> diff --git a/fs/ocfs2/extent_map.h b/fs/ocfs2/extent_map.h
>> index 67ea57d..fd9e86a 100644
>> --- a/fs/ocfs2/extent_map.h
>> +++ b/fs/ocfs2/extent_map.h
>> @@ -53,6 +53,9 @@ int ocfs2_extent_map_get_blocks(struct inode *inode, u64 
> v_blkno, u64 *p_blkno,
>>  int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>>   u64 map_start, u64 map_len);
>>  
>> +int ocfs2_overwrite_io(struct inode *inode, u64 map_start, u64 map_len,
>> +   int wait);
>> +
>>  int ocfs2_seek_data_hole_offset(struct file *file, loff_t *offset, int 
> origin);
>>  
>>  int ocfs2_xattr_get_clusters(struct inode *inode, u32 v_cluster,
>> 


___
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel


Re: [Ocfs2-devel] [PATCH 3/3] ocfs2: nowait aio support

2017-11-27 Thread alex chen
Hi Gang,

On 2017/11/27 17:46, Gang He wrote:
> Return EAGAIN if any of the following checks fail for direct I/O:
> Can not get the related locks immediately,
> Blocks are not allocated at the write location, it will trigger
> block allocation and block IO operations.
> 
> Signed-off-by: Gang He 
> ---
>  fs/ocfs2/dir.c |  2 +-
>  fs/ocfs2/dlmglue.c | 20 ++
>  fs/ocfs2/dlmglue.h |  2 +-
>  fs/ocfs2/file.c| 74 
> +-
>  fs/ocfs2/mmap.c|  2 +-
>  fs/ocfs2/ocfs2_trace.h | 10 ---
>  6 files changed, 79 insertions(+), 31 deletions(-)
> 
> diff --git a/fs/ocfs2/dir.c b/fs/ocfs2/dir.c
> index febe631..ea50901 100644
> --- a/fs/ocfs2/dir.c
> +++ b/fs/ocfs2/dir.c
> @@ -1957,7 +1957,7 @@ int ocfs2_readdir(struct file *file, struct dir_context 
> *ctx)
>  
>   trace_ocfs2_readdir((unsigned long long)OCFS2_I(inode)->ip_blkno);
>  
> - error = ocfs2_inode_lock_atime(inode, file->f_path.mnt, &lock_level);
> + error = ocfs2_inode_lock_atime(inode, file->f_path.mnt, &lock_level, 1);
>   if (lock_level && error >= 0) {
>   /* We release EX lock which used to update atime
>* and get PR lock again to reduce contention
> diff --git a/fs/ocfs2/dlmglue.c b/fs/ocfs2/dlmglue.c
> index 5cfbd04..feb8dbe 100644
> --- a/fs/ocfs2/dlmglue.c
> +++ b/fs/ocfs2/dlmglue.c
> @@ -2516,13 +2516,18 @@ int ocfs2_inode_lock_with_page(struct inode *inode,
>  
>  int ocfs2_inode_lock_atime(struct inode *inode,
> struct vfsmount *vfsmnt,
> -   int *level)
> +   int *level, int wait)
>  {
>   int ret;
>  
> - ret = ocfs2_inode_lock(inode, NULL, 0);
> + if (wait)
> + ret = ocfs2_inode_lock(inode, NULL, 0);
> + else
> + ret = ocfs2_try_inode_lock(inode, NULL, 0);
> +
>   if (ret < 0) {
> - mlog_errno(ret);
> + if (ret != -EAGAIN)
> + mlog_errno(ret);
>   return ret;
>   }
>  
> @@ -2534,9 +2539,14 @@ int ocfs2_inode_lock_atime(struct inode *inode,
>   struct buffer_head *bh = NULL;
>  
>   ocfs2_inode_unlock(inode, 0);
> - ret = ocfs2_inode_lock(inode, &bh, 1);
> + if (wait)
> + ret = ocfs2_inode_lock(inode, &bh, 1);
> + else
> + ret = ocfs2_try_inode_lock(inode, &bh, 1);
> +
>   if (ret < 0) {
> - mlog_errno(ret);
> + if (ret != -EAGAIN)
> + mlog_errno(ret);
>   return ret;
>   }
>   *level = 1;
> diff --git a/fs/ocfs2/dlmglue.h b/fs/ocfs2/dlmglue.h
> index 05910fc..c83dbb5 100644
> --- a/fs/ocfs2/dlmglue.h
> +++ b/fs/ocfs2/dlmglue.h
> @@ -123,7 +123,7 @@ void ocfs2_refcount_lock_res_init(struct ocfs2_lock_res 
> *lockres,
>  void ocfs2_open_unlock(struct inode *inode);
>  int ocfs2_inode_lock_atime(struct inode *inode,
> struct vfsmount *vfsmnt,
> -   int *level);
> +   int *level, int wait);
>  int ocfs2_inode_lock_full_nested(struct inode *inode,
>struct buffer_head **ret_bh,
>int ex,
> diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
> index dc455d4..900f04e 100644
> --- a/fs/ocfs2/file.c
> +++ b/fs/ocfs2/file.c
> @@ -140,6 +140,8 @@ static int ocfs2_file_open(struct inode *inode, struct 
> file *file)
>   spin_unlock(&oi->ip_lock);
>   }
>  
> + file->f_mode |= FMODE_NOWAIT;
> +
>  leave:
>   return status;
>  }
> @@ -2132,8 +2134,7 @@ static int ocfs2_prepare_inode_for_refcount(struct 
> inode *inode,
>  }
>  
>  static int ocfs2_prepare_inode_for_write(struct file *file,
> -  loff_t pos,
> -  size_t count)
> +  loff_t pos, size_t count, int wait)
>  {
>   int ret = 0, meta_level = 0;
>   struct dentry *dentry = file->f_path.dentry;
> @@ -2145,10 +2146,14 @@ static int ocfs2_prepare_inode_for_write(struct file 
> *file,
>* if we need to make modifications here.
>*/
>   for(;;) {
> - ret = ocfs2_inode_lock(inode, NULL, meta_level);
> + if (wait)
> + ret = ocfs2_inode_lock(inode, NULL, meta_level);
> + else
> + ret = ocfs2_try_inode_lock(inode, NULL, meta_level);
>   if (ret < 0) {
>   meta_level = -1;
> - mlog_errno(ret);
> + if (ret != -EAGAIN)
> + mlog_errno(ret);
>   goto out;
>   }
>

We will lock inode again in 
ocfs2_prepare_inode_for_write()->ocfs2_prepare_inode_for_refcount().
Should we add the check of 'nowait' flags?

> @@ -2199,7 +2204,7 @@ stati

Re: [Ocfs2-devel] [PATCH 2/3] ocfs2: add ocfs2_overwrite_io function

2017-11-27 Thread Changwei Ge
Hi,
Gang

On 2017/11/27 17:48, Gang He wrote:
> Add ocfs2_overwrite_io function, which is used to judge if
> overwrite allocated blocks, otherwise, the write will bring extra
> block allocation overhead.
> 

Can you elaborate how this overhead is introduced?
Forgive me, I don't figure it.

Thanks,
Changwei

> Signed-off-by: Gang He 
> ---
>   fs/ocfs2/extent_map.c | 67 
> +++
>   fs/ocfs2/extent_map.h |  3 +++
>   2 files changed, 70 insertions(+)
> 
> diff --git a/fs/ocfs2/extent_map.c b/fs/ocfs2/extent_map.c
> index e4719e0..98bf325 100644
> --- a/fs/ocfs2/extent_map.c
> +++ b/fs/ocfs2/extent_map.c
> @@ -832,6 +832,73 @@ int ocfs2_fiemap(struct inode *inode, struct 
> fiemap_extent_info *fieinfo,
>   return ret;
>   }
>   
> +/* Is IO overwriting allocated blocks? */
> +int ocfs2_overwrite_io(struct inode *inode, u64 map_start, u64 map_len,
> +int wait)
> +{
> + int ret = 0, is_last;
> + u32 mapping_end, cpos;
> + struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
> + struct buffer_head *di_bh = NULL;
> + struct ocfs2_extent_rec rec;
> +
> + if (wait)
> + ret = ocfs2_inode_lock(inode, &di_bh, 0);
> + else
> + ret = ocfs2_try_inode_lock(inode, &di_bh, 0);
> + if (ret)
> + goto out;
> +
> + if (wait)
> + down_read(&OCFS2_I(inode)->ip_alloc_sem);
> + else {
> + if (!down_read_trylock(&OCFS2_I(inode)->ip_alloc_sem)) {
> + ret = -EAGAIN;
> + goto out_unlock1;
> + }
> + }
> +
> + if ((OCFS2_I(inode)->ip_dyn_features & OCFS2_INLINE_DATA_FL) &&
> +((map_start + map_len) <= i_size_read(inode)))
> + goto out_unlock2;
> +
> + cpos = map_start >> osb->s_clustersize_bits;
> + mapping_end = ocfs2_clusters_for_bytes(inode->i_sb,
> +map_start + map_len);
> + is_last = 0;
> + while (cpos < mapping_end && !is_last) {
> + ret = ocfs2_get_clusters_nocache(inode, di_bh, cpos,
> +  NULL, &rec, &is_last);
> + if (ret) {
> + mlog_errno(ret);
> + goto out_unlock2;
> + }
> +
> + if (rec.e_blkno == 0ULL)
> + break;
> +
> + if (rec.e_flags & OCFS2_EXT_REFCOUNTED)
> + break;
> +
> + cpos = le32_to_cpu(rec.e_cpos) +
> + le16_to_cpu(rec.e_leaf_clusters);
> + }
> +
> + if (cpos < mapping_end)
> + ret = 1;
> +
> +out_unlock2:
> + brelse(di_bh);
> +
> + up_read(&OCFS2_I(inode)->ip_alloc_sem);
> +
> +out_unlock1:
> + ocfs2_inode_unlock(inode, 0);
> +
> +out:
> + return (ret ? 0 : 1);
> +}
> +
>   int ocfs2_seek_data_hole_offset(struct file *file, loff_t *offset, int 
> whence)
>   {
>   struct inode *inode = file->f_mapping->host;
> diff --git a/fs/ocfs2/extent_map.h b/fs/ocfs2/extent_map.h
> index 67ea57d..fd9e86a 100644
> --- a/fs/ocfs2/extent_map.h
> +++ b/fs/ocfs2/extent_map.h
> @@ -53,6 +53,9 @@ int ocfs2_extent_map_get_blocks(struct inode *inode, u64 
> v_blkno, u64 *p_blkno,
>   int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>u64 map_start, u64 map_len);
>   
> +int ocfs2_overwrite_io(struct inode *inode, u64 map_start, u64 map_len,
> +int wait);
> +
>   int ocfs2_seek_data_hole_offset(struct file *file, loff_t *offset, int 
> origin);
>   
>   int ocfs2_xattr_get_clusters(struct inode *inode, u32 v_cluster,
> 


___
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel


Re: [Ocfs2-devel] [PATCH 2/3] ocfs2: add ocfs2_overwrite_io function

2017-11-27 Thread alex chen
Hi Gang,

On 2017/11/27 17:46, Gang He wrote:
> Add ocfs2_overwrite_io function, which is used to judge if
> overwrite allocated blocks, otherwise, the write will bring extra
> block allocation overhead.
> 
> Signed-off-by: Gang He 
> ---
>  fs/ocfs2/extent_map.c | 67 
> +++
>  fs/ocfs2/extent_map.h |  3 +++
>  2 files changed, 70 insertions(+)
> 
> diff --git a/fs/ocfs2/extent_map.c b/fs/ocfs2/extent_map.c
> index e4719e0..98bf325 100644
> --- a/fs/ocfs2/extent_map.c
> +++ b/fs/ocfs2/extent_map.c
> @@ -832,6 +832,73 @@ int ocfs2_fiemap(struct inode *inode, struct 
> fiemap_extent_info *fieinfo,
>   return ret;
>  }
>  
> +/* Is IO overwriting allocated blocks? */
> +int ocfs2_overwrite_io(struct inode *inode, u64 map_start, u64 map_len,
> +int wait)
> +{
> + int ret = 0, is_last;
> + u32 mapping_end, cpos;
> + struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
> + struct buffer_head *di_bh = NULL;
> + struct ocfs2_extent_rec rec;
> +
> + if (wait)
> + ret = ocfs2_inode_lock(inode, &di_bh, 0);
> + else
> + ret = ocfs2_try_inode_lock(inode, &di_bh, 0);
> + if (ret)
> + goto out;
> +
> + if (wait)
> + down_read(&OCFS2_I(inode)->ip_alloc_sem);
> + else {
> + if (!down_read_trylock(&OCFS2_I(inode)->ip_alloc_sem)) {
> + ret = -EAGAIN;
> + goto out_unlock1;
> + }
> + }
> +
> + if ((OCFS2_I(inode)->ip_dyn_features & OCFS2_INLINE_DATA_FL) &&
> +((map_start + map_len) <= i_size_read(inode)))
> + goto out_unlock2;
> +
> + cpos = map_start >> osb->s_clustersize_bits;
> + mapping_end = ocfs2_clusters_for_bytes(inode->i_sb,
> +map_start + map_len);
> + is_last = 0;
> + while (cpos < mapping_end && !is_last) {
> + ret = ocfs2_get_clusters_nocache(inode, di_bh, cpos,
> +  NULL, &rec, &is_last);
> + if (ret) {
> + mlog_errno(ret);
> + goto out_unlock2;
> + }
> +
> + if (rec.e_blkno == 0ULL)
> + break;
I think here the blocks is not overwrite, because the hold is found and the 
blocks
should be allocated.
> +
> + if (rec.e_flags & OCFS2_EXT_REFCOUNTED)
> + break;
> +
> + cpos = le32_to_cpu(rec.e_cpos) +
> + le16_to_cpu(rec.e_leaf_clusters);
> + }
> +
> + if (cpos < mapping_end)
> + ret = 1;
> +
> +out_unlock2:

I think the 'out_up_read' is more readable than the 'out_unlock2' .

> + brelse(di_bh);
> +
> + up_read(&OCFS2_I(inode)->ip_alloc_sem);
> +
> +out_unlock1:

We should release buffer head here.

> + ocfs2_inode_unlock(inode, 0);
> +
> +out:
> + return (ret ? 0 : 1);
> +}
> +
>  int ocfs2_seek_data_hole_offset(struct file *file, loff_t *offset, int 
> whence)
>  {
>   struct inode *inode = file->f_mapping->host;
> diff --git a/fs/ocfs2/extent_map.h b/fs/ocfs2/extent_map.h
> index 67ea57d..fd9e86a 100644
> --- a/fs/ocfs2/extent_map.h
> +++ b/fs/ocfs2/extent_map.h
> @@ -53,6 +53,9 @@ int ocfs2_extent_map_get_blocks(struct inode *inode, u64 
> v_blkno, u64 *p_blkno,
>  int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>u64 map_start, u64 map_len);
>  
> +int ocfs2_overwrite_io(struct inode *inode, u64 map_start, u64 map_len,
> +int wait);
> +
>  int ocfs2_seek_data_hole_offset(struct file *file, loff_t *offset, int 
> origin);
>  
>  int ocfs2_xattr_get_clusters(struct inode *inode, u32 v_cluster,
> 


___
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel


Re: [Ocfs2-devel] [PATCH 2/3] ocfs2: add ocfs2_overwrite_io function

2017-11-27 Thread Changwei Ge
On 2017/11/28 9:52, piaojun wrote:
> Hi Gang,
> 
> If ocfs2_overwrite_io is only called in 'nowait' scenarios, I wonder if
> we can discard 'int wait' just as ext4 does:
> 
> static bool ext4_overwrite_io(struct inode *inode, loff_t pos, loff_t len);

Yes, Jun has a point.
It seems that ocfs2_overwrite_io is only involved in non-blocking aio 
and no other code spot is calling ocfs2_overwrite_io with wait=1 passed.

> 
> thans,
> Jun
> 
> On 2017/11/27 17:46, Gang He wrote:
>> Add ocfs2_overwrite_io function, which is used to judge if
>> overwrite allocated blocks, otherwise, the write will bring extra
>> block allocation overhead.
>>
>> Signed-off-by: Gang He 
>> ---
>>   fs/ocfs2/extent_map.c | 67 
>> +++
>>   fs/ocfs2/extent_map.h |  3 +++
>>   2 files changed, 70 insertions(+)
>>
>> diff --git a/fs/ocfs2/extent_map.c b/fs/ocfs2/extent_map.c
>> index e4719e0..98bf325 100644
>> --- a/fs/ocfs2/extent_map.c
>> +++ b/fs/ocfs2/extent_map.c
>> @@ -832,6 +832,73 @@ int ocfs2_fiemap(struct inode *inode, struct 
>> fiemap_extent_info *fieinfo,
>>  return ret;
>>   }
>>   
>> +/* Is IO overwriting allocated blocks? */
>> +int ocfs2_overwrite_io(struct inode *inode, u64 map_start, u64 map_len,
>> +   int wait)
>> +{
>> +int ret = 0, is_last;
>> +u32 mapping_end, cpos;
>> +struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
>> +struct buffer_head *di_bh = NULL;
>> +struct ocfs2_extent_rec rec;
>> +
>> +if (wait)
>> +ret = ocfs2_inode_lock(inode, &di_bh, 0);
>> +else
>> +ret = ocfs2_try_inode_lock(inode, &di_bh, 0);
>> +if (ret)
>> +goto out;
>> +
>> +if (wait)
>> +down_read(&OCFS2_I(inode)->ip_alloc_sem);
>> +else {
>> +if (!down_read_trylock(&OCFS2_I(inode)->ip_alloc_sem)) {
>> +ret = -EAGAIN;
Here is a little strange, it seems that you don't care much about how 
this function fails. Why evaluate _ret_ to  -EAGAIN here and ignore it 
later?

Thanks,
Changwei

>> +goto out_unlock1;
>> +}
>> +}
>> +
>> +if ((OCFS2_I(inode)->ip_dyn_features & OCFS2_INLINE_DATA_FL) &&
>> +   ((map_start + map_len) <= i_size_read(inode)))
>> +goto out_unlock2;
>> +
>> +cpos = map_start >> osb->s_clustersize_bits;
>> +mapping_end = ocfs2_clusters_for_bytes(inode->i_sb,
>> +   map_start + map_len);
>> +is_last = 0;
>> +while (cpos < mapping_end && !is_last) {
>> +ret = ocfs2_get_clusters_nocache(inode, di_bh, cpos,
>> + NULL, &rec, &is_last);
>> +if (ret) {
>> +mlog_errno(ret);
>> +goto out_unlock2;
>> +}
>> +
>> +if (rec.e_blkno == 0ULL)
>> +break;
>> +
>> +if (rec.e_flags & OCFS2_EXT_REFCOUNTED)
>> +break;
>> +
>> +cpos = le32_to_cpu(rec.e_cpos) +
>> +le16_to_cpu(rec.e_leaf_clusters);
>> +}
>> +
>> +if (cpos < mapping_end)
>> +ret = 1;
>> +
>> +out_unlock2:
>> +brelse(di_bh);
>> +
>> +up_read(&OCFS2_I(inode)->ip_alloc_sem);
>> +
>> +out_unlock1:
>> +ocfs2_inode_unlock(inode, 0);
>> +
>> +out:
>> +return (ret ? 0 : 1);
>> +}
>> +
>>   int ocfs2_seek_data_hole_offset(struct file *file, loff_t *offset, int 
>> whence)
>>   {
>>  struct inode *inode = file->f_mapping->host;
>> diff --git a/fs/ocfs2/extent_map.h b/fs/ocfs2/extent_map.h
>> index 67ea57d..fd9e86a 100644
>> --- a/fs/ocfs2/extent_map.h
>> +++ b/fs/ocfs2/extent_map.h
>> @@ -53,6 +53,9 @@ int ocfs2_extent_map_get_blocks(struct inode *inode, u64 
>> v_blkno, u64 *p_blkno,
>>   int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>>   u64 map_start, u64 map_len);
>>   
>> +int ocfs2_overwrite_io(struct inode *inode, u64 map_start, u64 map_len,
>> +   int wait);
>> +
>>   int ocfs2_seek_data_hole_offset(struct file *file, loff_t *offset, int 
>> origin);
>>   
>>   int ocfs2_xattr_get_clusters(struct inode *inode, u32 v_cluster,
>>
> 
> ___
> Ocfs2-devel mailing list
> Ocfs2-devel@oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/ocfs2-devel
> 


___
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel


Re: [Ocfs2-devel] [PATCH 1/3] ocfs2: add ocfs2_try_rw_lock and ocfs2_try_inode_lock

2017-11-27 Thread Changwei Ge
Hi Gang,

On 2017/11/27 17:48, Gang He wrote:
> Add ocfs2_try_rw_lock and ocfs2_try_inode_lock functions, which
> will be used in non-block IO scenarios.
> 
> Signed-off-by: Gang He 
> ---
>   fs/ocfs2/dlmglue.c | 22 ++
>   fs/ocfs2/dlmglue.h |  4 
>   2 files changed, 26 insertions(+)
> 
> diff --git a/fs/ocfs2/dlmglue.c b/fs/ocfs2/dlmglue.c
> index 4689940..5cfbd04 100644
> --- a/fs/ocfs2/dlmglue.c
> +++ b/fs/ocfs2/dlmglue.c
> @@ -1742,6 +1742,28 @@ int ocfs2_rw_lock(struct inode *inode, int write)
>   return status;
>   }
>   
> +int ocfs2_try_rw_lock(struct inode *inode, int write)
> +{
> + int status, level;
> + struct ocfs2_lock_res *lockres;
> + struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
> +
> + mlog(0, "inode %llu try to take %s RW lock\n",
> +  (unsigned long long)OCFS2_I(inode)->ip_blkno,
> +  write ? "EXMODE" : "PRMODE");
> +
> + if (ocfs2_mount_local(osb))
> + return 0;
> +
> + lockres = &OCFS2_I(inode)->ip_rw_lockres;
> +
> + level = write ? DLM_LOCK_EX : DLM_LOCK_PR;
> +
> + status = ocfs2_cluster_lock(OCFS2_SB(inode->i_sb), lockres, level,
> + DLM_LKF_NOQUEUE, 0);
> + return status;
> +}

The newly added function ocfs2_try_rw_lock almost has the same logic 
with ocfs2_rw_lock.Is it possible to combine them into an unique one?
That will be more elegant.

Moreover, can you elaborate further why we need a *NOQUEUE* lock for 
supporting non-block aio?

Why can't we wait for a while to grant a lock request? Is this necessary?

Thanks,
Changwei

> +
>   void ocfs2_rw_unlock(struct inode *inode, int write)
>   {
>   int level = write ? DLM_LOCK_EX : DLM_LOCK_PR;
> diff --git a/fs/ocfs2/dlmglue.h b/fs/ocfs2/dlmglue.h
> index a7fc18b..05910fc 100644
> --- a/fs/ocfs2/dlmglue.h
> +++ b/fs/ocfs2/dlmglue.h
> @@ -116,6 +116,7 @@ void ocfs2_refcount_lock_res_init(struct ocfs2_lock_res 
> *lockres,
>   int ocfs2_create_new_inode_locks(struct inode *inode);
>   int ocfs2_drop_inode_locks(struct inode *inode);
>   int ocfs2_rw_lock(struct inode *inode, int write);
> +int ocfs2_try_rw_lock(struct inode *inode, int write);
>   void ocfs2_rw_unlock(struct inode *inode, int write);
>   int ocfs2_open_lock(struct inode *inode);
>   int ocfs2_try_open_lock(struct inode *inode, int write);
> @@ -140,6 +141,9 @@ int ocfs2_inode_lock_with_page(struct inode *inode,
>   /* 99% of the time we don't want to supply any additional flags --
>* those are for very specific cases only. */
>   #define ocfs2_inode_lock(i, b, e) ocfs2_inode_lock_full_nested(i, b, e, 0, 
> OI_LS_NORMAL)
> +#define ocfs2_try_inode_lock(i, b, e)\
> + ocfs2_inode_lock_full_nested(i, b, e, OCFS2_META_LOCK_NOQUEUE,\
> + OI_LS_NORMAL)
>   void ocfs2_inode_unlock(struct inode *inode,
>  int ex);
>   int ocfs2_super_lock(struct ocfs2_super *osb,
> 


___
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel


Re: [Ocfs2-devel] [PATCH 2/3] ocfs2: add ocfs2_overwrite_io function

2017-11-27 Thread piaojun
Hi Gang,

If ocfs2_overwrite_io is only called in 'nowait' scenarios, I wonder if
we can discard 'int wait' just as ext4 does:

static bool ext4_overwrite_io(struct inode *inode, loff_t pos, loff_t len);

thans,
Jun

On 2017/11/27 17:46, Gang He wrote:
> Add ocfs2_overwrite_io function, which is used to judge if
> overwrite allocated blocks, otherwise, the write will bring extra
> block allocation overhead.
> 
> Signed-off-by: Gang He 
> ---
>  fs/ocfs2/extent_map.c | 67 
> +++
>  fs/ocfs2/extent_map.h |  3 +++
>  2 files changed, 70 insertions(+)
> 
> diff --git a/fs/ocfs2/extent_map.c b/fs/ocfs2/extent_map.c
> index e4719e0..98bf325 100644
> --- a/fs/ocfs2/extent_map.c
> +++ b/fs/ocfs2/extent_map.c
> @@ -832,6 +832,73 @@ int ocfs2_fiemap(struct inode *inode, struct 
> fiemap_extent_info *fieinfo,
>   return ret;
>  }
>  
> +/* Is IO overwriting allocated blocks? */
> +int ocfs2_overwrite_io(struct inode *inode, u64 map_start, u64 map_len,
> +int wait)
> +{
> + int ret = 0, is_last;
> + u32 mapping_end, cpos;
> + struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
> + struct buffer_head *di_bh = NULL;
> + struct ocfs2_extent_rec rec;
> +
> + if (wait)
> + ret = ocfs2_inode_lock(inode, &di_bh, 0);
> + else
> + ret = ocfs2_try_inode_lock(inode, &di_bh, 0);
> + if (ret)
> + goto out;
> +
> + if (wait)
> + down_read(&OCFS2_I(inode)->ip_alloc_sem);
> + else {
> + if (!down_read_trylock(&OCFS2_I(inode)->ip_alloc_sem)) {
> + ret = -EAGAIN;
> + goto out_unlock1;
> + }
> + }
> +
> + if ((OCFS2_I(inode)->ip_dyn_features & OCFS2_INLINE_DATA_FL) &&
> +((map_start + map_len) <= i_size_read(inode)))
> + goto out_unlock2;
> +
> + cpos = map_start >> osb->s_clustersize_bits;
> + mapping_end = ocfs2_clusters_for_bytes(inode->i_sb,
> +map_start + map_len);
> + is_last = 0;
> + while (cpos < mapping_end && !is_last) {
> + ret = ocfs2_get_clusters_nocache(inode, di_bh, cpos,
> +  NULL, &rec, &is_last);
> + if (ret) {
> + mlog_errno(ret);
> + goto out_unlock2;
> + }
> +
> + if (rec.e_blkno == 0ULL)
> + break;
> +
> + if (rec.e_flags & OCFS2_EXT_REFCOUNTED)
> + break;
> +
> + cpos = le32_to_cpu(rec.e_cpos) +
> + le16_to_cpu(rec.e_leaf_clusters);
> + }
> +
> + if (cpos < mapping_end)
> + ret = 1;
> +
> +out_unlock2:
> + brelse(di_bh);
> +
> + up_read(&OCFS2_I(inode)->ip_alloc_sem);
> +
> +out_unlock1:
> + ocfs2_inode_unlock(inode, 0);
> +
> +out:
> + return (ret ? 0 : 1);
> +}
> +
>  int ocfs2_seek_data_hole_offset(struct file *file, loff_t *offset, int 
> whence)
>  {
>   struct inode *inode = file->f_mapping->host;
> diff --git a/fs/ocfs2/extent_map.h b/fs/ocfs2/extent_map.h
> index 67ea57d..fd9e86a 100644
> --- a/fs/ocfs2/extent_map.h
> +++ b/fs/ocfs2/extent_map.h
> @@ -53,6 +53,9 @@ int ocfs2_extent_map_get_blocks(struct inode *inode, u64 
> v_blkno, u64 *p_blkno,
>  int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>u64 map_start, u64 map_len);
>  
> +int ocfs2_overwrite_io(struct inode *inode, u64 map_start, u64 map_len,
> +int wait);
> +
>  int ocfs2_seek_data_hole_offset(struct file *file, loff_t *offset, int 
> origin);
>  
>  int ocfs2_xattr_get_clusters(struct inode *inode, u32 v_cluster,
> 

___
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel


Re: [Ocfs2-devel] [PATCH 1/3] ocfs2: add ocfs2_try_rw_lock and ocfs2_try_inode_lock

2017-11-27 Thread piaojun
Hi Gang,

On 2017/11/27 17:46, Gang He wrote:
> Add ocfs2_try_rw_lock and ocfs2_try_inode_lock functions, which
> will be used in non-block IO scenarios.
> 
> Signed-off-by: Gang He 
> ---
>  fs/ocfs2/dlmglue.c | 22 ++
>  fs/ocfs2/dlmglue.h |  4 
>  2 files changed, 26 insertions(+)
> 
> diff --git a/fs/ocfs2/dlmglue.c b/fs/ocfs2/dlmglue.c
> index 4689940..5cfbd04 100644
> --- a/fs/ocfs2/dlmglue.c
> +++ b/fs/ocfs2/dlmglue.c
> @@ -1742,6 +1742,28 @@ int ocfs2_rw_lock(struct inode *inode, int write)
>   return status;
>  }
>  
> +int ocfs2_try_rw_lock(struct inode *inode, int write)
> +{
> + int status, level;
> + struct ocfs2_lock_res *lockres;
> + struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
> +
> + mlog(0, "inode %llu try to take %s RW lock\n",
> +  (unsigned long long)OCFS2_I(inode)->ip_blkno,
> +  write ? "EXMODE" : "PRMODE");
> +
> + if (ocfs2_mount_local(osb))
> + return 0;
> +
> + lockres = &OCFS2_I(inode)->ip_rw_lockres;
> +
> + level = write ? DLM_LOCK_EX : DLM_LOCK_PR;
> +
> + status = ocfs2_cluster_lock(OCFS2_SB(inode->i_sb), lockres, level,
> + DLM_LKF_NOQUEUE, 0);

we'd better use 'osb' instead of 'OCFS2_SB(inode->i_sb)'.

> + return status;
> +}
> +
>  void ocfs2_rw_unlock(struct inode *inode, int write)
>  {
>   int level = write ? DLM_LOCK_EX : DLM_LOCK_PR;
> diff --git a/fs/ocfs2/dlmglue.h b/fs/ocfs2/dlmglue.h
> index a7fc18b..05910fc 100644
> --- a/fs/ocfs2/dlmglue.h
> +++ b/fs/ocfs2/dlmglue.h
> @@ -116,6 +116,7 @@ void ocfs2_refcount_lock_res_init(struct ocfs2_lock_res 
> *lockres,
>  int ocfs2_create_new_inode_locks(struct inode *inode);
>  int ocfs2_drop_inode_locks(struct inode *inode);
>  int ocfs2_rw_lock(struct inode *inode, int write);
> +int ocfs2_try_rw_lock(struct inode *inode, int write);
>  void ocfs2_rw_unlock(struct inode *inode, int write);
>  int ocfs2_open_lock(struct inode *inode);
>  int ocfs2_try_open_lock(struct inode *inode, int write);
> @@ -140,6 +141,9 @@ int ocfs2_inode_lock_with_page(struct inode *inode,
>  /* 99% of the time we don't want to supply any additional flags --
>   * those are for very specific cases only. */
>  #define ocfs2_inode_lock(i, b, e) ocfs2_inode_lock_full_nested(i, b, e, 0, 
> OI_LS_NORMAL)
> +#define ocfs2_try_inode_lock(i, b, e)\
> + ocfs2_inode_lock_full_nested(i, b, e, OCFS2_META_LOCK_NOQUEUE,\
> + OI_LS_NORMAL)
>  void ocfs2_inode_unlock(struct inode *inode,
>  int ex);
>  int ocfs2_super_lock(struct ocfs2_super *osb,
> 

___
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel


Re: [Ocfs2-devel] [PATCH 2/3] ocfs2: add ocfs2_overwrite_io function

2017-11-27 Thread Joseph Qi
Hi Gang,

On 17/11/27 17:46, Gang He wrote:
> Add ocfs2_overwrite_io function, which is used to judge if
> overwrite allocated blocks, otherwise, the write will bring extra
> block allocation overhead.
> 
> Signed-off-by: Gang He 
> ---
>  fs/ocfs2/extent_map.c | 67 
> +++
>  fs/ocfs2/extent_map.h |  3 +++
>  2 files changed, 70 insertions(+)
> 
> diff --git a/fs/ocfs2/extent_map.c b/fs/ocfs2/extent_map.c
> index e4719e0..98bf325 100644
> --- a/fs/ocfs2/extent_map.c
> +++ b/fs/ocfs2/extent_map.c
> @@ -832,6 +832,73 @@ int ocfs2_fiemap(struct inode *inode, struct 
> fiemap_extent_info *fieinfo,
>   return ret;
>  }
>  
> +/* Is IO overwriting allocated blocks? */
> +int ocfs2_overwrite_io(struct inode *inode, u64 map_start, u64 map_len,
> +int wait)
> +{
> + int ret = 0, is_last;
> + u32 mapping_end, cpos;
> + struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
> + struct buffer_head *di_bh = NULL;
> + struct ocfs2_extent_rec rec;
> +
> + if (wait)
> + ret = ocfs2_inode_lock(inode, &di_bh, 0);
> + else
> + ret = ocfs2_try_inode_lock(inode, &di_bh, 0);
> + if (ret)
> + goto out;
> +
> + if (wait)
> + down_read(&OCFS2_I(inode)->ip_alloc_sem);
> + else {
> + if (!down_read_trylock(&OCFS2_I(inode)->ip_alloc_sem)) {
> + ret = -EAGAIN;
> + goto out_unlock1;
> + }
> + }
> +
> + if ((OCFS2_I(inode)->ip_dyn_features & OCFS2_INLINE_DATA_FL) &&
> +((map_start + map_len) <= i_size_read(inode)))
> + goto out_unlock2;
> +
> + cpos = map_start >> osb->s_clustersize_bits;
> + mapping_end = ocfs2_clusters_for_bytes(inode->i_sb,
> +map_start + map_len);
> + is_last = 0;
> + while (cpos < mapping_end && !is_last) {
> + ret = ocfs2_get_clusters_nocache(inode, di_bh, cpos,
> +  NULL, &rec, &is_last);
> + if (ret) {
> + mlog_errno(ret);
> + goto out_unlock2;
> + }
> +
> + if (rec.e_blkno == 0ULL)
> + break;
> +
> + if (rec.e_flags & OCFS2_EXT_REFCOUNTED)
> + break;
> +
> + cpos = le32_to_cpu(rec.e_cpos) +
> + le16_to_cpu(rec.e_leaf_clusters);
> + }
> +
> + if (cpos < mapping_end)
> + ret = 1;
> +
> +out_unlock2:
> + brelse(di_bh);
> +
> + up_read(&OCFS2_I(inode)->ip_alloc_sem);
> +
> +out_unlock1:
Should brelse(di_bh) be here?

> + ocfs2_inode_unlock(inode, 0);
> +
> +out:
> + return (ret ? 0 : 1);
I don't think EAGAIN and other error code can be handled the same. We
have to distinguish them.

Thanks,
Joseph

> +}
> +
>  int ocfs2_seek_data_hole_offset(struct file *file, loff_t *offset, int 
> whence)
>  {
>   struct inode *inode = file->f_mapping->host;
> diff --git a/fs/ocfs2/extent_map.h b/fs/ocfs2/extent_map.h
> index 67ea57d..fd9e86a 100644
> --- a/fs/ocfs2/extent_map.h
> +++ b/fs/ocfs2/extent_map.h
> @@ -53,6 +53,9 @@ int ocfs2_extent_map_get_blocks(struct inode *inode, u64 
> v_blkno, u64 *p_blkno,
>  int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>u64 map_start, u64 map_len);
>  
> +int ocfs2_overwrite_io(struct inode *inode, u64 map_start, u64 map_len,
> +int wait);
> +
>  int ocfs2_seek_data_hole_offset(struct file *file, loff_t *offset, int 
> origin);
>  
>  int ocfs2_xattr_get_clusters(struct inode *inode, u32 v_cluster,
> 

___
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel


[Ocfs2-devel] [PATCH 2/3] ocfs2: add ocfs2_overwrite_io function

2017-11-27 Thread Gang He
Add ocfs2_overwrite_io function, which is used to judge if
overwrite allocated blocks, otherwise, the write will bring extra
block allocation overhead.

Signed-off-by: Gang He 
---
 fs/ocfs2/extent_map.c | 67 +++
 fs/ocfs2/extent_map.h |  3 +++
 2 files changed, 70 insertions(+)

diff --git a/fs/ocfs2/extent_map.c b/fs/ocfs2/extent_map.c
index e4719e0..98bf325 100644
--- a/fs/ocfs2/extent_map.c
+++ b/fs/ocfs2/extent_map.c
@@ -832,6 +832,73 @@ int ocfs2_fiemap(struct inode *inode, struct 
fiemap_extent_info *fieinfo,
return ret;
 }
 
+/* Is IO overwriting allocated blocks? */
+int ocfs2_overwrite_io(struct inode *inode, u64 map_start, u64 map_len,
+  int wait)
+{
+   int ret = 0, is_last;
+   u32 mapping_end, cpos;
+   struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
+   struct buffer_head *di_bh = NULL;
+   struct ocfs2_extent_rec rec;
+
+   if (wait)
+   ret = ocfs2_inode_lock(inode, &di_bh, 0);
+   else
+   ret = ocfs2_try_inode_lock(inode, &di_bh, 0);
+   if (ret)
+   goto out;
+
+   if (wait)
+   down_read(&OCFS2_I(inode)->ip_alloc_sem);
+   else {
+   if (!down_read_trylock(&OCFS2_I(inode)->ip_alloc_sem)) {
+   ret = -EAGAIN;
+   goto out_unlock1;
+   }
+   }
+
+   if ((OCFS2_I(inode)->ip_dyn_features & OCFS2_INLINE_DATA_FL) &&
+  ((map_start + map_len) <= i_size_read(inode)))
+   goto out_unlock2;
+
+   cpos = map_start >> osb->s_clustersize_bits;
+   mapping_end = ocfs2_clusters_for_bytes(inode->i_sb,
+  map_start + map_len);
+   is_last = 0;
+   while (cpos < mapping_end && !is_last) {
+   ret = ocfs2_get_clusters_nocache(inode, di_bh, cpos,
+NULL, &rec, &is_last);
+   if (ret) {
+   mlog_errno(ret);
+   goto out_unlock2;
+   }
+
+   if (rec.e_blkno == 0ULL)
+   break;
+
+   if (rec.e_flags & OCFS2_EXT_REFCOUNTED)
+   break;
+
+   cpos = le32_to_cpu(rec.e_cpos) +
+   le16_to_cpu(rec.e_leaf_clusters);
+   }
+
+   if (cpos < mapping_end)
+   ret = 1;
+
+out_unlock2:
+   brelse(di_bh);
+
+   up_read(&OCFS2_I(inode)->ip_alloc_sem);
+
+out_unlock1:
+   ocfs2_inode_unlock(inode, 0);
+
+out:
+   return (ret ? 0 : 1);
+}
+
 int ocfs2_seek_data_hole_offset(struct file *file, loff_t *offset, int whence)
 {
struct inode *inode = file->f_mapping->host;
diff --git a/fs/ocfs2/extent_map.h b/fs/ocfs2/extent_map.h
index 67ea57d..fd9e86a 100644
--- a/fs/ocfs2/extent_map.h
+++ b/fs/ocfs2/extent_map.h
@@ -53,6 +53,9 @@ int ocfs2_extent_map_get_blocks(struct inode *inode, u64 
v_blkno, u64 *p_blkno,
 int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
 u64 map_start, u64 map_len);
 
+int ocfs2_overwrite_io(struct inode *inode, u64 map_start, u64 map_len,
+  int wait);
+
 int ocfs2_seek_data_hole_offset(struct file *file, loff_t *offset, int origin);
 
 int ocfs2_xattr_get_clusters(struct inode *inode, u32 v_cluster,
-- 
1.8.5.6


___
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel


[Ocfs2-devel] [PATCH 3/3] ocfs2: nowait aio support

2017-11-27 Thread Gang He
Return EAGAIN if any of the following checks fail for direct I/O:
Can not get the related locks immediately,
Blocks are not allocated at the write location, it will trigger
block allocation and block IO operations.

Signed-off-by: Gang He 
---
 fs/ocfs2/dir.c |  2 +-
 fs/ocfs2/dlmglue.c | 20 ++
 fs/ocfs2/dlmglue.h |  2 +-
 fs/ocfs2/file.c| 74 +-
 fs/ocfs2/mmap.c|  2 +-
 fs/ocfs2/ocfs2_trace.h | 10 ---
 6 files changed, 79 insertions(+), 31 deletions(-)

diff --git a/fs/ocfs2/dir.c b/fs/ocfs2/dir.c
index febe631..ea50901 100644
--- a/fs/ocfs2/dir.c
+++ b/fs/ocfs2/dir.c
@@ -1957,7 +1957,7 @@ int ocfs2_readdir(struct file *file, struct dir_context 
*ctx)
 
trace_ocfs2_readdir((unsigned long long)OCFS2_I(inode)->ip_blkno);
 
-   error = ocfs2_inode_lock_atime(inode, file->f_path.mnt, &lock_level);
+   error = ocfs2_inode_lock_atime(inode, file->f_path.mnt, &lock_level, 1);
if (lock_level && error >= 0) {
/* We release EX lock which used to update atime
 * and get PR lock again to reduce contention
diff --git a/fs/ocfs2/dlmglue.c b/fs/ocfs2/dlmglue.c
index 5cfbd04..feb8dbe 100644
--- a/fs/ocfs2/dlmglue.c
+++ b/fs/ocfs2/dlmglue.c
@@ -2516,13 +2516,18 @@ int ocfs2_inode_lock_with_page(struct inode *inode,
 
 int ocfs2_inode_lock_atime(struct inode *inode,
  struct vfsmount *vfsmnt,
- int *level)
+ int *level, int wait)
 {
int ret;
 
-   ret = ocfs2_inode_lock(inode, NULL, 0);
+   if (wait)
+   ret = ocfs2_inode_lock(inode, NULL, 0);
+   else
+   ret = ocfs2_try_inode_lock(inode, NULL, 0);
+
if (ret < 0) {
-   mlog_errno(ret);
+   if (ret != -EAGAIN)
+   mlog_errno(ret);
return ret;
}
 
@@ -2534,9 +2539,14 @@ int ocfs2_inode_lock_atime(struct inode *inode,
struct buffer_head *bh = NULL;
 
ocfs2_inode_unlock(inode, 0);
-   ret = ocfs2_inode_lock(inode, &bh, 1);
+   if (wait)
+   ret = ocfs2_inode_lock(inode, &bh, 1);
+   else
+   ret = ocfs2_try_inode_lock(inode, &bh, 1);
+
if (ret < 0) {
-   mlog_errno(ret);
+   if (ret != -EAGAIN)
+   mlog_errno(ret);
return ret;
}
*level = 1;
diff --git a/fs/ocfs2/dlmglue.h b/fs/ocfs2/dlmglue.h
index 05910fc..c83dbb5 100644
--- a/fs/ocfs2/dlmglue.h
+++ b/fs/ocfs2/dlmglue.h
@@ -123,7 +123,7 @@ void ocfs2_refcount_lock_res_init(struct ocfs2_lock_res 
*lockres,
 void ocfs2_open_unlock(struct inode *inode);
 int ocfs2_inode_lock_atime(struct inode *inode,
  struct vfsmount *vfsmnt,
- int *level);
+ int *level, int wait);
 int ocfs2_inode_lock_full_nested(struct inode *inode,
 struct buffer_head **ret_bh,
 int ex,
diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
index dc455d4..900f04e 100644
--- a/fs/ocfs2/file.c
+++ b/fs/ocfs2/file.c
@@ -140,6 +140,8 @@ static int ocfs2_file_open(struct inode *inode, struct file 
*file)
spin_unlock(&oi->ip_lock);
}
 
+   file->f_mode |= FMODE_NOWAIT;
+
 leave:
return status;
 }
@@ -2132,8 +2134,7 @@ static int ocfs2_prepare_inode_for_refcount(struct inode 
*inode,
 }
 
 static int ocfs2_prepare_inode_for_write(struct file *file,
-loff_t pos,
-size_t count)
+loff_t pos, size_t count, int wait)
 {
int ret = 0, meta_level = 0;
struct dentry *dentry = file->f_path.dentry;
@@ -2145,10 +2146,14 @@ static int ocfs2_prepare_inode_for_write(struct file 
*file,
 * if we need to make modifications here.
 */
for(;;) {
-   ret = ocfs2_inode_lock(inode, NULL, meta_level);
+   if (wait)
+   ret = ocfs2_inode_lock(inode, NULL, meta_level);
+   else
+   ret = ocfs2_try_inode_lock(inode, NULL, meta_level);
if (ret < 0) {
meta_level = -1;
-   mlog_errno(ret);
+   if (ret != -EAGAIN)
+   mlog_errno(ret);
goto out;
}
 
@@ -2199,7 +2204,7 @@ static int ocfs2_prepare_inode_for_write(struct file 
*file,
 
 out_unlock:
trace_ocfs2_prepare_inode_for_write(OCFS2_I(inode)->ip_blkno,
-   pos, count);
+   pos, count, wait);
 
if (meta_level >= 0)
ocfs2_inode_un

[Ocfs2-devel] [PATCH 1/3] ocfs2: add ocfs2_try_rw_lock and ocfs2_try_inode_lock

2017-11-27 Thread Gang He
Add ocfs2_try_rw_lock and ocfs2_try_inode_lock functions, which
will be used in non-block IO scenarios.

Signed-off-by: Gang He 
---
 fs/ocfs2/dlmglue.c | 22 ++
 fs/ocfs2/dlmglue.h |  4 
 2 files changed, 26 insertions(+)

diff --git a/fs/ocfs2/dlmglue.c b/fs/ocfs2/dlmglue.c
index 4689940..5cfbd04 100644
--- a/fs/ocfs2/dlmglue.c
+++ b/fs/ocfs2/dlmglue.c
@@ -1742,6 +1742,28 @@ int ocfs2_rw_lock(struct inode *inode, int write)
return status;
 }
 
+int ocfs2_try_rw_lock(struct inode *inode, int write)
+{
+   int status, level;
+   struct ocfs2_lock_res *lockres;
+   struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
+
+   mlog(0, "inode %llu try to take %s RW lock\n",
+(unsigned long long)OCFS2_I(inode)->ip_blkno,
+write ? "EXMODE" : "PRMODE");
+
+   if (ocfs2_mount_local(osb))
+   return 0;
+
+   lockres = &OCFS2_I(inode)->ip_rw_lockres;
+
+   level = write ? DLM_LOCK_EX : DLM_LOCK_PR;
+
+   status = ocfs2_cluster_lock(OCFS2_SB(inode->i_sb), lockres, level,
+   DLM_LKF_NOQUEUE, 0);
+   return status;
+}
+
 void ocfs2_rw_unlock(struct inode *inode, int write)
 {
int level = write ? DLM_LOCK_EX : DLM_LOCK_PR;
diff --git a/fs/ocfs2/dlmglue.h b/fs/ocfs2/dlmglue.h
index a7fc18b..05910fc 100644
--- a/fs/ocfs2/dlmglue.h
+++ b/fs/ocfs2/dlmglue.h
@@ -116,6 +116,7 @@ void ocfs2_refcount_lock_res_init(struct ocfs2_lock_res 
*lockres,
 int ocfs2_create_new_inode_locks(struct inode *inode);
 int ocfs2_drop_inode_locks(struct inode *inode);
 int ocfs2_rw_lock(struct inode *inode, int write);
+int ocfs2_try_rw_lock(struct inode *inode, int write);
 void ocfs2_rw_unlock(struct inode *inode, int write);
 int ocfs2_open_lock(struct inode *inode);
 int ocfs2_try_open_lock(struct inode *inode, int write);
@@ -140,6 +141,9 @@ int ocfs2_inode_lock_with_page(struct inode *inode,
 /* 99% of the time we don't want to supply any additional flags --
  * those are for very specific cases only. */
 #define ocfs2_inode_lock(i, b, e) ocfs2_inode_lock_full_nested(i, b, e, 0, 
OI_LS_NORMAL)
+#define ocfs2_try_inode_lock(i, b, e)\
+   ocfs2_inode_lock_full_nested(i, b, e, OCFS2_META_LOCK_NOQUEUE,\
+   OI_LS_NORMAL)
 void ocfs2_inode_unlock(struct inode *inode,
   int ex);
 int ocfs2_super_lock(struct ocfs2_super *osb,
-- 
1.8.5.6


___
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel


[Ocfs2-devel] [PATCH 0/3] ocfs2: add nowait aio support

2017-11-27 Thread Gang He
As you know, VFS layer has introduced non-block aio
flag IOCB_NOWAIT, which informs kernel to bail out 
if an AIO request will block for reasons such as file 
allocations, or a writeback triggered, or would block
while allocating requests while performing direct I/O.
Subsequent, pwritev2/preadv2 also can leverage this
part kernel code.
So far, ext4/xfs/btrfs have supported this feature,
I'd like to add the related code for ocfs2 file system. 

Gang He (3):
  ocfs2: add ocfs2_try_rw_lock and ocfs2_try_inode_lock
  ocfs2: add ocfs2_overwrite_io function
  ocfs2: nowait aio support

 fs/ocfs2/dir.c |  2 +-
 fs/ocfs2/dlmglue.c | 42 
 fs/ocfs2/dlmglue.h |  6 +++-
 fs/ocfs2/extent_map.c  | 67 +
 fs/ocfs2/extent_map.h  |  3 ++
 fs/ocfs2/file.c| 74 +-
 fs/ocfs2/mmap.c|  2 +-
 fs/ocfs2/ocfs2_trace.h | 10 ---
 8 files changed, 175 insertions(+), 31 deletions(-)

-- 
1.8.5.6


___
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel