2015-07-14 19:18:27

by Jaegeuk Kim

[permalink] [raw]
Subject: [PATCH 1/2] f2fs: handle error cases in move_encrypted_block

This patch fixes some missing error handlers.

Signed-off-by: Jaegeuk Kim <[email protected]>
---
fs/f2fs/gc.c | 25 ++++++++++++++++---------
1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index 11046db..db11861 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -556,27 +556,34 @@ static void move_encrypted_block(struct inode *inode, block_t bidx)
if (!fio.encrypted_page)
goto put_out;

- f2fs_submit_page_bio(&fio);
-
- /* allocate block address */
- f2fs_wait_on_page_writeback(dn.node_page, NODE);
-
- allocate_data_block(fio.sbi, NULL, fio.blk_addr,
- &fio.blk_addr, &sum, CURSEG_COLD_DATA);
- dn.data_blkaddr = fio.blk_addr;
+ err = f2fs_submit_page_bio(&fio);
+ if (err)
+ goto put_page_out;

/* write page */
lock_page(fio.encrypted_page);
+
+ if (unlikely(!PageUptodate(fio.encrypted_page)))
+ goto put_page_out;
+ if (unlikely(fio.encrypted_page->mapping != META_MAPPING(fio.sbi)))
+ goto put_page_out;
+
set_page_writeback(fio.encrypted_page);
fio.rw = WRITE_SYNC;
f2fs_submit_page_mbio(&fio);

+ /* allocate block address */
+ f2fs_wait_on_page_writeback(dn.node_page, NODE);
+ allocate_data_block(fio.sbi, NULL, fio.blk_addr,
+ &fio.blk_addr, &sum, CURSEG_COLD_DATA);
+ dn.data_blkaddr = fio.blk_addr;
+
set_data_blkaddr(&dn);
f2fs_update_extent_cache(&dn);
set_inode_flag(F2FS_I(inode), FI_APPEND_WRITE);
if (page->index == 0)
set_inode_flag(F2FS_I(inode), FI_FIRST_BLOCK_WRITTEN);
-
+put_page_out:
f2fs_put_page(fio.encrypted_page, 1);
put_out:
f2fs_put_dnode(&dn);
--
2.1.1


2015-07-14 19:18:30

by Jaegeuk Kim

[permalink] [raw]
Subject: [PATCH 2/2] f2fs: use a page temporarily for encrypted gced page

That encrypted page is used temporarily, so we don't need to mark it accessed.

Signed-off-by: Jaegeuk Kim <[email protected]>
---
fs/f2fs/gc.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index db11861..ca562df 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -552,7 +552,10 @@ static void move_encrypted_block(struct inode *inode, block_t bidx)
fio.page = page;
fio.blk_addr = dn.data_blkaddr;

- fio.encrypted_page = grab_cache_page(META_MAPPING(fio.sbi), fio.blk_addr);
+ fio.encrypted_page = pagecache_get_page(META_MAPPING(fio.sbi),
+ fio.blk_addr,
+ FGP_LOCK|FGP_CREAT,
+ GFP_NOFS);
if (!fio.encrypted_page)
goto put_out;

--
2.1.1

2015-07-15 01:40:38

by Chao Yu

[permalink] [raw]
Subject: RE: [f2fs-dev] [PATCH 1/2] f2fs: handle error cases in move_encrypted_block

Hi Jaegeuk,

> -----Original Message-----
> From: Jaegeuk Kim [mailto:[email protected]]
> Sent: Wednesday, July 15, 2015 3:18 AM
> To: [email protected]; [email protected];
> [email protected]
> Cc: Jaegeuk Kim
> Subject: [f2fs-dev] [PATCH 1/2] f2fs: handle error cases in move_encrypted_block
>
> This patch fixes some missing error handlers.
>
> Signed-off-by: Jaegeuk Kim <[email protected]>
> ---
> fs/f2fs/gc.c | 25 ++++++++++++++++---------
> 1 file changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> index 11046db..db11861 100644
> --- a/fs/f2fs/gc.c
> +++ b/fs/f2fs/gc.c
> @@ -556,27 +556,34 @@ static void move_encrypted_block(struct inode *inode, block_t bidx)
> if (!fio.encrypted_page)
> goto put_out;
>
> - f2fs_submit_page_bio(&fio);
> -
> - /* allocate block address */
> - f2fs_wait_on_page_writeback(dn.node_page, NODE);
> -
> - allocate_data_block(fio.sbi, NULL, fio.blk_addr,
> - &fio.blk_addr, &sum, CURSEG_COLD_DATA);
> - dn.data_blkaddr = fio.blk_addr;
> + err = f2fs_submit_page_bio(&fio);
> + if (err)
> + goto put_page_out;

f2fs_submit_page_bio will put the page when failed.

So goto put_out is enough?

>
> /* write page */
> lock_page(fio.encrypted_page);
> +
> + if (unlikely(!PageUptodate(fio.encrypted_page)))
> + goto put_page_out;
> + if (unlikely(fio.encrypted_page->mapping != META_MAPPING(fio.sbi)))
> + goto put_page_out;
> +
> set_page_writeback(fio.encrypted_page);
> fio.rw = WRITE_SYNC;
> f2fs_submit_page_mbio(&fio);
>
> + /* allocate block address */
> + f2fs_wait_on_page_writeback(dn.node_page, NODE);
> + allocate_data_block(fio.sbi, NULL, fio.blk_addr,
> + &fio.blk_addr, &sum, CURSEG_COLD_DATA);

move above f2fs_submit_page_mbio(&fio) to here?

Thanks,

> + dn.data_blkaddr = fio.blk_addr;
> +
> set_data_blkaddr(&dn);
> f2fs_update_extent_cache(&dn);
> set_inode_flag(F2FS_I(inode), FI_APPEND_WRITE);
> if (page->index == 0)
> set_inode_flag(F2FS_I(inode), FI_FIRST_BLOCK_WRITTEN);
> -
> +put_page_out:
> f2fs_put_page(fio.encrypted_page, 1);
> put_out:
> f2fs_put_dnode(&dn);
> --
> 2.1.1
>
>
> ------------------------------------------------------------------------------
> Don't Limit Your Business. Reach for the Cloud.
> GigeNET's Cloud Solutions provide you with the tools and support that
> you need to offload your IT needs and focus on growing your business.
> Configured For All Businesses. Start Your Cloud Today.
> https://www.gigenetcloud.com/
> _______________________________________________
> Linux-f2fs-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

2015-07-15 01:44:55

by Chao Yu

[permalink] [raw]
Subject: RE: [f2fs-dev] [PATCH 2/2] f2fs: use a page temporarily for encrypted gced page

> -----Original Message-----
> From: Jaegeuk Kim [mailto:[email protected]]
> Sent: Wednesday, July 15, 2015 3:18 AM
> To: [email protected]; [email protected];
> [email protected]
> Cc: Jaegeuk Kim
> Subject: [f2fs-dev] [PATCH 2/2] f2fs: use a page temporarily for encrypted gced page
>
> That encrypted page is used temporarily, so we don't need to mark it accessed.
>
> Signed-off-by: Jaegeuk Kim <[email protected]>

Reviewed-by: Chao Yu <[email protected]>

2015-07-15 22:42:53

by Jaegeuk Kim

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH 1/2] f2fs: handle error cases in move_encrypted_block

On Wed, Jul 15, 2015 at 09:39:47AM +0800, Chao Yu wrote:
> Hi Jaegeuk,
>
> > -----Original Message-----
> > From: Jaegeuk Kim [mailto:[email protected]]
> > Sent: Wednesday, July 15, 2015 3:18 AM
> > To: [email protected]; [email protected];
> > [email protected]
> > Cc: Jaegeuk Kim
> > Subject: [f2fs-dev] [PATCH 1/2] f2fs: handle error cases in move_encrypted_block
> >
> > This patch fixes some missing error handlers.
> >
> > Signed-off-by: Jaegeuk Kim <[email protected]>
> > ---
> > fs/f2fs/gc.c | 25 ++++++++++++++++---------
> > 1 file changed, 16 insertions(+), 9 deletions(-)
> >
> > diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> > index 11046db..db11861 100644
> > --- a/fs/f2fs/gc.c
> > +++ b/fs/f2fs/gc.c
> > @@ -556,27 +556,34 @@ static void move_encrypted_block(struct inode *inode, block_t bidx)
> > if (!fio.encrypted_page)
> > goto put_out;
> >
> > - f2fs_submit_page_bio(&fio);
> > -
> > - /* allocate block address */
> > - f2fs_wait_on_page_writeback(dn.node_page, NODE);
> > -
> > - allocate_data_block(fio.sbi, NULL, fio.blk_addr,
> > - &fio.blk_addr, &sum, CURSEG_COLD_DATA);
> > - dn.data_blkaddr = fio.blk_addr;
> > + err = f2fs_submit_page_bio(&fio);
> > + if (err)
> > + goto put_page_out;
>
> f2fs_submit_page_bio will put the page when failed.
>
> So goto put_out is enough?

Right.
BTW, I realized that this is somewhat wrong, since this function needs to
take care of its page for error cases too.
I wrote a patch dealing with errors of f2fs_submit_page_bio, and submitted
it right ago.

>
> >
> > /* write page */
> > lock_page(fio.encrypted_page);
> > +
> > + if (unlikely(!PageUptodate(fio.encrypted_page)))
> > + goto put_page_out;
> > + if (unlikely(fio.encrypted_page->mapping != META_MAPPING(fio.sbi)))
> > + goto put_page_out;
> > +
> > set_page_writeback(fio.encrypted_page);
> > fio.rw = WRITE_SYNC;
> > f2fs_submit_page_mbio(&fio);
> >
> > + /* allocate block address */
> > + f2fs_wait_on_page_writeback(dn.node_page, NODE);
> > + allocate_data_block(fio.sbi, NULL, fio.blk_addr,
> > + &fio.blk_addr, &sum, CURSEG_COLD_DATA);
>
> move above f2fs_submit_page_mbio(&fio) to here?

Fixed.

Thanks,