From mboxrd@z Thu Jan 1 00:00:00 1970 From: Namjae Jeon Subject: Re: [PATCH 4/5] f2fs: align f2fs maximum name length to linux based filesystem Date: Mon, 4 Mar 2013 15:25:11 +0900 Message-ID: References: <1362195678-20785-1-git-send-email-linkinjeon@gmail.com> <1362287099.14386.13.camel@kjgkr> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net, Namjae Jeon , Amit Sahrawat To: jaegeuk.kim@samsung.com Return-path: Received: from mail-oa0-f50.google.com ([209.85.219.50]:56866 "EHLO mail-oa0-f50.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750803Ab3CDGZL convert rfc822-to-8bit (ORCPT ); Mon, 4 Mar 2013 01:25:11 -0500 In-Reply-To: <1362287099.14386.13.camel@kjgkr> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: 2013/3/3, Jaegeuk Kim : > We should not change the on-disk layout. > Instead, simply we can deal with it by changing original condition ch= eck > like below. Even though the changes you suggested are ok. But there is one doubt. By not changing the on-disk layout and just taking care of the limits using the code is just causing confusion and looks a make-shift arrangement. Even though =E2=80=98256=E2=80=99 is the space reserved for the name =E2= =80=98on-disk=E2=80=99 but by changing the code =E2=80=93 we are limiting it to use =E2=80=98255=E2=80= =99. If we chance the on-disk to make use of =E2=80=98255=E2=80=99 bytes it = allows for keeping all code intact and also like the code changes suggested, it will still refer only the =E2=80=98255=E2=80=99 bytes. More so, changing the on-disk allows for =E2=80=981byte=E2=80=99 which = is still padded at the same position to be used in future. Otherwise, this =E2=80=98ext= ra=E2=80=99 byte will continue to exist without having that to be used for some extra work. Let me know your opinion. Thanks. > > --- > From ccc2546eded1efd2d6ed98f8aee7d7ce247cb4a2 Mon Sep 17 00:00:00 200= 1 > From: Jaegeuk Kim > Date: Sun, 3 Mar 2013 13:58:05 +0900 > Subject: [PATCH] f2fs: align f2fs maximum name length to linux based > filesystem > Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, > linux-f2fs-devel@lists.sourceforge.net > > The maximum filename length supported in linux is 255 characters. > So let's follow that. > > Signed-off-by: Namjae Jeon > Signed-off-by: Amit Sahrawat > Signed-off-by: Jaegeuk Kim > --- > fs/f2fs/dir.c | 3 +++ > fs/f2fs/namei.c | 2 +- > fs/f2fs/super.c | 2 +- > include/linux/f2fs_fs.h | 14 ++++++++------ > 4 files changed, 13 insertions(+), 8 deletions(-) > > diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c > index c395c50..4ac8a7b 100644 > --- a/fs/f2fs/dir.c > +++ b/fs/f2fs/dir.c > @@ -189,6 +189,9 @@ struct f2fs_dir_entry *f2fs_find_entry(struct ino= de > *dir, > unsigned int max_depth; > unsigned int level; > > + if (namelen > F2FS_NAME_LEN) > + return NULL; > + > if (npages =3D=3D 0) > return NULL; > > diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c > index 1a49b88..d4a171b 100644 > --- a/fs/f2fs/namei.c > +++ b/fs/f2fs/namei.c > @@ -197,7 +197,7 @@ static struct dentry *f2fs_lookup(struct inode *d= ir, > struct dentry *dentry, > struct f2fs_dir_entry *de; > struct page *page; > > - if (dentry->d_name.len > F2FS_MAX_NAME_LEN) > + if (dentry->d_name.len > F2FS_NAME_LEN) > return ERR_PTR(-ENAMETOOLONG); > > de =3D f2fs_find_entry(dir, &dentry->d_name, &page); > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c > index 8c11764..1c7f595 100644 > --- a/fs/f2fs/super.c > +++ b/fs/f2fs/super.c > @@ -180,7 +180,7 @@ static int f2fs_statfs(struct dentry *dentry, str= uct > kstatfs *buf) > buf->f_files =3D sbi->total_node_count; > buf->f_ffree =3D sbi->total_node_count - valid_inode_count(sbi); > > - buf->f_namelen =3D F2FS_MAX_NAME_LEN; > + buf->f_namelen =3D F2FS_NAME_LEN; > buf->f_fsid.val[0] =3D (u32)id; > buf->f_fsid.val[1] =3D (u32)(id >> 32); > > diff --git a/include/linux/f2fs_fs.h b/include/linux/f2fs_fs.h > index f9a12f6..7b50991 100644 > --- a/include/linux/f2fs_fs.h > +++ b/include/linux/f2fs_fs.h > @@ -139,6 +139,8 @@ struct f2fs_extent { > __le32 len; /* lengh of the extent */ > } __packed; > > +/* We can store F2FS_MAX_NAME_LEN, but lets follow linux convention.= */ > +#define F2FS_NAME_LEN 255 > #define F2FS_MAX_NAME_LEN 256 > #define ADDRS_PER_INODE 923 /* Address Pointers in an Inode = */ > #define ADDRS_PER_BLOCK 1018 /* Address Pointers in a Direct > Block */ > @@ -362,10 +364,10 @@ struct f2fs_summary_block { > typedef __le32 f2fs_hash_t; > > /* One directory entry slot covers 8bytes-long file name */ > -#define F2FS_NAME_LEN 8 > -#define F2FS_NAME_LEN_BITS 3 > +#define F2FS_SLOT_LEN 8 > +#define F2FS_SLOT_LEN_BITS 3 > > -#define GET_DENTRY_SLOTS(x) ((x + F2FS_NAME_LEN - 1) >> > F2FS_NAME_LEN_BITS) > +#define GET_DENTRY_SLOTS(x) ((x + F2FS_SLOT_LEN - 1) >> > F2FS_SLOT_LEN_BITS) > > /* the number of dentry in a block */ > #define NR_DENTRY_IN_BLOCK 214 > @@ -377,10 +379,10 @@ typedef __le32 f2fs_hash_t; > #define SIZE_OF_DENTRY_BITMAP ((NR_DENTRY_IN_BLOCK + BITS_PER_BYTE - > 1) / \ > BITS_PER_BYTE) > #define SIZE_OF_RESERVED (PAGE_SIZE - ((SIZE_OF_DIR_ENTRY + \ > - F2FS_NAME_LEN) * \ > + F2FS_SLOT_LEN) * \ > NR_DENTRY_IN_BLOCK + SIZE_OF_DENTRY_BITMAP)) > > -/* One directory entry slot representing F2FS_NAME_LEN-sized file na= me > */ > +/* One directory entry slot representing F2FS_SLOT_LEN-sized file na= me > */ > struct f2fs_dir_entry { > __le32 hash_code; /* hash code of file name */ > __le32 ino; /* inode number */ > @@ -394,7 +396,7 @@ struct f2fs_dentry_block { > __u8 dentry_bitmap[SIZE_OF_DENTRY_BITMAP]; > __u8 reserved[SIZE_OF_RESERVED]; > struct f2fs_dir_entry dentry[NR_DENTRY_IN_BLOCK]; > - __u8 filename[NR_DENTRY_IN_BLOCK][F2FS_NAME_LEN]; > + __u8 filename[NR_DENTRY_IN_BLOCK][F2FS_SLOT_LEN]; > } __packed; > > /* file types used in inode_info->flags */ > -- > 1.8.1.3.566.gaa39828 > > > > > -- > Jaegeuk Kim > Samsung > -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel= " in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html