[PATCH] erofs: fix erofs_module_init & exit
Chao Yu
yuchao0 at huawei.com
Fri Jul 6 11:30:00 AEST 2018
Hi Xiang,
On 2018/7/6 0:39, Gao Xiang wrote:
> Hi Chao,
>
> On 2018/7/6 0:19, Chao Yu wrote:
>> Hi Xiang,
>>
>> On 2018/7/5 13:03, Gao Xiang wrote:
>>> This patch adds missing parts and EROFS ondisk
>>> layout check as well.
>>>
>>> Signed-off-by: Gao Xiang <gaoxiang25 at huawei.com>
>>> ---
>>> fs/erofs/erofs_fs.h | 13 +++++++++++++
>>> fs/erofs/super.c | 28 +++++++++++++++++++++-------
>>> 2 files changed, 34 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/fs/erofs/erofs_fs.h b/fs/erofs/erofs_fs.h
>>> index ed943d4..0dbf08d 100644
>>> --- a/fs/erofs/erofs_fs.h
>>> +++ b/fs/erofs/erofs_fs.h
>>> @@ -253,5 +253,18 @@ enum {
>>>
>>> #define EROFS_NAME_LEN 255
>>>
>>> +/* check the EROFS on-disk layout strictly at compile time */
>>> +static inline void erofs_check_ondisk_layout(void)
>>> +{
>>> + BUILD_BUG_ON(sizeof(struct erofs_super_block) != 128);
>>> + BUILD_BUG_ON(sizeof(struct erofs_inode_v1) != 32);
>>> + BUILD_BUG_ON(sizeof(struct erofs_inode_v2) != 64);
>>> + BUILD_BUG_ON(sizeof(struct erofs_xattr_ibody_header) != 12);
>>> + BUILD_BUG_ON(sizeof(struct erofs_xattr_entry) != 4);
>>> + BUILD_BUG_ON(sizeof(struct erofs_extent_header) != 16);
>>> + BUILD_BUG_ON(sizeof(struct erofs_decompressed_index_vle) != 8);
>>> + BUILD_BUG_ON(sizeof(struct erofs_dirent) != 12);
>>
>> We'd better define some macros include above constants.
>>
>> Other parts look good to me.
>>
>> Thanks,
>>
>
> I was also thinking of wrapping in macros, but I thought over again,
> it seems not too necessary because:
>
> 1) the above code is already well straight-forward to view and change if needed...
>
> BUILD_BUG_ON(sizeof(struct erofs_super_block) != 128);
> <==> #define EROFS_SUPER_BLOCK_ONDISK_SIZE 128
>
> EROFS_SUPER_BLOCK_ONDISK_SIZE is not really needed since
> we could use sizeof(struct erofs_super_block) directly in code.
>
> 2) Besides, I also observed the other file systems, such as xfs, sysv, minix, and ubifs, eg.
>
> fs/jffs2/super.c:
> 366 /* Paranoia checks for on-medium structures. If we ask GCC
> 367 to pack them with __attribute__((packed)) then it _also_
> 368 assumes that they're not aligned -- so it emits crappy
> 369 code on some architectures. Ideally we want an attribute
> 370 which means just 'no padding', without the alignment
> 371 thing. But GCC doesn't have that -- we have to just
> 372 hope the structs are the right sizes, instead. */
> 373 BUILD_BUG_ON(sizeof(struct jffs2_unknown_node) != 12);
> 374 BUILD_BUG_ON(sizeof(struct jffs2_raw_dirent) != 40);
> 375 BUILD_BUG_ON(sizeof(struct jffs2_raw_inode) != 68);
> 376 BUILD_BUG_ON(sizeof(struct jffs2_raw_summary) != 32);
>
> fs/minix/inode.c:
> 174 BUILD_BUG_ON(32 != sizeof (struct minix_inode));
> 175 BUILD_BUG_ON(64 != sizeof(struct minix2_inode));
>
> fs/ubifs/super.c:
> 359 BUILD_BUG_ON(1024 != sizeof (struct xenix_super_block));
> 360 BUILD_BUG_ON(512 != sizeof (struct sysv4_super_block));
> 361 BUILD_BUG_ON(512 != sizeof (struct sysv2_super_block));
> 362 BUILD_BUG_ON(500 != sizeof (struct coh_super_block));
> 363 BUILD_BUG_ON(64 != sizeof (struct sysv_inode));
>
> fs/ubifs/super.c
> 2211 BUILD_BUG_ON(UBIFS_MAX_NODE_SZ & 7);
> 2212 BUILD_BUG_ON(MIN_WRITE_SZ & 7);
> 2213
> 2214 /* Check min. node size */
> 2215 BUILD_BUG_ON(UBIFS_INO_NODE_SZ < MIN_WRITE_SZ);
> 2216 BUILD_BUG_ON(UBIFS_DENT_NODE_SZ < MIN_WRITE_SZ);
> 2217 BUILD_BUG_ON(UBIFS_XENT_NODE_SZ < MIN_WRITE_SZ);
> 2218 BUILD_BUG_ON(UBIFS_TRUN_NODE_SZ < MIN_WRITE_SZ);
> 2219
> 2220 BUILD_BUG_ON(UBIFS_MAX_DENT_NODE_SZ > UBIFS_MAX_NODE_SZ);
> 2221 BUILD_BUG_ON(UBIFS_MAX_XENT_NODE_SZ > UBIFS_MAX_NODE_SZ);
> 2222 BUILD_BUG_ON(UBIFS_MAX_DATA_NODE_SZ > UBIFS_MAX_NODE_SZ);
> 2223 BUILD_BUG_ON(UBIFS_MAX_INO_NODE_SZ > UBIFS_MAX_NODE_SZ);
> 2224
> 2225 /* Defined node sizes */
> 2226 BUILD_BUG_ON(UBIFS_SB_NODE_SZ != 4096);
> 2227 BUILD_BUG_ON(UBIFS_MST_NODE_SZ != 512);
> 2228 BUILD_BUG_ON(UBIFS_INO_NODE_SZ != 160);
> 2229 BUILD_BUG_ON(UBIFS_REF_NODE_SZ != 64);
>
> how do you think? :) If you think it is necessary, I can change into marcos immediately~
Alright, it's trivial, let's keep the code as original state. :)
Reviewed-by: Chao Yu <yuchao0 at huawei.com>
Thanks,
>
> Thanks,
> Gao Xiang
>
>>> +}
>>> +
>>> #endif
>>>
>>> diff --git a/fs/erofs/super.c b/fs/erofs/super.c
>>> index a1826b9..d104d88 100644
>>> --- a/fs/erofs/super.c
>>> +++ b/fs/erofs/super.c
>>> @@ -37,6 +37,12 @@ static int erofs_init_inode_cache(void)
>>> return erofs_inode_cachep != NULL ? 0 : -ENOMEM;
>>> }
>>>
>>> +static void erofs_exit_inode_cache(void)
>>> +{
>>> + BUG_ON(erofs_inode_cachep == NULL);
>>> + kmem_cache_destroy(erofs_inode_cachep);
>>> +}
>>> +
>>> static struct inode *alloc_inode(struct super_block *sb)
>>> {
>>> struct erofs_vnode *vi =
>>> @@ -398,22 +404,30 @@ int __init erofs_module_init(void)
>>> {
>>> int err;
>>>
>>> + erofs_check_ondisk_layout();
>>> infoln("Initializing erofs " EROFS_VERSION);
>>>
>>> err = erofs_init_inode_cache();
>>> - if (!err) {
>>> - err = register_filesystem(&erofs_fs_type);
>>> - if (!err) {
>>> - infoln("Successfully to initialize erofs");
>>> - return 0;
>>> - }
>>> - }
>>> + if (err)
>>> + goto icache_err;
>>> +
>>> + err = register_filesystem(&erofs_fs_type);
>>> + if (err)
>>> + goto fs_err;
>>> +
>>> + infoln("Successfully to initialize erofs");
>>> + return 0;
>>> +
>>> +fs_err:
>>> + erofs_exit_inode_cache();
>>> +icache_err:
>>> return err;
>>> }
>>>
>>> void __exit erofs_module_exit(void)
>>> {
>>> unregister_filesystem(&erofs_fs_type);
>>> + erofs_exit_inode_cache();
>>> infoln("Successfully finalize erofs");
>>> }
>>>
>>>
>
> .
>
More information about the Linux-erofs
mailing list