Re: [PATCH] f2fs: prevent checkpoint once any IO failure is detected

[Jaegeuk Kim <jaegeuk.kim@samsung.com>]

[-- Attachment #1: Type: text/plain, Size: 2546 bytes --]

Hi,

2013-01-24 (목), 15:37 +0900, Namjae Jeon:
> 2013/1/24, Jaegeuk Kim <jaegeuk.kim@samsung.com>:
> > This patch enhances the checkpoint routine to cope with IO errors.
> >
> > Basically f2fs detects IO errors from end_io_write, and the errors are able
> > to
> > be occurred during one of data, node, and meta page writes.
> >
> > In the previous code, when an IO error is occurred during writes, f2fs sets
> > a
> > flag, CP_ERROR_FLAG, in the raw ckeckpoint buffer which will be written to
> > disk.
> > Afterwards, write_checkpoint() will check the flag and remount f2fs as a
> > read-only (ro) mode.
> >
> > However, even once f2fs is remounted as a ro mode, dirty checkpoint pages
> > are
> > freely able to be written to disk by flusher or kswapd in background.
> > In such a case, after cold reboot, f2fs would restore the checkpoint data
> > having
> > CP_ERROR_FLAG, resulting in disabling write_checkpoint and remounting f2fs
> > as
> > a ro mode again.
> >
> > Therefore, let's prevent any checkpoint page (meta) writes once an IO error
> > is
> > occurred, and remount f2fs as a ro mode right away at that moment.
> Hi Jaegeuk.
> 1. there is no meaingful return type in write_meta_page. how about
> change void function prototype ?

Agreed.

> 2. "A bug case: need to run fsck" => what is fsck ? Is there fsck for f2fs ?

We've developing a very naive fsck tool, but it's too messy to release
now. I hope to release it within a couple of months later.
BTW, I think it is no problem to add such a comment since fsck should be
implemented whether it exists or not.

> 3. do_checkpoint()->sync_meta_pages()->
>         while (index <= end) {
>                 int i, nr_pages;
>                 nr_pages = pagevec_lookup_tag(&pvec, mapping, &index,
>                                 PAGECACHE_TAG_DIRTY,
>                                 min(end - index, (pgoff_t)PAGEVEC_SIZE-1) + 1);
>                 if (nr_pages == 0)
>                         break;
> 
>                 for (i = 0; i < nr_pages; i++) {
>                         struct page *page = pvec.pages[i];
>                         lock_page(page);
>                         BUG_ON(page->mapping != mapping);
>                         BUG_ON(!PageDirty(page));
>                         clear_page_dirty_for_io(page);
>                         f2fs_write_meta_page(page, &wbc); -> At this
> point error need to be handled ?

Agreed. I'll send v2.

Thank you for reviewing. :)

-- 
Jaegeuk Kim
Samsung

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]