Messages in this thread | | | Date | Mon, 18 Feb 2013 23:03:45 +0900 | Subject | Re: [PATCH v6 4/7] fat: restructure export_operations | From | Namjae Jeon <> |
| |
2013/2/18 OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>: > Namjae Jeon <linkinjeon@gmail.com> writes: > >> + if (MSDOS_SB(inode->i_sb)->options.nfs == FAT_NFS_NOSTALE_RO) { >> + if (inode->i_ino == MSDOS_ROOT_INO) >> + stat->ino = MSDOS_ROOT_INO; > > Can we simply set i_pos = MSDOS_ROOT_INO in fat_read_root()? If so, I > think we can avoid this check. Yes, we can. I will change it.
> >> + else >> + /* Use i_pos for ino. This is used as fileid of nfs. */ >> + stat->ino = fat_i_pos_read(MSDOS_SB(inode->i_sb), >> + inode); >> + } >> return 0; >> } >> EXPORT_SYMBOL_GPL(fat_getattr); > > >> +struct fat_fid { >> + u32 i_gen; >> + u32 i_pos_low; >> + u16 i_pos_hi; >> + u16 parent_i_pos_hi; >> + u32 parent_i_pos_low; >> + u32 parent_i_gen; >> +} __packed; > > Do we need to use __packed? Unnecessary __packed can generate slower > code for alignment check on arch has unaligned fault. Actually, I referenced other filesystem's exportfs code like btrfs and xfs. they are packing their fids. your opinion is reasonable, so I can fix it as your opinion. > >> + if (parent && (len < FAT_FID_SIZE_WITH_PARENT)) { >> + *lenp = FAT_FID_SIZE_WITH_PARENT; >> + return 255; >> + } else if (len < FAT_FID_SIZE_WITHOUT_PARENT) { >> + *lenp = FAT_FID_SIZE_WITHOUT_PARENT; >> + return 255; >> + } > > This check strange. "parent && len == FAT_FID_SIZE_WITHOUT_PARENT" will > overwrite over limit of fh size? I need to check more. because I followed the logic in export_encode_fh() function.
Thanks for review! > -- > OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
| |