Re: [f2fs-dev] [f2fs-dev 3/5] f2fs: Add a new function: f2fs_reserve_block()

2013-10-29 Thread Huajun Li
On Tue, Oct 29, 2013 at 8:56 AM, Jaegeuk Kim jaegeuk@samsung.com wrote:
 Hi Huajun,

 2013-10-29 (화), 00:53 +0800, Huajun Li:
 Hi Jaegeuk,

 Thanks for your kindly review and comments.

 On Mon, Oct 28, 2013 at 8:28 PM, Jaegeuk Kim jaegeuk@samsung.com wrote:
  2013-10-28 (월), 21:16 +0900, Jaegeuk Kim:
  Hi,
 
 
  2013-10-26 (토), 00:01 +0800, Huajun Li:
   From: Huajun Li huajun...@intel.com
  
   Add the function f2fs_reserve_block() to easily reserve new blocks.
  
   Signed-off-by: Huajun Li huajun...@intel.com
   Signed-off-by: Haicheng Li haicheng...@linux.intel.com
   Signed-off-by: Weihong Xu weihong...@intel.com
   ---
fs/f2fs/data.c |   29 ++---
fs/f2fs/f2fs.h |1 +
2 files changed, 19 insertions(+), 11 deletions(-)
  
   diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
   index c8887d8..7b31911 100644
   --- a/fs/f2fs/data.c
   +++ b/fs/f2fs/data.c
   @@ -64,6 +64,23 @@ int reserve_new_block(struct dnode_of_data *dn)
   return 0;
}
  
   +int f2fs_reserve_block(struct inode *inode,
   +  struct dnode_of_data *dn, pgoff_t index)
 
 
  We don't need to get dnode_of_data from parameters, since it is
  used by this function only.

 After calling f2fs_reserve_block(), we need dn.data_blkaddr to check
 whether it is NEW_ADDR. So IMO, it's nice to keep this parameter.


 Ah, got it.
 BTW, I found another flows we can clean up wrt this issue.
 How about this?


This can clean up more codes, thanks a lot.

 ---
  fs/f2fs/data.c | 51 +++
  fs/f2fs/f2fs.h |  1 +
  fs/f2fs/file.c | 39 ++-
  3 files changed, 30 insertions(+), 61 deletions(-)

 diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
 index c8887d8..b8c4f76d 100644
 --- a/fs/f2fs/data.c
 +++ b/fs/f2fs/data.c
 @@ -64,6 +64,22 @@ int reserve_new_block(struct dnode_of_data *dn)
 return 0;
  }

 +int f2fs_reserve_block(struct dnode_of_data *dn, pgoff_t index)
 +{
 +   bool need_put = dn-inode_page ? false : true;
 +   int err;
 +
 +   err = get_dnode_of_data(dn, index, ALLOC_NODE);
 +   if (err)
 +   return err;
 +   if (dn-data_blkaddr == NULL_ADDR)
 +   err = reserve_new_block(dn);
 +
 +   if (need_put)
 +   f2fs_put_dnode(dn);
 +   return err;
 +}
 +
  static int check_extent_cache(struct inode *inode, pgoff_t pgofs,
 struct buffer_head *bh_result)
  {
 @@ -300,19 +316,9 @@ struct page *get_new_data_page(struct inode *inode,
 int err;

 set_new_dnode(dn, inode, npage, npage, 0);
 -   err = get_dnode_of_data(dn, index, ALLOC_NODE);
 +   err = f2fs_reserve_block(dn, index);
 if (err)
 return ERR_PTR(err);
 -
 -   if (dn.data_blkaddr == NULL_ADDR) {
 -   if (reserve_new_block(dn)) {
 -   if (!npage)
 -   f2fs_put_dnode(dn);
 -   return ERR_PTR(-ENOSPC);
 -   }
 -   }
 -   if (!npage)
 -   f2fs_put_dnode(dn);
  repeat:
 page = grab_cache_page(mapping, index);
 if (!page)
 @@ -644,21 +650,15 @@ repeat:
 *pagep = page;

 f2fs_lock_op(sbi);
 -
 set_new_dnode(dn, inode, NULL, NULL, 0);
 -   err = get_dnode_of_data(dn, index, ALLOC_NODE);
 -   if (err)
 -   goto err;
 -
 -   if (dn.data_blkaddr == NULL_ADDR)
 -   err = reserve_new_block(dn);
 -
 -   f2fs_put_dnode(dn);
 -   if (err)
 -   goto err;
 -
 +   err = f2fs_reserve_block(dn, index);
 f2fs_unlock_op(sbi);

 +   if (err) {
 +   f2fs_put_page(page, 1);
 +   return err;
 +   }
 +
 if ((len == PAGE_CACHE_SIZE) || PageUptodate(page))
 return 0;

 @@ -691,11 +691,6 @@ out:
 SetPageUptodate(page);
 clear_cold_data(page);
 return 0;
 -
 -err:
 -   f2fs_unlock_op(sbi);
 -   f2fs_put_page(page, 1);
 -   return err;
  }

  static int f2fs_write_end(struct file *file,
 diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
 index c9d394c..e77ca20 100644
 --- a/fs/f2fs/f2fs.h
 +++ b/fs/f2fs/f2fs.h
 @@ -1112,6 +1112,7 @@ void destroy_checkpoint_caches(void);
   * data.c
   */
  int reserve_new_block(struct dnode_of_data *);
 +int f2fs_reserve_block(struct dnode_of_data *, pgoff_t);
  void update_extent_cache(block_t, struct dnode_of_data *);
  struct page *find_data_page(struct inode *, pgoff_t, bool);
  struct page *get_lock_data_page(struct inode *, pgoff_t);
 diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
 index 2d4190a..21ef0d1 100644
 --- a/fs/f2fs/file.c
 +++ b/fs/f2fs/file.c
 @@ -33,7 +33,6 @@ static int f2fs_vm_page_mkwrite(struct vm_area_struct
 *vma,
 struct page *page = vmf-page;
 struct inode *inode = file_inode(vma-vm_file);
 struct f2fs_sb_info *sbi = F2FS_SB(inode-i_sb);
 -   

Re: [f2fs-dev] [f2fs-dev 3/5] f2fs: Add a new function: f2fs_reserve_block()

2013-10-28 Thread Jaegeuk Kim
2013-10-28 (월), 21:16 +0900, Jaegeuk Kim:
Hi,

 
 2013-10-26 (토), 00:01 +0800, Huajun Li:
  From: Huajun Li huajun...@intel.com
  
  Add the function f2fs_reserve_block() to easily reserve new blocks.
  
  Signed-off-by: Huajun Li huajun...@intel.com
  Signed-off-by: Haicheng Li haicheng...@linux.intel.com
  Signed-off-by: Weihong Xu weihong...@intel.com
  ---
   fs/f2fs/data.c |   29 ++---
   fs/f2fs/f2fs.h |1 +
   2 files changed, 19 insertions(+), 11 deletions(-)
  
  diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
  index c8887d8..7b31911 100644
  --- a/fs/f2fs/data.c
  +++ b/fs/f2fs/data.c
  @@ -64,6 +64,23 @@ int reserve_new_block(struct dnode_of_data *dn)
  return 0;
   }
   
  +int f2fs_reserve_block(struct inode *inode,
  +  struct dnode_of_data *dn, pgoff_t index)
 

We don't need to get dnode_of_data from parameters, since it is
used by this function only.

 
  +{
  +   int err;

+struct dnode_of_data dn;

 
  +
  +   set_new_dnode(dn, inode, NULL, NULL, 0);
  +   err = get_dnode_of_data(dn, index, ALLOC_NODE);
  +   if (err)
  +   return err;
  +   if (dn-data_blkaddr == NULL_ADDR)
  +   err = reserve_new_block(dn);
  +
  +   f2fs_put_dnode(dn);
  +
  +   return err;
  +}
  +
 

-- 
Jaegeuk Kim
Samsung



--
October Webinars: Code for Performance
Free Intel webinars can help you accelerate application performance.
Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most from 
the latest Intel processors and coprocessors. See abstracts and register 
http://pubads.g.doubleclick.net/gampad/clk?id=60135991iu=/4140/ostg.clktrk
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


Re: [f2fs-dev] [f2fs-dev 3/5] f2fs: Add a new function: f2fs_reserve_block()

2013-10-28 Thread Huajun Li
Hi Jaegeuk,

Thanks for your kindly review and comments.

On Mon, Oct 28, 2013 at 8:28 PM, Jaegeuk Kim jaegeuk@samsung.com wrote:
 2013-10-28 (월), 21:16 +0900, Jaegeuk Kim:
 Hi,


 2013-10-26 (토), 00:01 +0800, Huajun Li:
  From: Huajun Li huajun...@intel.com
 
  Add the function f2fs_reserve_block() to easily reserve new blocks.
 
  Signed-off-by: Huajun Li huajun...@intel.com
  Signed-off-by: Haicheng Li haicheng...@linux.intel.com
  Signed-off-by: Weihong Xu weihong...@intel.com
  ---
   fs/f2fs/data.c |   29 ++---
   fs/f2fs/f2fs.h |1 +
   2 files changed, 19 insertions(+), 11 deletions(-)
 
  diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
  index c8887d8..7b31911 100644
  --- a/fs/f2fs/data.c
  +++ b/fs/f2fs/data.c
  @@ -64,6 +64,23 @@ int reserve_new_block(struct dnode_of_data *dn)
  return 0;
   }
 
  +int f2fs_reserve_block(struct inode *inode,
  +  struct dnode_of_data *dn, pgoff_t index)


 We don't need to get dnode_of_data from parameters, since it is
 used by this function only.

After calling f2fs_reserve_block(), we need dn.data_blkaddr to check
whether it is NEW_ADDR. So IMO, it's nice to keep this parameter.



  +{
  +   int err;

 +struct dnode_of_data dn;


  +
  +   set_new_dnode(dn, inode, NULL, NULL, 0);
  +   err = get_dnode_of_data(dn, index, ALLOC_NODE);
  +   if (err)
  +   return err;
  +   if (dn-data_blkaddr == NULL_ADDR)
  +   err = reserve_new_block(dn);
  +
  +   f2fs_put_dnode(dn);
  +
  +   return err;
  +}
  +


 --
 Jaegeuk Kim
 Samsung


--
October Webinars: Code for Performance
Free Intel webinars can help you accelerate application performance.
Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most from 
the latest Intel processors and coprocessors. See abstracts and register 
http://pubads.g.doubleclick.net/gampad/clk?id=60135991iu=/4140/ostg.clktrk
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


Re: [f2fs-dev] [f2fs-dev 3/5] f2fs: Add a new function: f2fs_reserve_block()

2013-10-28 Thread Jaegeuk Kim
Hi Huajun,

2013-10-29 (화), 00:53 +0800, Huajun Li:
 Hi Jaegeuk,
 
 Thanks for your kindly review and comments.
 
 On Mon, Oct 28, 2013 at 8:28 PM, Jaegeuk Kim jaegeuk@samsung.com wrote:
  2013-10-28 (월), 21:16 +0900, Jaegeuk Kim:
  Hi,
 
 
  2013-10-26 (토), 00:01 +0800, Huajun Li:
   From: Huajun Li huajun...@intel.com
  
   Add the function f2fs_reserve_block() to easily reserve new blocks.
  
   Signed-off-by: Huajun Li huajun...@intel.com
   Signed-off-by: Haicheng Li haicheng...@linux.intel.com
   Signed-off-by: Weihong Xu weihong...@intel.com
   ---
fs/f2fs/data.c |   29 ++---
fs/f2fs/f2fs.h |1 +
2 files changed, 19 insertions(+), 11 deletions(-)
  
   diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
   index c8887d8..7b31911 100644
   --- a/fs/f2fs/data.c
   +++ b/fs/f2fs/data.c
   @@ -64,6 +64,23 @@ int reserve_new_block(struct dnode_of_data *dn)
   return 0;
}
  
   +int f2fs_reserve_block(struct inode *inode,
   +  struct dnode_of_data *dn, pgoff_t index)
 
 
  We don't need to get dnode_of_data from parameters, since it is
  used by this function only.
 
 After calling f2fs_reserve_block(), we need dn.data_blkaddr to check
 whether it is NEW_ADDR. So IMO, it's nice to keep this parameter.
 

Ah, got it.
BTW, I found another flows we can clean up wrt this issue.
How about this?

---
 fs/f2fs/data.c | 51 +++
 fs/f2fs/f2fs.h |  1 +
 fs/f2fs/file.c | 39 ++-
 3 files changed, 30 insertions(+), 61 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index c8887d8..b8c4f76d 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -64,6 +64,22 @@ int reserve_new_block(struct dnode_of_data *dn)
return 0;
 }
 
+int f2fs_reserve_block(struct dnode_of_data *dn, pgoff_t index)
+{
+   bool need_put = dn-inode_page ? false : true;
+   int err;
+
+   err = get_dnode_of_data(dn, index, ALLOC_NODE);
+   if (err)
+   return err;
+   if (dn-data_blkaddr == NULL_ADDR)
+   err = reserve_new_block(dn);
+
+   if (need_put)
+   f2fs_put_dnode(dn);
+   return err;
+}
+
 static int check_extent_cache(struct inode *inode, pgoff_t pgofs,
struct buffer_head *bh_result)
 {
@@ -300,19 +316,9 @@ struct page *get_new_data_page(struct inode *inode,
int err;
 
set_new_dnode(dn, inode, npage, npage, 0);
-   err = get_dnode_of_data(dn, index, ALLOC_NODE);
+   err = f2fs_reserve_block(dn, index);
if (err)
return ERR_PTR(err);
-
-   if (dn.data_blkaddr == NULL_ADDR) {
-   if (reserve_new_block(dn)) {
-   if (!npage)
-   f2fs_put_dnode(dn);
-   return ERR_PTR(-ENOSPC);
-   }
-   }
-   if (!npage)
-   f2fs_put_dnode(dn);
 repeat:
page = grab_cache_page(mapping, index);
if (!page)
@@ -644,21 +650,15 @@ repeat:
*pagep = page;
 
f2fs_lock_op(sbi);
-
set_new_dnode(dn, inode, NULL, NULL, 0);
-   err = get_dnode_of_data(dn, index, ALLOC_NODE);
-   if (err)
-   goto err;
-
-   if (dn.data_blkaddr == NULL_ADDR)
-   err = reserve_new_block(dn);
-
-   f2fs_put_dnode(dn);
-   if (err)
-   goto err;
-
+   err = f2fs_reserve_block(dn, index);
f2fs_unlock_op(sbi);
 
+   if (err) {
+   f2fs_put_page(page, 1);
+   return err;
+   }
+
if ((len == PAGE_CACHE_SIZE) || PageUptodate(page))
return 0;
 
@@ -691,11 +691,6 @@ out:
SetPageUptodate(page);
clear_cold_data(page);
return 0;
-
-err:
-   f2fs_unlock_op(sbi);
-   f2fs_put_page(page, 1);
-   return err;
 }
 
 static int f2fs_write_end(struct file *file,
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index c9d394c..e77ca20 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1112,6 +1112,7 @@ void destroy_checkpoint_caches(void);
  * data.c
  */
 int reserve_new_block(struct dnode_of_data *);
+int f2fs_reserve_block(struct dnode_of_data *, pgoff_t);
 void update_extent_cache(block_t, struct dnode_of_data *);
 struct page *find_data_page(struct inode *, pgoff_t, bool);
 struct page *get_lock_data_page(struct inode *, pgoff_t);
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 2d4190a..21ef0d1 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -33,7 +33,6 @@ static int f2fs_vm_page_mkwrite(struct vm_area_struct
*vma,
struct page *page = vmf-page;
struct inode *inode = file_inode(vma-vm_file);
struct f2fs_sb_info *sbi = F2FS_SB(inode-i_sb);
-   block_t old_blk_addr;
struct dnode_of_data dn;
int err;
 
@@ -44,24 +43,10 @@ static int f2fs_vm_page_mkwrite(struct
vm_area_struct *vma,
/* block allocation */
f2fs_lock_op(sbi);