> Thank you for your reply. > > >> @@ -171,7 +174,9 @@ struct exfat_entry_set_cache { > >> unsigned int start_off; > >> int num_bh; > >> struct buffer_head *bh[DIR_CACHE_SIZE]; > >> - unsigned int num_entries; > >> + int num_entries; > >> + struct exfat_de_file *de_file; > >> + struct exfat_de_stream *de_stream; > > I prefer to assign validated entries to **de and use it using enum value. > > struct exfat_dentry **de; > > I've tried several implementations that add a struct exfat_dentry type.(*de0 & *de1; *de[2]; etc...) > The problem with the struct exfat_dentry type is that it is too flexible for type. > This means weak typing. > Therefore, when using them, > de[XXX_FILE]->dentry.file.zzz ... > It is necessary to re-specify the type. (against the DRY principle) Strong typing prevents use with > wrong type, at compiling. > > I think the approach of using de_file/de_stream could be strongly typed. > I don't think we need excessive flexibility. Could you check the following change ? If you think it's okay, please add it to your patch series. diff --git a/fs/exfat/dir.c b/fs/exfat/dir.c index 573659bfbc55..37e0f92f74b3 100644 --- a/fs/exfat/dir.c +++ b/fs/exfat/dir.c @@ -44,14 +44,12 @@ static void exfat_get_uniname_from_ext_entry(struct super_block *sb, * Third entry : first file-name entry * So, the index of first file-name dentry should start from 2. */ - for (i = 2; i < es->num_entries; i++) { - struct exfat_dentry *ep = exfat_get_dentry_cached(es, i); - + for (i = ENTRY_NAME; i < es->num_entries; i++) { /* end of name entry */ - if (exfat_get_entry_type(ep) != TYPE_EXTEND) + if (exfat_get_entry_type(es->de[i]) != TYPE_NAME) break; - exfat_extract_uni_name(ep, uniname); + exfat_extract_uni_name(es->de[i], uniname); uniname += EXFAT_FILE_NAME_LEN; } @@ -372,7 +370,7 @@ unsigned int exfat_get_entry_type(struct exfat_dentry *ep) if (ep->type == EXFAT_STREAM) return TYPE_STREAM; if (ep->type == EXFAT_NAME) - return TYPE_EXTEND; + return TYPE_NAME; if (ep->type == EXFAT_ACL) return TYPE_ACL; return TYPE_CRITICAL_SEC; @@ -388,7 +386,7 @@ static void exfat_set_entry_type(struct exfat_dentry *ep, unsigned int type) ep->type &= EXFAT_DELETE; } else if (type == TYPE_STREAM) { ep->type = EXFAT_STREAM; - } else if (type == TYPE_EXTEND) { + } else if (type == TYPE_NAME) { ep->type = EXFAT_NAME; } else if (type == TYPE_BITMAP) { ep->type = EXFAT_BITMAP; @@ -421,7 +419,7 @@ static void exfat_init_name_entry(struct exfat_dentry *ep, { int i; - exfat_set_entry_type(ep, TYPE_EXTEND); + exfat_set_entry_type(ep, TYPE_NAME); ep->dentry.name.flags = 0x0; for (i = 0; i < EXFAT_FILE_NAME_LEN; i++) { @@ -591,16 +589,13 @@ void exfat_update_dir_chksum_with_entry_set(struct exfat_entry_set_cache *es) { int chksum_type = CS_DIR_ENTRY, i; unsigned short chksum = 0; - struct exfat_dentry *ep; for (i = 0; i < es->num_entries; i++) { - ep = exfat_get_dentry_cached(es, i); - chksum = exfat_calc_chksum16(ep, DENTRY_SIZE, chksum, + chksum = exfat_calc_chksum16(es->de[i], DENTRY_SIZE, chksum, chksum_type); chksum_type = CS_DEFAULT; } - ep = exfat_get_dentry_cached(es, 0); - ep->dentry.file.checksum = cpu_to_le16(chksum); + ES_FILE(es).checksum = cpu_to_le16(chksum); es->modified = true; } @@ -741,59 +736,8 @@ struct exfat_dentry *exfat_get_dentry(struct super_block *sb, return (struct exfat_dentry *)((*bh)->b_data + off); } -enum exfat_validate_dentry_mode { - ES_MODE_STARTED, - ES_MODE_GET_FILE_ENTRY, - ES_MODE_GET_STRM_ENTRY, - ES_MODE_GET_NAME_ENTRY, - ES_MODE_GET_CRITICAL_SEC_ENTRY, -}; - -static bool exfat_validate_entry(unsigned int type, - enum exfat_validate_dentry_mode *mode) -{ - if (type == TYPE_UNUSED || type == TYPE_DELETED) - return false; - - switch (*mode) { - case ES_MODE_STARTED: - if (type != TYPE_FILE && type != TYPE_DIR) - return false; - *mode = ES_MODE_GET_FILE_ENTRY; - return true; - case ES_MODE_GET_FILE_ENTRY: - if (type != TYPE_STREAM) - return false; - *mode = ES_MODE_GET_STRM_ENTRY; - return true; - case ES_MODE_GET_STRM_ENTRY: - if (type != TYPE_EXTEND) - return false; - *mode = ES_MODE_GET_NAME_ENTRY; - return true; - case ES_MODE_GET_NAME_ENTRY: - if (type == TYPE_STREAM) - return false; - if (type != TYPE_EXTEND) { - if (!(type & TYPE_CRITICAL_SEC)) - return false; - *mode = ES_MODE_GET_CRITICAL_SEC_ENTRY; - } - return true; - case ES_MODE_GET_CRITICAL_SEC_ENTRY: - if (type == TYPE_EXTEND || type == TYPE_STREAM) - return false; - if ((type & TYPE_CRITICAL_SEC) != TYPE_CRITICAL_SEC) - return false; - return true; - default: - WARN_ON_ONCE(1); - return false; - } -} - -struct exfat_dentry *exfat_get_dentry_cached( - struct exfat_entry_set_cache *es, int num) +struct exfat_dentry *exfat_get_dentry_cached(struct exfat_entry_set_cache *es, + int num) { int off = es->start_off + num * DENTRY_SIZE; struct buffer_head *bh = es->bh[EXFAT_B_TO_BLK(off, es->sb)]; @@ -817,16 +761,14 @@ struct exfat_dentry *exfat_get_dentry_cached( * NULL on failure. */ struct exfat_entry_set_cache *exfat_get_dentry_set(struct super_block *sb, - struct exfat_chain *p_dir, int entry, unsigned int type) + struct exfat_chain *p_dir, int entry, int max_entries) { - int ret, i, num_bh; - unsigned int off, byte_offset, clu = 0; + int i, num_bh, num_entries, last_name_entry; + unsigned int clu = 0; sector_t sec; struct exfat_sb_info *sbi = EXFAT_SB(sb); struct exfat_entry_set_cache *es; struct exfat_dentry *ep; - int num_entries; - enum exfat_validate_dentry_mode mode = ES_MODE_STARTED; struct buffer_head *bh; if (p_dir->dir == DIR_DELETED) { @@ -834,42 +776,31 @@ struct exfat_entry_set_cache *exfat_get_dentry_set(struct super_block *sb, return NULL; } - byte_offset = EXFAT_DEN_TO_B(entry); - ret = exfat_walk_fat_chain(sb, p_dir, byte_offset, &clu); - if (ret) + ep = exfat_get_dentry(sb, p_dir, entry, &bh, &sec); + if (!ep || ep->type != EXFAT_FILE) { + brelse(bh); return NULL; + } - es = kzalloc(sizeof(*es), GFP_KERNEL); - if (!es) + max_entries = max(max_entries, ES_2_ENTRIES); + num_entries = min(ep->dentry.file.num_ext + 1, max_entries); + es = kzalloc(sizeof(*es) + num_entries * sizeof(struct exfat_dentry *), + GFP_KERNEL); + if (!es) { + brelse(bh); return NULL; + } + es->sb = sb; es->modified = false; - - /* byte offset in cluster */ - byte_offset = EXFAT_CLU_OFFSET(byte_offset, sbi); - - /* byte offset in sector */ - off = EXFAT_BLK_OFFSET(byte_offset, sb); - es->start_off = off; - - /* sector offset in cluster */ - sec = EXFAT_B_TO_BLK(byte_offset, sb); - sec += exfat_cluster_to_sector(sbi, clu); - - bh = sb_bread(sb, sec); - if (!bh) - goto free_es; - es->bh[es->num_bh++] = bh; - - ep = exfat_get_dentry_cached(es, 0); - if (!exfat_validate_entry(exfat_get_entry_type(ep), &mode)) - goto free_es; - - num_entries = type == ES_ALL_ENTRIES ? - ep->dentry.file.num_ext + 1 : type; es->num_entries = num_entries; + es->de[ENTRY_FILE] = ep; + es->bh[es->num_bh++] = bh; + /* byte offset in sector */ + es->start_off = EXFAT_BLK_OFFSET(EXFAT_DEN_TO_B(entry), sb); - num_bh = EXFAT_B_TO_BLK_ROUND_UP(off + num_entries * DENTRY_SIZE, sb); + clu = exfat_sector_to_cluster(sbi, sec); + num_bh = EXFAT_B_TO_BLK_ROUND_UP(es->start_off + num_entries * DENTRY_SIZE, sb); for (i = 1; i < num_bh; i++) { /* get the next sector */ if (exfat_is_last_sector_in_cluster(sbi, sec)) { @@ -888,14 +819,27 @@ struct exfat_entry_set_cache *exfat_get_dentry_set(struct super_block *sb, es->bh[es->num_bh++] = bh; } - /* validiate cached dentries */ - for (i = 1; i < num_entries; i++) { - ep = exfat_get_dentry_cached(es, i); - if (!exfat_validate_entry(exfat_get_entry_type(ep), &mode)) + ep = exfat_get_dentry_cached(es, ENTRY_STREAM); + if (!ep || ep->type != EXFAT_STREAM) + goto free_es; + es->de[ENTRY_STREAM] = ep; + + last_name_entry = + ENTRY_NAME + ES_STREAM(es).name_len / EXFAT_FILE_NAME_LEN; + for (i = ENTRY_NAME; i < es->num_entries; i++) { + es->de[i] = exfat_get_dentry_cached(es, i); + + if (i <= last_name_entry) { + if (exfat_get_entry_type(es->de[i]) != TYPE_NAME) + goto free_es; + continue; + } + + if (!(exfat_get_entry_type(es->de[i]) & TYPE_SECONDARY)) goto free_es; } - return es; + return es; free_es: exfat_free_dentry_set(es, false); return NULL; @@ -1028,7 +972,7 @@ int exfat_find_dir_entry(struct super_block *sb, struct exfat_inode_info *ei, } brelse(bh); - if (entry_type == TYPE_EXTEND) { + if (entry_type == TYPE_NAME) { unsigned short entry_uniname[16], unichar; if (step != DIRENT_STEP_NAME) { @@ -1144,7 +1088,7 @@ int exfat_count_ext_entries(struct super_block *sb, struct exfat_chain *p_dir, type = exfat_get_entry_type(ext_ep); brelse(bh); - if (type == TYPE_EXTEND || type == TYPE_STREAM) + if (type == TYPE_NAME || type == TYPE_STREAM) count++; else break; diff --git a/fs/exfat/exfat_fs.h b/fs/exfat/exfat_fs.h index 44dc04520175..0e4cc8ba2f8e 100644 --- a/fs/exfat/exfat_fs.h +++ b/fs/exfat/exfat_fs.h @@ -33,6 +33,12 @@ enum { NLS_NAME_OVERLEN, /* the length is over than its limit */ }; +enum { + ENTRY_FILE, + ENTRY_STREAM, + ENTRY_NAME, +}; + #define EXFAT_HASH_BITS 8 #define EXFAT_HASH_SIZE (1UL << EXFAT_HASH_BITS) @@ -40,7 +46,7 @@ enum { * Type Definitions */ #define ES_2_ENTRIES 2 -#define ES_ALL_ENTRIES 0 +#define ES_ALL_ENTRIES 256 #define DIR_DELETED 0xFFFF0321 @@ -56,7 +62,7 @@ enum { #define TYPE_FILE 0x011F #define TYPE_CRITICAL_SEC 0x0200 #define TYPE_STREAM 0x0201 -#define TYPE_EXTEND 0x0202 +#define TYPE_NAME 0x0202 #define TYPE_ACL 0x0203 #define TYPE_BENIGN_PRI 0x0400 #define TYPE_GUID 0x0401 @@ -65,6 +71,8 @@ enum { #define TYPE_BENIGN_SEC 0x0800 #define TYPE_ALL 0x0FFF +#define TYPE_SECONDARY (TYPE_CRITICAL_SEC | TYPE_BENIGN_SEC) + #define MAX_CHARSET_SIZE 6 /* max size of multi-byte character */ #define MAX_NAME_LENGTH 255 /* max len of file name excluding NULL */ #define MAX_VFSNAME_BUF_SIZE ((MAX_NAME_LENGTH + 1) * MAX_CHARSET_SIZE) @@ -126,6 +134,9 @@ enum { #define BITS_PER_BYTE_MASK 0x7 #define IGNORED_BITS_REMAINED(clu, clu_base) ((1 << ((clu) - (clu_base))) - 1) +#define ES_FILE(es) (es->de[ENTRY_FILE]->dentry.file) +#define ES_STREAM(es) (es->de[ENTRY_STREAM]->dentry.stream) + struct exfat_dentry_namebuf { char *lfn; int lfnbuf_len; /* usually MAX_UNINAME_BUF_SIZE */ @@ -171,7 +182,8 @@ struct exfat_entry_set_cache { unsigned int start_off; int num_bh; struct buffer_head *bh[DIR_CACHE_SIZE]; - unsigned int num_entries; + int num_entries; + struct exfat_dentry *de[0]; }; struct exfat_dir_entry { @@ -461,7 +473,7 @@ struct exfat_dentry *exfat_get_dentry(struct super_block *sb, struct exfat_dentry *exfat_get_dentry_cached(struct exfat_entry_set_cache *es, int num); struct exfat_entry_set_cache *exfat_get_dentry_set(struct super_block *sb, - struct exfat_chain *p_dir, int entry, unsigned int type); + struct exfat_chain *p_dir, int entry, int max_entries); int exfat_free_dentry_set(struct exfat_entry_set_cache *es, int sync); int exfat_count_dir_entries(struct super_block *sb, struct exfat_chain *p_dir); diff --git a/fs/exfat/file.c b/fs/exfat/file.c index 4831a39632a1..504ffcaffacc 100644 --- a/fs/exfat/file.c +++ b/fs/exfat/file.c @@ -152,7 +152,6 @@ int __exfat_truncate(struct inode *inode, loff_t new_size) /* update the directory entry */ if (!evict) { struct timespec64 ts; - struct exfat_dentry *ep, *ep2; struct exfat_entry_set_cache *es; int err; @@ -160,32 +159,30 @@ int __exfat_truncate(struct inode *inode, loff_t new_size) ES_ALL_ENTRIES); if (!es) return -EIO; - ep = exfat_get_dentry_cached(es, 0); - ep2 = exfat_get_dentry_cached(es, 1); ts = current_time(inode); exfat_set_entry_time(sbi, &ts, - &ep->dentry.file.modify_tz, - &ep->dentry.file.modify_time, - &ep->dentry.file.modify_date, - &ep->dentry.file.modify_time_cs); - ep->dentry.file.attr = cpu_to_le16(ei->attr); + &ES_FILE(es).modify_tz, + &ES_FILE(es).modify_time, + &ES_FILE(es).modify_date, + &ES_FILE(es).modify_time_cs); + ES_FILE(es).attr = cpu_to_le16(ei->attr); /* File size should be zero if there is no cluster allocated */ if (ei->start_clu == EXFAT_EOF_CLUSTER) { - ep2->dentry.stream.valid_size = 0; - ep2->dentry.stream.size = 0; + ES_STREAM(es).valid_size = 0; + ES_STREAM(es).size = 0; } else { - ep2->dentry.stream.valid_size = cpu_to_le64(new_size); - ep2->dentry.stream.size = ep2->dentry.stream.valid_size; + ES_STREAM(es).valid_size = cpu_to_le64(new_size); + ES_STREAM(es).size = ES_STREAM(es).valid_size; } if (new_size == 0) { /* Any directory can not be truncated to zero */ WARN_ON(ei->type != TYPE_FILE); - ep2->dentry.stream.flags = ALLOC_FAT_CHAIN; - ep2->dentry.stream.start_clu = EXFAT_FREE_CLUSTER; + ES_STREAM(es).flags = ALLOC_FAT_CHAIN; + ES_STREAM(es).start_clu = EXFAT_FREE_CLUSTER; } exfat_update_dir_chksum_with_entry_set(es); diff --git a/fs/exfat/inode.c b/fs/exfat/inode.c index 7f90204adef5..a6be7ab76a72 100644 --- a/fs/exfat/inode.c +++ b/fs/exfat/inode.c @@ -20,7 +20,6 @@ static int __exfat_write_inode(struct inode *inode, int sync) { unsigned long long on_disk_size; - struct exfat_dentry *ep, *ep2; struct exfat_entry_set_cache *es = NULL; struct super_block *sb = inode->i_sb; struct exfat_sb_info *sbi = EXFAT_SB(sb); @@ -45,26 +44,24 @@ static int __exfat_write_inode(struct inode *inode, int sync) es = exfat_get_dentry_set(sb, &(ei->dir), ei->entry, ES_ALL_ENTRIES); if (!es) return -EIO; - ep = exfat_get_dentry_cached(es, 0); - ep2 = exfat_get_dentry_cached(es, 1); - ep->dentry.file.attr = cpu_to_le16(exfat_make_attr(inode)); + ES_FILE(es).attr = cpu_to_le16(exfat_make_attr(inode)); /* set FILE_INFO structure using the acquired struct exfat_dentry */ exfat_set_entry_time(sbi, &ei->i_crtime, - &ep->dentry.file.create_tz, - &ep->dentry.file.create_time, - &ep->dentry.file.create_date, - &ep->dentry.file.create_time_cs); + &ES_FILE(es).create_tz, + &ES_FILE(es).create_time, + &ES_FILE(es).create_date, + &ES_FILE(es).create_time_cs); exfat_set_entry_time(sbi, &inode->i_mtime, - &ep->dentry.file.modify_tz, - &ep->dentry.file.modify_time, - &ep->dentry.file.modify_date, - &ep->dentry.file.modify_time_cs); + &ES_FILE(es).modify_tz, + &ES_FILE(es).modify_time, + &ES_FILE(es).modify_date, + &ES_FILE(es).modify_time_cs); exfat_set_entry_time(sbi, &inode->i_atime, - &ep->dentry.file.access_tz, - &ep->dentry.file.access_time, - &ep->dentry.file.access_date, + &ES_FILE(es).access_tz, + &ES_FILE(es).access_time, + &ES_FILE(es).access_date, NULL); /* File size should be zero if there is no cluster allocated */ @@ -73,8 +70,8 @@ static int __exfat_write_inode(struct inode *inode, int sync) if (ei->start_clu == EXFAT_EOF_CLUSTER) on_disk_size = 0; - ep2->dentry.stream.valid_size = cpu_to_le64(on_disk_size); - ep2->dentry.stream.size = ep2->dentry.stream.valid_size; + ES_STREAM(es).valid_size = cpu_to_le64(on_disk_size); + ES_STREAM(es).size = ES_STREAM(es).valid_size; exfat_update_dir_chksum_with_entry_set(es); return exfat_free_dentry_set(es, sync); @@ -219,7 +216,6 @@ static int exfat_map_cluster(struct inode *inode, unsigned int clu_offset, *clu = new_clu.dir; if (ei->dir.dir != DIR_DELETED && modified) { - struct exfat_dentry *ep; struct exfat_entry_set_cache *es; int err; @@ -227,17 +223,12 @@ static int exfat_map_cluster(struct inode *inode, unsigned int clu_offset, ES_ALL_ENTRIES); if (!es) return -EIO; - /* get stream entry */ - ep = exfat_get_dentry_cached(es, 1); /* update directory entry */ - ep->dentry.stream.flags = ei->flags; - ep->dentry.stream.start_clu = - cpu_to_le32(ei->start_clu); - ep->dentry.stream.valid_size = - cpu_to_le64(i_size_read(inode)); - ep->dentry.stream.size = - ep->dentry.stream.valid_size; + ES_STREAM(es).flags = ei->flags; + ES_STREAM(es).start_clu = cpu_to_le32(ei->start_clu); + ES_STREAM(es).valid_size = cpu_to_le64(i_size_read(inode)); + ES_STREAM(es).size = ES_STREAM(es).valid_size; exfat_update_dir_chksum_with_entry_set(es); err = exfat_free_dentry_set(es, inode_needs_sync(inode)); diff --git a/fs/exfat/namei.c b/fs/exfat/namei.c index 2aff6605fecc..469fe075dc1f 100644 --- a/fs/exfat/namei.c +++ b/fs/exfat/namei.c @@ -658,25 +658,21 @@ static int exfat_find(struct inode *dir, struct qstr *qname, info->num_subdirs = count; } else { - struct exfat_dentry *ep, *ep2; struct exfat_entry_set_cache *es; es = exfat_get_dentry_set(sb, &cdir, dentry, ES_2_ENTRIES); if (!es) return -EIO; - ep = exfat_get_dentry_cached(es, 0); - ep2 = exfat_get_dentry_cached(es, 1); - info->type = exfat_get_entry_type(ep); - info->attr = le16_to_cpu(ep->dentry.file.attr); - info->size = le64_to_cpu(ep2->dentry.stream.valid_size); + info->type = exfat_get_entry_type(es->de[ENTRY_FILE]); + info->attr = le16_to_cpu(ES_FILE(es).attr); + info->size = le64_to_cpu(ES_STREAM(es).valid_size); if ((info->type == TYPE_FILE) && (info->size == 0)) { info->flags = ALLOC_NO_FAT_CHAIN; info->start_clu = EXFAT_EOF_CLUSTER; } else { - info->flags = ep2->dentry.stream.flags; - info->start_clu = - le32_to_cpu(ep2->dentry.stream.start_clu); + info->flags = ES_STREAM(es).flags; + info->start_clu = le32_to_cpu(ES_STREAM(es).start_clu); } if (ei->start_clu == EXFAT_FREE_CLUSTER) { @@ -688,19 +684,19 @@ static int exfat_find(struct inode *dir, struct qstr *qname, } exfat_get_entry_time(sbi, &info->crtime, - ep->dentry.file.create_tz, - ep->dentry.file.create_time, - ep->dentry.file.create_date, - ep->dentry.file.create_time_cs); + ES_FILE(es).create_tz, + ES_FILE(es).create_time, + ES_FILE(es).create_date, + ES_FILE(es).create_time_cs); exfat_get_entry_time(sbi, &info->mtime, - ep->dentry.file.modify_tz, - ep->dentry.file.modify_time, - ep->dentry.file.modify_date, - ep->dentry.file.modify_time_cs); + ES_FILE(es).modify_tz, + ES_FILE(es).modify_time, + ES_FILE(es).modify_date, + ES_FILE(es).modify_time_cs); exfat_get_entry_time(sbi, &info->atime, - ep->dentry.file.access_tz, - ep->dentry.file.access_time, - ep->dentry.file.access_date, + ES_FILE(es).access_tz, + ES_FILE(es).access_time, + ES_FILE(es).access_date, 0); exfat_free_dentry_set(es, false); -- 2.17.1 > > > BR > --- > Tetsuhiro Kohada <kohada.t2@xxxxxxxxx>