On 03/01, Chao Yu wrote:
> Hi Jianan,

Merged 1/3 and 2/3, so please post v2 on 3/3.

Thanks,

> 
> On 2021/2/27 20:02, Huang Jianan via Linux-f2fs-devel wrote:
> > If the swapfile isn't created by pin and fallocate, it cann't be
> 
> Typo:
> 
> can't
> 
> > guaranteed section-aligned, so it may be selected by f2fs gc. When
> > gc_pin_file_threshold is reached, the address of swapfile may change,
> > but won't be synchroniz to swap_extent, so swap will write to wrong
> 
> synchronized
> 
> > address, which will cause data corruption.
> > 
> > Signed-off-by: Huang Jianan <[email protected]>
> > Signed-off-by: Guo Weichao <[email protected]>
> > ---
> >   fs/f2fs/data.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 63 insertions(+)
> > 
> > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> > index 4dbc1cafc55d..3e523d6e4643 100644
> > --- a/fs/f2fs/data.c
> > +++ b/fs/f2fs/data.c
> > @@ -3781,11 +3781,63 @@ int f2fs_migrate_page(struct address_space *mapping,
> >   #endif
> >   #ifdef CONFIG_SWAP
> > +static int f2fs_check_file_aligned(struct inode *inode)
> 
> f2fs_check_file_alignment() or f2fs_is_file_aligned()?
> 
> > +{
> > +   struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
> > +   block_t main_blkaddr = SM_I(sbi)->main_blkaddr;
> > +   block_t cur_lblock;
> > +   block_t last_lblock;
> > +   block_t pblock;
> > +   unsigned long len;
> > +   unsigned long nr_pblocks;
> > +   unsigned int blocks_per_sec = sbi->blocks_per_seg * sbi->segs_per_sec;
> 
> unsigned int blocks_per_sec = BLKS_PER_SEC(sbi);
> 
> > +   int ret;
> > +
> > +   cur_lblock = 0;
> > +   last_lblock = bytes_to_blks(inode, i_size_read(inode));
> > +   len = i_size_read(inode);
> > +
> > +   while (cur_lblock < last_lblock) {
> > +           struct f2fs_map_blocks map;
> > +           pgoff_t next_pgofs;
> > +
> > +           memset(&map, 0, sizeof(map));
> > +           map.m_lblk = cur_lblock;
> > +           map.m_len = bytes_to_blks(inode, len) - cur_lblock;
> 
> map.m_len = last_lblock - cur_lblock;
> 
> > +           map.m_next_pgofs = &next_pgofs;
> 
> map.m_next_pgofs = NULL;
> map.m_next_extent = NULL;
> 
> > +           map.m_seg_type = NO_CHECK_TYPE;
> 
> map.m_may_create = false;
> 
> > +
> > +           ret = f2fs_map_blocks(inode, &map, 0, F2FS_GET_BLOCK_FIEMAP);
> > +
> 
> Unneeded blank line.
> 
> > +           if (ret)
> > +                   goto err_out;
> > +
> > +           /* hole */
> > +           if (!(map.m_flags & F2FS_MAP_FLAGS))
> 
> ret = -ENOENT;
> 
> > +                   goto err_out;
> > +
> > +           pblock = map.m_pblk;
> > +           nr_pblocks = map.m_len;
> > +
> > +           if ((pblock - main_blkaddr) & (blocks_per_sec - 1) ||
> > +                   nr_pblocks & (blocks_per_sec - 1))
> 
> ret = -EINVAL;
> 
> > +                   goto err_out;
> > +
> > +           cur_lblock += nr_pblocks;
> > +   }
> > +
> > +   return 0;
> > +err_out:
> > +   pr_err("swapon: swapfile isn't section-aligned\n");
> 
> We should show above message only after we fail in check condition:
> 
>       if ((pblock - main_blkaddr) & (blocks_per_sec - 1) ||
>               nr_pblocks & (blocks_per_sec - 1)) {
>               f2fs_err(sbi, "Swapfile does not align to section");
>               goto err_out;
>       }
> 
> And please use f2fs_{err,warn,info..} macro rather than pr_{err,warn,info..}.
> 
> Could you please fix above related issues in check_swap_activate_fast() as 
> well.
> 
> > +   return -EINVAL;
> 
> return ret;
> 
> > +}
> > +
> >   static int check_swap_activate_fast(struct swap_info_struct *sis,
> >                             struct file *swap_file, sector_t *span)
> >   {
> >     struct address_space *mapping = swap_file->f_mapping;
> >     struct inode *inode = mapping->host;
> > +   struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
> >     sector_t cur_lblock;
> >     sector_t last_lblock;
> >     sector_t pblock;
> > @@ -3793,6 +3845,7 @@ static int check_swap_activate_fast(struct 
> > swap_info_struct *sis,
> >     sector_t highest_pblock = 0;
> >     int nr_extents = 0;
> >     unsigned long nr_pblocks;
> > +   unsigned int blocks_per_sec = sbi->blocks_per_seg * sbi->segs_per_sec;
> 
> Ditto,
> 
> >     u64 len;
> >     int ret;
> > @@ -3827,6 +3880,13 @@ static int check_swap_activate_fast(struct 
> > swap_info_struct *sis,
> >             pblock = map.m_pblk;
> >             nr_pblocks = map.m_len;
> > +           if ((pblock - SM_I(sbi)->main_blkaddr) & (blocks_per_sec - 1) ||
> > +                   nr_pblocks & (blocks_per_sec - 1)) {
> > +                   pr_err("swapon: swapfile isn't section-aligned\n");
> 
> Ditto,
> 
> > +                   ret = -EINVAL;
> > +                   goto out;
> > +           }
> > +
> >             if (cur_lblock + nr_pblocks >= sis->max)
> >                     nr_pblocks = sis->max - cur_lblock;
> > @@ -3878,6 +3938,9 @@ static int check_swap_activate(struct 
> > swap_info_struct *sis,
> >     if (PAGE_SIZE == F2FS_BLKSIZE)
> >             return check_swap_activate_fast(sis, swap_file, span);
> > +   if (f2fs_check_file_aligned(inode))
> 
> ret = f2fs_check_file_aligned();
> if (ret)
>       return ret;
> 
> Thanks,
> 
> > +           return -EINVAL;
> > +
> >     blocks_per_page = bytes_to_blks(inode, PAGE_SIZE);
> >     /*
> > 
> 
> 
> _______________________________________________
> Linux-f2fs-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


_______________________________________________
Linux-f2fs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Reply via email to