Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   "Namjae Jeon" <namjae.jeon@samsung.com>
To:     "'Tetsuhiro Kohada'" <kohada.t2@gmail.com>
Cc:     <kohada.tetsuhiro@dc.mitsubishielectric.co.jp>,
        <mori.takahiro@ab.mitsubishielectric.co.jp>,
        <motai.hirotaka@aj.mitsubishielectric.co.jp>,
        "'Sungjong Seo'" <sj1557.seo@samsung.com>,
        <linux-fsdevel@vger.kernel.org>, <linux-kernel@vger.kernel.org>
In-Reply-To: <20200525115052.19243-4-kohada.t2@gmail.com>
Subject: RE: [PATCH 4/4] exfat: standardize checksum calculation
Date:   Tue, 26 May 2020 16:32:37 +0900
Message-ID: <00d301d6332f$d4a52300$7def6900$@samsung.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Thread-Index: AQEl0VwLAqGTGno4seWfP7lwGg+RAAKJW6foAcSd2fWp+EvHYA==
Content-Language: ko
Content-Type: text/plain; charset="utf-8"
CMS-TYPE: 101P
DLP-Filter: Pass
References: <20200525115052.19243-1-kohada.t2@gmail.com>
        <CGME20200525115121epcas1p2843be2c4af35d5d7e176c68af95052f8@epcas1p2.samsung.com>
        <20200525115052.19243-4-kohada.t2@gmail.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

> To clarify that it is a 16-bit checksum, the parts related to the 16-bit checksum are renamed and
> change type to u16.
> Furthermore, replace checksum calculation in exfat_load_upcase_table() with exfat_calc_checksum32().
> 
> Signed-off-by: Tetsuhiro Kohada <kohada.t2@gmail.com>

I can not apply this patch to exfat dev tree. Could you please check it ?
patching file fs/exfat/dir.c
Hunk #1 succeeded at 491 (offset -5 lines).
Hunk #2 succeeded at 500 (offset -5 lines).
Hunk #3 succeeded at 508 (offset -5 lines).
Hunk #4 FAILED at 600.
Hunk #5 succeeded at 1000 (offset -47 lines).
1 out of 5 hunks FAILED -- saving rejects to file fs/exfat/dir.c.rej
patching file fs/exfat/exfat_fs.h
Hunk #1 succeeded at 137 (offset -2 lines).
Hunk #2 succeeded at 512 (offset -3 lines).
patching file fs/exfat/misc.c
patching file fs/exfat/nls.c

Thanks!
> ---
>  fs/exfat/dir.c      | 12 ++++++------
>  fs/exfat/exfat_fs.h |  5 ++---
>  fs/exfat/misc.c     | 10 ++++------
>  fs/exfat/nls.c      | 19 +++++++------------
>  4 files changed, 19 insertions(+), 27 deletions(-)
> 
> diff --git a/fs/exfat/dir.c b/fs/exfat/dir.c index b5a237c33d50..b673362a895c 100644
> --- a/fs/exfat/dir.c
> +++ b/fs/exfat/dir.c
> @@ -496,7 +496,7 @@ int exfat_update_dir_chksum(struct inode *inode, struct exfat_chain *p_dir,
>  	int ret = 0;
>  	int i, num_entries;
>  	sector_t sector;
> -	unsigned short chksum;
> +	u16 chksum;
>  	struct exfat_dentry *ep, *fep;
>  	struct buffer_head *fbh, *bh;
> 
> @@ -505,7 +505,7 @@ int exfat_update_dir_chksum(struct inode *inode, struct exfat_chain *p_dir,
>  		return -EIO;
> 
>  	num_entries = fep->dentry.file.num_ext + 1;
> -	chksum = exfat_calc_chksum_2byte(fep, DENTRY_SIZE, 0, CS_DIR_ENTRY);
> +	chksum = exfat_calc_chksum16(fep, DENTRY_SIZE, 0, CS_DIR_ENTRY);
> 
>  	for (i = 1; i < num_entries; i++) {
>  		ep = exfat_get_dentry(sb, p_dir, entry + i, &bh, NULL); @@ -513,7 +513,7 @@ int
> exfat_update_dir_chksum(struct inode *inode, struct exfat_chain *p_dir,
>  			ret = -EIO;
>  			goto release_fbh;
>  		}
> -		chksum = exfat_calc_chksum_2byte(ep, DENTRY_SIZE, chksum,
> +		chksum = exfat_calc_chksum16(ep, DENTRY_SIZE, chksum,
>  				CS_DEFAULT);
>  		brelse(bh);
>  	}
> @@ -600,10 +600,10 @@ int exfat_update_dir_chksum_with_entry_set(struct super_block *sb,
>  	int chksum_type = CS_DIR_ENTRY, i, num_entries = es->num_entries;
>  	unsigned int buf_off = (off - es->offset);
>  	unsigned int remaining_byte_in_sector, copy_entries, clu;
> -	unsigned short chksum = 0;
> +	u16 chksum = 0;
> 
>  	for (i = 0; i < num_entries; i++) {
> -		chksum = exfat_calc_chksum_2byte(&es->entries[i], DENTRY_SIZE,
> +		chksum = exfat_calc_chksum16(&es->entries[i], DENTRY_SIZE,
>  			chksum, chksum_type);
>  		chksum_type = CS_DEFAULT;
>  	}
> @@ -1047,7 +1047,7 @@ int exfat_find_dir_entry(struct super_block *sb, struct exfat_inode_info *ei,
>  			}
> 
>  			if (entry_type == TYPE_STREAM) {
> -				unsigned short name_hash;
> +				u16 name_hash;
> 
>  				if (step != DIRENT_STEP_STRM) {
>  					step = DIRENT_STEP_FILE;
> diff --git a/fs/exfat/exfat_fs.h b/fs/exfat/exfat_fs.h index 15817281b3c8..993d13bbebec 100644
> --- a/fs/exfat/exfat_fs.h
> +++ b/fs/exfat/exfat_fs.h
> @@ -139,7 +139,7 @@ struct exfat_dentry_namebuf {  struct exfat_uni_name {
>  	/* +3 for null and for converting */
>  	unsigned short name[MAX_NAME_LENGTH + 3];
> -	unsigned short name_hash;
> +	u16 name_hash;
>  	unsigned char name_len;
>  };
> 
> @@ -515,8 +515,7 @@ void exfat_get_entry_time(struct exfat_sb_info *sbi, struct timespec64 *ts,  void
> exfat_truncate_atime(struct timespec64 *ts);  void exfat_set_entry_time(struct exfat_sb_info *sbi,
> struct timespec64 *ts,
>  		u8 *tz, __le16 *time, __le16 *date, u8 *time_cs); -unsigned short
> exfat_calc_chksum_2byte(void *data, int len,
> -		unsigned short chksum, int type);
> +u16 exfat_calc_chksum16(void *data, int len, u16 chksum, int type);
>  u32 exfat_calc_chksum32(void *data, int len, u32 chksum, int type);  void exfat_update_bh(struct
> super_block *sb, struct buffer_head *bh, int sync);  void exfat_chain_set(struct exfat_chain *ec,
> unsigned int dir, diff --git a/fs/exfat/misc.c b/fs/exfat/misc.c index b82d2dd5bd7c..17d41f3d3709
> 100644
> --- a/fs/exfat/misc.c
> +++ b/fs/exfat/misc.c
> @@ -136,17 +136,15 @@ void exfat_truncate_atime(struct timespec64 *ts)
>  	ts->tv_nsec = 0;
>  }
> 
> -unsigned short exfat_calc_chksum_2byte(void *data, int len,
> -		unsigned short chksum, int type)
> +u16 exfat_calc_chksum16(void *data, int len, u16 chksum, int type)
>  {
>  	int i;
> -	unsigned char *c = (unsigned char *)data;
> +	u8 *c = (u8 *)data;
> 
>  	for (i = 0; i < len; i++, c++) {
> -		if (((i == 2) || (i == 3)) && (type == CS_DIR_ENTRY))
> +		if (unlikely(type == CS_DIR_ENTRY && (i == 2 || i == 3)))
>  			continue;
> -		chksum = (((chksum & 1) << 15) | ((chksum & 0xFFFE) >> 1)) +
> -			(unsigned short)*c;
> +		chksum = ((chksum << 15) | (chksum >> 1)) + *c;
>  	}
>  	return chksum;
>  }
> diff --git a/fs/exfat/nls.c b/fs/exfat/nls.c index 1ebda90cbdd7..19321773dd07 100644
> --- a/fs/exfat/nls.c
> +++ b/fs/exfat/nls.c
> @@ -527,7 +527,7 @@ static int exfat_utf8_to_utf16(struct super_block *sb,
> 
>  	*uniname = '\0';
>  	p_uniname->name_len = unilen;
> -	p_uniname->name_hash = exfat_calc_chksum_2byte(upname, unilen << 1, 0,
> +	p_uniname->name_hash = exfat_calc_chksum16(upname, unilen << 1, 0,
>  			CS_DEFAULT);
> 
>  	if (p_lossy)
> @@ -623,7 +623,7 @@ static int exfat_nls_to_ucs2(struct super_block *sb,
> 
>  	*uniname = '\0';
>  	p_uniname->name_len = unilen;
> -	p_uniname->name_hash = exfat_calc_chksum_2byte(upname, unilen << 1, 0,
> +	p_uniname->name_hash = exfat_calc_chksum16(upname, unilen << 1, 0,
>  			CS_DEFAULT);
> 
>  	if (p_lossy)
> @@ -655,7 +655,8 @@ static int exfat_load_upcase_table(struct super_block *sb,  {
>  	struct exfat_sb_info *sbi = EXFAT_SB(sb);
>  	unsigned int sect_size = sb->s_blocksize;
> -	unsigned int i, index = 0, checksum = 0;
> +	unsigned int i, index = 0;
> +	u32 chksum = 0;
>  	int ret;
>  	unsigned char skip = false;
>  	unsigned short *upcase_table;
> @@ -681,13 +682,6 @@ static int exfat_load_upcase_table(struct super_block *sb,
>  		for (i = 0; i < sect_size && index <= 0xFFFF; i += 2) {
>  			unsigned short uni = get_unaligned_le16(bh->b_data + i);
> 
> -			checksum = ((checksum & 1) ? 0x80000000 : 0) +
> -				(checksum >> 1) +
> -				*(((unsigned char *)bh->b_data) + i);
> -			checksum = ((checksum & 1) ? 0x80000000 : 0) +
> -				(checksum >> 1) +
> -				*(((unsigned char *)bh->b_data) + (i + 1));
> -
>  			if (skip) {
>  				index += uni;
>  				skip = false;
> @@ -701,13 +695,14 @@ static int exfat_load_upcase_table(struct super_block *sb,
>  			}
>  		}
>  		brelse(bh);
> +		chksum = exfat_calc_chksum32(bh->b_data, i, chksum, CS_DEFAULT);
>  	}
> 
> -	if (index >= 0xFFFF && utbl_checksum == checksum)
> +	if (index >= 0xFFFF && utbl_checksum == chksum)
>  		return 0;
> 
>  	exfat_err(sb, "failed to load upcase table (idx : 0x%08x, chksum : 0x%08x, utbl_chksum :
> 0x%08x)",
> -		  index, checksum, utbl_checksum);
> +		  index, chksum, utbl_checksum);
>  	ret = -EINVAL;
>  free_table:
>  	exfat_free_upcase_table(sbi);
> --
> 2.25.1