[Ocfs2-devel] [PATCH 0/2] ocfs2: two bug fixes about xattr and inline-data V2

[Ocfs2-devel] [PATCH 0/2] ocfs2: two bug fixes about xattr and inline-data V2

[Tiger Yang]

Hi,

Thanks Tao and Joel's review, These are the second version of the patches.

Mark and Joel, you can correct my comments if you find they don't suitable.

thanks,
tiger

[Ocfs2-devel] [PATCH 1/2] ocfs2: reserve xattr block for new directory with inline data

[Tiger Yang]

If this is a new directory with inline data, we choose to reserve
the entire inline area for directory contents and force an external
xattr block.

Signed-off-by: Tiger Yang <tiger.yang@oracle.com>
---
 fs/ocfs2/xattr.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index 4ddd788..053ae3d 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -549,6 +549,7 @@ int ocfs2_calc_xattr_init(struct inode *dir,
 	 * for them is ok.
 	 */
 	if (dir->i_sb->s_blocksize == OCFS2_MIN_BLOCKSIZE ||
+	    (S_ISDIR(mode) && ocfs2_supports_inline_data(osb)) ||
 	    (s_size + a_size) > OCFS2_XATTR_FREE_IN_IBODY) {
 		ret = ocfs2_reserve_new_metadata_blocks(osb, 1, xattr_ac);
 		if (ret) {
-- 
1.5.4.1

[Ocfs2-devel] [PATCH 1/2] ocfs2: reserve xattr block for new directory with inline data

[Tao Ma]

Hi tiger,
	I guess Joel's idea is that you add comment above the code. Not in the 
commit log. ;)

Regards,
Tao

Tiger Yang wrote:
> If this is a new directory with inline data, we choose to reserve
> the entire inline area for directory contents and force an external
> xattr block.
> 
> Signed-off-by: Tiger Yang <tiger.yang@oracle.com>
> ---
>  fs/ocfs2/xattr.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
> index 4ddd788..053ae3d 100644
> --- a/fs/ocfs2/xattr.c
> +++ b/fs/ocfs2/xattr.c
> @@ -549,6 +549,7 @@ int ocfs2_calc_xattr_init(struct inode *dir,
>  	 * for them is ok.
>  	 */
>  	if (dir->i_sb->s_blocksize == OCFS2_MIN_BLOCKSIZE ||
> +	    (S_ISDIR(mode) && ocfs2_supports_inline_data(osb)) ||
>  	    (s_size + a_size) > OCFS2_XATTR_FREE_IN_IBODY) {
>  		ret = ocfs2_reserve_new_metadata_blocks(osb, 1, xattr_ac);
>  		if (ret) {

[Ocfs2-devel] [PATCH 1/2] ocfs2: reserve xattr block for new directory with inline data

[Joel Becker]

On Thu, Mar 05, 2009 at 11:17:14AM +0800, Tao Ma wrote:
> Hi tiger,
> 	I guess Joel's idea is that you add comment above the code. Not in the 
> commit log. ;)

	The commit log is good, but the comment should also be added to
the code.  Tao is right that's what I was asking for :-)  You'll note
that the comment above the if() describes the cases in which we skip
inline xattrs.  You've added another reason to skip inline xattrs, so
that comment should include your reason.
	Literally, cut and past your commit log into that comment block.

Joel

-- 

"The question of whether computers can think is just like the question
 of whether submarines can swim."
	- Edsger W. Dijkstra

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127

[Ocfs2-devel] [PATCH 1/2] ocfs2: reserve xattr block for new directory with inline data

[Tiger Yang]

If this is a new directory with inline data, we choose to
reserve the entire inline area for directory contents and
force an external xattr block.

Signed-off-by: Tiger Yang <tiger.yang@oracle.com>
---
 fs/ocfs2/xattr.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index 4ddd788..c63efb5 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -547,8 +547,12 @@ int ocfs2_calc_xattr_init(struct inode *dir,
 	 * when blocksize = 512, may reserve one more cluser for
 	 * xattr bucket, otherwise reserve one metadata block
 	 * for them is ok.
+	 * If this is a new directory with inline data,
+	 * we choose to reserve the entire inline area for
+	 * directory contents and force an external xattr block.
 	 */
 	if (dir->i_sb->s_blocksize == OCFS2_MIN_BLOCKSIZE ||
+	    (S_ISDIR(mode) && ocfs2_supports_inline_data(osb)) ||
 	    (s_size + a_size) > OCFS2_XATTR_FREE_IN_IBODY) {
 		ret = ocfs2_reserve_new_metadata_blocks(osb, 1, xattr_ac);
 		if (ret) {
-- 
1.5.4.1

[Ocfs2-devel] [PATCH 1/2] ocfs2: reserve xattr block for new directory with inline data

[Joel Becker]

On Fri, Mar 06, 2009 at 10:19:30AM +0800, Tiger Yang wrote:
> If this is a new directory with inline data, we choose to
> reserve the entire inline area for directory contents and
> force an external xattr block.
> 
> Signed-off-by: Tiger Yang <tiger.yang@oracle.com>
Acked-by: Joel Becker <joel.becker@oracle.com>


-- 

"To fall in love is to create a religion that has a fallible god."
        -Jorge Luis Borges

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127

[Ocfs2-devel] [PATCH 2/2] ocfs2: tweak to get the maximum inline data size with xattr

[Tiger Yang]

Replace max_inline_data with max_inline_data_with_xattr
to ensure it correct when xattr inlined.

Signed-off-by: Tiger Yang <tiger.yang@oracle.com>
---
 fs/ocfs2/aops.c     |    7 +++++--
 fs/ocfs2/namei.c    |    3 ++-
 fs/ocfs2/ocfs2_fs.h |    6 ------
 3 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c
index a067a6c..8e1709a 100644
--- a/fs/ocfs2/aops.c
+++ b/fs/ocfs2/aops.c
@@ -227,7 +227,7 @@ int ocfs2_read_inline_data(struct inode *inode, struct page *page,
 	size = i_size_read(inode);
 
 	if (size > PAGE_CACHE_SIZE ||
-	    size > ocfs2_max_inline_data(inode->i_sb)) {
+	    size > ocfs2_max_inline_data_with_xattr(inode->i_sb, di)) {
 		ocfs2_error(inode->i_sb,
 			    "Inode %llu has with inline data has bad size: %Lu",
 			    (unsigned long long)OCFS2_I(inode)->ip_blkno,
@@ -1555,6 +1555,7 @@ static int ocfs2_try_to_write_inline_data(struct address_space *mapping,
 	int ret, written = 0;
 	loff_t end = pos + len;
 	struct ocfs2_inode_info *oi = OCFS2_I(inode);
+	struct ocfs2_dinode *di = NULL;
 
 	mlog(0, "Inode %llu, write of %u bytes at off %llu. features: 0x%x\n",
 	     (unsigned long long)oi->ip_blkno, len, (unsigned long long)pos,
@@ -1587,7 +1588,9 @@ static int ocfs2_try_to_write_inline_data(struct address_space *mapping,
 	/*
 	 * Check whether the write can fit.
 	 */
-	if (mmap_page || end > ocfs2_max_inline_data(inode->i_sb))
+	di = (struct ocfs2_dinode *)wc->w_di_bh->b_data;
+	if (mmap_page ||
+	    end > ocfs2_max_inline_data_with_xattr(inode->i_sb, di))
 		return 0;
 
 do_inline_write:
diff --git a/fs/ocfs2/namei.c b/fs/ocfs2/namei.c
index 084aba8..4b11762 100644
--- a/fs/ocfs2/namei.c
+++ b/fs/ocfs2/namei.c
@@ -532,7 +532,8 @@ static int ocfs2_mknod_locked(struct ocfs2_super *osb,
 
 		fe->i_dyn_features = cpu_to_le16(feat | OCFS2_INLINE_DATA_FL);
 
-		fe->id2.i_data.id_count = cpu_to_le16(ocfs2_max_inline_data(osb->sb));
+		fe->id2.i_data.id_count = cpu_to_le16(
+				ocfs2_max_inline_data_with_xattr(osb->sb, fe));
 	} else {
 		fel = &fe->id2.i_list;
 		fel->l_tree_depth = 0;
diff --git a/fs/ocfs2/ocfs2_fs.h b/fs/ocfs2/ocfs2_fs.h
index c7ae45a..2332ef7 100644
--- a/fs/ocfs2/ocfs2_fs.h
+++ b/fs/ocfs2/ocfs2_fs.h
@@ -1070,12 +1070,6 @@ static inline int ocfs2_fast_symlink_chars(struct super_block *sb)
 		 offsetof(struct ocfs2_dinode, id2.i_symlink);
 }
 
-static inline int ocfs2_max_inline_data(struct super_block *sb)
-{
-	return sb->s_blocksize -
-		offsetof(struct ocfs2_dinode, id2.i_data.id_data);
-}
-
 static inline int ocfs2_max_inline_data_with_xattr(struct super_block *sb,
 						   struct ocfs2_dinode *di)
 {
-- 
1.5.4.1

[Ocfs2-devel] [PATCH 2/2] ocfs2: tweak to get the maximum inline data size with xattr

[Joel Becker]

On Thu, Mar 05, 2009 at 11:06:15AM +0800, Tiger Yang wrote:
> Replace max_inline_data with max_inline_data_with_xattr
> to ensure it correct when xattr inlined.
> 
> Signed-off-by: Tiger Yang <tiger.yang@oracle.com>
Acked-by: Joel Becker <joel.becker@oracle.com>

Probably want to make sure the same problem doesn't affect ocfs2-tools.

Joel

-- 

"If the human brain were so simple we could understand it, we would
 be so simple that we could not."
	- W. A. Clouston

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127