[PATCH RESEND] f2fs: optimize f2fs_write_cache_pages

2015-07-16 Thread Tiezhu Yang
The if statement goto continue_unlock is exactly the same when
each if condition is true that is depended on the value of both
step and is_cold_data(page) are 0 or 1. That means when the
value of step equals to is_cold_data(page), the if condition
is true and the if statement goto continue_unlock appears only
once, so it can be optimized to reduce the duplicated code.

Signed-off-by: Tiezhu Yang kernelpa...@126.com
---
 fs/f2fs/data.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index b621c08..0219bd0 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1214,9 +1214,7 @@ continue_unlock:
goto continue_unlock;
}
 
-   if (step == 0  !is_cold_data(page))
-   goto continue_unlock;
-   if (step == 1  is_cold_data(page))
+   if (step == is_cold_data(page))
goto continue_unlock;
 
if (PageWriteback(page)) {
-- 
1.8.3.1

[PATCH] scsi: ufs: fix potential memory leak

2016-06-07 Thread Tiezhu Yang
There exists potential memory leak in ufshcd_parse_clock_info(),
this patch fixes it.

Signed-off-by: Tiezhu Yang <kernelpa...@126.com>
---
 drivers/scsi/ufs/ufshcd-pltfrm.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c
index db53f38..8b057f4 100644
--- a/drivers/scsi/ufs/ufshcd-pltfrm.c
+++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
@@ -100,19 +100,19 @@ static int ufshcd_parse_clock_info(struct ufs_hba *hba)
if (ret && (ret != -EINVAL)) {
dev_err(dev, "%s: error reading array %d\n",
"freq-table-hz", ret);
-   return ret;
+   goto out_free;
}
 
for (i = 0; i < sz; i += 2) {
ret = of_property_read_string_index(np,
"clock-names", i/2, (const char **));
if (ret)
-   goto out;
+   goto out_free;
 
clki = devm_kzalloc(dev, sizeof(*clki), GFP_KERNEL);
if (!clki) {
ret = -ENOMEM;
-   goto out;
+   goto out_free;
}
 
clki->min_freq = clkfreq[i];
@@ -122,6 +122,10 @@ static int ufshcd_parse_clock_info(struct ufs_hba *hba)
clki->min_freq, clki->max_freq, clki->name);
list_add_tail(>list, >clk_list_head);
}
+
+out_free:
+   devm_kfree(dev, clkfreq);
+   clkfreq = NULL;
 out:
return ret;
 }
-- 
1.8.3.1

Re: [PATCH] scsi: ufs: fix potential memory leak

2016-06-07 Thread Tiezhu Yang
At 2016-06-08 03:12:08, "James Bottomley" 
<james.bottom...@hansenpartnership.com> wrote:
>On Wed, 2016-06-08 at 02:00 +0800, Tiezhu Yang wrote:
>> There exists potential memory leak in ufshcd_parse_clock_info(),
>> this patch fixes it.
>
>What makes you think there's a leak here?  These are all devm_
>allocations, so they're all freed when the device is.  If an error is
>returned, the device is released and the memory freed.
>
>You can argue that on successful initialization, there's no need to
>keep the clkfreq array but this patch isn't the way you'd change that.
>
>James
>
OK, thanks. I will send a v2 patch.

[PATCH v2] scsi: ufs: fix potential memory leak

2016-06-07 Thread Tiezhu Yang
If the function ufshcd_parse_clock_info returns an error, the memory clkfreq
allocated by devm_kzalloc will be freed at that time. But when the function
ufshcd_parse_clock_info returns 0 on success, there exists potential memory
leak, this patch fixes it.

Signed-off-by: Tiezhu Yang <kernelpa...@126.com>
---
 drivers/scsi/ufs/ufshcd-pltfrm.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c
index db53f38..83f757f 100644
--- a/drivers/scsi/ufs/ufshcd-pltfrm.c
+++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
@@ -122,6 +122,10 @@ static int ufshcd_parse_clock_info(struct ufs_hba *hba)
clki->min_freq, clki->max_freq, clki->name);
list_add_tail(>list, >clk_list_head);
}
+
+   devm_kfree(dev, clkfreq);
+   clkfreq = NULL;
+
 out:
return ret;
 }
-- 
1.8.3.1

[PATCH] Documentation: fix wrong value in md.txt

2016-06-16 Thread Tiezhu Yang
In the current Documentation/md.txt, the lower limit value of
stripe_cache_size is 16 and the default value is 128, but when
I update kernel to the latest mainline version and RAID5 array
is created by mdadm, then execute the following commands, it
shows an error and a difference respectively.

1) set stripe_cache_size to 16
[root@localhost ~]# echo 16 > /sys/block/md0/md/stripe_cache_size
bash: echo: write error: Invalid argument
2) read the default value of stripe_cache_size
[root@localhost ~]# cat /sys/block/md0/md/stripe_cache_size
256

I read drivers/md/raid5.c and find the following related code:
1) in function 'raid5_set_cache_size':
if (size <= 16 || size > 32768)
return -EINVAL;
2) #define NR_STRIPES   256

So the lower limit value of stripe_cache_size should be 17 and
the default value should be 256.

Signed-off-by: Tiezhu Yang <kernelpa...@126.com>
---
 Documentation/md.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/md.txt b/Documentation/md.txt
index 1a2ada4..d6e2fcf 100644
--- a/Documentation/md.txt
+++ b/Documentation/md.txt
@@ -602,7 +602,7 @@ These currently include
 
   stripe_cache_size  (currently raid5 only)
   number of entries in the stripe cache.  This is writable, but
-  there are upper and lower limits (32768, 16).  Default is 128.
+  there are upper and lower limits (32768, 17).  Default is 256.
   strip_cache_active (currently raid5 only)
   number of active entries in the stripe cache
   preread_bypass_threshold (currently raid5 only)
-- 
1.8.3.1

[PATCH] f2fs: remove unnecessary goto statement

2016-06-27 Thread Tiezhu Yang
When base_addr is NULL, there is no need to call kzfree,
it should return -ENOMEM directly. Additionally, it is
better to initialize variable 'error' with 0.

Signed-off-by: Tiezhu Yang <kernelpa...@126.com>
---
 fs/f2fs/xattr.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c
index 5564e0f..e530254 100644
--- a/fs/f2fs/xattr.c
+++ b/fs/f2fs/xattr.c
@@ -447,7 +447,7 @@ static int __f2fs_setxattr(struct inode *inode, int index,
int found, newsize;
size_t len;
__u32 new_hsize;
-   int error = -ENOMEM;
+   int error = 0;
 
if (name == NULL)
return -EINVAL;
@@ -465,7 +465,7 @@ static int __f2fs_setxattr(struct inode *inode, int index,
 
base_addr = read_all_xattrs(inode, ipage);
if (!base_addr)
-   goto exit;
+   return -ENOMEM;
 
/* find entry with wanted name. */
here = __find_xattr(base_addr, index, len, name);
-- 
1.8.3.1

[PATCH] scsi: ufs: remove unnecessary goto label

2016-06-24 Thread Tiezhu Yang
When buff_ascii kmalloc failed, there is no need to call kfree,
it should return -ENOMEM directly, this patch fixes it.

Signed-off-by: Tiezhu Yang <kernelpa...@126.com>
---
 drivers/scsi/ufs/ufshcd.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 8e8989a..f08d41a 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -2137,7 +2137,7 @@ int ufshcd_read_string_desc(struct ufs_hba *hba, int 
desc_index, u8 *buf,
buff_ascii = kmalloc(ascii_len, GFP_KERNEL);
if (!buff_ascii) {
err = -ENOMEM;
-   goto out_free_buff;
+   goto out;
}
 
/*
@@ -2156,7 +2156,6 @@ int ufshcd_read_string_desc(struct ufs_hba *hba, int 
desc_index, u8 *buf,
size - QUERY_DESC_HDR_SIZE);
memcpy(buf + QUERY_DESC_HDR_SIZE, buff_ascii, ascii_len);
buf[QUERY_DESC_LENGTH_OFFSET] = ascii_len + QUERY_DESC_HDR_SIZE;
-out_free_buff:
kfree(buff_ascii);
}
 out:
-- 
1.8.3.1

[PATCH] f2fs: make exit_f2fs_fs more clear

2016-05-17 Thread Tiezhu Yang
init_f2fs_fs does:
1) f2fs_build_trace_ios
2) init_inodecache
3) create_node_manager_caches
4) create_segment_manager_caches
5) create_checkpoint_caches
6) create_extent_cache
7) kset_create_and_add
8) kobject_init_and_add
9) register_shrinker
10) register_filesystem
11) f2fs_create_root_stats
12) proc_mkdir

exit_f2fs_fs should do cleanup in the reverse order
to make the code more clear.

Signed-off-by: Tiezhu Yang <kernelpa...@126.com>
---
 fs/f2fs/super.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index bbfeb6a..741fba1 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -1928,17 +1928,17 @@ static void __exit exit_f2fs_fs(void)
 {
remove_proc_entry("fs/f2fs", NULL);
f2fs_destroy_root_stats();
-   unregister_shrinker(_shrinker_info);
unregister_filesystem(_fs_type);
+   unregister_shrinker(_shrinker_info);
+#ifdef CONFIG_F2FS_FAULT_INJECTION
+   kobject_put(_fault_inject);
+#endif
+   kset_unregister(f2fs_kset);
destroy_extent_cache();
destroy_checkpoint_caches();
destroy_segment_manager_caches();
destroy_node_manager_caches();
destroy_inodecache();
-#ifdef CONFIG_F2FS_FAULT_INJECTION
-   kobject_put(_fault_inject);
-#endif
-   kset_unregister(f2fs_kset);
f2fs_destroy_trace_ios();
 }
 
-- 
1.8.3.1

[PATCH] md: make the code more readable in the for-loop

2016-05-08 Thread Tiezhu Yang
This patch modifies raid1.c, raid10.c and raid5.c
to make the code more readable in the for-loop
and also fixes the scripts/checkpatch.pl error:
ERROR: trailing statements should be on next line.

Signed-off-by: Tiezhu Yang <kernelpa...@126.com>
---
 drivers/md/raid1.c  |  6 +++---
 drivers/md/raid10.c |  2 +-
 drivers/md/raid5.c  | 59 +++--
 3 files changed, 34 insertions(+), 33 deletions(-)

diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index a7f2b9c..760de56 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -109,7 +109,7 @@ static void * r1buf_pool_alloc(gfp_t gfp_flags, void *data)
/*
 * Allocate bios : 1 for reading, n-1 for writing
 */
-   for (j = pi->raid_disks ; j-- ; ) {
+   for (j = pi->raid_disks - 1; j >= 0; j--) {
bio = bio_kmalloc(gfp_flags, RESYNC_PAGES);
if (!bio)
goto out_free_bio;
@@ -166,7 +166,7 @@ static void r1buf_pool_free(void *__r1_bio, void *data)
struct r1bio *r1bio = __r1_bio;
 
for (i = 0; i < RESYNC_PAGES; i++)
-   for (j = pi->raid_disks; j-- ;) {
+   for (j = pi->raid_disks - 1; j >= 0; j--) {
if (j == 0 ||
r1bio->bios[j]->bi_io_vec[i].bv_page !=
r1bio->bios[0]->bi_io_vec[i].bv_page)
@@ -1976,7 +1976,7 @@ static void process_checks(struct r1bio *r1_bio)
sbio->bi_error = 0;
 
if (!error) {
-   for (j = vcnt; j-- ; ) {
+   for (j = vcnt - 1; j >= 0; j--) {
struct page *p, *s;
p = pbio->bi_io_vec[j].bv_page;
s = sbio->bi_io_vec[j].bv_page;
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 84e24e6..52f1e07 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -157,7 +157,7 @@ static void * r10buf_pool_alloc(gfp_t gfp_flags, void *data)
/*
 * Allocate bios.
 */
-   for (j = nalloc ; j-- ; ) {
+   for (j = nalloc - 1; j >= 0; j--) {
bio = bio_kmalloc(gfp_flags, RESYNC_PAGES);
if (!bio)
goto out_free_bio;
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 4d31b23..db978ab 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -536,7 +536,7 @@ retry:
stripe_set_idx(sector, conf, previous, sh);
sh->state = 0;
 
-   for (i = sh->disks; i--; ) {
+   for (i = sh->disks - 1; i >= 0; i--) {
struct r5dev *dev = >dev[i];
 
if (dev->toread || dev->read || dev->towrite || dev->written ||
@@ -890,7 +890,7 @@ static void ops_run_io(struct stripe_head *sh, struct 
stripe_head_state *s)
 
if (r5l_write_stripe(conf->log, sh) == 0)
return;
-   for (i = disks; i--; ) {
+   for (i = disks - 1; i >= 0; i--) {
int rw;
int replace_only = 0;
struct bio *bi, *rbi;
@@ -1175,7 +1175,7 @@ static void ops_complete_biofill(void *stripe_head_ref)
(unsigned long long)sh->sector);
 
/* clear completed biofills */
-   for (i = sh->disks; i--; ) {
+   for (i = sh->disks - 1; i >= 0; i--) {
struct r5dev *dev = >dev[i];
 
/* acknowledge completion of a biofill operation */
@@ -1216,7 +1216,7 @@ static void ops_run_biofill(struct stripe_head *sh)
pr_debug("%s: stripe %llu\n", __func__,
(unsigned long long)sh->sector);
 
-   for (i = sh->disks; i--; ) {
+   for (i = sh->disks - 1; i >= 0; i--) {
struct r5dev *dev = >dev[i];
if (test_bit(R5_Wantfill, >flags)) {
struct bio *rbi;
@@ -1307,7 +1307,7 @@ ops_run_compute5(struct stripe_head *sh, struct 
raid5_percpu *percpu)
__func__, (unsigned long long)sh->sector, target);
BUG_ON(!test_bit(R5_Wantcompute, >flags));
 
-   for (i = disks; i--; )
+   for (i = disks - 1; i >= 0; i--)
if (i != target)
xor_srcs[count++] = sh->dev[i].page;
 
@@ -1407,7 +1407,7 @@ ops_run_compute6_1(struct stripe_head *sh, struct 
raid5_percpu *percpu)
} else {
/* Compute any data- or p-drive using XOR */
count = 0;
-   for (i = disks; i-- ; ) {
+   for (i = disks - 1; i >= 0; i--) {
if (i == target || i == qd_idx)
continue;
blocks[count++] = sh->dev[i].page;
@@ -1492,7 +1492,7 @@ ops_run_compute6_2(struct stripe_head *sh, struct 
raid5_percpu *percpu)
data_target = 

Re:Re: [f2fs-dev] [PATCH] f2fs: return proper error code

2016-07-12 Thread Tiezhu Yang
At 2016-07-12 09:45:43, "Chao Yu" <yuch...@huawei.com> wrote:
>On 2016/7/11 7:20, Tiezhu Yang wrote:
>> When the length of file name is more than F2FS_NAME_LEN,
>
>Seem @name indicates a xattr/key name, not a file name.

Yes, you are right. Sorry for the noise.

Thanks,

[PATCH] f2fs: return proper error code

2016-07-10 Thread Tiezhu Yang
When the length of file name is more than F2FS_NAME_LEN,
it should return -ENAMETOOLONG.

Signed-off-by: Tiezhu Yang <kernelpa...@126.com>
---
 fs/f2fs/xattr.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c
index 28a5023..b225062 100644
--- a/fs/f2fs/xattr.c
+++ b/fs/f2fs/xattr.c
@@ -364,7 +364,7 @@ int f2fs_getxattr(struct inode *inode, int index, const 
char *name,
 
len = strlen(name);
if (len > F2FS_NAME_LEN)
-   return -ERANGE;
+   return -ENAMETOOLONG;
 
base_addr = read_all_xattrs(inode, ipage);
if (!base_addr)
@@ -458,7 +458,7 @@ static int __f2fs_setxattr(struct inode *inode, int index,
len = strlen(name);
 
if (len > F2FS_NAME_LEN)
-   return -ERANGE;
+   return -ENAMETOOLONG;
 
if (size > MAX_VALUE_LEN(inode))
return -E2BIG;
-- 
1.8.3.1

[PATCH] f2fs: fix a typo in f2fs.txt

2017-02-07 Thread Tiezhu Yang
There is a typo "f2f2" in f2fs.txt, this patch fixes it.

Signed-off-by: Tiezhu Yang <kernelpa...@126.com>
---
 Documentation/filesystems/f2fs.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/filesystems/f2fs.txt 
b/Documentation/filesystems/f2fs.txt
index 0ab33d4..3d7e12d 100644
--- a/Documentation/filesystems/f2fs.txt
+++ b/Documentation/filesystems/f2fs.txt
@@ -176,7 +176,7 @@ f2fs. Each file shows the whole f2fs information.
 SYSFS ENTRIES
 

 
-Information about mounted f2f2 file systems can be found in
+Information about mounted f2fs file systems can be found in
 /sys/fs/f2fs.  Each mounted filesystem will have a directory in
 /sys/fs/f2fs based on its device name (i.e., /sys/fs/f2fs/sda).
 The files in each per-device directory are shown in table below.
-- 
1.8.3.1

[PATCH] f2fs: remove dead code f2fs_check_acl

2016-09-13 Thread Tiezhu Yang
The macro f2fs_check_acl is defined but never used since
the initial commit, this patch removes the code that has
been dead for several years.

Signed-off-by: Tiezhu Yang <kernelpa...@126.com>
---
 fs/f2fs/acl.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/f2fs/acl.h b/fs/f2fs/acl.h
index b2334d1..2c68518 100644
--- a/fs/f2fs/acl.h
+++ b/fs/f2fs/acl.h
@@ -41,7 +41,6 @@ extern int f2fs_set_acl(struct inode *, struct posix_acl *, 
int);
 extern int f2fs_init_acl(struct inode *, struct inode *, struct page *,
struct page *);
 #else
-#define f2fs_check_acl NULL
 #define f2fs_get_acl   NULL
 #define f2fs_set_acl   NULL
 
-- 
1.8.3.1

[PATCH v2] f2fs: introduce get_checkpoint_version for cleanup

2016-09-26 Thread Tiezhu Yang
There exists almost same codes when get the value of pre_version
and cur_version in function validate_checkpoint, this patch adds
get_checkpoint_version to clean up redundant codes.

Signed-off-by: Tiezhu Yang <kernelpa...@126.com>
---
 fs/f2fs/checkpoint.c | 63 ++--
 1 file changed, 37 insertions(+), 26 deletions(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index de8693c..2dbc834 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -663,45 +663,56 @@ static void write_orphan_inodes(struct f2fs_sb_info *sbi, 
block_t start_blk)
}
 }
 
-static struct page *validate_checkpoint(struct f2fs_sb_info *sbi,
-   block_t cp_addr, unsigned long long *version)
+static int get_checkpoint_version(struct f2fs_sb_info *sbi, block_t cp_addr,
+   struct f2fs_checkpoint *cp_block, struct page *cp_page,
+   unsigned long long *version)
 {
-   struct page *cp_page_1, *cp_page_2 = NULL;
unsigned long blk_size = sbi->blocksize;
-   struct f2fs_checkpoint *cp_block;
-   unsigned long long cur_version = 0, pre_version = 0;
-   size_t crc_offset;
+   size_t crc_offset = 0;
__u32 crc = 0;
 
-   /* Read the 1st cp block in this CP pack */
-   cp_page_1 = get_meta_page(sbi, cp_addr);
+   cp_page = get_meta_page(sbi, cp_addr);
+   cp_block = (struct f2fs_checkpoint *)page_address(cp_page);
 
-   /* get the version number */
-   cp_block = (struct f2fs_checkpoint *)page_address(cp_page_1);
crc_offset = le32_to_cpu(cp_block->checksum_offset);
-   if (crc_offset >= blk_size)
-   goto invalid_cp1;
+   if (crc_offset >= blk_size) {
+   f2fs_msg(sbi->sb, KERN_WARNING,
+   "%s: crc_offset is greater than or equal to blk_size.",
+   __func__);
+   return -EINVAL;
+   }
 
crc = le32_to_cpu(*((__le32 *)((unsigned char *)cp_block + 
crc_offset)));
-   if (!f2fs_crc_valid(sbi, crc, cp_block, crc_offset))
-   goto invalid_cp1;
+   if (!f2fs_crc_valid(sbi, crc, cp_block, crc_offset)) {
+   f2fs_msg(sbi->sb, KERN_WARNING,
+   "%s: f2fs_crc_valid returns false.", __func__);
+   return -EINVAL;
+   }
 
-   pre_version = cur_cp_version(cp_block);
+   *version = cur_cp_version(cp_block);
+   return 0;
+}
 
-   /* Read the 2nd cp block in this CP pack */
-   cp_addr += le32_to_cpu(cp_block->cp_pack_total_block_count) - 1;
-   cp_page_2 = get_meta_page(sbi, cp_addr);
+static struct page *validate_checkpoint(struct f2fs_sb_info *sbi,
+   block_t cp_addr, unsigned long long *version)
+{
+   struct page *cp_page_1 = NULL, *cp_page_2 = NULL;
+   struct f2fs_checkpoint *cp_block = NULL;
+   unsigned long long cur_version = 0, pre_version = 0;
+   int err;
 
-   cp_block = (struct f2fs_checkpoint *)page_address(cp_page_2);
-   crc_offset = le32_to_cpu(cp_block->checksum_offset);
-   if (crc_offset >= blk_size)
-   goto invalid_cp2;
+   err = get_checkpoint_version(sbi, cp_addr, cp_block,
+   cp_page_1, version);
+   if (err)
+   goto invalid_cp1;
+   pre_version = *version;
 
-   crc = le32_to_cpu(*((__le32 *)((unsigned char *)cp_block + 
crc_offset)));
-   if (!f2fs_crc_valid(sbi, crc, cp_block, crc_offset))
+   cp_addr += le32_to_cpu(cp_block->cp_pack_total_block_count) - 1;
+   err = get_checkpoint_version(sbi, cp_addr, cp_block,
+   cp_page_2, version);
+   if (err)
goto invalid_cp2;
-
-   cur_version = cur_cp_version(cp_block);
+   cur_version = *version;
 
if (cur_version == pre_version) {
*version = cur_version;
-- 
1.8.3.1

Re: [f2fs-dev] [PATCH v2] f2fs: introduce get_checkpoint_version for cleanup

2016-09-27 Thread Tiezhu Yang
Hi Chao,

At 2016-09-27 17:46:30, "Chao Yu" <yuch...@huawei.com> wrote:
>On 2016/9/27 10:05, Tiezhu Yang wrote:
>> There exists almost same codes when get the value of pre_version
>> and cur_version in function validate_checkpoint, this patch adds
>> get_checkpoint_version to clean up redundant codes.
>> 
>> Signed-off-by: Tiezhu Yang <kernelpa...@126.com>
>> ---
>>  fs/f2fs/checkpoint.c | 63 
>> ++--
>>  1 file changed, 37 insertions(+), 26 deletions(-)
>> 
>> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
>> index de8693c..2dbc834 100644
>> --- a/fs/f2fs/checkpoint.c
>> +++ b/fs/f2fs/checkpoint.c
>> @@ -663,45 +663,56 @@ static void write_orphan_inodes(struct f2fs_sb_info 
>> *sbi, block_t start_blk)
>>  }
>>  }
>>  
>> -static struct page *validate_checkpoint(struct f2fs_sb_info *sbi,
>> -block_t cp_addr, unsigned long long *version)
>> +static int get_checkpoint_version(struct f2fs_sb_info *sbi, block_t cp_addr,
>> +struct f2fs_checkpoint *cp_block, struct page *cp_page,
>
>struct f2fs_checkpoint **cp_block, struct page **cp_page,

Thank you for pointing out my mistake, I will send a v3 patch.

Thanks,
>
>> +unsigned long long *version)
>>  {
>> -struct page *cp_page_1, *cp_page_2 = NULL;
>>  unsigned long blk_size = sbi->blocksize;
>> -struct f2fs_checkpoint *cp_block;
>> -unsigned long long cur_version = 0, pre_version = 0;
>> -size_t crc_offset;
>> +size_t crc_offset = 0;
>>  __u32 crc = 0;
>>  
>> -/* Read the 1st cp block in this CP pack */
>> -cp_page_1 = get_meta_page(sbi, cp_addr);
>> +cp_page = get_meta_page(sbi, cp_addr);
>> +cp_block = (struct f2fs_checkpoint *)page_address(cp_page);
>
>*cp_page = get_meta_page(sbi, cp_addr);
>*cp_block = (struct f2fs_checkpoint *)page_address(*cp_page);
>
>>  
>> -/* get the version number */
>> -cp_block = (struct f2fs_checkpoint *)page_address(cp_page_1);
>>  crc_offset = le32_to_cpu(cp_block->checksum_offset);
>
>ditto
>
>> -if (crc_offset >= blk_size)
>> -goto invalid_cp1;
>> +if (crc_offset >= blk_size) {
>> +f2fs_msg(sbi->sb, KERN_WARNING,
>> +"%s: crc_offset is greater than or equal to blk_size.",
>> +__func__);
>> +return -EINVAL;
>> +}
>>  
>>  crc = le32_to_cpu(*((__le32 *)((unsigned char *)cp_block + 
>> crc_offset)));
>> -if (!f2fs_crc_valid(sbi, crc, cp_block, crc_offset))
>> -goto invalid_cp1;
>> +if (!f2fs_crc_valid(sbi, crc, cp_block, crc_offset)) {
>
>ditto
>
>> +f2fs_msg(sbi->sb, KERN_WARNING,
>> +"%s: f2fs_crc_valid returns false.", __func__);
>> +return -EINVAL;
>> +}
>>  
>> -pre_version = cur_cp_version(cp_block);
>> +*version = cur_cp_version(cp_block);
>> +return 0;
>> +}
>>  
>> -/* Read the 2nd cp block in this CP pack */
>> -cp_addr += le32_to_cpu(cp_block->cp_pack_total_block_count) - 1;
>> -cp_page_2 = get_meta_page(sbi, cp_addr);
>> +static struct page *validate_checkpoint(struct f2fs_sb_info *sbi,
>> +block_t cp_addr, unsigned long long *version)
>> +{
>> +struct page *cp_page_1 = NULL, *cp_page_2 = NULL;
>> +struct f2fs_checkpoint *cp_block = NULL;
>> +unsigned long long cur_version = 0, pre_version = 0;
>> +int err;
>>  
>> -cp_block = (struct f2fs_checkpoint *)page_address(cp_page_2);
>> -crc_offset = le32_to_cpu(cp_block->checksum_offset);
>> -if (crc_offset >= blk_size)
>> -goto invalid_cp2;
>> +err = get_checkpoint_version(sbi, cp_addr, cp_block,
>> +cp_page_1, version);
>
>err = get_checkpoint_version(sbi, cp_addr, _block, _page_1, version);
>
>> +if (err)
>> +goto invalid_cp1;
>> +pre_version = *version;
>>  
>> -crc = le32_to_cpu(*((__le32 *)((unsigned char *)cp_block + 
>> crc_offset)));
>> -if (!f2fs_crc_valid(sbi, crc, cp_block, crc_offset))
>> +cp_addr += le32_to_cpu(cp_block->cp_pack_total_block_count) - 1;
>> +err = get_checkpoint_version(sbi, cp_addr, cp_block,
>> +cp_page_2, version);
>
>ditto
>
>Thanks,
>
>> +if (err)
>>  goto invalid_cp2;
>> -
>> -cur_version = cur_cp_version(cp_block);
>> +cur_version = *version;
>>  
>>  if (cur_version == pre_version) {
>>  *version = cur_version;
>> 
>

Re: [PATCH] f2fs: introduce get_checkpoint_version for cleanup

2016-09-26 Thread Tiezhu Yang
At 2016-09-27 02:57:16, "Jaegeuk Kim" <jaeg...@kernel.org> wrote:
>Hi Tiezhu,
>
>On Sun, Sep 25, 2016 at 05:50:44PM +0800, Tiezhu Yang wrote:
>> There exists almost same codes when get the value of pre_version
>> and cur_version in function validate_checkpoint, this patch adds
>> get_checkpoint_version to clean up redundant codes.
>> 
>> Signed-off-by: Tiezhu Yang <kernelpa...@126.com>
>> ---
>>  fs/f2fs/checkpoint.c | 52 
>> 
>>  1 file changed, 28 insertions(+), 24 deletions(-)
>> 
>> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
>> index a655d75..facede2 100644
>> --- a/fs/f2fs/checkpoint.c
>> +++ b/fs/f2fs/checkpoint.c
>> @@ -666,45 +666,49 @@ static void write_orphan_inodes(struct f2fs_sb_info 
>> *sbi, block_t start_blk)
>>  }
>>  }
>>  
>> -static struct page *validate_checkpoint(struct f2fs_sb_info *sbi,
>> -block_t cp_addr, unsigned long long *version)
>> +static int get_checkpoint_version(struct f2fs_sb_info *sbi, block_t cp_addr,
>> +struct f2fs_checkpoint *cp_block, struct page *cp_page,
>> +unsigned long long *version)
>>  {
>> -struct page *cp_page_1, *cp_page_2 = NULL;
>>  unsigned long blk_size = sbi->blocksize;
>> -struct f2fs_checkpoint *cp_block;
>> -unsigned long long cur_version = 0, pre_version = 0;
>> -size_t crc_offset;
>> +size_t crc_offset = 0;
>>  __u32 crc = 0;
>>  
>> -/* Read the 1st cp block in this CP pack */
>> -cp_page_1 = get_meta_page(sbi, cp_addr);
>> +cp_page = get_meta_page(sbi, cp_addr);
>> +cp_block = (struct f2fs_checkpoint *)page_address(cp_page);
>>  
>> -/* get the version number */
>> -cp_block = (struct f2fs_checkpoint *)page_address(cp_page_1);
>>  crc_offset = le32_to_cpu(cp_block->checksum_offset);
>>  if (crc_offset >= blk_size)
>> -goto invalid_cp1;
>> +return -1;
>
>How about using -EINVAL with a kernel message?

OK, thanks for your suggestion, I will send a v2 patch.

Thanks,
>
>>  
>>  crc = le32_to_cpu(*((__le32 *)((unsigned char *)cp_block + 
>> crc_offset)));
>>  if (!f2fs_crc_valid(sbi, crc, cp_block, crc_offset))
>> -goto invalid_cp1;
>> +return -1;
>
>ditto.
>
>> -pre_version = cur_cp_version(cp_block);
>> +*version = cur_cp_version(cp_block);
>> +return 0;
>> +}
>>  
>> -/* Read the 2nd cp block in this CP pack */
>> -cp_addr += le32_to_cpu(cp_block->cp_pack_total_block_count) - 1;
>> -cp_page_2 = get_meta_page(sbi, cp_addr);
>> +static struct page *validate_checkpoint(struct f2fs_sb_info *sbi,
>> +block_t cp_addr, unsigned long long *version)
>> +{
>> +struct page *cp_page_1 = NULL, *cp_page_2 = NULL;
>> +struct f2fs_checkpoint *cp_block = NULL;
>> +unsigned long long cur_version = 0, pre_version = 0;
>> +int ret = 0;
>
>   int err;
>
>>  
>> -cp_block = (struct f2fs_checkpoint *)page_address(cp_page_2);
>> -crc_offset = le32_to_cpu(cp_block->checksum_offset);
>> -if (crc_offset >= blk_size)
>> -goto invalid_cp2;
>> +ret = get_checkpoint_version(sbi, cp_addr, cp_block,
>> +cp_page_1, version);
>> +if (ret == -1)
>
>   if (err)
>
>> +goto invalid_cp1;
>> +pre_version = *version;
>>  
>> -crc = le32_to_cpu(*((__le32 *)((unsigned char *)cp_block + 
>> crc_offset)));
>> -if (!f2fs_crc_valid(sbi, crc, cp_block, crc_offset))
>> +cp_addr += le32_to_cpu(cp_block->cp_pack_total_block_count) - 1;
>> +ret = get_checkpoint_version(sbi, cp_addr, cp_block,
>> +cp_page_2, version);
>> +if (ret == -1)
>
>   if (err)
>
>>  goto invalid_cp2;
>> -
>> -cur_version = cur_cp_version(cp_block);
>> +cur_version = *version;
>>  
>>  if (cur_version == pre_version) {
>>  *version = cur_version;
>> -- 
>> 1.8.3.1


[PATCH v3] f2fs: introduce get_checkpoint_version for cleanup

2016-09-27 Thread Tiezhu Yang
There exists almost same codes when get the value of pre_version
and cur_version in function validate_checkpoint, this patch adds
get_checkpoint_version to clean up redundant codes.

Signed-off-by: Tiezhu Yang <kernelpa...@126.com>
---
 fs/f2fs/checkpoint.c | 68 ++--
 1 file changed, 40 insertions(+), 28 deletions(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index de8693c..06992a7 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -663,45 +663,57 @@ static void write_orphan_inodes(struct f2fs_sb_info *sbi, 
block_t start_blk)
}
 }
 
-static struct page *validate_checkpoint(struct f2fs_sb_info *sbi,
-   block_t cp_addr, unsigned long long *version)
+static int get_checkpoint_version(struct f2fs_sb_info *sbi, block_t cp_addr,
+   struct f2fs_checkpoint **cp_block, struct page **cp_page,
+   unsigned long long *version)
 {
-   struct page *cp_page_1, *cp_page_2 = NULL;
unsigned long blk_size = sbi->blocksize;
-   struct f2fs_checkpoint *cp_block;
-   unsigned long long cur_version = 0, pre_version = 0;
-   size_t crc_offset;
+   size_t crc_offset = 0;
__u32 crc = 0;
 
-   /* Read the 1st cp block in this CP pack */
-   cp_page_1 = get_meta_page(sbi, cp_addr);
+   *cp_page = get_meta_page(sbi, cp_addr);
+   *cp_block = (struct f2fs_checkpoint *)page_address(*cp_page);
 
-   /* get the version number */
-   cp_block = (struct f2fs_checkpoint *)page_address(cp_page_1);
-   crc_offset = le32_to_cpu(cp_block->checksum_offset);
-   if (crc_offset >= blk_size)
-   goto invalid_cp1;
+   crc_offset = le32_to_cpu((*cp_block)->checksum_offset);
+   if (crc_offset >= blk_size) {
+   f2fs_msg(sbi->sb, KERN_WARNING,
+   "%s: crc_offset is greater than or equal to blk_size.",
+   __func__);
+   return -EINVAL;
+   }
 
-   crc = le32_to_cpu(*((__le32 *)((unsigned char *)cp_block + 
crc_offset)));
-   if (!f2fs_crc_valid(sbi, crc, cp_block, crc_offset))
-   goto invalid_cp1;
+   crc = le32_to_cpu(*((__le32 *)((unsigned char *)*cp_block
+   + crc_offset)));
+   if (!f2fs_crc_valid(sbi, crc, *cp_block, crc_offset)) {
+   f2fs_msg(sbi->sb, KERN_WARNING,
+   "%s: f2fs_crc_valid returns false.", __func__);
+   return -EINVAL;
+   }
 
-   pre_version = cur_cp_version(cp_block);
+   *version = cur_cp_version(*cp_block);
+   return 0;
+}
 
-   /* Read the 2nd cp block in this CP pack */
-   cp_addr += le32_to_cpu(cp_block->cp_pack_total_block_count) - 1;
-   cp_page_2 = get_meta_page(sbi, cp_addr);
+static struct page *validate_checkpoint(struct f2fs_sb_info *sbi,
+   block_t cp_addr, unsigned long long *version)
+{
+   struct page *cp_page_1 = NULL, *cp_page_2 = NULL;
+   struct f2fs_checkpoint *cp_block = NULL;
+   unsigned long long cur_version = 0, pre_version = 0;
+   int err;
 
-   cp_block = (struct f2fs_checkpoint *)page_address(cp_page_2);
-   crc_offset = le32_to_cpu(cp_block->checksum_offset);
-   if (crc_offset >= blk_size)
-   goto invalid_cp2;
+   err = get_checkpoint_version(sbi, cp_addr, _block,
+   _page_1, version);
+   if (err)
+   goto invalid_cp1;
+   pre_version = *version;
 
-   crc = le32_to_cpu(*((__le32 *)((unsigned char *)cp_block + 
crc_offset)));
-   if (!f2fs_crc_valid(sbi, crc, cp_block, crc_offset))
+   cp_addr += le32_to_cpu(cp_block->cp_pack_total_block_count) - 1;
+   err = get_checkpoint_version(sbi, cp_addr, _block,
+   _page_2, version);
+   if (err)
goto invalid_cp2;
-
-   cur_version = cur_cp_version(cp_block);
+   cur_version = *version;
 
if (cur_version == pre_version) {
*version = cur_version;
-- 
1.8.3.1

Re:[PATCH v4] f2fs: introduce get_checkpoint_version for cleanup

2016-09-29 Thread Tiezhu Yang
At 2016-09-30 07:47:58, "Tiezhu Yang" <kernelpa...@126.com> wrote:
>There exists almost same codes when get the value of pre_version
>and cur_version in function validate_checkpoint, this patch adds
>get_checkpoint_version to clean up redundant codes.
>
>Signed-off-by: Tiezhu Yang <kernelpa...@126.com>
>---
> fs/f2fs/checkpoint.c | 66 ++--
> 1 file changed, 38 insertions(+), 28 deletions(-)
>
>diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
>index de8693c..764ed0b 100644
>--- a/fs/f2fs/checkpoint.c
>+++ b/fs/f2fs/checkpoint.c
>@@ -663,45 +663,55 @@ static void write_orphan_inodes(struct f2fs_sb_info 
>*sbi, block_t start_blk)
>   }
> }
> 
>-static struct page *validate_checkpoint(struct f2fs_sb_info *sbi,
>-  block_t cp_addr, unsigned long long *version)
>+static int get_checkpoint_version(struct f2fs_sb_info *sbi, block_t cp_addr,
>+  struct f2fs_checkpoint **cp_block, struct page **cp_page,
>+  unsigned long long *version)
> {
>-  struct page *cp_page_1, *cp_page_2 = NULL;
>   unsigned long blk_size = sbi->blocksize;
>-  struct f2fs_checkpoint *cp_block;
>-  unsigned long long cur_version = 0, pre_version = 0;
>-  size_t crc_offset;
>+  size_t crc_offset = 0;
>   __u32 crc = 0;
> 
>-  /* Read the 1st cp block in this CP pack */
>-  cp_page_1 = get_meta_page(sbi, cp_addr);
>+  *cp_page = get_meta_page(sbi, cp_addr);
>+  *cp_block = (struct f2fs_checkpoint *)page_address(*cp_page);
> 
>-  /* get the version number */
>-  cp_block = (struct f2fs_checkpoint *)page_address(cp_page_1);
>-  crc_offset = le32_to_cpu(cp_block->checksum_offset);
>-  if (crc_offset >= blk_size)
>-  goto invalid_cp1;
>+  crc_offset = le32_to_cpu((*cp_block)->checksum_offset);
>+  if (crc_offset >= blk_size) {
>+  f2fs_msg(sbi->sb, KERN_WARNING,
>+  "invalid crc_offset: %zu.", crc_offset);

Sorry, The full stop '.' is needless, I will resend it.

Thanks,
>+  return -EINVAL;
>+  }
> 
>-  crc = le32_to_cpu(*((__le32 *)((unsigned char *)cp_block + 
>crc_offset)));
>-  if (!f2fs_crc_valid(sbi, crc, cp_block, crc_offset))
>-  goto invalid_cp1;
>+  crc = le32_to_cpu(*((__le32 *)((unsigned char *)*cp_block
>+  + crc_offset)));
>+  if (!f2fs_crc_valid(sbi, crc, *cp_block, crc_offset)) {
>+  f2fs_msg(sbi->sb, KERN_WARNING, "invalid crc value.");

ditto

>+  return -EINVAL;
>+  }
> 
>-  pre_version = cur_cp_version(cp_block);
>+  *version = cur_cp_version(*cp_block);
>+  return 0;
>+}
> 
>-  /* Read the 2nd cp block in this CP pack */
>-  cp_addr += le32_to_cpu(cp_block->cp_pack_total_block_count) - 1;
>-  cp_page_2 = get_meta_page(sbi, cp_addr);
>+static struct page *validate_checkpoint(struct f2fs_sb_info *sbi,
>+  block_t cp_addr, unsigned long long *version)
>+{
>+  struct page *cp_page_1 = NULL, *cp_page_2 = NULL;
>+  struct f2fs_checkpoint *cp_block = NULL;
>+  unsigned long long cur_version = 0, pre_version = 0;
>+  int err;
> 
>-  cp_block = (struct f2fs_checkpoint *)page_address(cp_page_2);
>-  crc_offset = le32_to_cpu(cp_block->checksum_offset);
>-  if (crc_offset >= blk_size)
>-  goto invalid_cp2;
>+  err = get_checkpoint_version(sbi, cp_addr, _block,
>+  _page_1, version);
>+  if (err)
>+  goto invalid_cp1;
>+  pre_version = *version;
> 
>-  crc = le32_to_cpu(*((__le32 *)((unsigned char *)cp_block + 
>crc_offset)));
>-  if (!f2fs_crc_valid(sbi, crc, cp_block, crc_offset))
>+  cp_addr += le32_to_cpu(cp_block->cp_pack_total_block_count) - 1;
>+  err = get_checkpoint_version(sbi, cp_addr, _block,
>+  _page_2, version);
>+  if (err)
>   goto invalid_cp2;
>-
>-  cur_version = cur_cp_version(cp_block);
>+  cur_version = *version;
> 
>   if (cur_version == pre_version) {
>   *version = cur_version;
>-- 
>1.8.3.1


[PATCH v4 RESEND] f2fs: introduce get_checkpoint_version for cleanup

2016-09-29 Thread Tiezhu Yang
There exists almost same codes when get the value of pre_version
and cur_version in function validate_checkpoint, this patch adds
get_checkpoint_version to clean up redundant codes.

Signed-off-by: Tiezhu Yang <kernelpa...@126.com>
---
 fs/f2fs/checkpoint.c | 66 ++--
 1 file changed, 38 insertions(+), 28 deletions(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index de8693c..764ed0b 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -663,45 +663,55 @@ static void write_orphan_inodes(struct f2fs_sb_info *sbi, 
block_t start_blk)
}
 }
 
-static struct page *validate_checkpoint(struct f2fs_sb_info *sbi,
-   block_t cp_addr, unsigned long long *version)
+static int get_checkpoint_version(struct f2fs_sb_info *sbi, block_t cp_addr,
+   struct f2fs_checkpoint **cp_block, struct page **cp_page,
+   unsigned long long *version)
 {
-   struct page *cp_page_1, *cp_page_2 = NULL;
unsigned long blk_size = sbi->blocksize;
-   struct f2fs_checkpoint *cp_block;
-   unsigned long long cur_version = 0, pre_version = 0;
-   size_t crc_offset;
+   size_t crc_offset = 0;
__u32 crc = 0;
 
-   /* Read the 1st cp block in this CP pack */
-   cp_page_1 = get_meta_page(sbi, cp_addr);
+   *cp_page = get_meta_page(sbi, cp_addr);
+   *cp_block = (struct f2fs_checkpoint *)page_address(*cp_page);
 
-   /* get the version number */
-   cp_block = (struct f2fs_checkpoint *)page_address(cp_page_1);
-   crc_offset = le32_to_cpu(cp_block->checksum_offset);
-   if (crc_offset >= blk_size)
-   goto invalid_cp1;
+   crc_offset = le32_to_cpu((*cp_block)->checksum_offset);
+   if (crc_offset >= blk_size) {
+   f2fs_msg(sbi->sb, KERN_WARNING,
+   "invalid crc_offset: %zu", crc_offset);
+   return -EINVAL;
+   }
 
-   crc = le32_to_cpu(*((__le32 *)((unsigned char *)cp_block + 
crc_offset)));
-   if (!f2fs_crc_valid(sbi, crc, cp_block, crc_offset))
-   goto invalid_cp1;
+   crc = le32_to_cpu(*((__le32 *)((unsigned char *)*cp_block
+   + crc_offset)));
+   if (!f2fs_crc_valid(sbi, crc, *cp_block, crc_offset)) {
+   f2fs_msg(sbi->sb, KERN_WARNING, "invalid crc value");
+   return -EINVAL;
+   }
 
-   pre_version = cur_cp_version(cp_block);
+   *version = cur_cp_version(*cp_block);
+   return 0;
+}
 
-   /* Read the 2nd cp block in this CP pack */
-   cp_addr += le32_to_cpu(cp_block->cp_pack_total_block_count) - 1;
-   cp_page_2 = get_meta_page(sbi, cp_addr);
+static struct page *validate_checkpoint(struct f2fs_sb_info *sbi,
+   block_t cp_addr, unsigned long long *version)
+{
+   struct page *cp_page_1 = NULL, *cp_page_2 = NULL;
+   struct f2fs_checkpoint *cp_block = NULL;
+   unsigned long long cur_version = 0, pre_version = 0;
+   int err;
 
-   cp_block = (struct f2fs_checkpoint *)page_address(cp_page_2);
-   crc_offset = le32_to_cpu(cp_block->checksum_offset);
-   if (crc_offset >= blk_size)
-   goto invalid_cp2;
+   err = get_checkpoint_version(sbi, cp_addr, _block,
+   _page_1, version);
+   if (err)
+   goto invalid_cp1;
+   pre_version = *version;
 
-   crc = le32_to_cpu(*((__le32 *)((unsigned char *)cp_block + 
crc_offset)));
-   if (!f2fs_crc_valid(sbi, crc, cp_block, crc_offset))
+   cp_addr += le32_to_cpu(cp_block->cp_pack_total_block_count) - 1;
+   err = get_checkpoint_version(sbi, cp_addr, _block,
+   _page_2, version);
+   if (err)
goto invalid_cp2;
-
-   cur_version = cur_cp_version(cp_block);
+   cur_version = *version;
 
if (cur_version == pre_version) {
*version = cur_version;
-- 
1.8.3.1

[PATCH v4] f2fs: introduce get_checkpoint_version for cleanup

2016-09-29 Thread Tiezhu Yang
There exists almost same codes when get the value of pre_version
and cur_version in function validate_checkpoint, this patch adds
get_checkpoint_version to clean up redundant codes.

Signed-off-by: Tiezhu Yang <kernelpa...@126.com>
---
 fs/f2fs/checkpoint.c | 66 ++--
 1 file changed, 38 insertions(+), 28 deletions(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index de8693c..764ed0b 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -663,45 +663,55 @@ static void write_orphan_inodes(struct f2fs_sb_info *sbi, 
block_t start_blk)
}
 }
 
-static struct page *validate_checkpoint(struct f2fs_sb_info *sbi,
-   block_t cp_addr, unsigned long long *version)
+static int get_checkpoint_version(struct f2fs_sb_info *sbi, block_t cp_addr,
+   struct f2fs_checkpoint **cp_block, struct page **cp_page,
+   unsigned long long *version)
 {
-   struct page *cp_page_1, *cp_page_2 = NULL;
unsigned long blk_size = sbi->blocksize;
-   struct f2fs_checkpoint *cp_block;
-   unsigned long long cur_version = 0, pre_version = 0;
-   size_t crc_offset;
+   size_t crc_offset = 0;
__u32 crc = 0;
 
-   /* Read the 1st cp block in this CP pack */
-   cp_page_1 = get_meta_page(sbi, cp_addr);
+   *cp_page = get_meta_page(sbi, cp_addr);
+   *cp_block = (struct f2fs_checkpoint *)page_address(*cp_page);
 
-   /* get the version number */
-   cp_block = (struct f2fs_checkpoint *)page_address(cp_page_1);
-   crc_offset = le32_to_cpu(cp_block->checksum_offset);
-   if (crc_offset >= blk_size)
-   goto invalid_cp1;
+   crc_offset = le32_to_cpu((*cp_block)->checksum_offset);
+   if (crc_offset >= blk_size) {
+   f2fs_msg(sbi->sb, KERN_WARNING,
+   "invalid crc_offset: %zu.", crc_offset);
+   return -EINVAL;
+   }
 
-   crc = le32_to_cpu(*((__le32 *)((unsigned char *)cp_block + 
crc_offset)));
-   if (!f2fs_crc_valid(sbi, crc, cp_block, crc_offset))
-   goto invalid_cp1;
+   crc = le32_to_cpu(*((__le32 *)((unsigned char *)*cp_block
+   + crc_offset)));
+   if (!f2fs_crc_valid(sbi, crc, *cp_block, crc_offset)) {
+   f2fs_msg(sbi->sb, KERN_WARNING, "invalid crc value.");
+   return -EINVAL;
+   }
 
-   pre_version = cur_cp_version(cp_block);
+   *version = cur_cp_version(*cp_block);
+   return 0;
+}
 
-   /* Read the 2nd cp block in this CP pack */
-   cp_addr += le32_to_cpu(cp_block->cp_pack_total_block_count) - 1;
-   cp_page_2 = get_meta_page(sbi, cp_addr);
+static struct page *validate_checkpoint(struct f2fs_sb_info *sbi,
+   block_t cp_addr, unsigned long long *version)
+{
+   struct page *cp_page_1 = NULL, *cp_page_2 = NULL;
+   struct f2fs_checkpoint *cp_block = NULL;
+   unsigned long long cur_version = 0, pre_version = 0;
+   int err;
 
-   cp_block = (struct f2fs_checkpoint *)page_address(cp_page_2);
-   crc_offset = le32_to_cpu(cp_block->checksum_offset);
-   if (crc_offset >= blk_size)
-   goto invalid_cp2;
+   err = get_checkpoint_version(sbi, cp_addr, _block,
+   _page_1, version);
+   if (err)
+   goto invalid_cp1;
+   pre_version = *version;
 
-   crc = le32_to_cpu(*((__le32 *)((unsigned char *)cp_block + 
crc_offset)));
-   if (!f2fs_crc_valid(sbi, crc, cp_block, crc_offset))
+   cp_addr += le32_to_cpu(cp_block->cp_pack_total_block_count) - 1;
+   err = get_checkpoint_version(sbi, cp_addr, _block,
+   _page_2, version);
+   if (err)
goto invalid_cp2;
-
-   cur_version = cur_cp_version(cp_block);
+   cur_version = *version;
 
if (cur_version == pre_version) {
*version = cur_version;
-- 
1.8.3.1

[PATCH] driver core: restrict buffer length in online_store()

2017-07-20 Thread Tiezhu Yang
According to Documentation/ABI/testing/sysfs-devices-online, in order to
control CPU N's hotplug state, we should write one of 'Yy1Nn0' to the file
/sys/devices/system/cpu/cpuN/online, other values should be invalid. so the
buffer length should be 2, buf[0] is one of 'Yy1Nn0' and buf[1] is '\n'.

without patch:
[root@localhost home]# echo 0test > /sys/devices/system/cpu/cpu1/online
[root@localhost home]# cat /sys/devices/system/cpu/cpu1/online
0
[root@localhost home]# echo 1test > /sys/devices/system/cpu/cpu1/online
[root@localhost home]# cat /sys/devices/system/cpu/cpu1/online
1
[root@localhost home]# echo ntest > /sys/devices/system/cpu/cpu1/online
[root@localhost home]# cat /sys/devices/system/cpu/cpu1/online
0
[root@localhost home]# echo ytest > /sys/devices/system/cpu/cpu1/online
[root@localhost home]# cat /sys/devices/system/cpu/cpu1/online
1
[root@localhost home]# echo Ntest > /sys/devices/system/cpu/cpu1/online
[root@localhost home]# cat /sys/devices/system/cpu/cpu1/online
0
[root@localhost home]# echo Ytest > /sys/devices/system/cpu/cpu1/online
[root@localhost home]# cat /sys/devices/system/cpu/cpu1/online
1

with patch:
[root@localhost home]# echo 0test > /sys/devices/system/cpu/cpu1/online
bash: echo: write error: Invalid argument
[root@localhost home]# echo 1test > /sys/devices/system/cpu/cpu1/online
bash: echo: write error: Invalid argument
[root@localhost home]# echo ntest > /sys/devices/system/cpu/cpu1/online
bash: echo: write error: Invalid argument
[root@localhost home]# echo ytest > /sys/devices/system/cpu/cpu1/online
bash: echo: write error: Invalid argument
[root@localhost home]# echo Ntest > /sys/devices/system/cpu/cpu1/online
bash: echo: write error: Invalid argument
[root@localhost home]# echo Ytest > /sys/devices/system/cpu/cpu1/online
bash: echo: write error: Invalid argument

Signed-off-by: Tiezhu Yang <kernelpa...@126.com>
---
 drivers/base/core.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 755451f..6588ed5 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -1005,6 +1005,12 @@ static ssize_t online_store(struct device *dev, struct 
device_attribute *attr,
bool val;
int ret;
 
+   /* According to Documentation/ABI/testing/sysfs-devices-online,
+* the buf length should be 2, buf[0]: one of 'Yy1Nn0', buf[1]: '\n'.
+*/
+   if (strlen(buf) != 2)
+   return -EINVAL;
+
ret = strtobool(buf, );
if (ret < 0)
return ret;
-- 
1.8.3.1