This patch changes for a caller to handle the page after its bio gets an error.
Signed-off-by: Jaegeuk Kim <[email protected]>
---
fs/f2fs/checkpoint.c | 4 +++-
fs/f2fs/data.c | 27 +++++++++++++--------------
fs/f2fs/node.c | 21 ++++++++++-----------
3 files changed, 26 insertions(+), 26 deletions(-)
diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index 6032702..6fb696d 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -69,8 +69,10 @@ repeat:
fio.page = page;
- if (f2fs_submit_page_bio(&fio))
+ if (f2fs_submit_page_bio(&fio)) {
+ f2fs_put_page(page, 1);
goto repeat;
+ }
lock_page(page);
if (unlikely(page->mapping != mapping)) {
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 57042bd..b621c08 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -156,7 +156,6 @@ int f2fs_submit_page_bio(struct f2fs_io_info *fio)
if (bio_add_page(bio, page, PAGE_CACHE_SIZE, 0) < PAGE_CACHE_SIZE) {
bio_put(bio);
- f2fs_put_page(page, 1);
return -EFAULT;
}
@@ -292,15 +291,13 @@ struct page *get_read_data_page(struct inode *inode, pgoff_t index, int rw)
set_new_dnode(&dn, inode, NULL, NULL, 0);
err = get_dnode_of_data(&dn, index, LOOKUP_NODE);
- if (err) {
- f2fs_put_page(page, 1);
- return ERR_PTR(err);
- }
+ if (err)
+ goto put_err;
f2fs_put_dnode(&dn);
if (unlikely(dn.data_blkaddr == NULL_ADDR)) {
- f2fs_put_page(page, 1);
- return ERR_PTR(-ENOENT);
+ err = -ENOENT;
+ goto put_err;
}
got_it:
if (PageUptodate(page)) {
@@ -325,8 +322,12 @@ got_it:
fio.page = page;
err = f2fs_submit_page_bio(&fio);
if (err)
- return ERR_PTR(err);
+ goto put_err;
return page;
+
+put_err:
+ f2fs_put_page(page, 1);
+ return ERR_PTR(err);
}
struct page *find_data_page(struct inode *inode, pgoff_t index)
@@ -1329,7 +1330,8 @@ static int f2fs_write_begin(struct file *file, struct address_space *mapping,
{
struct inode *inode = mapping->host;
struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
- struct page *page, *ipage;
+ struct page *page = NULL;
+ struct page *ipage;
pgoff_t index = ((unsigned long long) pos) >> PAGE_CACHE_SHIFT;
struct dnode_of_data dn;
int err = 0;
@@ -1419,7 +1421,6 @@ put_next:
lock_page(page);
if (unlikely(!PageUptodate(page))) {
- f2fs_put_page(page, 1);
err = -EIO;
goto fail;
}
@@ -1431,10 +1432,8 @@ put_next:
/* avoid symlink page */
if (f2fs_encrypted_inode(inode) && S_ISREG(inode->i_mode)) {
err = f2fs_decrypt_one(inode, page);
- if (err) {
- f2fs_put_page(page, 1);
+ if (err)
goto fail;
- }
}
}
out_update:
@@ -1447,8 +1446,8 @@ put_fail:
f2fs_put_dnode(&dn);
unlock_fail:
f2fs_unlock_op(sbi);
- f2fs_put_page(page, 1);
fail:
+ f2fs_put_page(page, 1);
f2fs_write_failed(mapping, pos + len);
return err;
}
diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index a05eb35..7dd2b9d 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -991,8 +991,7 @@ fail:
/*
* Caller should do after getting the following values.
* 0: f2fs_put_page(page, 0)
- * LOCKED_PAGE: f2fs_put_page(page, 1)
- * error: nothing
+ * LOCKED_PAGE or error: f2fs_put_page(page, 1)
*/
static int read_node_page(struct page *page, int rw)
{
@@ -1010,7 +1009,6 @@ static int read_node_page(struct page *page, int rw)
if (unlikely(ni.blk_addr == NULL_ADDR)) {
ClearPageUptodate(page);
- f2fs_put_page(page, 1);
return -ENOENT;
}
@@ -1041,10 +1039,7 @@ void ra_node_page(struct f2fs_sb_info *sbi, nid_t nid)
return;
err = read_node_page(apage, READA);
- if (err == 0)
- f2fs_put_page(apage, 0);
- else if (err == LOCKED_PAGE)
- f2fs_put_page(apage, 1);
+ f2fs_put_page(apage, err ? 1 : 0);
}
struct page *get_node_page(struct f2fs_sb_info *sbi, pgoff_t nid)
@@ -1057,10 +1052,12 @@ repeat:
return ERR_PTR(-ENOMEM);
err = read_node_page(page, READ_SYNC);
- if (err < 0)
+ if (err < 0) {
+ f2fs_put_page(page, 1);
return ERR_PTR(err);
- else if (err != LOCKED_PAGE)
+ } else if (err != LOCKED_PAGE) {
lock_page(page);
+ }
if (unlikely(!PageUptodate(page) || nid != nid_of_node(page))) {
ClearPageUptodate(page);
@@ -1096,10 +1093,12 @@ repeat:
return ERR_PTR(-ENOMEM);
err = read_node_page(page, READ_SYNC);
- if (err < 0)
+ if (err < 0) {
+ f2fs_put_page(page, 1);
return ERR_PTR(err);
- else if (err == LOCKED_PAGE)
+ } else if (err == LOCKED_PAGE) {
goto page_hit;
+ }
blk_start_plug(&plug);
--
2.1.1
This patch fixes some missing error handlers.
Signed-off-by: Jaegeuk Kim <[email protected]>
---
fs/f2fs/gc.c | 23 +++++++++++++++--------
1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index 93aff5b..22007ad 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -559,27 +559,34 @@ static void move_encrypted_block(struct inode *inode, block_t bidx)
if (!fio.encrypted_page)
goto put_out;
- f2fs_submit_page_bio(&fio);
+ 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);
/* 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;
-
- /* write page */
- lock_page(fio.encrypted_page);
- set_page_writeback(fio.encrypted_page);
fio.rw = WRITE_SYNC;
f2fs_submit_page_mbio(&fio);
+ 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
Since find_dentry doesn't lock its dentry page, it can traverse intermediate
bit positions consisting of a big dentry.
For these bit positions, this patch fills the intermediate name length fields
as zeros and skips them during look-up.
Signed-off-by: Jaegeuk Kim <[email protected]>
---
fs/f2fs/dir.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index a34ebd8..71195f5 100644
--- a/fs/f2fs/dir.c
+++ b/fs/f2fs/dir.c
@@ -141,11 +141,15 @@ struct f2fs_dir_entry *find_target_dentry(struct f2fs_filename *fname,
*max_slots = max_len;
max_len = 0;
- /* remain bug on condition */
- if (unlikely(!de->name_len))
- d->max = -1;
-
- bit_pos += GET_DENTRY_SLOTS(le16_to_cpu(de->name_len));
+ /*
+ * While a large dentry is adding, it can traverse its valid bit
+ * positions, since find_entry does not lock its dentry page.
+ * In that case, let's skip the bit position.
+ */
+ if (de_name.len == 0)
+ bit_pos++;
+ else
+ bit_pos += GET_DENTRY_SLOTS(de_name.len);
}
de = NULL;
@@ -505,8 +509,12 @@ void f2fs_update_dentry(nid_t ino, umode_t mode, struct f2fs_dentry_ptr *d,
memcpy(d->filename[bit_pos], name->name, name->len);
de->ino = cpu_to_le32(ino);
set_de_type(de, mode);
- for (i = 0; i < slots; i++)
+ for (i = 0; i < slots; i++) {
+ /* fill zeros for intermediate name_len slots */
+ if (i >= 1)
+ d->dentry[bit_pos + i].name_len = 0;
test_and_set_bit_le(bit_pos + i, (void *)d->bitmap);
+ }
}
/*
--
2.1.1
> -----Original Message-----
> From: Jaegeuk Kim [mailto:[email protected]]
> Sent: Thursday, July 16, 2015 6:38 AM
> To: [email protected]; [email protected];
> [email protected]
> Cc: Jaegeuk Kim
> Subject: [f2fs-dev] [PATCH 1/3] f2fs: callers take care of the page from bio error
>
> This patch changes for a caller to handle the page after its bio gets an error.
More clear!
>
> Signed-off-by: Jaegeuk Kim <[email protected]>
Reviewed-by: Chao Yu <[email protected]>
> -----Original Message-----
> From: Jaegeuk Kim [mailto:[email protected]]
> Sent: Thursday, July 16, 2015 6:38 AM
> To: [email protected]; [email protected];
> [email protected]
> Cc: Jaegeuk Kim
> Subject: [f2fs-dev] [PATCH 2/3] f2fs: handle error cases in move_encrypted_block
>
> This patch fixes some missing error handlers.
>
> Signed-off-by: Jaegeuk Kim <[email protected]>
Reviewed-by: Chao Yu <[email protected]>
Hi Jaegeuk,
> -----Original Message-----
> From: Jaegeuk Kim [mailto:[email protected]]
> Sent: Thursday, July 16, 2015 6:38 AM
> To: [email protected]; [email protected];
> [email protected]
> Cc: Jaegeuk Kim
> Subject: [f2fs-dev] [PATCH 3/3] f2fs: allow zeroed name length during find_dentry
>
> Since find_dentry doesn't lock its dentry page, it can traverse intermediate
> bit positions consisting of a big dentry.
> For these bit positions, this patch fills the intermediate name length fields
> as zeros and skips them during look-up.
I didn't get it, AFAIK, dentry page will be accessed under protection of
i_mutex, if our inode operation has no bug, why this zeroed name_len can
exist?
Thanks,
>
> Signed-off-by: Jaegeuk Kim <[email protected]>
> ---
> fs/f2fs/dir.c | 20 ++++++++++++++------
> 1 file changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
> index a34ebd8..71195f5 100644
> --- a/fs/f2fs/dir.c
> +++ b/fs/f2fs/dir.c
> @@ -141,11 +141,15 @@ struct f2fs_dir_entry *find_target_dentry(struct f2fs_filename *fname,
> *max_slots = max_len;
> max_len = 0;
>
> - /* remain bug on condition */
> - if (unlikely(!de->name_len))
> - d->max = -1;
> -
> - bit_pos += GET_DENTRY_SLOTS(le16_to_cpu(de->name_len));
> + /*
> + * While a large dentry is adding, it can traverse its valid bit
> + * positions, since find_entry does not lock its dentry page.
> + * In that case, let's skip the bit position.
> + */
> + if (de_name.len == 0)
> + bit_pos++;
> + else
> + bit_pos += GET_DENTRY_SLOTS(de_name.len);
> }
>
> de = NULL;
> @@ -505,8 +509,12 @@ void f2fs_update_dentry(nid_t ino, umode_t mode, struct f2fs_dentry_ptr
> *d,
> memcpy(d->filename[bit_pos], name->name, name->len);
> de->ino = cpu_to_le32(ino);
> set_de_type(de, mode);
> - for (i = 0; i < slots; i++)
> + for (i = 0; i < slots; i++) {
> + /* fill zeros for intermediate name_len slots */
> + if (i >= 1)
> + d->dentry[bit_pos + i].name_len = 0;
> test_and_set_bit_le(bit_pos + i, (void *)d->bitmap);
> + }
> }
>
> /*
> --
> 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
Hi Chao,
On Thu, Jul 16, 2015 at 06:03:46PM +0800, Chao Yu wrote:
> Hi Jaegeuk,
>
> > -----Original Message-----
> > From: Jaegeuk Kim [mailto:[email protected]]
> > Sent: Thursday, July 16, 2015 6:38 AM
> > To: [email protected]; [email protected];
> > [email protected]
> > Cc: Jaegeuk Kim
> > Subject: [f2fs-dev] [PATCH 3/3] f2fs: allow zeroed name length during find_dentry
> >
> > Since find_dentry doesn't lock its dentry page, it can traverse intermediate
> > bit positions consisting of a big dentry.
> > For these bit positions, this patch fills the intermediate name length fields
> > as zeros and skips them during look-up.
>
> I didn't get it, AFAIK, dentry page will be accessed under protection of
> i_mutex, if our inode operation has no bug, why this zeroed name_len can
> exist?
Indeed. I've checked that again, and i_mutex covers this.
I'll drop this patch, and let me keep an eye on any further bug report about
this. :)
Thanks,
>
> Thanks,
>
> >
> > Signed-off-by: Jaegeuk Kim <[email protected]>
> > ---
> > fs/f2fs/dir.c | 20 ++++++++++++++------
> > 1 file changed, 14 insertions(+), 6 deletions(-)
> >
> > diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
> > index a34ebd8..71195f5 100644
> > --- a/fs/f2fs/dir.c
> > +++ b/fs/f2fs/dir.c
> > @@ -141,11 +141,15 @@ struct f2fs_dir_entry *find_target_dentry(struct f2fs_filename *fname,
> > *max_slots = max_len;
> > max_len = 0;
> >
> > - /* remain bug on condition */
> > - if (unlikely(!de->name_len))
> > - d->max = -1;
> > -
> > - bit_pos += GET_DENTRY_SLOTS(le16_to_cpu(de->name_len));
> > + /*
> > + * While a large dentry is adding, it can traverse its valid bit
> > + * positions, since find_entry does not lock its dentry page.
> > + * In that case, let's skip the bit position.
> > + */
> > + if (de_name.len == 0)
> > + bit_pos++;
> > + else
> > + bit_pos += GET_DENTRY_SLOTS(de_name.len);
> > }
> >
> > de = NULL;
> > @@ -505,8 +509,12 @@ void f2fs_update_dentry(nid_t ino, umode_t mode, struct f2fs_dentry_ptr
> > *d,
> > memcpy(d->filename[bit_pos], name->name, name->len);
> > de->ino = cpu_to_le32(ino);
> > set_de_type(de, mode);
> > - for (i = 0; i < slots; i++)
> > + for (i = 0; i < slots; i++) {
> > + /* fill zeros for intermediate name_len slots */
> > + if (i >= 1)
> > + d->dentry[bit_pos + i].name_len = 0;
> > test_and_set_bit_le(bit_pos + i, (void *)d->bitmap);
> > + }
> > }
> >
> > /*
> > --
> > 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