* [PATCH] f2fs: fix to determine start_cp_addr by sbi->cur_cp_pack
@ 2016-11-25 2:32 Jaegeuk Kim
2016-12-10 7:40 ` Chao Yu
0 siblings, 1 reply; 3+ messages in thread
From: Jaegeuk Kim @ 2016-11-25 2:32 UTC (permalink / raw)
To: linux-kernel, linux-fsdevel, linux-f2fs-devel; +Cc: Jaegeuk Kim, stable
We don't guarantee cp_addr is fixed by cp_version.
This is to sync with f2fs-tools.
Cc: stable@vger.kernel.org
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
fs/f2fs/checkpoint.c | 5 +++++
fs/f2fs/f2fs.h | 14 +++-----------
2 files changed, 8 insertions(+), 11 deletions(-)
diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index 54cc6a9..bf2f44c 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -789,6 +789,11 @@ int get_valid_checkpoint(struct f2fs_sb_info *sbi)
f2fs_put_page(cur_page, 1);
}
done:
+ if (cur_page == cp1)
+ sbi->cur_cp_pack = 1;
+ else
+ sbi->cur_cp_pack = 2;
+
f2fs_put_page(cp1, 1);
f2fs_put_page(cp2, 1);
return 0;
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 62383d2..e22e7e1 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -792,6 +792,7 @@ struct f2fs_sb_info {
/* for checkpoint */
struct f2fs_checkpoint *ckpt; /* raw checkpoint pointer */
+ int cur_cp_pack; /* remain current cp pack */
spinlock_t cp_lock; /* for flag in ckpt */
struct inode *meta_inode; /* cache meta blocks */
struct mutex cp_mutex; /* checkpoint procedure lock */
@@ -1352,19 +1353,10 @@ static inline void *__bitmap_ptr(struct f2fs_sb_info *sbi, int flag)
static inline block_t __start_cp_addr(struct f2fs_sb_info *sbi)
{
- block_t start_addr;
- struct f2fs_checkpoint *ckpt = F2FS_CKPT(sbi);
- unsigned long long ckpt_version = cur_cp_version(ckpt);
-
- start_addr = le32_to_cpu(F2FS_RAW_SUPER(sbi)->cp_blkaddr);
+ block_t start_addr = le32_to_cpu(F2FS_RAW_SUPER(sbi)->cp_blkaddr);
- /*
- * odd numbered checkpoint should at cp segment 0
- * and even segment must be at cp segment 1
- */
- if (!(ckpt_version & 1))
+ if (sbi->cur_cp_pack == 2)
start_addr += sbi->blocks_per_seg;
-
return start_addr;
}
--
2.8.3
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] f2fs: fix to determine start_cp_addr by sbi->cur_cp_pack
2016-11-25 2:32 [PATCH] f2fs: fix to determine start_cp_addr by sbi->cur_cp_pack Jaegeuk Kim
@ 2016-12-10 7:40 ` Chao Yu
2016-12-12 18:56 ` Jaegeuk Kim
0 siblings, 1 reply; 3+ messages in thread
From: Chao Yu @ 2016-12-10 7:40 UTC (permalink / raw)
To: Jaegeuk Kim, linux-kernel, linux-fsdevel, linux-f2fs-devel; +Cc: stable
Hi Jaegeuk,
Let me try to understand this, in some cases, we can write a checkpoint pack
which has wrong cp_ver, like in 1st cp pack which has even version number or 2nd
cp pack which has odd version number, so if we load that kind of cp pack during
fill_super, we may load wrong summary data from another older cp pack which cp
version pointed to, is that right?
Thanks,
On 2016/11/25 10:32, Jaegeuk Kim wrote:
> We don't guarantee cp_addr is fixed by cp_version.
> This is to sync with f2fs-tools.
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
> fs/f2fs/checkpoint.c | 5 +++++
> fs/f2fs/f2fs.h | 14 +++-----------
> 2 files changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> index 54cc6a9..bf2f44c 100644
> --- a/fs/f2fs/checkpoint.c
> +++ b/fs/f2fs/checkpoint.c
> @@ -789,6 +789,11 @@ int get_valid_checkpoint(struct f2fs_sb_info *sbi)
> f2fs_put_page(cur_page, 1);
> }
> done:
> + if (cur_page == cp1)
> + sbi->cur_cp_pack = 1;
> + else
> + sbi->cur_cp_pack = 2;
> +
> f2fs_put_page(cp1, 1);
> f2fs_put_page(cp2, 1);
> return 0;
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 62383d2..e22e7e1 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -792,6 +792,7 @@ struct f2fs_sb_info {
>
> /* for checkpoint */
> struct f2fs_checkpoint *ckpt; /* raw checkpoint pointer */
> + int cur_cp_pack; /* remain current cp pack */
> spinlock_t cp_lock; /* for flag in ckpt */
> struct inode *meta_inode; /* cache meta blocks */
> struct mutex cp_mutex; /* checkpoint procedure lock */
> @@ -1352,19 +1353,10 @@ static inline void *__bitmap_ptr(struct f2fs_sb_info *sbi, int flag)
>
> static inline block_t __start_cp_addr(struct f2fs_sb_info *sbi)
> {
> - block_t start_addr;
> - struct f2fs_checkpoint *ckpt = F2FS_CKPT(sbi);
> - unsigned long long ckpt_version = cur_cp_version(ckpt);
> -
> - start_addr = le32_to_cpu(F2FS_RAW_SUPER(sbi)->cp_blkaddr);
> + block_t start_addr = le32_to_cpu(F2FS_RAW_SUPER(sbi)->cp_blkaddr);
>
> - /*
> - * odd numbered checkpoint should at cp segment 0
> - * and even segment must be at cp segment 1
> - */
> - if (!(ckpt_version & 1))
> + if (sbi->cur_cp_pack == 2)
> start_addr += sbi->blocks_per_seg;
> -
> return start_addr;
> }
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] f2fs: fix to determine start_cp_addr by sbi->cur_cp_pack
2016-12-10 7:40 ` Chao Yu
@ 2016-12-12 18:56 ` Jaegeuk Kim
0 siblings, 0 replies; 3+ messages in thread
From: Jaegeuk Kim @ 2016-12-12 18:56 UTC (permalink / raw)
To: Chao Yu; +Cc: linux-kernel, linux-fsdevel, linux-f2fs-devel, stable
On 12/10, Chao Yu wrote:
> Hi Jaegeuk,
>
> Let me try to understand this, in some cases, we can write a checkpoint pack
> which has wrong cp_ver, like in 1st cp pack which has even version number or 2nd
> cp pack which has odd version number, so if we load that kind of cp pack during
> fill_super, we may load wrong summary data from another older cp pack which cp
> version pointed to, is that right?
Yup. Especially we can use __start_cp_addr() when finding summary blocks. So, I
suspect it was able to cause missing summary blocks.
Thanks,
>
> Thanks,
>
> On 2016/11/25 10:32, Jaegeuk Kim wrote:
> > We don't guarantee cp_addr is fixed by cp_version.
> > This is to sync with f2fs-tools.
> >
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> > ---
> > fs/f2fs/checkpoint.c | 5 +++++
> > fs/f2fs/f2fs.h | 14 +++-----------
> > 2 files changed, 8 insertions(+), 11 deletions(-)
> >
> > diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> > index 54cc6a9..bf2f44c 100644
> > --- a/fs/f2fs/checkpoint.c
> > +++ b/fs/f2fs/checkpoint.c
> > @@ -789,6 +789,11 @@ int get_valid_checkpoint(struct f2fs_sb_info *sbi)
> > f2fs_put_page(cur_page, 1);
> > }
> > done:
> > + if (cur_page == cp1)
> > + sbi->cur_cp_pack = 1;
> > + else
> > + sbi->cur_cp_pack = 2;
> > +
> > f2fs_put_page(cp1, 1);
> > f2fs_put_page(cp2, 1);
> > return 0;
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > index 62383d2..e22e7e1 100644
> > --- a/fs/f2fs/f2fs.h
> > +++ b/fs/f2fs/f2fs.h
> > @@ -792,6 +792,7 @@ struct f2fs_sb_info {
> >
> > /* for checkpoint */
> > struct f2fs_checkpoint *ckpt; /* raw checkpoint pointer */
> > + int cur_cp_pack; /* remain current cp pack */
> > spinlock_t cp_lock; /* for flag in ckpt */
> > struct inode *meta_inode; /* cache meta blocks */
> > struct mutex cp_mutex; /* checkpoint procedure lock */
> > @@ -1352,19 +1353,10 @@ static inline void *__bitmap_ptr(struct f2fs_sb_info *sbi, int flag)
> >
> > static inline block_t __start_cp_addr(struct f2fs_sb_info *sbi)
> > {
> > - block_t start_addr;
> > - struct f2fs_checkpoint *ckpt = F2FS_CKPT(sbi);
> > - unsigned long long ckpt_version = cur_cp_version(ckpt);
> > -
> > - start_addr = le32_to_cpu(F2FS_RAW_SUPER(sbi)->cp_blkaddr);
> > + block_t start_addr = le32_to_cpu(F2FS_RAW_SUPER(sbi)->cp_blkaddr);
> >
> > - /*
> > - * odd numbered checkpoint should at cp segment 0
> > - * and even segment must be at cp segment 1
> > - */
> > - if (!(ckpt_version & 1))
> > + if (sbi->cur_cp_pack == 2)
> > start_addr += sbi->blocks_per_seg;
> > -
> > return start_addr;
> > }
> >
> >
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-12-12 18:56 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-25 2:32 [PATCH] f2fs: fix to determine start_cp_addr by sbi->cur_cp_pack Jaegeuk Kim
2016-12-10 7:40 ` Chao Yu
2016-12-12 18:56 ` Jaegeuk Kim
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).