Re: [PATCH 4/5] f2fs: align f2fs maximum name length to linux based filesystem

[Namjae Jeon]

2013/3/3, Jaegeuk Kim <jaegeuk.kim@xxxxxxxxxxx>:
> We should not change the on-disk layout.
> Instead, simply we can deal with it by changing original condition check
> 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 â256â is the space reserved for the name âon-diskâ but by
changing the code â we are limiting it to use â255â.
If we chance the on-disk to make use of â255â bytes it allows for
keeping all code intact and also like the code changes suggested, it
will still refer only the â255â bytes.

More so, changing the on-disk allows for â1byteâ which is still padded
at the same position to be used in future. Otherwise, this âextraâ
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 2001
> From: Jaegeuk Kim <jaegeuk.kim@xxxxxxxxxxx>
> Date: Sun, 3 Mar 2013 13:58:05 +0900
> Subject: [PATCH] f2fs: align f2fs maximum name length to linux based
>  filesystem
> Cc: linux-fsdevel@xxxxxxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx,
> linux-f2fs-devel@xxxxxxxxxxxxxxxxxxxxx
>
> The maximum filename length supported in linux is 255 characters.
> So let's follow that.
>
> Signed-off-by: Namjae Jeon <namjae.jeon@xxxxxxxxxxx>
> Signed-off-by: Amit Sahrawat <a.sahrawat@xxxxxxxxxxx>
> Signed-off-by: Jaegeuk Kim <jaegeuk.kim@xxxxxxxxxxx>
> ---
>  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 inode
> *dir,
>  	unsigned int max_depth;
>  	unsigned int level;
>
> +	if (namelen > F2FS_NAME_LEN)
> +		return NULL;
> +
>  	if (npages == 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 *dir,
> 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 = 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, struct
> kstatfs *buf)
>  	buf->f_files = sbi->total_node_count;
>  	buf->f_ffree = sbi->total_node_count - valid_inode_count(sbi);
>
> -	buf->f_namelen = F2FS_MAX_NAME_LEN;
> +	buf->f_namelen = F2FS_NAME_LEN;
>  	buf->f_fsid.val[0] = (u32)id;
>  	buf->f_fsid.val[1] = (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 name
> */
> +/* One directory entry slot representing F2FS_SLOT_LEN-sized file name
> */
>  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-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/