linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jaegeuk Kim <jaegeuk.kim@samsung.com>
To: Namjae Jeon <linkinjeon@gmail.com>
Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-f2fs-devel@lists.sourceforge.net,
	Namjae Jeon <namjae.jeon@samsung.com>,
	Amit Sahrawat <a.sahrawat@samsung.com>
Subject: Re: [PATCH 4/5] f2fs: align f2fs maximum name length to linux based filesystem
Date: Fri, 08 Mar 2013 11:38:52 +0900	[thread overview]
Message-ID: <1362710332.14386.38.camel@kjgkr> (raw)
In-Reply-To: <CAKYAXd8VzmxdrUq-vCbhDpLWirmUUxRjj_-5FUmNutBG5cojVA@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 5672 bytes --]

2013-03-04 (월), 15:25 +0900, Namjae Jeon:
> 2013/3/3, Jaegeuk Kim <jaegeuk.kim@samsung.com>:
> > 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.

Agreed, but it still needs to change a couple of names.
How about this?

From 2ce7f345559d967bc1c79959885dff9f0a713e32 Mon Sep 17 00:00:00 2001
From: Jaegeuk Kim <jaegeuk.kim@samsung.com>
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 <namjae.jeon@samsung.com>
Signed-off-by: Amit Sahrawat <a.sahrawat@samsung.com>
Signed-off-by: Jaegeuk Kim <jaegeuk.kim@samsung.com>
---
 fs/f2fs/dir.c           |  3 +++
 fs/f2fs/namei.c         |  2 +-
 fs/f2fs/super.c         |  2 +-
 include/linux/f2fs_fs.h | 17 +++++++++--------
 4 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index a1f3844..2851ae6 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..df6fab8 100644
--- a/include/linux/f2fs_fs.h
+++ b/include/linux/f2fs_fs.h
@@ -139,7 +139,7 @@ struct f2fs_extent {
 	__le32 len;		/* lengh of the extent */
 } __packed;
 
-#define F2FS_MAX_NAME_LEN	256
+#define F2FS_NAME_LEN		255
 #define ADDRS_PER_INODE         923	/* Address Pointers in an Inode */
 #define ADDRS_PER_BLOCK         1018	/* Address Pointers in a Direct
Block */
 #define NIDS_PER_BLOCK          1018	/* Node IDs in an Indirect Block
*/
@@ -165,7 +165,8 @@ struct f2fs_inode {
 	__le32 i_flags;			/* file attributes */
 	__le32 i_pino;			/* parent inode number */
 	__le32 i_namelen;		/* file name length */
-	__u8 i_name[F2FS_MAX_NAME_LEN];	/* file name for SPOR */
+	__u8 i_name[F2FS_NAME_LEN];	/* file name for SPOR */
+	__u8 i_reserved2;		/* for backward compatibility */
 
 	struct f2fs_extent i_ext;	/* caching a largest extent */
 
@@ -362,10 +363,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 +378,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 +395,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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

  reply	other threads:[~2013-03-08  2:38 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-02  3:41 [PATCH 4/5] f2fs: align f2fs maximum name length to linux based filesystem Namjae Jeon
2013-03-03  5:04 ` Jaegeuk Kim
2013-03-04  6:25   ` Namjae Jeon
2013-03-08  2:38     ` Jaegeuk Kim [this message]
2013-03-08  4:15       ` Namjae Jeon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1362710332.14386.38.camel@kjgkr \
    --to=jaegeuk.kim@samsung.com \
    --cc=a.sahrawat@samsung.com \
    --cc=linkinjeon@gmail.com \
    --cc=linux-f2fs-devel@lists.sourceforge.net \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=namjae.jeon@samsung.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).