ocfs2: add security EA and ACL support V4

PATCH New Tiger Yang 4 participants 37 messages Oct 30, 2008 – Nov 7, 2008

Subsystems (3)

Documentation VFS OCFS2

lore

Share & tools

lore

Tiger Yang

author

2008-10-30 11:31 UTC #0 lore raw

cover ocfs2: add security EA and ACL support V4

hi,
This is the version 4 patches for add security EA and ACL support in ocfs2. We have fixed many problems in version 3 and I split the previous big patch into some small patches. Them based on joel's xattr_buckets branch.

The first five patches from Tao Ma.
1: only extend xattr bucket in need
2: add cluster free in dealloc ctxt
3: move clusters free into dealloc
4: reserve meta data at the begining of ocfs2_xattr_set 5: merge xattr set transactions

The next four patches add security EA support in ocfs2. 6: move new_inode out of transaction
7: add ocfs2_xattr_set_handle
8: add security xattr API
9: add ocfs2_init_security in mknod

The final six patches add ACL support in ocfs2.
10: add xattr_get_nolock
11: add posix acl API
12: add ocfs2_check_acl
13: add ocfs2_acl_chmod
14: add mount and Kconfig option for acl

Best regards,
tiger

Tao Ma

2008-10-30 05:42 UTC #1 lore raw

PATCH 1/15 ocfs2/xattr: Only extend xattr bucket in need.

When the first block of a bucket is filled up with xattr entries, we normally extend the bucket. But if we are just replace one xattr with small length, we don't need to extend it. This is important since we will calculate what we need before the transaction and in this situation no resources will be allocated.

Signed-off-by Tao Ma <[email protected]>

---
 fs/ocfs2/xattr.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

fs/ocfs2/xattr.c +3 -1

index b0a73c2..dc90ffb 100644
@@ -4569,7 +4569,9 @@ try_again:
 	     free, need, max_free, le16_to_cpu(xh->xh_free_start),
 	     le16_to_cpu(xh->xh_name_value_len));
 
-	if (free < need || count == ocfs2_xattr_max_xe_in_bucket(inode->i_sb)) {
+	if (free < need ||
+	    (xs->not_found &&
+	     count == ocfs2_xattr_max_xe_in_bucket(inode->i_sb))) {
 		if (need <= max_free &&
 		    count < ocfs2_xattr_max_xe_in_bucket(inode->i_sb)) {
 			/*

Signature · 1 line

1.5.4.GIT

Mark Fasheh

2008-11-06 19:45 UTC #2 lore raw

PATCH 1/15 ocfs2/xattr: Only extend xattr bucket in need.

Quoted 6 lines · Tao Ma

When the first block of a bucket is filled up with xattr entries, we normally extend the bucket. But if we are just replace one xattr with small length, we don't need to extend it. This is important since we will calculate what we need before the transaction and in this situation no resources will be allocated.

Ok, looks good. Am I correct in thinking that this is a fix which should go upstream before 2.6.28 is out?

	--Mark

--
Mark Fasheh

Tao Ma

2008-11-07 01:05 UTC #3 lore raw

PATCH 1/15 ocfs2/xattr: Only extend xattr bucket in need.

Quoted 10 lines · Mark Fasheh

Quoted 6 lines · Tao Ma
When the first block of a bucket is filled up with xattr entries, we normally extend the bucket. But if we are just replace one xattr with small length, we don't need to extend it. This is important since we will calculate what we need before the transaction and in this situation no resources will be allocated.
Ok, looks good. Am I correct in thinking that this is a fix which should go upstream before 2.6.28 is out?

en, this should not be called a bug fix. Maybe an improvement. In old implementation, we extend the bucket with a less restriction. And since we allocate resource in ocfs2_extend_xattr_bucket, it will not cause a problem. But if we want to merge the transactions into one, I calculate the resource needed in a more strict way, so it is a problem at that time.

anyway, you can push it before 2.6.28 as you wish.

Regards,
Tao

Tao Ma

2008-10-30 05:42 UTC #4 lore raw

PATCH 2/15 ocfs2: Add clusters free in dealloc_ctxt.

In reply to Tiger Yang

Now in ocfs2 xattr set, the whole process are divided into many small parts and they are wrapped into diffrent transactions and it make the set doesn't look like a real transaction. So we want to integrate it into a real one.

In some cases we will allocate some clusters and free some in just one transaction. e.g, one xattr is larger than inline size, so it and its value root is stored within the inode while the value is outside in a cluster. Then we try to update it with a smaller value(larger than the size of root but smaller than inline size), we may need to free the outside cluster while allocate a new bucket(one cluster) since now the inode may be full. The old solution will lock the global_bitmap(if the local alloc failed in stress test) and then the truncate log. This will cause a ABBA lock with truncate log flush.

This patch add the clusters free in dealloc_ctxt, so that we can record the free clusters during the transaction and then free it after we release the global_bitmap in xattr set.

Signed-off-by Tao Ma <[email protected]>

---
 fs/ocfs2/alloc.c |   80 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/ocfs2/alloc.h |    4 +++
 2 files changed, 84 insertions(+), 0 deletions(-)

fs/ocfs2/alloc.c +80 -0

index 0cc2deb..3c54572 100644
@@ -5801,6 +5801,11 @@ int ocfs2_truncate_log_init(struct ocfs2_super *osb)
 
 /*
  * Describes a single block free from a suballocator
+ *
+ * XXX:
+ * This structure is reused when we free a number of clusters
+ * during xattr set. In that case, free_bit indicates the number
+ * of the free clusters.
  */
 struct ocfs2_cached_block_free {
 	struct ocfs2_cached_block_free		*free_next;

@@ -5893,6 +5898,73 @@ out:
 	return ret;
 }
 
+int ocfs2_cache_clusters_dealloc(struct ocfs2_cached_dealloc_ctxt *ctxt,
+				 u64 blkno, unsigned int bit)
+{
+	int ret = 0;
+	struct ocfs2_cached_block_free *item;
+
+	item = kmalloc(sizeof(*item), GFP_NOFS);
+	if (item == NULL) {
+		ret = -ENOMEM;
+		mlog_errno(ret);
+		return ret;
+	}
+
+	mlog(0, "Insert clusters: (bit %u, blk %llu)\n",
+	     bit, (unsigned long long)blkno);
+
+	item->free_blk = blkno;
+	item->free_bit = bit;
+	item->free_next = ctxt->c_clusters_list;
+
+	return ret;
+}
+
+static int ocfs2_free_clusters_list(struct ocfs2_super *osb,
+				    struct ocfs2_cached_block_free *head)
+{
+	struct ocfs2_cached_block_free *tmp;
+	struct inode *tl_inode = osb->osb_tl_inode;
+	handle_t *handle;
+	int ret = 0;
+
+	mutex_lock(&tl_inode->i_mutex);
+
+	handle = ocfs2_start_trans(osb, OCFS2_TRUNCATE_LOG_UPDATE);
+	if (IS_ERR(handle)) {
+		ret = PTR_ERR(handle);
+		mlog_errno(ret);
+		goto out_mutex;
+	}
+
+	while (head) {
+		ret = ocfs2_truncate_log_append(osb, handle, head->free_blk,
+						head->free_bit);
+		if (ret) {
+			mlog_errno(ret);
+			break;
+		}
+		tmp = head;
+		head = head->free_next;
+		kfree(tmp);
+	}
+
+	ocfs2_commit_trans(osb, handle);
+
+out_mutex:
+	mutex_unlock(&tl_inode->i_mutex);
+
+	while (head) {
+		/* Premature exit may have left some dangling items. */
+		tmp = head;
+		head = head->free_next;
+		kfree(tmp);
+	}
+
+	return ret;
+}
+
 int ocfs2_run_deallocs(struct ocfs2_super *osb,
 		       struct ocfs2_cached_dealloc_ctxt *ctxt)
 {

@@ -5920,6 +5992,14 @@ int ocfs2_run_deallocs(struct ocfs2_super *osb,
 		kfree(fl);
 	}
 
+	if (ctxt->c_clusters_list) {
+		ret2 = ocfs2_free_clusters_list(osb, ctxt->c_clusters_list);
+		if (ret2)
+			mlog_errno(ret2);
+		if (!ret)
+			ret = ret2;
+	}
+
 	return ret;
 }

fs/ocfs2/alloc.h +4 -0

index 70257c8..1333a92 100644
@@ -167,11 +167,15 @@ int __ocfs2_flush_truncate_log(struct ocfs2_super *osb);
  */
 struct ocfs2_cached_dealloc_ctxt {
 	struct ocfs2_per_slot_free_list		*c_first_suballocator;
+	struct ocfs2_cached_block_free 		*c_clusters_list;
 };
 static inline void ocfs2_init_dealloc_ctxt(struct ocfs2_cached_dealloc_ctxt *c)
 {
 	c->c_first_suballocator = NULL;
+	c->c_clusters_list = NULL;
 }
+int ocfs2_cache_clusters_dealloc(struct ocfs2_cached_dealloc_ctxt *ctxt,
+				 u64 blkno, unsigned int bit);
 int ocfs2_run_deallocs(struct ocfs2_super *osb,
 		       struct ocfs2_cached_dealloc_ctxt *ctxt);

Signature · 1 line

1.5.4.GIT

Joel Becker

2008-11-04 01:16 UTC #5 lore raw

PATCH 2/15 ocfs2: Add clusters free in dealloc_ctxt.

Quoted 15 lines · Tao Ma

Now in ocfs2 xattr set, the whole process are divided into many small parts and they are wrapped into diffrent transactions and it make the set doesn't look like a real transaction. So we want to integrate it into a real one.
In some cases we will allocate some clusters and free some in just one transaction. e.g, one xattr is larger than inline size, so it and its value root is stored within the inode while the value is outside in a cluster. Then we try to update it with a smaller value(larger than the size of root but smaller than inline size), we may need to free the outside cluster while allocate a new bucket(one cluster) since now the inode may be full. The old solution will lock the global_bitmap(if the local alloc failed in stress test) and then the truncate log. This will cause a ABBA lock with truncate log flush.

Quoted 3 lines

This patch add the clusters free in dealloc_ctxt, so that we can record the free clusters during the transaction and then free it after we release the global_bitmap in xattr set.

So, you're really adding the global cluster allocator as a type
of allocator that can live on the dealloc context.

Quoted 5 lines

fs/ocfs2/alloc.c +0 -0
index 0cc2deb..3c54572 100644

fs/ocfs2/alloc.c int ocfs2_truncate_log_init(struct ocfs2_super *osb) · line 5809

Quoted 9 lines

fs/ocfs2/alloc.c +5 -0

@@ -5801,6 +5801,11 @@ int ocfs2_truncate_log_init(struct ocfs2_super *osb)
 
 /*
  * Describes a single block free from a suballocator
+ *
+ * XXX:
+ * This structure is reused when we free a number of clusters
+ * during xattr set. In that case, free_bit indicates the number
+ * of the free clusters.
  */

How about:

/*
* Describe a single bit freed from a suballocator. For the block
* suballocators, it represents one block. For the global cluster
* allocator, it represents one cluster.
*/

fs/ocfs2/alloc.c out: · line 5902

Quoted 8 lines

fs/ocfs2/alloc.c +2 -0

 struct ocfs2_cached_block_free {
 	struct ocfs2_cached_block_free		*free_next;

@@ -5893,6 +5898,73 @@ out:
 	return ret;
 }
 
+int ocfs2_cache_clusters_dealloc(struct ocfs2_cached_dealloc_ctxt *ctxt,
+				 u64 blkno, unsigned int bit)

Call it ocfs2_cache_cluster_dealloc() for consistency (the
singular form).

Quoted 7 lines

fs/ocfs2/alloc.c +7 -0

+{
+	int ret = 0;
+	struct ocfs2_cached_block_free *item;
+
+	item = kmalloc(sizeof(*item), GFP_NOFS);
+	if (item == NULL) {
+		ret = -ENOMEM;

fs/ocfs2/alloc.c out: · line 5919

Quoted 10 lines

fs/ocfs2/alloc.c +10 -0

+		mlog_errno(ret);
+		return ret;
+	}
+
+	mlog(0, "Insert clusters: (bit %u, blk %llu)\n",
+	     bit, (unsigned long long)blkno);
+
+	item->free_blk = blkno;
+	item->free_bit = bit;
+	item->free_next = ctxt->c_clusters_list;

You need to push item on to the list too.

  +	ctxt->c_clusters_list = item;

fs/ocfs2/alloc.c out: · line 5925

Quoted 6 lines

fs/ocfs2/alloc.c +6 -0

+
+	return ret;
+}
+
+static int ocfs2_free_clusters_list(struct ocfs2_super *osb,
+				    struct ocfs2_cached_block_free *head)

Call this 'ocfs2_free_cached_clusters' and change
'ocfs2_free_cached_items' to 'ocfs2_free_cached_blocks'.

Quoted 12 lines

fs/ocfs2/alloc.c +12 -0

+{
+	struct ocfs2_cached_block_free *tmp;
+	struct inode *tl_inode = osb->osb_tl_inode;
+	handle_t *handle;
+	int ret = 0;
+
+	mutex_lock(&tl_inode->i_mutex);
+
+	handle = ocfs2_start_trans(osb, OCFS2_TRUNCATE_LOG_UPDATE);
+	if (IS_ERR(handle)) {
+		ret = PTR_ERR(handle);
+		mlog_errno(ret);

fs/ocfs2/alloc.c out: · line 5947

Quoted 10 lines

fs/ocfs2/alloc.c +10 -0

+		goto out_mutex;
+	}
+
+	while (head) {
+		ret = ocfs2_truncate_log_append(osb, handle, head->free_blk,
+						head->free_bit);
+		if (ret) {
+			mlog_errno(ret);
+			break;
+		}

What happens if the truncate log fills up?

Quoted 23 lines

fs/ocfs2/alloc.c +20 -0

+		tmp = head;
+		head = head->free_next;
+		kfree(tmp);
+	}
+
+	ocfs2_commit_trans(osb, handle);
+
+out_mutex:
+	mutex_unlock(&tl_inode->i_mutex);
+
+	while (head) {
+		/* Premature exit may have left some dangling items. */
+		tmp = head;
+		head = head->free_next;
+		kfree(tmp);
+	}
+
+	return ret;
+}
+
 int ocfs2_run_deallocs(struct ocfs2_super *osb,
 		       struct ocfs2_cached_dealloc_ctxt *ctxt)
 {

<snip>

fs/ocfs2/alloc.h int __ocfs2_flush_truncate_log(struct ocfs2_super *osb); · line 170

Quoted 9 lines

fs/ocfs2/alloc.h +1 -0

index 70257c8..1333a92 100644
@@ -167,11 +167,15 @@ int __ocfs2_flush_truncate_log(struct ocfs2_super *osb);
  */
 struct ocfs2_cached_dealloc_ctxt {
 	struct ocfs2_per_slot_free_list		*c_first_suballocator;
+	struct ocfs2_cached_block_free 		*c_clusters_list;

Change 'c_clusters_list' to 'c_global_allocator'.

Joel

Signature · 9 lines

"If you took all of the grains of sand in the world, and lined
 them up end to end in a row, you'd be working for the government!"
	- Mr. Interesting

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

Tao Ma

2008-11-04 01:34 UTC #6 lore raw

PATCH 2/15 ocfs2: Add clusters free in dealloc_ctxt.

Quoted 20 lines · Joel Becker

Quoted 7 lines

fs/ocfs2/alloc.c +7 -0

+{
+	int ret = 0;
+	struct ocfs2_cached_block_free *item;
+
+	item = kmalloc(sizeof(*item), GFP_NOFS);
+	if (item == NULL) {
+		ret = -ENOMEM;

fs/ocfs2/alloc.c out: · line 5919

Quoted 10 lines

fs/ocfs2/alloc.c +10 -0

+		mlog_errno(ret);
+		return ret;
+	}
+
+	mlog(0, "Insert clusters: (bit %u, blk %llu)\n",
+	     bit, (unsigned long long)blkno);
+
+	item->free_blk = blkno;
+	item->free_bit = bit;
+	item->free_next = ctxt->c_clusters_list;

You need to push item on to the list too.

Quoted 1 line

  +	ctxt->c_clusters_list = item;

yeah, this is obviously a bug. thanks.

Quoted 27 lines

fs/ocfs2/alloc.c out: · line 5925

Quoted 7 lines

fs/ocfs2/alloc.c +6 -0

+
+	return ret;
+}
+
+static int ocfs2_free_clusters_list(struct ocfs2_super *osb,
+				    struct ocfs2_cached_block_free *head)

Call this 'ocfs2_free_cached_clusters' and change
'ocfs2_free_cached_items' to 'ocfs2_free_cached_blocks'.

Quoted 16 lines

fs/ocfs2/alloc.c +16 -0

+{
+	struct ocfs2_cached_block_free *tmp;
+	struct inode *tl_inode = osb->osb_tl_inode;
+	handle_t *handle;
+	int ret = 0;
+
+	mutex_lock(&tl_inode->i_mutex);
+
+	handle = ocfs2_start_trans(osb, OCFS2_TRUNCATE_LOG_UPDATE);
+	if (IS_ERR(handle)) {
+		ret = PTR_ERR(handle);
+		mlog_errno(ret);
+		goto out_mutex;
+	}
+
+	while (head) {

Quoted 8 lines

fs/ocfs2/alloc.c out: · line 5947
Quoted 6 lines
fs/ocfs2/alloc.c +6 -0
+		ret = ocfs2_truncate_log_append(osb, handle, head->free_blk,
+						head->free_bit);
+		if (ret) {
+			mlog_errno(ret);
+			break;
+		}
What happens if the truncate log fills up?

currently, it is only used in one xattr set. So the total size is only 64K bytes and I don't think the chance is very high. I have also gone through ocfs2_commit_truncate, no error handler there, and even if it is filled up and error out, fsck should be able to find them. But you are probably right, if in future we use it in truncating file, it may be a very high risk. But it could be in another separate patch, agree?

btw, I will fix other typos according to your advice, thanks.

Regards,
Tao

Joel Becker

2008-11-04 03:01 UTC #7 lore raw

PATCH 2/15 ocfs2: Add clusters free in dealloc_ctxt.

Quoted 25 lines · Tao Ma

Quoted 16 lines · Joel Becker

Quoted 16 lines

fs/ocfs2/alloc.c +16 -0

+{
+	struct ocfs2_cached_block_free *tmp;
+	struct inode *tl_inode = osb->osb_tl_inode;
+	handle_t *handle;
+	int ret = 0;
+
+	mutex_lock(&tl_inode->i_mutex);
+
+	handle = ocfs2_start_trans(osb, OCFS2_TRUNCATE_LOG_UPDATE);
+	if (IS_ERR(handle)) {
+		ret = PTR_ERR(handle);
+		mlog_errno(ret);
+		goto out_mutex;
+	}
+
+	while (head) {

Quoted 8 lines

fs/ocfs2/alloc.c out: · line 5947
Quoted 6 lines
fs/ocfs2/alloc.c +6 -0
+		ret = ocfs2_truncate_log_append(osb, handle, head->free_blk,
+						head->free_bit);
+		if (ret) {
+			mlog_errno(ret);
+			break;
+		}
What happens if the truncate log fills up?

Quoted 6 lines

currently, it is only used in one xattr set. So the total size is only 64K bytes and I don't think the chance is very high. I have also gone through ocfs2_commit_truncate, no error handler there, and even if it is filled up and error out, fsck should be able to find them. But you are probably right, if in future we use it in truncating file, it may be a very high risk. But it could be in another separate patch, agree?

ocfs2_commit_truncate() is a loop from start: to bail:. It
calls ocfs2_do_truncate() for each extent it removes. Before each call to ocfs2_do_truncate(), it checks ocfs2_truncate_log_needs_flush(). That is, it never adds more than one record to the truncate log at a time, and it checks for a full log before adding each record.
Your 64K can be 16 clusters at a 4K cluster size. That's 16
records. What if the truncate log had only space for three records from a previous operation? Your fourth call to ocfs2_truncate_log_append() will mlog_bug_on_msg() like so:

5319 mlog_bug_on_msg(tl_count > ocfs2_truncate_recs_per_inode(osb->sb) ||

That is not acceptable. We don't introduce known bugs and say
they are OK if they are rare.

Joel

Signature · 8 lines

"Sometimes one pays most for the things one gets for nothing."
        - Albert Einstein

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

Tao Ma

2008-11-04 03:07 UTC #8 lore raw

PATCH 2/15 ocfs2: Add clusters free in dealloc_ctxt.

Quoted 42 lines · Joel Becker

Quoted 24 lines · Tao Ma
Quoted 15 lines · Joel Becker
Quoted 15 lines
fs/ocfs2/alloc.c +15 -0
+{
+	struct ocfs2_cached_block_free *tmp;
+	struct inode *tl_inode = osb->osb_tl_inode;
+	handle_t *handle;
+	int ret = 0;
+
+	mutex_lock(&tl_inode->i_mutex);
+
+	handle = ocfs2_start_trans(osb, OCFS2_TRUNCATE_LOG_UPDATE);
+	if (IS_ERR(handle)) {
+		ret = PTR_ERR(handle);
+		mlog_errno(ret);
+		goto out_mutex;
+	}
+
Quoted 8 lines
fs/ocfs2/alloc.c out: · line 5947
Quoted 7 lines
fs/ocfs2/alloc.c +7 -0
+	while (head) {
+		ret = ocfs2_truncate_log_append(osb, handle, head->free_blk,
+						head->free_bit);
+		if (ret) {
+			mlog_errno(ret);
+			break;
+		}
What happens if the truncate log fills up?
Quoted 6 lines
currently, it is only used in one xattr set. So the total size is only 64K bytes and I don't think the chance is very high. I have also gone through ocfs2_commit_truncate, no error handler there, and even if it is filled up and error out, fsck should be able to find them. But you are probably right, if in future we use it in truncating file, it may be a very high risk. But it could be in another separate patch, agree?
ocfs2_commit_truncate() is a loop from start: to bail:. It
calls ocfs2_do_truncate() for each extent it removes. Before each call to ocfs2_do_truncate(), it checks ocfs2_truncate_log_needs_flush(). That is, it never adds more than one record to the truncate log at a time, and it checks for a full log before adding each record.
Your 64K can be 16 clusters at a 4K cluster size. That's 16
records. What if the truncate log had only space for three records from a previous operation? Your fourth call to ocfs2_truncate_log_append() will mlog_bug_on_msg() like so:

Quoted 4 lines

5319 mlog_bug_on_msg(tl_count > ocfs2_truncate_recs_per_inode(osb->sb) ||
That is not acceptable. We don't introduce known bugs and say
they are OK if they are rare.

Fair enough. I will try to find a way for it. thanks.

Regards,
Tao

Joel Becker

2008-11-04 07:40 UTC #9 lore raw

PATCH 2/15 ocfs2: Add clusters free in dealloc_ctxt.

In reply to Joel Becker

Quoted 9 lines · Joel Becker

Your 64K can be 16 clusters at a 4K cluster size. That's 16
records. What if the truncate log had only space for three records from a previous operation? Your fourth call to ocfs2_truncate_log_append() will mlog_bug_on_msg() like so:
5319 mlog_bug_on_msg(tl_count > ocfs2_truncate_recs_per_inode(osb->sb) ||
That is not acceptable. We don't introduce known bugs and say
they are OK if they are rare.

I want to clarify what I'm talking about here.
There are a few places in ocfs2 where we can't clean up from an
error perfectly. A recent example was in truncate. In ocfs2_do_truncate(), we can possibly get an error like -EIO or -ENOMEM inside ocfs2_trim_tree(). If that happens, the inode will have more clusters allocated to it than specified by ip_clusters. Since there is no easy way to either complete the truncate or restore the allocation, we leave the inode as-is. The allocation code is taught to handle the case where i_clusters is smaller than the actual allocated number.
The key fact is that the error is an unexpected case. -EIO
happens because the disk array went offline or there was a bad sector. It's not part of normal operation. Even with it not being part of normal operation, the filesystem can continue normal operations. There is no corruption, no BUG() is tripped, and if the rest of the disk is fine, things continue to work.
In the code above, adding records to the truncate log is normal
operation. The truncate log can be almost full as a part of normal operation. Thus, it should never cause an error or a crash.

Joel

Signature · 9 lines

Life's Little Instruction Book #444

	"Never underestimate the power of a kind word or deed."

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

Tao Ma

2008-11-04 23:25 UTC #10 lore raw

PATCH 2/15 ocfs2: Add clusters free in dealloc_ctxt.v2

In reply to Joel Becker

Modification from V1 to V2:
1. Some typo fix from Joel.
2. Adjust ocfs2_free_cached_clusters to handle the situation of truncate log full. Most of codes are copied from ocfs2_complete_truncate_log_recovery.

This patch add the clusters free in dealloc_ctxt, so that we can record the free clusters during the transaction and then free it after we release the global_bitmap in xattr set.

Signed-off-by Tao Ma <[email protected]>

---
 fs/ocfs2/alloc.c |  104 ++++++++++++++++++++++++++++++++++++++++++++++++++----
 fs/ocfs2/alloc.h |    4 ++
 2 files changed, 101 insertions(+), 7 deletions(-)

fs/ocfs2/alloc.c +97 -7

index 0cc2deb..0894b68 100644
@@ -5800,7 +5800,10 @@ int ocfs2_truncate_log_init(struct ocfs2_super *osb)
  */
 
 /*
- * Describes a single block free from a suballocator
+ * Describe a single bit freed from a suballocator.  For the block
+ * suballocators, it represents one block.  For the global cluster
+ * allocator, it represents some clusters and free_bit indicates
+ * clusters number.
  */
 struct ocfs2_cached_block_free {
 	struct ocfs2_cached_block_free		*free_next;

@@ -5815,10 +5818,10 @@ struct ocfs2_per_slot_free_list {
 	struct ocfs2_cached_block_free		*f_first;
 };
 
-static int ocfs2_free_cached_items(struct ocfs2_super *osb,
-				   int sysfile_type,
-				   int slot,
-				   struct ocfs2_cached_block_free *head)
+static int ocfs2_free_cached_blocks(struct ocfs2_super *osb,
+				    int sysfile_type,
+				    int slot,
+				    struct ocfs2_cached_block_free *head)
 {
 	int ret;
 	u64 bg_blkno;

@@ -5893,6 +5896,82 @@ out:
 	return ret;
 }
 
+int ocfs2_cache_cluster_dealloc(struct ocfs2_cached_dealloc_ctxt *ctxt,
+				u64 blkno, unsigned int bit)
+{
+	int ret = 0;
+	struct ocfs2_cached_block_free *item;
+
+	item = kmalloc(sizeof(*item), GFP_NOFS);
+	if (item == NULL) {
+		ret = -ENOMEM;
+		mlog_errno(ret);
+		return ret;
+	}
+
+	mlog(0, "Insert clusters: (bit %u, blk %llu)\n",
+	     bit, (unsigned long long)blkno);
+
+	item->free_blk = blkno;
+	item->free_bit = bit;
+	item->free_next = ctxt->c_global_allocator;
+
+	ctxt->c_global_allocator = item;
+	return ret;
+}
+
+static int ocfs2_free_cached_clusters(struct ocfs2_super *osb,
+				      struct ocfs2_cached_block_free *head)
+{
+	struct ocfs2_cached_block_free *tmp;
+	struct inode *tl_inode = osb->osb_tl_inode;
+	handle_t *handle;
+	int ret = 0;
+
+	mutex_lock(&tl_inode->i_mutex);
+
+	while (head) {
+		if (ocfs2_truncate_log_needs_flush(osb)) {
+			ret = __ocfs2_flush_truncate_log(osb);
+			if (ret < 0) {
+				mlog_errno(ret);
+				break;
+			}
+		}
+
+		handle = ocfs2_start_trans(osb, OCFS2_TRUNCATE_LOG_UPDATE);
+		if (IS_ERR(handle)) {
+			ret = PTR_ERR(handle);
+			mlog_errno(ret);
+			break;
+		}
+
+		ret = ocfs2_truncate_log_append(osb, handle, head->free_blk,
+						head->free_bit);
+
+		ocfs2_commit_trans(osb, handle);
+		tmp = head;
+		head = head->free_next;
+		kfree(tmp);
+
+		if (ret < 0) {
+			mlog_errno(ret);
+			break;
+		}
+	}
+
+	mutex_unlock(&tl_inode->i_mutex);
+
+	while (head) {
+		/* Premature exit may have left some dangling items. */
+		tmp = head;
+		head = head->free_next;
+		kfree(tmp);
+	}
+
+	return ret;
+}
+
 int ocfs2_run_deallocs(struct ocfs2_super *osb,
 		       struct ocfs2_cached_dealloc_ctxt *ctxt)
 {

@@ -5908,8 +5987,10 @@ int ocfs2_run_deallocs(struct ocfs2_super *osb,
 		if (fl->f_first) {
 			mlog(0, "Free items: (type %u, slot %d)\n",
 			     fl->f_inode_type, fl->f_slot);
-			ret2 = ocfs2_free_cached_items(osb, fl->f_inode_type,
-						       fl->f_slot, fl->f_first);
+			ret2 = ocfs2_free_cached_blocks(osb,
+							fl->f_inode_type,
+							fl->f_slot,
+							fl->f_first);
 			if (ret2)
 				mlog_errno(ret2);
 			if (!ret)

@@ -5920,6 +6001,15 @@ int ocfs2_run_deallocs(struct ocfs2_super *osb,
 		kfree(fl);
 	}
 
+	if (ctxt->c_global_allocator) {
+		ret2 = ocfs2_free_cached_clusters(osb,
+						  ctxt->c_global_allocator);
+		if (ret2)
+			mlog_errno(ret2);
+		if (!ret)
+			ret = ret2;
+	}
+
 	return ret;
 }

fs/ocfs2/alloc.h +4 -0

index 70257c8..c301cf2 100644
@@ -167,11 +167,15 @@ int __ocfs2_flush_truncate_log(struct ocfs2_super *osb);
  */
 struct ocfs2_cached_dealloc_ctxt {
 	struct ocfs2_per_slot_free_list		*c_first_suballocator;
+	struct ocfs2_cached_block_free 		*c_global_allocator;
 };
 static inline void ocfs2_init_dealloc_ctxt(struct ocfs2_cached_dealloc_ctxt *c)
 {
 	c->c_first_suballocator = NULL;
+	c->c_global_allocator = NULL;
 }
+int ocfs2_cache_cluster_dealloc(struct ocfs2_cached_dealloc_ctxt *ctxt,
+				u64 blkno, unsigned int bit);
 int ocfs2_run_deallocs(struct ocfs2_super *osb,
 		       struct ocfs2_cached_dealloc_ctxt *ctxt);

Signature · 1 line

1.5.5

Joel Becker

2008-11-05 22:25 UTC #11 lore raw

PATCH 2/15 ocfs2: Add clusters free in dealloc_ctxt.v2

Quoted 5 lines · Tao Ma

Modification from V1 to V2:
1. Some typo fix from Joel.
2. Adjust ocfs2_free_cached_clusters to handle the situation of truncate log full. Most of codes are copied from ocfs2_complete_truncate_log_recovery.

Looking good, one comment.

fs/ocfs2/alloc.c int ocfs2_run_deallocs(struct ocfs2_super *osb, · line 6010

Quoted 11 lines

fs/ocfs2/alloc.c +7 -0

@@ -5920,6 +6001,15 @@ int ocfs2_run_deallocs(struct ocfs2_super *osb,
 		kfree(fl);
 	}
 
+	if (ctxt->c_global_allocator) {
+		ret2 = ocfs2_free_cached_clusters(osb,
+						  ctxt->c_global_allocator);
+		if (ret2)
+			mlog_errno(ret2);
+		if (!ret)
+			ret = ret2;

	Be sure to set ctxt->c_global_allocator to NULL here;  The block

free list ends in a NULL, so c_first_suballocator ends up as NULL. This cluster list should be set NULL too after free_cached_clusters() returns.
Otherwise, this patch is good to go.

Joel

Signature · 9 lines

"The nice thing about egotists is that they don't talk about other
 people."
         - Lucille S. Harper

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

Tao Ma

2008-11-06 06:08 UTC #12 lore raw

PATCH 2/15 ocfs2: Add clusters free in dealloc_ctxt.v2

Quoted 22 lines · Joel Becker

Quoted 5 lines · Tao Ma
Modification from V1 to V2:
1. Some typo fix from Joel.
2. Adjust ocfs2_free_cached_clusters to handle the situation of truncate log full. Most of codes are copied from ocfs2_complete_truncate_log_recovery.
Looking good, one comment.
fs/ocfs2/alloc.c int ocfs2_run_deallocs(struct ocfs2_super *osb, · line 6010
Quoted 11 lines
fs/ocfs2/alloc.c +7 -0
@@ -5920,6 +6001,15 @@ int ocfs2_run_deallocs(struct ocfs2_super *osb,
 		kfree(fl);
 	}
 
+	if (ctxt->c_global_allocator) {
+		ret2 = ocfs2_free_cached_clusters(osb,
+						  ctxt->c_global_allocator);
+		if (ret2)
+			mlog_errno(ret2);
+		if (!ret)
+			ret = ret2;
	Be sure to set ctxt->c_global_allocator to NULL here;  The block

Quoted 4 lines

free list ends in a NULL, so c_first_suballocator ends up as NULL. This cluster list should be set NULL too after free_cached_clusters() returns.
Otherwise, this patch is good to go.

no problem. I will modify it in my own box. It will show up when Tiger sent his next round of acl. Thanks.

Regards,
Tao

Tao Ma

2008-10-30 05:42 UTC #13 lore raw

PATCH 3/15 ocfs2/xattr: Move clusters free into dealloc.

In reply to Tiger Yang

Move clusters free process into dealloc context so that they can be freed after the transaction.

Signed-off-by Tao Ma <[email protected]>

---
 fs/ocfs2/xattr.c |   14 +-------------
 1 files changed, 1 insertions(+), 13 deletions(-)

fs/ocfs2/xattr.c +1 ~~-13~~

index dc90ffb..7605677 100644
@@ -455,7 +455,6 @@ static int __ocfs2_remove_xattr_range(struct inode *inode,
 	int ret;
 	u64 phys_blkno = ocfs2_clusters_to_blocks(inode->i_sb, phys_cpos);
 	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
-	struct inode *tl_inode = osb->osb_tl_inode;
 	handle_t *handle;
 	struct ocfs2_alloc_context *meta_ac = NULL;
 	struct ocfs2_extent_tree et;

@@ -468,16 +467,6 @@ static int __ocfs2_remove_xattr_range(struct inode *inode,
 		return ret;
 	}
 
-	mutex_lock(&tl_inode->i_mutex);
-
-	if (ocfs2_truncate_log_needs_flush(osb)) {
-		ret = __ocfs2_flush_truncate_log(osb);
-		if (ret < 0) {
-			mlog_errno(ret);
-			goto out;
-		}
-	}
-
 	handle = ocfs2_start_trans(osb, OCFS2_REMOVE_EXTENT_CREDITS);
 	if (IS_ERR(handle)) {
 		ret = PTR_ERR(handle);

@@ -507,14 +496,13 @@ static int __ocfs2_remove_xattr_range(struct inode *inode,
 		goto out_commit;
 	}
 
-	ret = ocfs2_truncate_log_append(osb, handle, phys_blkno, len);
+	ret = ocfs2_cache_clusters_dealloc(dealloc, phys_blkno, len);
 	if (ret)
 		mlog_errno(ret);
 
 out_commit:
 	ocfs2_commit_trans(osb, handle);
 out:
-	mutex_unlock(&tl_inode->i_mutex);
 
 	if (meta_ac)
 		ocfs2_free_alloc_context(meta_ac);

Signature · 1 line

1.5.4.GIT

Joel Becker

2008-11-04 03:17 UTC #14 lore raw

PATCH 3/15 ocfs2/xattr: Move clusters free into dealloc.

Quoted 38 lines · Tao Ma

Move clusters free process into dealloc context so that they can be freed after the transaction.

Signed-off-by Tao Ma <[email protected]>

---
 fs/ocfs2/xattr.c |   14 +-------------
 1 files changed, 1 insertions(+), 13 deletions(-)

fs/ocfs2/xattr.c +0 ~~-11~~

index dc90ffb..7605677 100644
@@ -455,7 +455,6 @@ static int __ocfs2_remove_xattr_range(struct inode *inode,
 	int ret;
 	u64 phys_blkno = ocfs2_clusters_to_blocks(inode->i_sb, phys_cpos);
 	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
-	struct inode *tl_inode = osb->osb_tl_inode;
 	handle_t *handle;
 	struct ocfs2_alloc_context *meta_ac = NULL;
 	struct ocfs2_extent_tree et;

@@ -468,16 +467,6 @@ static int __ocfs2_remove_xattr_range(struct inode *inode,
 		return ret;
 	}
 
-	mutex_lock(&tl_inode->i_mutex);
-
-	if (ocfs2_truncate_log_needs_flush(osb)) {
-		ret = __ocfs2_flush_truncate_log(osb);
-		if (ret < 0) {
-			mlog_errno(ret);
-			goto out;
-		}
-	}
-
 	handle = ocfs2_start_trans(osb, OCFS2_REMOVE_EXTENT_CREDITS);
 	if (IS_ERR(handle)) {
 		ret = PTR_ERR(handle);

fs/ocfs2/xattr.c static int __ocfs2_remove_xattr_range(struct inode *inode, · line 499

Quoted 5 lines

fs/ocfs2/xattr.c +1 -1

@@ -507,14 +496,13 @@ static int __ocfs2_remove_xattr_range(struct inode *inode,
 		goto out_commit;
 	}
 
-	ret = ocfs2_truncate_log_append(osb, handle, phys_blkno, len);
+	ret = ocfs2_cache_clusters_dealloc(dealloc, phys_blkno, len);

[Referring to the discussion of the previous patch]
See, in the old code you called __ocfs2_remove_xattr_range()
once per extend, so the check for needs_flush() and the call to log_append() were just like ocfs2_commit_truncate(). In the new code, you move the handling into the dealloc context, but the actual mechanics of deleting the clusters inside ocfs2_run_deallocs() need to be correct.
This patch is fine, btw.

Joel

Signature · 10 lines

"All alone at the end of the evening
 When the bright lights have faded to blue.
 I was thinking about a woman who had loved me
 And I never knew"

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

Tao Ma

2008-10-30 05:42 UTC #15 lore raw

PATCH 4/15 ocfs2/xattr: Reserve meta/data at the beginning of ocfs2_xattr_set.

In reply to Tiger Yang

In ocfs2 xattr set, we reserve metadata and clusters in any place they are needed. It is time-consuming and ineffective, so this patch try to reserve metadata and clusters at the beginning of ocfs2_xattr_set.

Signed-off-by Tao Ma <[email protected]>

---
 fs/ocfs2/xattr.c |  486 ++++++++++++++++++++++++++++++++++++++++--------------
 1 files changed, 360 insertions(+), 126 deletions(-)

fs/ocfs2/xattr.c +360 ~~-126~~

index 7605677..f6881b4 100644
@@ -71,6 +71,12 @@ struct ocfs2_xattr_bucket {
 	int bu_blocks;
 };
 
+struct ocfs2_xattr_set_ctxt {
+	struct ocfs2_alloc_context *meta_ac;
+	struct ocfs2_alloc_context *data_ac;
+	struct ocfs2_cached_dealloc_ctxt dealloc;
+};
+
 #define OCFS2_XATTR_ROOT_SIZE	(sizeof(struct ocfs2_xattr_def_value_root))
 #define OCFS2_XATTR_INLINE_SIZE	80

@@ -133,11 +139,13 @@ static int ocfs2_xattr_tree_list_index_block(struct inode *inode,
 					size_t buffer_size);
 
 static int ocfs2_xattr_create_index_block(struct inode *inode,
-					  struct ocfs2_xattr_search *xs);
+					  struct ocfs2_xattr_search *xs,
+					  struct ocfs2_xattr_set_ctxt *ctxt);
 
 static int ocfs2_xattr_set_entry_index_block(struct inode *inode,
 					     struct ocfs2_xattr_info *xi,
-					     struct ocfs2_xattr_search *xs);
+					     struct ocfs2_xattr_search *xs,
+					     struct ocfs2_xattr_set_ctxt *ctxt);
 
 static int ocfs2_delete_xattr_index_block(struct inode *inode,
 					  struct buffer_head *xb_bh);

@@ -332,14 +340,13 @@ static void ocfs2_xattr_hash_entry(struct inode *inode,
 static int ocfs2_xattr_extend_allocation(struct inode *inode,
 					 u32 clusters_to_add,
 					 struct buffer_head *xattr_bh,
-					 struct ocfs2_xattr_value_root *xv)
+					 struct ocfs2_xattr_value_root *xv,
+					 struct ocfs2_xattr_set_ctxt *ctxt)
 {
 	int status = 0;
 	int restart_func = 0;
 	int credits = 0;
 	handle_t *handle = NULL;
-	struct ocfs2_alloc_context *data_ac = NULL;
-	struct ocfs2_alloc_context *meta_ac = NULL;
 	enum ocfs2_alloc_restarted why;
 	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
 	u32 prev_clusters, logical_start = le32_to_cpu(xv->xr_clusters);

@@ -351,13 +358,6 @@ static int ocfs2_xattr_extend_allocation(struct inode *inode,
 
 restart_all:
 
-	status = ocfs2_lock_allocators(inode, &et, clusters_to_add, 0,
-				       &data_ac, &meta_ac);
-	if (status) {
-		mlog_errno(status);
-		goto leave;
-	}
-
 	credits = ocfs2_calc_extend_credits(osb->sb, et.et_root_el,
 					    clusters_to_add);
 	handle = ocfs2_start_trans(osb, credits);

@@ -384,8 +384,8 @@ restarted_transaction:
 					     0,
 					     &et,
 					     handle,
-					     data_ac,
-					     meta_ac,
+					     ctxt->data_ac,
+					     ctxt->meta_ac,
 					     &why);
 	if ((status < 0) && (status != -EAGAIN)) {
 		if (status != -ENOSPC)

@@ -430,14 +430,6 @@ leave:
 		ocfs2_commit_trans(osb, handle);
 		handle = NULL;
 	}
-	if (data_ac) {
-		ocfs2_free_alloc_context(data_ac);
-		data_ac = NULL;
-	}
-	if (meta_ac) {
-		ocfs2_free_alloc_context(meta_ac);
-		meta_ac = NULL;
-	}
 	if ((!status) && restart_func) {
 		restart_func = 0;
 		goto restart_all;

@@ -450,23 +442,16 @@ static int __ocfs2_remove_xattr_range(struct inode *inode,
 				      struct buffer_head *root_bh,
 				      struct ocfs2_xattr_value_root *xv,
 				      u32 cpos, u32 phys_cpos, u32 len,
-				      struct ocfs2_cached_dealloc_ctxt *dealloc)
+				      struct ocfs2_xattr_set_ctxt *ctxt)
 {
 	int ret;
 	u64 phys_blkno = ocfs2_clusters_to_blocks(inode->i_sb, phys_cpos);
 	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
 	handle_t *handle;
-	struct ocfs2_alloc_context *meta_ac = NULL;
 	struct ocfs2_extent_tree et;
 
 	ocfs2_init_xattr_value_extent_tree(&et, inode, root_bh, xv);
 
-	ret = ocfs2_lock_allocators(inode, &et, 0, 1, NULL, &meta_ac);
-	if (ret) {
-		mlog_errno(ret);
-		return ret;
-	}
-
 	handle = ocfs2_start_trans(osb, OCFS2_REMOVE_EXTENT_CREDITS);
 	if (IS_ERR(handle)) {
 		ret = PTR_ERR(handle);

@@ -481,8 +466,8 @@ static int __ocfs2_remove_xattr_range(struct inode *inode,
 		goto out_commit;
 	}
 
-	ret = ocfs2_remove_extent(inode, &et, cpos, len, handle, meta_ac,
-				  dealloc);
+	ret = ocfs2_remove_extent(inode, &et, cpos, len, handle, ctxt->meta_ac,
+				  &ctxt->dealloc);
 	if (ret) {
 		mlog_errno(ret);
 		goto out_commit;

@@ -496,17 +481,13 @@ static int __ocfs2_remove_xattr_range(struct inode *inode,
 		goto out_commit;
 	}
 
-	ret = ocfs2_cache_clusters_dealloc(dealloc, phys_blkno, len);
+	ret = ocfs2_cache_clusters_dealloc(&ctxt->dealloc, phys_blkno, len);
 	if (ret)
 		mlog_errno(ret);
 
 out_commit:
 	ocfs2_commit_trans(osb, handle);
 out:
-
-	if (meta_ac)
-		ocfs2_free_alloc_context(meta_ac);
-
 	return ret;
 }

@@ -514,15 +495,12 @@ static int ocfs2_xattr_shrink_size(struct inode *inode,
 				   u32 old_clusters,
 				   u32 new_clusters,
 				   struct buffer_head *root_bh,
-				   struct ocfs2_xattr_value_root *xv)
+				   struct ocfs2_xattr_value_root *xv,
+				   struct ocfs2_xattr_set_ctxt *ctxt)
 {
 	int ret = 0;
 	u32 trunc_len, cpos, phys_cpos, alloc_size;
 	u64 block;
-	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
-	struct ocfs2_cached_dealloc_ctxt dealloc;
-
-	ocfs2_init_dealloc_ctxt(&dealloc);
 
 	if (old_clusters <= new_clusters)
 		return 0;

@@ -542,7 +520,7 @@ static int ocfs2_xattr_shrink_size(struct inode *inode,
 
 		ret = __ocfs2_remove_xattr_range(inode, root_bh, xv, cpos,
 						 phys_cpos, alloc_size,
-						 &dealloc);
+						 ctxt);
 		if (ret) {
 			mlog_errno(ret);
 			goto out;

@@ -556,16 +534,14 @@ static int ocfs2_xattr_shrink_size(struct inode *inode,
 	}
 
 out:
-	ocfs2_schedule_truncate_log_flush(osb, 1);
-	ocfs2_run_deallocs(osb, &dealloc);
-
 	return ret;
 }
 
 static int ocfs2_xattr_value_truncate(struct inode *inode,
 				      struct buffer_head *root_bh,
 				      struct ocfs2_xattr_value_root *xv,
-				      int len)
+				      int len,
+				      struct ocfs2_xattr_set_ctxt *ctxt)
 {
 	int ret;
 	u32 new_clusters = ocfs2_clusters_for_bytes(inode->i_sb, len);

@@ -577,11 +553,11 @@ static int ocfs2_xattr_value_truncate(struct inode *inode,
 	if (new_clusters > old_clusters)
 		ret = ocfs2_xattr_extend_allocation(inode,
 						    new_clusters - old_clusters,
-						    root_bh, xv);
+						    root_bh, xv, ctxt);
 	else
 		ret = ocfs2_xattr_shrink_size(inode,
 					      old_clusters, new_clusters,
-					      root_bh, xv);
+					      root_bh, xv, ctxt);
 
 	return ret;
 }

@@ -1160,6 +1136,7 @@ out:
 static int ocfs2_xattr_set_value_outside(struct inode *inode,
 					 struct ocfs2_xattr_info *xi,
 					 struct ocfs2_xattr_search *xs,
+					 struct ocfs2_xattr_set_ctxt *ctxt,
 					 size_t offs)
 {
 	size_t name_len = strlen(xi->name);

@@ -1179,7 +1156,7 @@ static int ocfs2_xattr_set_value_outside(struct inode *inode,
 	xv->xr_list.l_next_free_rec = 0;
 
 	ret = ocfs2_xattr_value_truncate(inode, xs->xattr_bh, xv,
-					 xi->value_len);
+					 xi->value_len, ctxt);
 	if (ret < 0) {
 		mlog_errno(ret);
 		return ret;

@@ -1310,6 +1287,7 @@ static void ocfs2_xattr_set_entry_local(struct inode *inode,
 static int ocfs2_xattr_set_entry(struct inode *inode,
 				 struct ocfs2_xattr_info *xi,
 				 struct ocfs2_xattr_search *xs,
+				 struct ocfs2_xattr_set_ctxt *ctxt,
 				 int flag)
 {
 	struct ocfs2_xattr_entry *last;

@@ -1380,7 +1358,7 @@ static int ocfs2_xattr_set_entry(struct inode *inode,
 		if (ocfs2_xattr_is_local(xs->here) && size == size_l) {
 			/* Replace existing local xattr with tree root */
 			ret = ocfs2_xattr_set_value_outside(inode, xi, xs,
-							    offs);
+							    ctxt, offs);
 			if (ret < 0)
 				mlog_errno(ret);
 			goto out;

@@ -1399,7 +1377,8 @@ static int ocfs2_xattr_set_entry(struct inode *inode,
 				ret = ocfs2_xattr_value_truncate(inode,
 								 xs->xattr_bh,
 								 xv,
-								 xi->value_len);
+								 xi->value_len,
+								 ctxt);
 				if (ret < 0) {
 					mlog_errno(ret);
 					goto out;

@@ -1429,7 +1408,8 @@ static int ocfs2_xattr_set_entry(struct inode *inode,
 				 ret = ocfs2_xattr_value_truncate(inode,
 								 xs->xattr_bh,
 								 xv,
-								 0);
+								 0,
+								 ctxt);
 				if (ret < 0)
 					mlog_errno(ret);
 			}

@@ -1524,7 +1504,7 @@ out_commit:
 		 * This is the second step for value size > INLINE_SIZE.
 		 */
 		size_t offs = le16_to_cpu(xs->here->xe_name_offset);
-		ret = ocfs2_xattr_set_value_outside(inode, xi, xs, offs);
+		ret = ocfs2_xattr_set_value_outside(inode, xi, xs, ctxt, offs);
 		if (ret < 0) {
 			int ret2;

@@ -1548,6 +1528,11 @@ static int ocfs2_remove_value_outside(struct inode*inode,
 				      struct ocfs2_xattr_header *header)
 {
 	int ret = 0, i;
+	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
+	struct ocfs2_xattr_set_ctxt ctxt;
+
+	memset(&ctxt, 0, sizeof(ctxt));
+	ocfs2_init_dealloc_ctxt(&ctxt.dealloc);
 
 	for (i = 0; i < le16_to_cpu(header->xh_count); i++) {
 		struct ocfs2_xattr_entry *entry = &header->xh_entries[i];

@@ -1560,14 +1545,17 @@ static int ocfs2_remove_value_outside(struct inode*inode,
 				le16_to_cpu(entry->xe_name_offset);
 			xv = (struct ocfs2_xattr_value_root *)
 				(val + OCFS2_XATTR_SIZE(entry->xe_name_len));
-			ret = ocfs2_xattr_value_truncate(inode, bh, xv, 0);
+			ret = ocfs2_xattr_value_truncate(inode, bh, xv,
+							 0, &ctxt);
 			if (ret < 0) {
 				mlog_errno(ret);
-				return ret;
+				break;
 			}
 		}
 	}
 
+	ocfs2_schedule_truncate_log_flush(osb, 1);
+	ocfs2_run_deallocs(osb, &ctxt.dealloc);
 	return ret;
 }

@@ -1829,7 +1817,8 @@ static int ocfs2_xattr_ibody_find(struct inode *inode,
  */
 static int ocfs2_xattr_ibody_set(struct inode *inode,
 				 struct ocfs2_xattr_info *xi,
-				 struct ocfs2_xattr_search *xs)
+				 struct ocfs2_xattr_search *xs,
+				 struct ocfs2_xattr_set_ctxt *ctxt)
 {
 	struct ocfs2_inode_info *oi = OCFS2_I(inode);
 	struct ocfs2_dinode *di = (struct ocfs2_dinode *)xs->inode_bh->b_data;

@@ -1846,7 +1835,7 @@ static int ocfs2_xattr_ibody_set(struct inode *inode,
 		}
 	}
 
-	ret = ocfs2_xattr_set_entry(inode, xi, xs,
+	ret = ocfs2_xattr_set_entry(inode, xi, xs, ctxt,
 				(OCFS2_INLINE_XATTR_FL | OCFS2_HAS_XATTR_FL));
 out:
 	up_write(&oi->ip_alloc_sem);

@@ -1919,12 +1908,12 @@ cleanup:
  */
 static int ocfs2_xattr_block_set(struct inode *inode,
 				 struct ocfs2_xattr_info *xi,
-				 struct ocfs2_xattr_search *xs)
+				 struct ocfs2_xattr_search *xs,
+				 struct ocfs2_xattr_set_ctxt *ctxt)
 {
 	struct buffer_head *new_bh = NULL;
 	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
 	struct ocfs2_dinode *di =  (struct ocfs2_dinode *)xs->inode_bh->b_data;
-	struct ocfs2_alloc_context *meta_ac = NULL;
 	handle_t *handle = NULL;
 	struct ocfs2_xattr_block *xblk = NULL;
 	u16 suballoc_bit_start;

@@ -1933,15 +1922,6 @@ static int ocfs2_xattr_block_set(struct inode *inode,
 	int ret;
 
 	if (!xs->xattr_bh) {
-		/*
-		 * Alloc one external block for extended attribute
-		 * outside of inode.
-		 */
-		ret = ocfs2_reserve_new_metadata_blocks(osb, 1, &meta_ac);
-		if (ret < 0) {
-			mlog_errno(ret);
-			goto out;
-		}
 		handle = ocfs2_start_trans(osb,
 					   OCFS2_XATTR_BLOCK_CREATE_CREDITS);
 		if (IS_ERR(handle)) {

@@ -1956,7 +1936,7 @@ static int ocfs2_xattr_block_set(struct inode *inode,
 			goto out_commit;
 		}
 
-		ret = ocfs2_claim_metadata(osb, handle, meta_ac, 1,
+		ret = ocfs2_claim_metadata(osb, handle, ctxt->meta_ac, 1,
 					   &suballoc_bit_start, &num_got,
 					   &first_blkno);
 		if (ret < 0) {

@@ -1989,7 +1969,6 @@ static int ocfs2_xattr_block_set(struct inode *inode,
 		xs->end = (void *)xblk + inode->i_sb->s_blocksize;
 		xs->here = xs->header->xh_entries;
 
-
 		ret = ocfs2_journal_dirty(handle, new_bh);
 		if (ret < 0) {
 			mlog_errno(ret);

@@ -2002,8 +1981,6 @@ static int ocfs2_xattr_block_set(struct inode *inode,
 out_commit:
 		ocfs2_commit_trans(osb, handle);
 out:
-		if (meta_ac)
-			ocfs2_free_alloc_context(meta_ac);
 		if (ret < 0)
 			return ret;
 	} else

@@ -2011,22 +1988,266 @@ out:
 
 	if (!(le16_to_cpu(xblk->xb_flags) & OCFS2_XATTR_INDEXED)) {
 		/* Set extended attribute into external block */
-		ret = ocfs2_xattr_set_entry(inode, xi, xs, OCFS2_HAS_XATTR_FL);
+		ret = ocfs2_xattr_set_entry(inode, xi, xs, ctxt,
+					    OCFS2_HAS_XATTR_FL);
 		if (!ret || ret != -ENOSPC)
 			goto end;
 
-		ret = ocfs2_xattr_create_index_block(inode, xs);
+		ret = ocfs2_xattr_create_index_block(inode, xs, ctxt);
 		if (ret)
 			goto end;
 	}
 
-	ret = ocfs2_xattr_set_entry_index_block(inode, xi, xs);
+	ret = ocfs2_xattr_set_entry_index_block(inode, xi, xs, ctxt);
 
 end:
 
 	return ret;
 }
 
+/* Check whether the new xattr can be inserted into the inode. */
+static int ocfs2_xattr_can_be_in_inode(struct inode *inode,
+				       struct ocfs2_xattr_info *xi,
+				       struct ocfs2_xattr_search *xs)
+{
+	u64 value_size;
+	struct ocfs2_xattr_entry *last;
+	int free, i;
+	size_t min_offs = xs->end - xs->base;
+
+	if (!xs->header)
+		return 0;
+
+	last = xs->header->xh_entries;
+
+	for (i = 0; i < le16_to_cpu(xs->header->xh_count); i++) {
+		size_t offs = le16_to_cpu(last->xe_name_offset);
+		if (offs < min_offs)
+			min_offs = offs;
+		last += 1;
+	}
+
+	free = min_offs - ((void *)last - xs->base) - sizeof(__u32);
+	if (free < 0)
+		return 0;
+
+	BUG_ON(!xs->not_found);
+
+	if (xi->value_len > OCFS2_XATTR_INLINE_SIZE)
+		value_size = OCFS2_XATTR_ROOT_SIZE;
+	else
+		value_size = OCFS2_XATTR_SIZE(xi->value_len);
+
+	if (free >= sizeof(struct ocfs2_xattr_entry) +
+		   OCFS2_XATTR_SIZE(strlen(xi->name)) + value_size)
+		return 1;
+
+	return 0;
+}
+
+static int ocfs2_calc_xattr_set_need(struct inode *inode,
+				     struct ocfs2_dinode *di,
+				     struct ocfs2_xattr_info *xi,
+				     struct ocfs2_xattr_search *xis,
+				     struct ocfs2_xattr_search *xbs,
+				     int *clusters_need,
+				     int *meta_need)
+{
+	int ret = 0, old_in_xb = 0;
+	int clusters_add = 0, meta_add = 0;
+	struct buffer_head *bh = NULL;
+	struct ocfs2_xattr_block *xb = NULL;
+	struct ocfs2_xattr_entry *xe = NULL;
+	struct ocfs2_xattr_value_root *xv = NULL;
+	char *base = NULL;
+	int name_offset, name_len = 0;
+	u32 new_clusters = ocfs2_clusters_for_bytes(inode->i_sb,
+						    xi->value_len);
+	u64 value_size;
+
+	/*
+	 * delete a xattr doesn't need metadata and cluster allocation.
+	 * so return.
+	 */
+	if (!xi->value)
+		goto out;
+
+	if (xis->not_found && xbs->not_found) {
+		if (xi->value_len > OCFS2_XATTR_INLINE_SIZE)
+			clusters_add += new_clusters;
+
+		goto meta_guess;
+	}
+
+	if (!xis->not_found) {
+		xe = xis->here;
+		name_offset = le16_to_cpu(xe->xe_name_offset);
+		name_len = OCFS2_XATTR_SIZE(xe->xe_name_len);
+		base = xis->base;
+	} else {
+		int i, block_off;
+		xb = (struct ocfs2_xattr_block *)xbs->xattr_bh->b_data;
+		xe = xbs->here;
+		name_offset = le16_to_cpu(xe->xe_name_offset);
+		name_len = OCFS2_XATTR_SIZE(xe->xe_name_len);
+		i = xbs->here - xbs->header->xh_entries;
+		old_in_xb = 1;
+
+		if (le16_to_cpu(xb->xb_flags) & OCFS2_XATTR_INDEXED) {
+			ret = ocfs2_xattr_bucket_get_name_value(inode,
+							bucket_xh(xbs->bucket),
+							i, &block_off,
+							&name_offset);
+			base = bucket_block(xbs->bucket, block_off);
+		} else
+			base = xbs->base;
+	}
+
+	/* do cluster allocation guess first. */
+	value_size = le64_to_cpu(xe->xe_value_size);
+
+	if (old_in_xb) {
+		/*
+		 * In xattr set, we always try to set the xe in inode first,
+		 * so if it can be inserted into inode successfully, the old
+		 * one will be removed from the xattr block, and this xattr
+		 * will be inserted into inode as a new xattr in inode.
+		 */
+		if (ocfs2_xattr_can_be_in_inode(inode, xi, xis)) {
+			clusters_add += new_clusters;
+			goto out;
+		}
+	}
+
+	if (xi->value_len > OCFS2_XATTR_INLINE_SIZE) {
+		/* the new values will be stored outside. */
+		u32 old_clusters = 0;
+
+		if (!ocfs2_xattr_is_local(xe)) {
+			old_clusters =	ocfs2_clusters_for_bytes(inode->i_sb,
+								 value_size);
+			xv = (struct ocfs2_xattr_value_root *)
+			     (base + name_offset + name_len);
+		} else
+			xv = &def_xv.xv;
+
+		if (old_clusters >= new_clusters)
+			goto out;
+		else {
+			meta_add += ocfs2_extend_meta_needed(&xv->xr_list);
+			clusters_add += new_clusters - old_clusters;
+			goto out;
+		}
+	} else {
+		/*
+		 * Now the new value will be stored inside. So if the new
+		 * value is smaller than the size of value root or the old
+		 * value, we don't need any allocation, otherwise we have
+		 * to guess metadata allocation.
+		 */
+		if ((ocfs2_xattr_is_local(xe) && value_size >= xi->value_len) ||
+		    (!ocfs2_xattr_is_local(xe) &&
+		     OCFS2_XATTR_ROOT_SIZE >= xi->value_len))
+			goto out;
+	}
+
+meta_guess:
+	/* calculate metadata allocation. */
+	if (di->i_xattr_loc) {
+		if (!xbs->xattr_bh) {
+			ret = ocfs2_read_block(inode,
+					       le64_to_cpu(di->i_xattr_loc),
+					       &bh);
+			if (ret) {
+				mlog_errno(ret);
+				goto out;
+			}
+
+			xb = (struct ocfs2_xattr_block *)bh->b_data;
+		} else
+			xb = (struct ocfs2_xattr_block *)xbs->xattr_bh->b_data;
+
+		if (le16_to_cpu(xb->xb_flags) & OCFS2_XATTR_INDEXED) {
+			struct ocfs2_extent_list *el =
+				 &xb->xb_attrs.xb_root.xt_list;
+			meta_add += ocfs2_extend_meta_needed(el);
+		}
+
+		/*
+		 * This cluster will be used either for new bucket or for
+		 * new xattr block.
+		 * If the cluster size is the same as the bucket size, one
+		 * more is needed since we may need to extend the bucket
+		 * also.
+		 */
+		clusters_add += 1;
+		if (OCFS2_XATTR_BUCKET_SIZE ==
+			OCFS2_SB(inode->i_sb)->s_clustersize)
+			clusters_add += 1;
+	} else
+		meta_add += 1;
+out:
+	if (clusters_need)
+		*clusters_need = clusters_add;
+	if (meta_need)
+		*meta_need = meta_add;
+	brelse(bh);
+	return ret;
+}
+
+static int ocfs2_init_xattr_set_ctxt(struct inode *inode,
+				     struct ocfs2_dinode *di,
+				     struct ocfs2_xattr_info *xi,
+				     struct ocfs2_xattr_search *xis,
+				     struct ocfs2_xattr_search *xbs,
+				     struct ocfs2_xattr_set_ctxt *ctxt)
+{
+	int clusters_add, meta_add, ret;
+	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
+
+	memset(ctxt, 0, sizeof(struct ocfs2_xattr_set_ctxt));
+
+	ocfs2_init_dealloc_ctxt(&ctxt->dealloc);
+
+	ret = ocfs2_calc_xattr_set_need(inode, di, xi, xis, xbs,
+					&clusters_add, &meta_add);
+	if (ret) {
+		mlog_errno(ret);
+		return ret;
+	}
+
+	mlog(0, "Set xattr %s, reserve meta blocks = %d, clusters = %d\n",
+	     xi->name, meta_add, clusters_add);
+
+	if (meta_add) {
+		ret = ocfs2_reserve_new_metadata_blocks(osb, meta_add,
+							&ctxt->meta_ac);
+		if (ret) {
+			mlog_errno(ret);
+			goto out;
+		}
+	}
+
+	if (clusters_add) {
+		ret = ocfs2_reserve_clusters(osb, clusters_add, &ctxt->data_ac);
+		if (ret)
+			mlog_errno(ret);
+	}
+out:
+	if (ret) {
+		if (ctxt->meta_ac) {
+			ocfs2_free_alloc_context(ctxt->meta_ac);
+			ctxt->meta_ac = NULL;
+		}
+
+		/*
+		 * We cannot have an error and a non null ctxt->data_ac.
+		 */
+	}
+
+	return ret;
+}
+
 /*
  * ocfs2_xattr_set()
  *

@@ -2044,6 +2265,8 @@ int ocfs2_xattr_set(struct inode *inode,
 	struct buffer_head *di_bh = NULL;
 	struct ocfs2_dinode *di;
 	int ret;
+	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
+	struct ocfs2_xattr_set_ctxt ctxt;
 
 	struct ocfs2_xattr_info xi = {
 		.name_index = name_index,

@@ -2060,6 +2283,8 @@ int ocfs2_xattr_set(struct inode *inode,
 		.not_found = -ENODATA,
 	};
 
+	memset(&ctxt, 0, sizeof(ctxt));
+
 	if (!ocfs2_supports_xattr(OCFS2_SB(inode->i_sb)))
 		return -EOPNOTSUPP;

@@ -2108,15 +2333,21 @@ int ocfs2_xattr_set(struct inode *inode,
 			goto cleanup;
 	}
 
+	ret = ocfs2_init_xattr_set_ctxt(inode, di, &xi, &xis, &xbs, &ctxt);
+	if (ret) {
+		mlog_errno(ret);
+		goto cleanup;
+	}
+
 	if (!value) {
 		/* Remove existing extended attribute */
 		if (!xis.not_found)
-			ret = ocfs2_xattr_ibody_set(inode, &xi, &xis);
+			ret = ocfs2_xattr_ibody_set(inode, &xi, &xis, &ctxt);
 		else if (!xbs.not_found)
-			ret = ocfs2_xattr_block_set(inode, &xi, &xbs);
+			ret = ocfs2_xattr_block_set(inode, &xi, &xbs, &ctxt);
 	} else {
 		/* We always try to set extended attribute into inode first*/
-		ret = ocfs2_xattr_ibody_set(inode, &xi, &xis);
+		ret = ocfs2_xattr_ibody_set(inode, &xi, &xis, &ctxt);
 		if (!ret && !xbs.not_found) {
 			/*
 			 * If succeed and that extended attribute existing in

@@ -2124,7 +2355,7 @@ int ocfs2_xattr_set(struct inode *inode,
 			 */
 			xi.value = NULL;
 			xi.value_len = 0;
-			ret = ocfs2_xattr_block_set(inode, &xi, &xbs);
+			ret = ocfs2_xattr_block_set(inode, &xi, &xbs, &ctxt);
 		} else if (ret == -ENOSPC) {
 			if (di->i_xattr_loc && !xbs.xattr_bh) {
 				ret = ocfs2_xattr_block_find(inode, name_index,

@@ -2136,9 +2367,9 @@ int ocfs2_xattr_set(struct inode *inode,
 			 * If no space in inode, we will set extended attribute
 			 * into external block.
 			 */
-			ret = ocfs2_xattr_block_set(inode, &xi, &xbs);
+			ret = ocfs2_xattr_block_set(inode, &xi, &xbs, &ctxt);
 			if (ret)
-				goto cleanup;
+				goto free;
 			if (!xis.not_found) {
 				/*
 				 * If succeed and that extended attribute

@@ -2146,10 +2377,18 @@ int ocfs2_xattr_set(struct inode *inode,
 				 */
 				xi.value = NULL;
 				xi.value_len = 0;
-				ret = ocfs2_xattr_ibody_set(inode, &xi, &xis);
+				ret = ocfs2_xattr_ibody_set(inode, &xi,
+							    &xis, &ctxt);
 			}
 		}
 	}
+free:
+	if (ctxt.data_ac)
+		ocfs2_free_alloc_context(ctxt.data_ac);
+	if (ctxt.meta_ac)
+		ocfs2_free_alloc_context(ctxt.meta_ac);
+	ocfs2_schedule_truncate_log_flush(osb, 1);
+	ocfs2_run_deallocs(osb, &ctxt.dealloc);
 cleanup:
 	up_write(&OCFS2_I(inode)->ip_xattr_sem);
 	ocfs2_inode_unlock(inode, 1);

@@ -2731,7 +2970,8 @@ static void ocfs2_xattr_update_xattr_search(struct inode *inode,
 }
 
 static int ocfs2_xattr_create_index_block(struct inode *inode,
-					  struct ocfs2_xattr_search *xs)
+					  struct ocfs2_xattr_search *xs,
+					  struct ocfs2_xattr_set_ctxt *ctxt)
 {
 	int ret, credits = OCFS2_SUBALLOC_ALLOC;
 	u32 bit_off, len;

@@ -2739,7 +2979,6 @@ static int ocfs2_xattr_create_index_block(struct inode *inode,
 	handle_t *handle;
 	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
 	struct ocfs2_inode_info *oi = OCFS2_I(inode);
-	struct ocfs2_alloc_context *data_ac;
 	struct buffer_head *xb_bh = xs->xattr_bh;
 	struct ocfs2_xattr_block *xb =
 			(struct ocfs2_xattr_block *)xb_bh->b_data;

@@ -2752,12 +2991,6 @@ static int ocfs2_xattr_create_index_block(struct inode *inode,
 	BUG_ON(xb_flags & OCFS2_XATTR_INDEXED);
 	BUG_ON(!xs->bucket);
 
-	ret = ocfs2_reserve_clusters(osb, 1, &data_ac);
-	if (ret) {
-		mlog_errno(ret);
-		goto out;
-	}
-
 	/*
 	 * XXX:
 	 * We can use this lock for now, and maybe move to a dedicated mutex

@@ -2784,7 +3017,8 @@ static int ocfs2_xattr_create_index_block(struct inode *inode,
 		goto out_commit;
 	}
 
-	ret = ocfs2_claim_clusters(osb, handle, data_ac, 1, &bit_off, &len);
+	ret = __ocfs2_claim_clusters(osb, handle, ctxt->data_ac,
+				     1, 1, &bit_off, &len);
 	if (ret) {
 		mlog_errno(ret);
 		goto out_commit;

@@ -2846,10 +3080,6 @@ out_commit:
 out_sem:
 	up_write(&oi->ip_alloc_sem);
 
-out:
-	if (data_ac)
-		ocfs2_free_alloc_context(data_ac);
-
 	return ret;
 }

@@ -3619,7 +3849,8 @@ static int ocfs2_add_new_xattr_cluster(struct inode *inode,
 				       u32 *num_clusters,
 				       u32 prev_cpos,
 				       u64 prev_blkno,
-				       int *extend)
+				       int *extend,
+				       struct ocfs2_xattr_set_ctxt *ctxt)
 {
 	int ret, credits;
 	u16 bpc = ocfs2_clusters_to_blocks(inode->i_sb, 1);

@@ -3627,8 +3858,6 @@ static int ocfs2_add_new_xattr_cluster(struct inode *inode,
 	u32 clusters_to_add = 1, bit_off, num_bits, v_start = 0;
 	u64 block;
 	handle_t *handle = NULL;
-	struct ocfs2_alloc_context *data_ac = NULL;
-	struct ocfs2_alloc_context *meta_ac = NULL;
 	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
 	struct ocfs2_extent_tree et;

@@ -3639,13 +3868,6 @@ static int ocfs2_add_new_xattr_cluster(struct inode *inode,
 
 	ocfs2_init_xattr_tree_extent_tree(&et, inode, root_bh);
 
-	ret = ocfs2_lock_allocators(inode, &et, clusters_to_add, 0,
-				    &data_ac, &meta_ac);
-	if (ret) {
-		mlog_errno(ret);
-		goto leave;
-	}
-
 	credits = ocfs2_calc_extend_credits(osb->sb, et.et_root_el,
 					    clusters_to_add);
 	handle = ocfs2_start_trans(osb, credits);

@@ -3663,7 +3885,7 @@ static int ocfs2_add_new_xattr_cluster(struct inode *inode,
 		goto leave;
 	}
 
-	ret = __ocfs2_claim_clusters(osb, handle, data_ac, 1,
+	ret = __ocfs2_claim_clusters(osb, handle, ctxt->data_ac, 1,
 				     clusters_to_add, &bit_off, &num_bits);
 	if (ret < 0) {
 		if (ret != -ENOSPC)

@@ -3724,7 +3946,7 @@ static int ocfs2_add_new_xattr_cluster(struct inode *inode,
 	mlog(0, "Insert %u clusters at block %llu for xattr@%u\n",
 	     num_bits, block, v_start);
 	ret = ocfs2_insert_extent(osb, handle, inode, &et, v_start, block,
-				  num_bits, 0, meta_ac);
+				  num_bits, 0, ctxt->meta_ac);
 	if (ret < 0) {
 		mlog_errno(ret);
 		goto leave;

@@ -3739,10 +3961,6 @@ static int ocfs2_add_new_xattr_cluster(struct inode *inode,
 leave:
 	if (handle)
 		ocfs2_commit_trans(osb, handle);
-	if (data_ac)
-		ocfs2_free_alloc_context(data_ac);
-	if (meta_ac)
-		ocfs2_free_alloc_context(meta_ac);
 
 	return ret;
 }

@@ -3826,7 +4044,8 @@ out:
  */
 static int ocfs2_add_new_xattr_bucket(struct inode *inode,
 				      struct buffer_head *xb_bh,
-				      struct buffer_head *header_bh)
+				      struct buffer_head *header_bh,
+				      struct ocfs2_xattr_set_ctxt *ctxt)
 {
 	struct ocfs2_xattr_header *first_xh = NULL;
 	struct buffer_head *first_bh = NULL;

@@ -3877,7 +4096,8 @@ static int ocfs2_add_new_xattr_bucket(struct inode *inode,
 						  &num_clusters,
 						  e_cpos,
 						  p_blkno,
-						  &extend);
+						  &extend,
+						  ctxt);
 		if (ret) {
 			mlog_errno(ret);
 			goto out;

@@ -4152,7 +4372,8 @@ out:
 static int ocfs2_xattr_bucket_value_truncate(struct inode *inode,
 					     struct buffer_head *header_bh,
 					     int xe_off,
-					     int len)
+					     int len,
+					     struct ocfs2_xattr_set_ctxt *ctxt)
 {
 	int ret, offset;
 	u64 value_blk;

@@ -4187,7 +4408,7 @@ static int ocfs2_xattr_bucket_value_truncate(struct inode *inode,
 
 	mlog(0, "truncate %u in xattr bucket %llu to %d bytes.\n",
 	     xe_off, (unsigned long long)header_bh->b_blocknr, len);
-	ret = ocfs2_xattr_value_truncate(inode, value_bh, xv, len);
+	ret = ocfs2_xattr_value_truncate(inode, value_bh, xv, len, ctxt);
 	if (ret) {
 		mlog_errno(ret);
 		goto out;

@@ -4205,8 +4426,9 @@ out:
 }
 
 static int ocfs2_xattr_bucket_value_truncate_xs(struct inode *inode,
-						struct ocfs2_xattr_search *xs,
-						int len)
+					struct ocfs2_xattr_search *xs,
+					int len,
+					struct ocfs2_xattr_set_ctxt *ctxt)
 {
 	int ret, offset;
 	struct ocfs2_xattr_entry *xe = xs->here;

@@ -4216,7 +4438,7 @@ static int ocfs2_xattr_bucket_value_truncate_xs(struct inode *inode,
 
 	offset = xe - xh->xh_entries;
 	ret = ocfs2_xattr_bucket_value_truncate(inode, xs->bucket->bu_bhs[0],
-						offset, len);
+						offset, len, ctxt);
 	if (ret)
 		mlog_errno(ret);

@@ -4380,7 +4602,8 @@ out_commit:
  */
 static int ocfs2_xattr_set_in_bucket(struct inode *inode,
 				     struct ocfs2_xattr_info *xi,
-				     struct ocfs2_xattr_search *xs)
+				     struct ocfs2_xattr_search *xs,
+				     struct ocfs2_xattr_set_ctxt *ctxt)
 {
 	int ret, local = 1;
 	size_t value_len;

@@ -4408,7 +4631,8 @@ static int ocfs2_xattr_set_in_bucket(struct inode *inode,
 			value_len = 0;
 
 		ret = ocfs2_xattr_bucket_value_truncate_xs(inode, xs,
-							   value_len);
+							   value_len,
+							   ctxt);
 		if (ret)
 			goto out;

@@ -4439,7 +4663,7 @@ static int ocfs2_xattr_set_in_bucket(struct inode *inode,
 
 	/* allocate the space now for the outside block storage. */
 	ret = ocfs2_xattr_bucket_value_truncate_xs(inode, xs,
-						   value_len);
+						   value_len, ctxt);
 	if (ret) {
 		mlog_errno(ret);

@@ -4490,7 +4714,8 @@ static int ocfs2_check_xattr_bucket_collision(struct inode *inode,
 
 static int ocfs2_xattr_set_entry_index_block(struct inode *inode,
 					     struct ocfs2_xattr_info *xi,
-					     struct ocfs2_xattr_search *xs)
+					     struct ocfs2_xattr_search *xs,
+					     struct ocfs2_xattr_set_ctxt *ctxt)
 {
 	struct ocfs2_xattr_header *xh;
 	struct ocfs2_xattr_entry *xe;

@@ -4608,7 +4833,8 @@ try_again:
 
 		ret = ocfs2_add_new_xattr_bucket(inode,
 						 xs->xattr_bh,
-						 xs->bucket->bu_bhs[0]);
+						 xs->bucket->bu_bhs[0],
+						 ctxt);
 		if (ret) {
 			mlog_errno(ret);
 			goto out;

@@ -4627,7 +4853,7 @@ try_again:
 	}
 
 xattr_set:
-	ret = ocfs2_xattr_set_in_bucket(inode, xi, xs);
+	ret = ocfs2_xattr_set_in_bucket(inode, xi, xs, ctxt);
 out:
 	mlog_exit(ret);
 	return ret;

@@ -4641,6 +4867,11 @@ static int ocfs2_delete_xattr_in_bucket(struct inode *inode,
 	struct ocfs2_xattr_header *xh = bucket_xh(bucket);
 	u16 i;
 	struct ocfs2_xattr_entry *xe;
+	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
+	struct ocfs2_xattr_set_ctxt ctxt;
+
+	memset(&ctxt, 0, sizeof(ctxt));
+	ocfs2_init_dealloc_ctxt(&ctxt.dealloc);
 
 	for (i = 0; i < le16_to_cpu(xh->xh_count); i++) {
 		xe = &xh->xh_entries[i];

@@ -4649,13 +4880,16 @@ static int ocfs2_delete_xattr_in_bucket(struct inode *inode,
 
 		ret = ocfs2_xattr_bucket_value_truncate(inode,
 							bucket->bu_bhs[0],
-							i, 0);
+							i, 0, &ctxt);
 		if (ret) {
 			mlog_errno(ret);
 			break;
 		}
 	}
 
+	ocfs2_schedule_truncate_log_flush(osb, 1);
+	ocfs2_run_deallocs(osb, &ctxt.dealloc);
+
 	return ret;
 }

Signature · 1 line

1.5.4.GIT

Mark Fasheh

2008-11-07 00:11 UTC #16 lore raw

PATCH 4/15 ocfs2/xattr: Reserve meta/data at the beginning of ocfs2_xattr_set.

fs/ocfs2/xattr.c static int ocfs2_remove_value_outside(struct inode*inode, · line 1535

Quoted 9 lines · Tao Ma

fs/ocfs2/xattr.c +5 -0

@@ -1548,6 +1528,11 @@ static int ocfs2_remove_value_outside(struct inode*inode,
 				      struct ocfs2_xattr_header *header)
 {
 	int ret = 0, i;
+	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
+	struct ocfs2_xattr_set_ctxt ctxt;
+
+	memset(&ctxt, 0, sizeof(ctxt));
+	ocfs2_init_dealloc_ctxt(&ctxt.dealloc);

You can lose the memset if you just do:

	struct ocfs2_xattr_set_ctxt ctxt = { NULL, };

fs/ocfs2/xattr.c out: · line 2204

Quoted 12 lines

fs/ocfs2/xattr.c +7 -0

 	for (i = 0; i < le16_to_cpu(header->xh_count); i++) {
 		struct ocfs2_xattr_entry *entry = &header->xh_entries[i];
 
 
+static int ocfs2_init_xattr_set_ctxt(struct inode *inode,
+				     struct ocfs2_dinode *di,
+				     struct ocfs2_xattr_info *xi,
+				     struct ocfs2_xattr_search *xis,
+				     struct ocfs2_xattr_search *xbs,
+				     struct ocfs2_xattr_set_ctxt *ctxt)
+{

This function looks suspiciously like ocfs2_lock_allocators()... That said, I think it's probably fine to leave it on it's own for now ;)

Quoted 33 lines

fs/ocfs2/xattr.c +26 -1

+	int clusters_add, meta_add, ret;
+	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
+
+	memset(ctxt, 0, sizeof(struct ocfs2_xattr_set_ctxt));
+
+	ocfs2_init_dealloc_ctxt(&ctxt->dealloc);
+
+	ret = ocfs2_calc_xattr_set_need(inode, di, xi, xis, xbs,
+					&clusters_add, &meta_add);
+	if (ret) {
+		mlog_errno(ret);
+		return ret;
+	}
+
+	mlog(0, "Set xattr %s, reserve meta blocks = %d, clusters = %d\n",
+	     xi->name, meta_add, clusters_add);
+
+	if (meta_add) {
+		ret = ocfs2_reserve_new_metadata_blocks(osb, meta_add,
+							&ctxt->meta_ac);
+		if (ret) {
+			mlog_errno(ret);
+			goto out;
+		}
+	}

@@ -2146,10 +2377,18 @@ int ocfs2_xattr_set(struct inode *inode,
 				 */
 				xi.value = NULL;
 				xi.value_len = 0;
-				ret = ocfs2_xattr_ibody_set(inode, &xi, &xis);
+				ret = ocfs2_xattr_ibody_set(inode, &xi,

fs/ocfs2/xattr.c int ocfs2_xattr_set(struct inode *inode, · line 2390

Quoted 10 lines

fs/ocfs2/xattr.c +7 -0

+							    &xis, &ctxt);
 			}
 		}
 	}
+free:
+	if (ctxt.data_ac)
+		ocfs2_free_alloc_context(ctxt.data_ac);
+	if (ctxt.meta_ac)
+		ocfs2_free_alloc_context(ctxt.meta_ac);
+	ocfs2_schedule_truncate_log_flush(osb, 1);

Since this is a set function, which only a small part of the time will be deleteing clusters, I think it would be better not to unconditionally start a truncate log flush. Maybe you can do that only in the case where we have clusters to free?

	--Mark

--
Mark Fasheh

Tao Ma

2008-11-07 01:10 UTC #17 lore raw

PATCH 4/15 ocfs2/xattr: Reserve meta/data at the beginning of ocfs2_xattr_set.

Quoted 15 lines · Mark Fasheh

fs/ocfs2/xattr.c static int ocfs2_remove_value_outside(struct inode*inode, · line 1535

Quoted 9 lines · Tao Ma

fs/ocfs2/xattr.c +5 -0

@@ -1548,6 +1528,11 @@ static int ocfs2_remove_value_outside(struct inode*inode,
 				      struct ocfs2_xattr_header *header)
 {
 	int ret = 0, i;
+	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
+	struct ocfs2_xattr_set_ctxt ctxt;
+
+	memset(&ctxt, 0, sizeof(ctxt));
+	ocfs2_init_dealloc_ctxt(&ctxt.dealloc);

You can lose the memset if you just do:

Quoted 1 line

	struct ocfs2_xattr_set_ctxt ctxt = { NULL, };

Sure. Thanks.

Quoted 6 lines

Quoted 4 lines

fs/ocfs2/xattr.c +0 -0

 	for (i = 0; i < le16_to_cpu(header->xh_count); i++) {
 		struct ocfs2_xattr_entry *entry = &header->xh_entries[i];

Quoted 10 lines

fs/ocfs2/xattr.c out: · line 2204
Quoted 7 lines
fs/ocfs2/xattr.c +7 -0
+static int ocfs2_init_xattr_set_ctxt(struct inode *inode,
+				     struct ocfs2_dinode *di,
+				     struct ocfs2_xattr_info *xi,
+				     struct ocfs2_xattr_search *xis,
+				     struct ocfs2_xattr_search *xbs,
+				     struct ocfs2_xattr_set_ctxt *ctxt)
+{
This function looks suspiciously like ocfs2_lock_allocators()... That said, I think it's probably fine to leave it on it's own for now ;)

yeah, actually I refer to that function. ;) So maybe we can intergrate them later.

Quoted 42 lines

fs/ocfs2/xattr.c +30 -1

+	int clusters_add, meta_add, ret;
+	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
+
+	memset(ctxt, 0, sizeof(struct ocfs2_xattr_set_ctxt));
+
+	ocfs2_init_dealloc_ctxt(&ctxt->dealloc);
+
+	ret = ocfs2_calc_xattr_set_need(inode, di, xi, xis, xbs,
+					&clusters_add, &meta_add);
+	if (ret) {
+		mlog_errno(ret);
+		return ret;
+	}
+
+	mlog(0, "Set xattr %s, reserve meta blocks = %d, clusters = %d\n",
+	     xi->name, meta_add, clusters_add);
+
+	if (meta_add) {
+		ret = ocfs2_reserve_new_metadata_blocks(osb, meta_add,
+							&ctxt->meta_ac);
+		if (ret) {
+			mlog_errno(ret);
+			goto out;
+		}
+	}

@@ -2146,10 +2377,18 @@ int ocfs2_xattr_set(struct inode *inode,
 				 */
 				xi.value = NULL;
 				xi.value_len = 0;
-				ret = ocfs2_xattr_ibody_set(inode, &xi, &xis);
+				ret = ocfs2_xattr_ibody_set(inode, &xi,
+							    &xis, &ctxt);
 			}
 		}
 	}
+free:
+	if (ctxt.data_ac)
+		ocfs2_free_alloc_context(ctxt.data_ac);

Quoted 8 lines

fs/ocfs2/xattr.c int ocfs2_xattr_set(struct inode *inode, · line 2390
Quoted 3 lines
fs/ocfs2/xattr.c +3 -0
+	if (ctxt.meta_ac)
+		ocfs2_free_alloc_context(ctxt.meta_ac);
+	ocfs2_schedule_truncate_log_flush(osb, 1);
Since this is a set function, which only a small part of the time will be deleteing clusters, I think it would be better not to unconditionally start a truncate log flush. Maybe you can do that only in the case where we have clusters to free?

No problem, it is easy for us to detect whether ctxt.dealloc has some clusters to free.

Regards,
Tao

Tao Ma

2008-10-30 05:42 UTC #18 lore raw

PATCH 5/15 ocfs2/xattr: Merge xattr set transaction.

In reply to Tiger Yang

In current ocfs2/xattr, the whole xattr set is divided into many steps are many transaction are used, this make the xattr set process isn't like a real transaction, so this patch try to merge all the transaction into one. Another benefit is that acl can use it easily now.

I don't merge the transaction of deleting xattr when we remove an inode. The reason is that if we have a large number of xattrs and every xattrs has large values(large enough for outside storage), the whole transaction will be very huge and it looks like jbd can't handle it(I meet with a jbd complain once). And the old inode removal is also divided into many steps, so I'd like to leave as it is.

Note:
In xattr set, I try to avoid ocfs2_extend_trans since if the credits aren't enough for the extension, it will commit all the dirty blocks and create a new transaction which may lead to inconsistency in metadata. All ocfs2_extend_trans remained are safe now.

Signed-off-by Tao Ma <[email protected]>

---
 fs/ocfs2/xattr.c |  673 ++++++++++++++++++++++++++----------------------------
 1 files changed, 325 insertions(+), 348 deletions(-)

fs/ocfs2/xattr.c +325 ~~-348~~

index f6881b4..0e380c4 100644
@@ -72,6 +72,7 @@ struct ocfs2_xattr_bucket {
 };
 
 struct ocfs2_xattr_set_ctxt {
+	handle_t *handle;
 	struct ocfs2_alloc_context *meta_ac;
 	struct ocfs2_alloc_context *data_ac;
 	struct ocfs2_cached_dealloc_ctxt dealloc;

@@ -344,9 +345,7 @@ static int ocfs2_xattr_extend_allocation(struct inode *inode,
 					 struct ocfs2_xattr_set_ctxt *ctxt)
 {
 	int status = 0;
-	int restart_func = 0;
-	int credits = 0;
-	handle_t *handle = NULL;
+	handle_t *handle = ctxt->handle;
 	enum ocfs2_alloc_restarted why;
 	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
 	u32 prev_clusters, logical_start = le32_to_cpu(xv->xr_clusters);

@@ -356,19 +355,6 @@ static int ocfs2_xattr_extend_allocation(struct inode *inode,
 
 	ocfs2_init_xattr_value_extent_tree(&et, inode, xattr_bh, xv);
 
-restart_all:
-
-	credits = ocfs2_calc_extend_credits(osb->sb, et.et_root_el,
-					    clusters_to_add);
-	handle = ocfs2_start_trans(osb, credits);
-	if (IS_ERR(handle)) {
-		status = PTR_ERR(handle);
-		handle = NULL;
-		mlog_errno(status);
-		goto leave;
-	}
-
-restarted_transaction:
 	status = ocfs2_journal_access(handle, inode, xattr_bh,
 				      OCFS2_JOURNAL_ACCESS_WRITE);
 	if (status < 0) {

@@ -387,9 +373,8 @@ restarted_transaction:
 					     ctxt->data_ac,
 					     ctxt->meta_ac,
 					     &why);
-	if ((status < 0) && (status != -EAGAIN)) {
-		if (status != -ENOSPC)
-			mlog_errno(status);
+	if (status < 0) {
+		mlog_errno(status);
 		goto leave;
 	}

@@ -401,39 +386,13 @@ restarted_transaction:
 
 	clusters_to_add -= le32_to_cpu(xv->xr_clusters) - prev_clusters;
 
-	if (why != RESTART_NONE && clusters_to_add) {
-		if (why == RESTART_META) {
-			mlog(0, "restarting function.\n");
-			restart_func = 1;
-		} else {
-			BUG_ON(why != RESTART_TRANS);
-
-			mlog(0, "restarting transaction.\n");
-			/* TODO: This can be more intelligent. */
-			credits = ocfs2_calc_extend_credits(osb->sb,
-							    et.et_root_el,
-							    clusters_to_add);
-			status = ocfs2_extend_trans(handle, credits);
-			if (status < 0) {
-				/* handle still has to be committed at
-				 * this point. */
-				status = -ENOMEM;
-				mlog_errno(status);
-				goto leave;
-			}
-			goto restarted_transaction;
-		}
-	}
+	/*
+	 * We should have already allocated enough space before the transaction,
+	 * so no need to restart.
+	 */
+	BUG_ON(why != RESTART_NONE || clusters_to_add);
 
 leave:
-	if (handle) {
-		ocfs2_commit_trans(osb, handle);
-		handle = NULL;
-	}
-	if ((!status) && restart_func) {
-		restart_func = 0;
-		goto restart_all;
-	}
 
 	return status;
 }

@@ -446,31 +405,23 @@ static int __ocfs2_remove_xattr_range(struct inode *inode,
 {
 	int ret;
 	u64 phys_blkno = ocfs2_clusters_to_blocks(inode->i_sb, phys_cpos);
-	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
-	handle_t *handle;
+	handle_t *handle = ctxt->handle;
 	struct ocfs2_extent_tree et;
 
 	ocfs2_init_xattr_value_extent_tree(&et, inode, root_bh, xv);
 
-	handle = ocfs2_start_trans(osb, OCFS2_REMOVE_EXTENT_CREDITS);
-	if (IS_ERR(handle)) {
-		ret = PTR_ERR(handle);
-		mlog_errno(ret);
-		goto out;
-	}
-
 	ret = ocfs2_journal_access(handle, inode, root_bh,
 				   OCFS2_JOURNAL_ACCESS_WRITE);
 	if (ret) {
 		mlog_errno(ret);
-		goto out_commit;
+		goto out;
 	}
 
 	ret = ocfs2_remove_extent(inode, &et, cpos, len, handle, ctxt->meta_ac,
 				  &ctxt->dealloc);
 	if (ret) {
 		mlog_errno(ret);
-		goto out_commit;
+		goto out;
 	}
 
 	le32_add_cpu(&xv->xr_clusters, -len);

@@ -478,15 +429,13 @@ static int __ocfs2_remove_xattr_range(struct inode *inode,
 	ret = ocfs2_journal_dirty(handle, root_bh);
 	if (ret) {
 		mlog_errno(ret);
-		goto out_commit;
+		goto out;
 	}
 
 	ret = ocfs2_cache_clusters_dealloc(&ctxt->dealloc, phys_blkno, len);
 	if (ret)
 		mlog_errno(ret);
 
-out_commit:
-	ocfs2_commit_trans(osb, handle);
 out:
 	return ret;
 }

@@ -968,6 +917,7 @@ static int ocfs2_xattr_get(struct inode *inode,
 }
 
 static int __ocfs2_xattr_set_value_outside(struct inode *inode,
+					   handle_t *handle,
 					   struct ocfs2_xattr_value_root *xv,
 					   const void *value,
 					   int value_len)

@@ -979,14 +929,17 @@ static int __ocfs2_xattr_set_value_outside(struct inode *inode,
 	u32 clusters = ocfs2_clusters_for_bytes(inode->i_sb, value_len);
 	u64 blkno;
 	struct buffer_head *bh = NULL;
-	handle_t *handle;
 
 	BUG_ON(clusters > le32_to_cpu(xv->xr_clusters));
 
+	/*
+	 * In __ocfs2_xattr_set_value_outside has already been dirtied,
+	 * so we don't need to worry about whether ocfs2_extend_trans
+	 * will create a new transactio for us or not.
+	 */
 	credits = clusters * bpc;
-	handle = ocfs2_start_trans(OCFS2_SB(inode->i_sb), credits);
-	if (IS_ERR(handle)) {
-		ret = PTR_ERR(handle);
+	ret = ocfs2_extend_trans(handle, credits);
+	if (ret) {
 		mlog_errno(ret);
 		goto out;
 	}

@@ -996,7 +949,7 @@ static int __ocfs2_xattr_set_value_outside(struct inode *inode,
 					       &num_clusters, &xv->xr_list);
 		if (ret) {
 			mlog_errno(ret);
-			goto out_commit;
+			goto out;
 		}
 
 		blkno = ocfs2_clusters_to_blocks(inode->i_sb, p_cluster);

@@ -1005,7 +958,7 @@ static int __ocfs2_xattr_set_value_outside(struct inode *inode,
 			ret = ocfs2_read_block(inode, blkno, &bh);
 			if (ret) {
 				mlog_errno(ret);
-				goto out_commit;
+				goto out;
 			}
 
 			ret = ocfs2_journal_access(handle,

@@ -1014,7 +967,7 @@ static int __ocfs2_xattr_set_value_outside(struct inode *inode,
 						   OCFS2_JOURNAL_ACCESS_WRITE);
 			if (ret < 0) {
 				mlog_errno(ret);
-				goto out_commit;
+				goto out;
 			}
 
 			cp_len = value_len > blocksize ? blocksize : value_len;

@@ -1028,7 +981,7 @@ static int __ocfs2_xattr_set_value_outside(struct inode *inode,
 			ret = ocfs2_journal_dirty(handle, bh);
 			if (ret < 0) {
 				mlog_errno(ret);
-				goto out_commit;
+				goto out;
 			}
 			brelse(bh);
 			bh = NULL;

@@ -1042,8 +995,6 @@ static int __ocfs2_xattr_set_value_outside(struct inode *inode,
 		}
 		cpos += num_clusters;
 	}
-out_commit:
-	ocfs2_commit_trans(OCFS2_SB(inode->i_sb), handle);
 out:
 	brelse(bh);

@@ -1051,28 +1002,21 @@ out:
 }
 
 static int ocfs2_xattr_cleanup(struct inode *inode,
+			       handle_t *handle,
 			       struct ocfs2_xattr_info *xi,
 			       struct ocfs2_xattr_search *xs,
 			       size_t offs)
 {
-	handle_t *handle = NULL;
 	int ret = 0;
 	size_t name_len = strlen(xi->name);
 	void *val = xs->base + offs;
 	size_t size = OCFS2_XATTR_SIZE(name_len) + OCFS2_XATTR_ROOT_SIZE;
 
-	handle = ocfs2_start_trans((OCFS2_SB(inode->i_sb)),
-				   OCFS2_XATTR_BLOCK_UPDATE_CREDITS);
-	if (IS_ERR(handle)) {
-		ret = PTR_ERR(handle);
-		mlog_errno(ret);
-		goto out;
-	}
 	ret = ocfs2_journal_access(handle, inode, xs->xattr_bh,
 				   OCFS2_JOURNAL_ACCESS_WRITE);
 	if (ret) {
 		mlog_errno(ret);
-		goto out_commit;
+		goto out;
 	}
 	/* Decrease xattr count */
 	le16_add_cpu(&xs->header->xh_count, -1);

@@ -1083,32 +1027,23 @@ static int ocfs2_xattr_cleanup(struct inode *inode,
 	ret = ocfs2_journal_dirty(handle, xs->xattr_bh);
 	if (ret < 0)
 		mlog_errno(ret);
-out_commit:
-	ocfs2_commit_trans(OCFS2_SB(inode->i_sb), handle);
 out:
 	return ret;
 }
 
 static int ocfs2_xattr_update_entry(struct inode *inode,
+				    handle_t *handle,
 				    struct ocfs2_xattr_info *xi,
 				    struct ocfs2_xattr_search *xs,
 				    size_t offs)
 {
-	handle_t *handle = NULL;
-	int ret = 0;
+	int ret;
 
-	handle = ocfs2_start_trans((OCFS2_SB(inode->i_sb)),
-				   OCFS2_XATTR_BLOCK_UPDATE_CREDITS);
-	if (IS_ERR(handle)) {
-		ret = PTR_ERR(handle);
-		mlog_errno(ret);
-		goto out;
-	}
 	ret = ocfs2_journal_access(handle, inode, xs->xattr_bh,
 				   OCFS2_JOURNAL_ACCESS_WRITE);
 	if (ret) {
 		mlog_errno(ret);
-		goto out_commit;
+		goto out;
 	}
 
 	xs->here->xe_name_offset = cpu_to_le16(offs);

@@ -1122,8 +1057,6 @@ static int ocfs2_xattr_update_entry(struct inode *inode,
 	ret = ocfs2_journal_dirty(handle, xs->xattr_bh);
 	if (ret < 0)
 		mlog_errno(ret);
-out_commit:
-	ocfs2_commit_trans(OCFS2_SB(inode->i_sb), handle);
 out:
 	return ret;
 }

@@ -1161,13 +1094,13 @@ static int ocfs2_xattr_set_value_outside(struct inode *inode,
 		mlog_errno(ret);
 		return ret;
 	}
-	ret = __ocfs2_xattr_set_value_outside(inode, xv, xi->value,
-					      xi->value_len);
+	ret = ocfs2_xattr_update_entry(inode, ctxt->handle, xi, xs, offs);
 	if (ret < 0) {
 		mlog_errno(ret);
 		return ret;
 	}
-	ret = ocfs2_xattr_update_entry(inode, xi, xs, offs);
+	ret = __ocfs2_xattr_set_value_outside(inode, ctxt->handle, xv,
+					      xi->value, xi->value_len);
 	if (ret < 0)
 		mlog_errno(ret);

@@ -1295,7 +1228,7 @@ static int ocfs2_xattr_set_entry(struct inode *inode,
 	struct ocfs2_dinode *di = (struct ocfs2_dinode *)xs->inode_bh->b_data;
 	size_t min_offs = xs->end - xs->base, name_len = strlen(xi->name);
 	size_t size_l = 0;
-	handle_t *handle = NULL;
+	handle_t *handle = ctxt->handle;
 	int free, i, ret;
 	struct ocfs2_xattr_info xi_l = {
 		.name_index = xi->name_index,

@@ -1384,19 +1317,21 @@ static int ocfs2_xattr_set_entry(struct inode *inode,
 					goto out;
 				}
 
-				ret = __ocfs2_xattr_set_value_outside(inode,
-								xv,
-								xi->value,
-								xi->value_len);
+				ret = ocfs2_xattr_update_entry(inode,
+							       handle,
+							       xi,
+							       xs,
+							       offs);
 				if (ret < 0) {
 					mlog_errno(ret);
 					goto out;
 				}
 
-				ret = ocfs2_xattr_update_entry(inode,
-							       xi,
-							       xs,
-							       offs);
+				ret = __ocfs2_xattr_set_value_outside(inode,
+								handle,
+								xv,
+								xi->value,
+								xi->value_len);
 				if (ret < 0)
 					mlog_errno(ret);
 				goto out;

@@ -1406,45 +1341,29 @@ static int ocfs2_xattr_set_entry(struct inode *inode,
 				 * just trucate old value to zero.
 				 */
 				 ret = ocfs2_xattr_value_truncate(inode,
-								 xs->xattr_bh,
-								 xv,
-								 0,
-								 ctxt);
+								  xs->xattr_bh,
+								  xv,
+								  0,
+								  ctxt);
 				if (ret < 0)
 					mlog_errno(ret);
 			}
 		}
 	}
 
-	handle = ocfs2_start_trans((OCFS2_SB(inode->i_sb)),
-				   OCFS2_INODE_UPDATE_CREDITS);
-	if (IS_ERR(handle)) {
-		ret = PTR_ERR(handle);
-		mlog_errno(ret);
-		goto out;
-	}
-
 	ret = ocfs2_journal_access(handle, inode, xs->inode_bh,
 				   OCFS2_JOURNAL_ACCESS_WRITE);
 	if (ret) {
 		mlog_errno(ret);
-		goto out_commit;
+		goto out;
 	}
 
 	if (!(flag & OCFS2_INLINE_XATTR_FL)) {
-		/* set extended attribute in external block. */
-		ret = ocfs2_extend_trans(handle,
-					 OCFS2_INODE_UPDATE_CREDITS +
-					 OCFS2_XATTR_BLOCK_UPDATE_CREDITS);
-		if (ret) {
-			mlog_errno(ret);
-			goto out_commit;
-		}
 		ret = ocfs2_journal_access(handle, inode, xs->xattr_bh,
 					   OCFS2_JOURNAL_ACCESS_WRITE);
 		if (ret) {
 			mlog_errno(ret);
-			goto out_commit;
+			goto out;
 		}
 	}

@@ -1458,7 +1377,7 @@ static int ocfs2_xattr_set_entry(struct inode *inode,
 		ret = ocfs2_journal_dirty(handle, xs->xattr_bh);
 		if (ret < 0) {
 			mlog_errno(ret);
-			goto out_commit;
+			goto out;
 		}
 	}

@@ -1495,9 +1414,6 @@ static int ocfs2_xattr_set_entry(struct inode *inode,
 	if (ret < 0)
 		mlog_errno(ret);
 
-out_commit:
-	ocfs2_commit_trans(OCFS2_SB(inode->i_sb), handle);
-
 	if (!ret && xi->value_len > OCFS2_XATTR_INLINE_SIZE) {
 		/*
 		 * Set value outside in B tree.

@@ -1513,14 +1429,14 @@ out_commit:
 			 * If set value outside failed, we have to clean
 			 * the junk tree root we have already set in local.
 			 */
-			ret2 = ocfs2_xattr_cleanup(inode, xi, xs, offs);
+			ret2 = ocfs2_xattr_cleanup(inode, ctxt->handle,
+						   xi, xs, offs);
 			if (ret2 < 0)
 				mlog_errno(ret2);
 		}
 	}
 out:
 	return ret;
-
 }
 
 static int ocfs2_remove_value_outside(struct inode*inode,

@@ -1534,6 +1450,13 @@ static int ocfs2_remove_value_outside(struct inode*inode,
 	memset(&ctxt, 0, sizeof(ctxt));
 	ocfs2_init_dealloc_ctxt(&ctxt.dealloc);
 
+	ctxt.handle = ocfs2_start_trans(osb, OCFS2_REMOVE_EXTENT_CREDITS);
+	if (IS_ERR(ctxt.handle)) {
+		ret = PTR_ERR(ctxt.handle);
+		mlog_errno(ret);
+		goto out;
+	}
+
 	for (i = 0; i < le16_to_cpu(header->xh_count); i++) {
 		struct ocfs2_xattr_entry *entry = &header->xh_entries[i];

@@ -1554,8 +1477,10 @@ static int ocfs2_remove_value_outside(struct inode*inode,
 		}
 	}
 
+	ocfs2_commit_trans(osb, ctxt.handle);
 	ocfs2_schedule_truncate_log_flush(osb, 1);
 	ocfs2_run_deallocs(osb, &ctxt.dealloc);
+out:
 	return ret;
 }

@@ -1914,7 +1839,7 @@ static int ocfs2_xattr_block_set(struct inode *inode,
 	struct buffer_head *new_bh = NULL;
 	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
 	struct ocfs2_dinode *di =  (struct ocfs2_dinode *)xs->inode_bh->b_data;
-	handle_t *handle = NULL;
+	handle_t *handle = ctxt->handle;
 	struct ocfs2_xattr_block *xblk = NULL;
 	u16 suballoc_bit_start;
 	u32 num_got;

@@ -1922,18 +1847,11 @@ static int ocfs2_xattr_block_set(struct inode *inode,
 	int ret;
 
 	if (!xs->xattr_bh) {
-		handle = ocfs2_start_trans(osb,
-					   OCFS2_XATTR_BLOCK_CREATE_CREDITS);
-		if (IS_ERR(handle)) {
-			ret = PTR_ERR(handle);
-			mlog_errno(ret);
-			goto out;
-		}
 		ret = ocfs2_journal_access(handle, inode, xs->inode_bh,
 					   OCFS2_JOURNAL_ACCESS_CREATE);
 		if (ret < 0) {
 			mlog_errno(ret);
-			goto out_commit;
+			goto end;
 		}
 
 		ret = ocfs2_claim_metadata(osb, handle, ctxt->meta_ac, 1,

@@ -1941,7 +1859,7 @@ static int ocfs2_xattr_block_set(struct inode *inode,
 					   &first_blkno);
 		if (ret < 0) {
 			mlog_errno(ret);
-			goto out_commit;
+			goto end;
 		}
 
 		new_bh = sb_getblk(inode->i_sb, first_blkno);

@@ -1951,7 +1869,7 @@ static int ocfs2_xattr_block_set(struct inode *inode,
 					   OCFS2_JOURNAL_ACCESS_CREATE);
 		if (ret < 0) {
 			mlog_errno(ret);
-			goto out_commit;
+			goto end;
 		}
 
 		/* Initialize ocfs2_xattr_block */

@@ -1972,17 +1890,10 @@ static int ocfs2_xattr_block_set(struct inode *inode,
 		ret = ocfs2_journal_dirty(handle, new_bh);
 		if (ret < 0) {
 			mlog_errno(ret);
-			goto out_commit;
+			goto end;
 		}
 		di->i_xattr_loc = cpu_to_le64(first_blkno);
-		ret = ocfs2_journal_dirty(handle, xs->inode_bh);
-		if (ret < 0)
-			mlog_errno(ret);
-out_commit:
-		ocfs2_commit_trans(osb, handle);
-out:
-		if (ret < 0)
-			return ret;
+		ocfs2_journal_dirty(handle, xs->inode_bh);
 	} else
 		xblk = (struct ocfs2_xattr_block *)xs->xattr_bh->b_data;

@@ -2051,10 +1962,11 @@ static int ocfs2_calc_xattr_set_need(struct inode *inode,
 				     struct ocfs2_xattr_search *xis,
 				     struct ocfs2_xattr_search *xbs,
 				     int *clusters_need,
-				     int *meta_need)
+				     int *meta_need,
+				     int *credits_need)
 {
 	int ret = 0, old_in_xb = 0;
-	int clusters_add = 0, meta_add = 0;
+	int clusters_add = 0, meta_add = 0, credits = 0;
 	struct buffer_head *bh = NULL;
 	struct ocfs2_xattr_block *xb = NULL;
 	struct ocfs2_xattr_entry *xe = NULL;

@@ -2065,16 +1977,15 @@ static int ocfs2_calc_xattr_set_need(struct inode *inode,
 						    xi->value_len);
 	u64 value_size;
 
-	/*
-	 * delete a xattr doesn't need metadata and cluster allocation.
-	 * so return.
-	 */
-	if (!xi->value)
-		goto out;
-
 	if (xis->not_found && xbs->not_found) {
-		if (xi->value_len > OCFS2_XATTR_INLINE_SIZE)
+		credits += ocfs2_blocks_per_xattr_bucket(inode->i_sb);
+
+		if (xi->value_len > OCFS2_XATTR_INLINE_SIZE) {
 			clusters_add += new_clusters;
+			credits += ocfs2_calc_extend_credits(inode->i_sb,
+							&def_xv.xv.xr_list,
+							new_clusters);
+		}
 
 		goto meta_guess;
 	}

@@ -2084,6 +1995,7 @@ static int ocfs2_calc_xattr_set_need(struct inode *inode,
 		name_offset = le16_to_cpu(xe->xe_name_offset);
 		name_len = OCFS2_XATTR_SIZE(xe->xe_name_len);
 		base = xis->base;
+		credits += OCFS2_INODE_UPDATE_CREDITS;
 	} else {
 		int i, block_off;
 		xb = (struct ocfs2_xattr_block *)xbs->xattr_bh->b_data;

@@ -2099,8 +2011,25 @@ static int ocfs2_calc_xattr_set_need(struct inode *inode,
 							i, &block_off,
 							&name_offset);
 			base = bucket_block(xbs->bucket, block_off);
-		} else
+			credits += ocfs2_blocks_per_xattr_bucket(inode->i_sb);
+		} else {
 			base = xbs->base;
+			credits += OCFS2_XATTR_BLOCK_UPDATE_CREDITS;
+		}
+	}
+
+	/*
+	 * delete a xattr doesn't need metadata and cluster allocation.
+	 * so just calculate the credits and return.
+	 *
+	 * The credits for removing the value tree will be extended
+	 * by ocfs2_remove_extent itself.
+	 */
+	if (!xi->value) {
+		if (!ocfs2_xattr_is_local(xe))
+			credits += OCFS2_REMOVE_EXTENT_CREDITS;
+
+		goto out;
 	}
 
 	/* do cluster allocation guess first. */

@@ -2115,6 +2044,13 @@ static int ocfs2_calc_xattr_set_need(struct inode *inode,
 		 */
 		if (ocfs2_xattr_can_be_in_inode(inode, xi, xis)) {
 			clusters_add += new_clusters;
+			credits += OCFS2_REMOVE_EXTENT_CREDITS +
+				    OCFS2_INODE_UPDATE_CREDITS;
+			if (!ocfs2_xattr_is_local(xe))
+				credits += ocfs2_calc_extend_credits(
+							inode->i_sb,
+							&def_xv.xv.xr_list,
+							new_clusters);
 			goto out;
 		}
 	}

@@ -2131,11 +2067,16 @@ static int ocfs2_calc_xattr_set_need(struct inode *inode,
 		} else
 			xv = &def_xv.xv;
 
-		if (old_clusters >= new_clusters)
+		if (old_clusters >= new_clusters) {
+			credits += OCFS2_REMOVE_EXTENT_CREDITS;
 			goto out;
-		else {
+		} else {
 			meta_add += ocfs2_extend_meta_needed(&xv->xr_list);
 			clusters_add += new_clusters - old_clusters;
+			credits += ocfs2_calc_extend_credits(inode->i_sb,
+							     &xv->xr_list,
+							     new_clusters -
+							     old_clusters);
 			goto out;
 		}
 	} else {

@@ -2171,6 +2112,8 @@ meta_guess:
 			struct ocfs2_extent_list *el =
 				 &xb->xb_attrs.xb_root.xt_list;
 			meta_add += ocfs2_extend_meta_needed(el);
+			credits += ocfs2_calc_extend_credits(inode->i_sb,
+							     el, 1);
 		}
 
 		/*

@@ -2181,16 +2124,23 @@ meta_guess:
 		 * also.
 		 */
 		clusters_add += 1;
+		credits += ocfs2_blocks_per_xattr_bucket(inode->i_sb);
 		if (OCFS2_XATTR_BUCKET_SIZE ==
-			OCFS2_SB(inode->i_sb)->s_clustersize)
+			OCFS2_SB(inode->i_sb)->s_clustersize) {
+			credits += ocfs2_blocks_per_xattr_bucket(inode->i_sb);
 			clusters_add += 1;
-	} else
+		}
+	} else {
 		meta_add += 1;
+		credits += OCFS2_XATTR_BLOCK_CREATE_CREDITS;
+	}
 out:
 	if (clusters_need)
 		*clusters_need = clusters_add;
 	if (meta_need)
 		*meta_need = meta_add;
+	if (credits_need)
+		*credits_need = credits;
 	brelse(bh);
 	return ret;
 }

@@ -2200,7 +2150,8 @@ static int ocfs2_init_xattr_set_ctxt(struct inode *inode,
 				     struct ocfs2_xattr_info *xi,
 				     struct ocfs2_xattr_search *xis,
 				     struct ocfs2_xattr_search *xbs,
-				     struct ocfs2_xattr_set_ctxt *ctxt)
+				     struct ocfs2_xattr_set_ctxt *ctxt,
+				     int *credits)
 {
 	int clusters_add, meta_add, ret;
 	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);

@@ -2210,14 +2161,14 @@ static int ocfs2_init_xattr_set_ctxt(struct inode *inode,
 	ocfs2_init_dealloc_ctxt(&ctxt->dealloc);
 
 	ret = ocfs2_calc_xattr_set_need(inode, di, xi, xis, xbs,
-					&clusters_add, &meta_add);
+					&clusters_add, &meta_add, credits);
 	if (ret) {
 		mlog_errno(ret);
 		return ret;
 	}
 
-	mlog(0, "Set xattr %s, reserve meta blocks = %d, clusters = %d\n",
-	     xi->name, meta_add, clusters_add);
+	mlog(0, "Set xattr %s, reserve meta blocks = %d, clusters = %d, "
+	     "credits = %d\n", xi->name, meta_add, clusters_add, *credits);
 
 	if (meta_add) {
 		ret = ocfs2_reserve_new_metadata_blocks(osb, meta_add,

@@ -2248,6 +2199,126 @@ out:
 	return ret;
 }
 
+static int __ocfs2_xattr_set_handle(struct inode *inode,
+				    struct ocfs2_dinode *di,
+				    struct ocfs2_xattr_info *xi,
+				    struct ocfs2_xattr_search *xis,
+				    struct ocfs2_xattr_search *xbs,
+				    struct ocfs2_xattr_set_ctxt *ctxt)
+{
+	int ret = 0, credits;
+
+	if (!xi->value) {
+		/* Remove existing extended attribute */
+		if (!xis->not_found)
+			ret = ocfs2_xattr_ibody_set(inode, xi, xis, ctxt);
+		else if (!xbs->not_found)
+			ret = ocfs2_xattr_block_set(inode, xi, xbs, ctxt);
+	} else {
+		/* We always try to set extended attribute into inode first*/
+		ret = ocfs2_xattr_ibody_set(inode, xi, xis, ctxt);
+		if (!ret && !xbs->not_found) {
+			/*
+			 * If succeed and that extended attribute existing in
+			 * external block, then we will remove it.
+			 */
+			xi->value = NULL;
+			xi->value_len = 0;
+
+			xis->not_found = -ENODATA;
+			ret = ocfs2_calc_xattr_set_need(inode,
+							di,
+							xi,
+							xis,
+							xbs,
+							NULL,
+							NULL,
+							&credits);
+			if (ret) {
+				mlog_errno(ret);
+				goto out;
+			}
+
+			ret = ocfs2_extend_trans(ctxt->handle, credits +
+					ctxt->handle->h_buffer_credits);
+			if (ret) {
+				mlog_errno(ret);
+				goto out;
+			}
+			ret = ocfs2_xattr_block_set(inode, xi, xbs, ctxt);
+		} else if (ret == -ENOSPC) {
+			if (di->i_xattr_loc && !xbs->xattr_bh) {
+				ret = ocfs2_xattr_block_find(inode,
+							     xi->name_index,
+							     xi->name, xbs);
+				if (ret)
+					goto out;
+
+				xis->not_found = -ENODATA;
+				ret = ocfs2_calc_xattr_set_need(inode,
+								di,
+								xi,
+								xis,
+								xbs,
+								NULL,
+								NULL,
+								&credits);
+				if (ret) {
+					mlog_errno(ret);
+					goto out;
+				}
+
+				ret = ocfs2_extend_trans(ctxt->handle, credits +
+					ctxt->handle->h_buffer_credits);
+				if (ret) {
+					mlog_errno(ret);
+					goto out;
+				}
+			}
+			/*
+			 * If no space in inode, we will set extended attribute
+			 * into external block.
+			 */
+			ret = ocfs2_xattr_block_set(inode, xi, xbs, ctxt);
+			if (ret)
+				goto out;
+			if (!xis->not_found) {
+				/*
+				 * If succeed and that extended attribute
+				 * existing in inode, we will remove it.
+				 */
+				xi->value = NULL;
+				xi->value_len = 0;
+				xbs->not_found = -ENODATA;
+				ret = ocfs2_calc_xattr_set_need(inode,
+								di,
+								xi,
+								xis,
+								xbs,
+								NULL,
+								NULL,
+								&credits);
+				if (ret) {
+					mlog_errno(ret);
+					goto out;
+				}
+
+				ret = ocfs2_extend_trans(ctxt->handle, credits +
+						ctxt->handle->h_buffer_credits);
+				if (ret) {
+					mlog_errno(ret);
+					goto out;
+				}
+				ret = ocfs2_xattr_ibody_set(inode, xi,
+							    xis, ctxt);
+			}
+		}
+	}
+
+out:
+	return ret;
+}
+
 /*
  * ocfs2_xattr_set()
  *

@@ -2264,9 +2335,10 @@ int ocfs2_xattr_set(struct inode *inode,
 {
 	struct buffer_head *di_bh = NULL;
 	struct ocfs2_dinode *di;
-	int ret;
+	int ret, credits;
 	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
 	struct ocfs2_xattr_set_ctxt ctxt;
+	struct inode *tl_inode = osb->osb_tl_inode;
 
 	struct ocfs2_xattr_info xi = {
 		.name_index = name_index,

@@ -2333,56 +2405,37 @@ int ocfs2_xattr_set(struct inode *inode,
 			goto cleanup;
 	}
 
-	ret = ocfs2_init_xattr_set_ctxt(inode, di, &xi, &xis, &xbs, &ctxt);
+
+	mutex_lock(&tl_inode->i_mutex);
+
+	if (ocfs2_truncate_log_needs_flush(osb)) {
+		ret = __ocfs2_flush_truncate_log(osb);
+		if (ret < 0) {
+			mutex_unlock(&tl_inode->i_mutex);
+			mlog_errno(ret);
+			goto cleanup;
+		}
+	}
+	mutex_unlock(&tl_inode->i_mutex);
+
+	ret = ocfs2_init_xattr_set_ctxt(inode, di, &xi, &xis,
+					&xbs, &ctxt, &credits);
 	if (ret) {
 		mlog_errno(ret);
 		goto cleanup;
 	}
 
-	if (!value) {
-		/* Remove existing extended attribute */
-		if (!xis.not_found)
-			ret = ocfs2_xattr_ibody_set(inode, &xi, &xis, &ctxt);
-		else if (!xbs.not_found)
-			ret = ocfs2_xattr_block_set(inode, &xi, &xbs, &ctxt);
-	} else {
-		/* We always try to set extended attribute into inode first*/
-		ret = ocfs2_xattr_ibody_set(inode, &xi, &xis, &ctxt);
-		if (!ret && !xbs.not_found) {
-			/*
-			 * If succeed and that extended attribute existing in
-			 * external block, then we will remove it.
-			 */
-			xi.value = NULL;
-			xi.value_len = 0;
-			ret = ocfs2_xattr_block_set(inode, &xi, &xbs, &ctxt);
-		} else if (ret == -ENOSPC) {
-			if (di->i_xattr_loc && !xbs.xattr_bh) {
-				ret = ocfs2_xattr_block_find(inode, name_index,
-							     name, &xbs);
-				if (ret)
-					goto cleanup;
-			}
-			/*
-			 * If no space in inode, we will set extended attribute
-			 * into external block.
-			 */
-			ret = ocfs2_xattr_block_set(inode, &xi, &xbs, &ctxt);
-			if (ret)
-				goto free;
-			if (!xis.not_found) {
-				/*
-				 * If succeed and that extended attribute
-				 * existing in inode, we will remove it.
-				 */
-				xi.value = NULL;
-				xi.value_len = 0;
-				ret = ocfs2_xattr_ibody_set(inode, &xi,
-							    &xis, &ctxt);
-			}
-		}
+	ctxt.handle = ocfs2_start_trans(osb, credits);
+	if (IS_ERR(ctxt.handle)) {
+		ret = PTR_ERR(ctxt.handle);
+		mlog_errno(ret);
+		goto cleanup;
 	}
-free:
+
+	ret = __ocfs2_xattr_set_handle(inode, di, &xi, &xis, &xbs, &ctxt);
+
+	ocfs2_commit_trans(osb, ctxt.handle);
+
 	if (ctxt.data_ac)
 		ocfs2_free_alloc_context(ctxt.data_ac);
 	if (ctxt.meta_ac)

@@ -2973,10 +3026,10 @@ static int ocfs2_xattr_create_index_block(struct inode *inode,
 					  struct ocfs2_xattr_search *xs,
 					  struct ocfs2_xattr_set_ctxt *ctxt)
 {
-	int ret, credits = OCFS2_SUBALLOC_ALLOC;
+	int ret;
 	u32 bit_off, len;
 	u64 blkno;
-	handle_t *handle;
+	handle_t *handle = ctxt->handle;
 	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
 	struct ocfs2_inode_info *oi = OCFS2_I(inode);
 	struct buffer_head *xb_bh = xs->xattr_bh;

@@ -2998,30 +3051,18 @@ static int ocfs2_xattr_create_index_block(struct inode *inode,
 	 */
 	down_write(&oi->ip_alloc_sem);
 
-	/*
-	 * We need more credits.  One for the xattr block update and one
-	 * for each block of the new xattr bucket.
-	 */
-	credits += 1 + ocfs2_blocks_per_xattr_bucket(inode->i_sb);
-	handle = ocfs2_start_trans(osb, credits);
-	if (IS_ERR(handle)) {
-		ret = PTR_ERR(handle);
-		mlog_errno(ret);
-		goto out_sem;
-	}
-
 	ret = ocfs2_journal_access(handle, inode, xb_bh,
 				   OCFS2_JOURNAL_ACCESS_WRITE);
 	if (ret) {
 		mlog_errno(ret);
-		goto out_commit;
+		goto out;
 	}
 
 	ret = __ocfs2_claim_clusters(osb, handle, ctxt->data_ac,
 				     1, 1, &bit_off, &len);
 	if (ret) {
 		mlog_errno(ret);
-		goto out_commit;
+		goto out;
 	}
 
 	/*

@@ -3036,14 +3077,14 @@ static int ocfs2_xattr_create_index_block(struct inode *inode,
 	ret = ocfs2_init_xattr_bucket(xs->bucket, blkno);
 	if (ret) {
 		mlog_errno(ret);
-		goto out_commit;
+		goto out;
 	}
 
 	ret = ocfs2_xattr_bucket_journal_access(handle, xs->bucket,
 						OCFS2_JOURNAL_ACCESS_CREATE);
 	if (ret) {
 		mlog_errno(ret);
-		goto out_commit;
+		goto out;
 	}
 
 	ocfs2_cp_xattr_block_to_bucket(inode, xb_bh, xs->bucket);

@@ -3068,16 +3109,9 @@ static int ocfs2_xattr_create_index_block(struct inode *inode,
 
 	xb->xb_flags = cpu_to_le16(xb_flags | OCFS2_XATTR_INDEXED);
 
-	ret = ocfs2_journal_dirty(handle, xb_bh);
-	if (ret) {
-		mlog_errno(ret);
-		goto out_commit;
-	}
+	ocfs2_journal_dirty(handle, xb_bh);
 
-out_commit:
-	ocfs2_commit_trans(osb, handle);
-
-out_sem:
+out:
 	up_write(&oi->ip_alloc_sem);
 
 	return ret;

@@ -3103,6 +3137,7 @@ static int cmp_xe_offset(const void *a, const void *b)
  * so that we can spare some space for insertion.
  */
 static int ocfs2_defrag_xattr_bucket(struct inode *inode,
+				     handle_t *handle,
 				     struct ocfs2_xattr_bucket *bucket)
 {
 	int ret, i;

@@ -3112,7 +3147,6 @@ static int ocfs2_defrag_xattr_bucket(struct inode *inode,
 	u64 blkno = bucket_blkno(bucket);
 	u16 xh_free_start;
 	size_t blocksize = inode->i_sb->s_blocksize;
-	handle_t *handle;
 	struct ocfs2_xattr_entry *xe;
 	struct ocfs2_xattr_bucket *wb = NULL;

@@ -3142,19 +3176,11 @@ static int ocfs2_defrag_xattr_bucket(struct inode *inode,
 	for (i = 0; i < wb->bu_blocks; i++, buf += blocksize)
 		memcpy(buf, bucket_block(wb, i), blocksize);
 
-	handle = ocfs2_start_trans((OCFS2_SB(inode->i_sb)), wb->bu_blocks);
-	if (IS_ERR(handle)) {
-		ret = PTR_ERR(handle);
-		handle = NULL;
-		mlog_errno(ret);
-		goto out;
-	}
-
 	ret = ocfs2_xattr_bucket_journal_access(handle, wb,
 						OCFS2_JOURNAL_ACCESS_WRITE);
 	if (ret < 0) {
 		mlog_errno(ret);
-		goto commit;
+		goto out;
 	}
 
 	xh = (struct ocfs2_xattr_header *)bucket_buf;

@@ -3212,7 +3238,7 @@ static int ocfs2_defrag_xattr_bucket(struct inode *inode,
 			"bucket %llu\n", (unsigned long long)blkno);
 
 	if (xh_free_start == end)
-		goto commit;
+		goto out;
 
 	memset(bucket_buf + xh_free_start, 0, end - xh_free_start);
 	xh->xh_free_start = cpu_to_le16(end);

@@ -3227,8 +3253,6 @@ static int ocfs2_defrag_xattr_bucket(struct inode *inode,
 		memcpy(bucket_block(wb, i), buf, blocksize);
 	ocfs2_xattr_bucket_journal_dirty(handle, wb);
 
-commit:
-	ocfs2_commit_trans(OCFS2_SB(inode->i_sb), handle);
 out:
 	ocfs2_xattr_bucket_free(wb);
 	kfree(bucket_buf);

@@ -3280,7 +3304,7 @@ static int ocfs2_mv_xattr_bucket_cross_cluster(struct inode *inode,
 	 * 1 more for the update of the 1st bucket of the previous
 	 * extent record.
 	 */
-	credits = bpc / 2 + 1;
+	credits = bpc / 2 + 1 + handle->h_buffer_credits;
 	ret = ocfs2_extend_trans(handle, credits);
 	if (ret) {
 		mlog_errno(ret);

@@ -3670,7 +3694,7 @@ static int ocfs2_cp_xattr_cluster(struct inode *inode,
 	 * We need to update the new cluster and 1 more for the update of
 	 * the 1st bucket of the previous extent rec.
 	 */
-	credits = bpc + 1;
+	credits = bpc + 1 + handle->h_buffer_credits;
 	ret = ocfs2_extend_trans(handle, credits);
 	if (ret) {
 		mlog_errno(ret);

@@ -3740,7 +3764,7 @@ static int ocfs2_divide_xattr_cluster(struct inode *inode,
 				      u32 *first_hash)
 {
 	u16 blk_per_bucket = ocfs2_blocks_per_xattr_bucket(inode->i_sb);
-	int ret, credits = 2 * blk_per_bucket;
+	int ret, credits = 2 * blk_per_bucket + handle->h_buffer_credits;
 
 	BUG_ON(OCFS2_XATTR_BUCKET_SIZE < OCFS2_SB(inode->i_sb)->s_clustersize);

@@ -3852,12 +3876,12 @@ static int ocfs2_add_new_xattr_cluster(struct inode *inode,
 				       int *extend,
 				       struct ocfs2_xattr_set_ctxt *ctxt)
 {
-	int ret, credits;
+	int ret;
 	u16 bpc = ocfs2_clusters_to_blocks(inode->i_sb, 1);
 	u32 prev_clusters = *num_clusters;
 	u32 clusters_to_add = 1, bit_off, num_bits, v_start = 0;
 	u64 block;
-	handle_t *handle = NULL;
+	handle_t *handle = ctxt->handle;
 	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
 	struct ocfs2_extent_tree et;

@@ -3868,16 +3892,6 @@ static int ocfs2_add_new_xattr_cluster(struct inode *inode,
 
 	ocfs2_init_xattr_tree_extent_tree(&et, inode, root_bh);
 
-	credits = ocfs2_calc_extend_credits(osb->sb, et.et_root_el,
-					    clusters_to_add);
-	handle = ocfs2_start_trans(osb, credits);
-	if (IS_ERR(handle)) {
-		ret = PTR_ERR(handle);
-		handle = NULL;
-		mlog_errno(ret);
-		goto leave;
-	}
-
 	ret = ocfs2_journal_access(handle, inode, root_bh,
 				   OCFS2_JOURNAL_ACCESS_WRITE);
 	if (ret < 0) {

@@ -3931,18 +3945,6 @@ static int ocfs2_add_new_xattr_cluster(struct inode *inode,
 		}
 	}
 
-	if (handle->h_buffer_credits < credits) {
-		/*
-		 * The journal has been restarted before, and don't
-		 * have enough space for the insertion, so extend it
-		 * here.
-		 */
-		ret = ocfs2_extend_trans(handle, credits);
-		if (ret) {
-			mlog_errno(ret);
-			goto leave;
-		}
-	}
 	mlog(0, "Insert %u clusters at block %llu for xattr@%u\n",
 	     num_bits, block, v_start);
 	ret = ocfs2_insert_extent(osb, handle, inode, &et, v_start, block,

@@ -3953,15 +3955,10 @@ static int ocfs2_add_new_xattr_cluster(struct inode *inode,
 	}
 
 	ret = ocfs2_journal_dirty(handle, root_bh);
-	if (ret < 0) {
+	if (ret < 0)
 		mlog_errno(ret);
-		goto leave;
-	}
 
 leave:
-	if (handle)
-		ocfs2_commit_trans(osb, handle);
-
 	return ret;
 }

@@ -3970,6 +3967,7 @@ leave:
  * We meet with start_bh. Only move half of the xattrs to the bucket after it.
  */
 static int ocfs2_extend_xattr_bucket(struct inode *inode,
+				     handle_t *handle,
 				     struct buffer_head *first_bh,
 				     struct buffer_head *start_bh,
 				     u32 num_clusters)

@@ -3979,7 +3977,6 @@ static int ocfs2_extend_xattr_bucket(struct inode *inode,
 	u16 blk_per_bucket = ocfs2_blocks_per_xattr_bucket(inode->i_sb);
 	u64 start_blk = start_bh->b_blocknr, end_blk;
 	u32 num_buckets = num_clusters * ocfs2_xattr_buckets_per_cluster(osb);
-	handle_t *handle;
 	struct ocfs2_xattr_header *first_xh =
 				(struct ocfs2_xattr_header *)first_bh->b_data;
 	u16 bucket = le16_to_cpu(first_xh->xh_num_buckets);

@@ -3996,11 +3993,10 @@ static int ocfs2_extend_xattr_bucket(struct inode *inode,
 	 * We will touch all the buckets after the start_bh(include it).
 	 * Then we add one more bucket.
 	 */
-	credits = end_blk - start_blk + 3 * blk_per_bucket + 1;
-	handle = ocfs2_start_trans(osb, credits);
-	if (IS_ERR(handle)) {
-		ret = PTR_ERR(handle);
-		handle = NULL;
+	credits = end_blk - start_blk + 3 * blk_per_bucket + 1 +
+		  handle->h_buffer_credits;
+	ret = ocfs2_extend_trans(handle, credits);
+	if (ret) {
 		mlog_errno(ret);
 		goto out;
 	}

@@ -4009,14 +4005,14 @@ static int ocfs2_extend_xattr_bucket(struct inode *inode,
 				   OCFS2_JOURNAL_ACCESS_WRITE);
 	if (ret) {
 		mlog_errno(ret);
-		goto commit;
+		goto out;
 	}
 
 	while (end_blk != start_blk) {
 		ret = ocfs2_cp_xattr_bucket(inode, handle, end_blk,
 					    end_blk + blk_per_bucket, 0);
 		if (ret)
-			goto commit;
+			goto out;
 		end_blk -= blk_per_bucket;
 	}

@@ -4027,8 +4023,6 @@ static int ocfs2_extend_xattr_bucket(struct inode *inode,
 	le16_add_cpu(&first_xh->xh_num_buckets, 1);
 	ocfs2_journal_dirty(handle, first_bh);
 
-commit:
-	ocfs2_commit_trans(osb, handle);
 out:
 	return ret;
 }

@@ -4106,6 +4100,7 @@ static int ocfs2_add_new_xattr_bucket(struct inode *inode,
 
 	if (extend)
 		ret = ocfs2_extend_xattr_bucket(inode,
+						ctxt->handle,
 						first_bh,
 						header_bh,
 						num_clusters);

@@ -4279,14 +4274,13 @@ set_new_name_value:
  * space for the xattr insertion.
  */
 static int ocfs2_xattr_set_entry_in_bucket(struct inode *inode,
+					   handle_t *handle,
 					   struct ocfs2_xattr_info *xi,
 					   struct ocfs2_xattr_search *xs,
 					   u32 name_hash,
 					   int local)
 {
 	int ret;
-	handle_t *handle = NULL;
-	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
 	u64 blkno;
 
 	mlog(0, "Set xattr entry len = %lu index = %d in bucket %llu\n",

@@ -4303,14 +4297,6 @@ static int ocfs2_xattr_set_entry_in_bucket(struct inode *inode,
 		}
 	}
 
-	handle = ocfs2_start_trans(osb, xs->bucket->bu_blocks);
-	if (IS_ERR(handle)) {
-		ret = PTR_ERR(handle);
-		handle = NULL;
-		mlog_errno(ret);
-		goto out;
-	}
-
 	ret = ocfs2_xattr_bucket_journal_access(handle, xs->bucket,
 						OCFS2_JOURNAL_ACCESS_WRITE);
 	if (ret < 0) {

@@ -4322,32 +4308,22 @@ static int ocfs2_xattr_set_entry_in_bucket(struct inode *inode,
 	ocfs2_xattr_bucket_journal_dirty(handle, xs->bucket);
 
 out:
-	ocfs2_commit_trans(osb, handle);
-
 	return ret;
 }
 
 static int ocfs2_xattr_value_update_size(struct inode *inode,
+					 handle_t *handle,
 					 struct buffer_head *xe_bh,
 					 struct ocfs2_xattr_entry *xe,
 					 u64 new_size)
 {
 	int ret;
-	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
-	handle_t *handle = NULL;
-
-	handle = ocfs2_start_trans(osb, 1);
-	if (IS_ERR(handle)) {
-		ret = -ENOMEM;
-		mlog_errno(ret);
-		goto out;
-	}
 
 	ret = ocfs2_journal_access(handle, inode, xe_bh,
 				   OCFS2_JOURNAL_ACCESS_WRITE);
 	if (ret < 0) {
 		mlog_errno(ret);
-		goto out_commit;
+		goto out;
 	}
 
 	xe->xe_value_size = cpu_to_le64(new_size);

@@ -4356,8 +4332,6 @@ static int ocfs2_xattr_value_update_size(struct inode *inode,
 	if (ret < 0)
 		mlog_errno(ret);
 
-out_commit:
-	ocfs2_commit_trans(osb, handle);
 out:
 	return ret;
 }

@@ -4414,7 +4388,8 @@ static int ocfs2_xattr_bucket_value_truncate(struct inode *inode,
 		goto out;
 	}
 
-	ret = ocfs2_xattr_value_update_size(inode, header_bh, xe, len);
+	ret = ocfs2_xattr_value_update_size(inode, ctxt->handle,
+					    header_bh, xe, len);
 	if (ret) {
 		mlog_errno(ret);
 		goto out;

@@ -4446,6 +4421,7 @@ static int ocfs2_xattr_bucket_value_truncate_xs(struct inode *inode,
 }
 
 static int ocfs2_xattr_bucket_set_value_outside(struct inode *inode,
+						handle_t *handle,
 						struct ocfs2_xattr_search *xs,
 						char *val,
 						int value_len)

@@ -4461,7 +4437,8 @@ static int ocfs2_xattr_bucket_set_value_outside(struct inode *inode,
 
 	xv = (struct ocfs2_xattr_value_root *)(xs->base + offset);
 
-	return __ocfs2_xattr_set_value_outside(inode, xv, val, value_len);
+	return __ocfs2_xattr_set_value_outside(inode, handle,
+					       xv, val, value_len);
 }
 
 static int ocfs2_rm_xattr_cluster(struct inode *inode,

@@ -4554,27 +4531,19 @@ out:
 }
 
 static void ocfs2_xattr_bucket_remove_xs(struct inode *inode,
+					 handle_t *handle,
 					 struct ocfs2_xattr_search *xs)
 {
-	handle_t *handle = NULL;
 	struct ocfs2_xattr_header *xh = bucket_xh(xs->bucket);
 	struct ocfs2_xattr_entry *last = &xh->xh_entries[
 						le16_to_cpu(xh->xh_count) - 1];
 	int ret = 0;
 
-	handle = ocfs2_start_trans((OCFS2_SB(inode->i_sb)),
-				   ocfs2_blocks_per_xattr_bucket(inode->i_sb));
-	if (IS_ERR(handle)) {
-		ret = PTR_ERR(handle);
-		mlog_errno(ret);
-		return;
-	}
-
 	ret = ocfs2_xattr_bucket_journal_access(handle, xs->bucket,
 						OCFS2_JOURNAL_ACCESS_WRITE);
 	if (ret) {
 		mlog_errno(ret);
-		goto out_commit;
+		return;
 	}
 
 	/* Remove the old entry. */

@@ -4584,9 +4553,6 @@ static void ocfs2_xattr_bucket_remove_xs(struct inode *inode,
 	le16_add_cpu(&xh->xh_count, -1);
 
 	ocfs2_xattr_bucket_journal_dirty(handle, xs->bucket);
-
-out_commit:
-	ocfs2_commit_trans(OCFS2_SB(inode->i_sb), handle);
 }
 
 /*

@@ -4652,7 +4618,8 @@ static int ocfs2_xattr_set_in_bucket(struct inode *inode,
 		xi->value_len = OCFS2_XATTR_ROOT_SIZE;
 	}
 
-	ret = ocfs2_xattr_set_entry_in_bucket(inode, xi, xs, name_hash, local);
+	ret = ocfs2_xattr_set_entry_in_bucket(inode, ctxt->handle, xi, xs,
+					      name_hash, local);
 	if (ret) {
 		mlog_errno(ret);
 		goto out;

@@ -4673,13 +4640,14 @@ static int ocfs2_xattr_set_in_bucket(struct inode *inode,
 			 * storage and we have allocated xattr already,
 			 * so need to remove it.
 			 */
-			ocfs2_xattr_bucket_remove_xs(inode, xs);
+			ocfs2_xattr_bucket_remove_xs(inode, ctxt->handle, xs);
 		}
 		goto out;
 	}
 
 set_value_outside:
-	ret = ocfs2_xattr_bucket_set_value_outside(inode, xs, val, value_len);
+	ret = ocfs2_xattr_bucket_set_value_outside(inode, ctxt->handle,
+						   xs, val, value_len);
 out:
 	return ret;
 }

@@ -4792,7 +4760,8 @@ try_again:
 			 * name/value will be moved, the xe shouldn't be changed
 			 * in xs.
 			 */
-			ret = ocfs2_defrag_xattr_bucket(inode, xs->bucket);
+			ret = ocfs2_defrag_xattr_bucket(inode, ctxt->handle,
+							xs->bucket);
 			if (ret) {
 				mlog_errno(ret);
 				goto out;

@@ -4873,6 +4842,13 @@ static int ocfs2_delete_xattr_in_bucket(struct inode *inode,
 	memset(&ctxt, 0, sizeof(ctxt));
 	ocfs2_init_dealloc_ctxt(&ctxt.dealloc);
 
+	ctxt.handle = ocfs2_start_trans(osb, OCFS2_REMOVE_EXTENT_CREDITS);
+	if (IS_ERR(ctxt.handle)) {
+		ret = PTR_ERR(ctxt.handle);
+		mlog_errno(ret);
+		goto out;
+	}
+
 	for (i = 0; i < le16_to_cpu(xh->xh_count); i++) {
 		xe = &xh->xh_entries[i];
 		if (ocfs2_xattr_is_local(xe))

@@ -4887,9 +4863,10 @@ static int ocfs2_delete_xattr_in_bucket(struct inode *inode,
 		}
 	}
 
+	ret = ocfs2_commit_trans(osb, ctxt.handle);
 	ocfs2_schedule_truncate_log_flush(osb, 1);
 	ocfs2_run_deallocs(osb, &ctxt.dealloc);
-
+out:
 	return ret;
 }

Signature · 1 line

1.5.4.GIT

Joel Becker

2008-11-07 22:46 UTC #19 lore raw

PATCH 5/15 ocfs2/xattr: Merge xattr set transaction.

Quoted 5 lines · Tao Ma

In current ocfs2/xattr, the whole xattr set is divided into many steps are many transaction are used, this make the xattr set process isn't like a real transaction, so this patch try to merge all the transaction into one. Another benefit is that acl can use it easily now.

There's nothing in this patch that looks horribly wrong. I will
admit, I don't have enough of a grasp of this code to evaluate it deeply. I think we should apply it, and as I finish up my bucket work on the new _set() path I'll mention anything I see as a go there.

Quoted 7 lines

I don't merge the transaction of deleting xattr when we remove an inode. The reason is that if we have a large number of xattrs and every xattrs has large values(large enough for outside storage), the whole transaction will be very huge and it looks like jbd can't handle it(I meet with a jbd complain once). And the old inode removal is also divided into many steps, so I'd like to leave as it is.

I think this is the right way to go.

Joel

Signature · 8 lines

"When ideas fail, words come in very handy." 
         - Goethe

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

Tiger Yang

author

2008-10-30 11:40 UTC #20 lore raw

PATCH 6/15 ocfs2: move new_inode out of the transaction

In reply to Tiger Yang

we need know the security xattr size before start transaction, so mov new_inode out of mknod_unlock and populate i_mode to get the exact security xattr size.

Signed-off-by Tiger Yang <[email protected]>

---
 fs/ocfs2/namei.c |   53 +++++++++++++++++++++++++++++++++--------------------
 1 files changed, 33 insertions(+), 20 deletions(-)

fs/ocfs2/namei.c +33 ~~-20~~

index f4967e6..dd4ee52 100644
@@ -71,7 +71,7 @@ static int ocfs2_mknod_locked(struct ocfs2_super *osb,
 			      struct buffer_head **new_fe_bh,
 			      struct buffer_head *parent_fe_bh,
 			      handle_t *handle,
-			      struct inode **ret_inode,
+			      struct inode *inode,
 			      struct ocfs2_alloc_context *inode_ac);
 
 static int ocfs2_prepare_orphan_dir(struct ocfs2_super *osb,

@@ -186,6 +186,21 @@ bail:
 	return ret;
 }
 
+static inline struct inode *ocfs2_make_init_inode(struct inode *dir, int mode)
+{
+	struct inode *inode;
+
+	/*
+	 * move new_inode out of the transanction and populate i_mode
+	 * to get exact security xattr size.
+	 */
+	inode = new_inode(dir->i_sb);
+	if (!IS_ERR(inode))
+		inode->i_mode = mode;
+
+	return inode;
+}
+
 static int ocfs2_mknod(struct inode *dir,
 		       struct dentry *dentry,
 		       int mode,

@@ -250,6 +265,13 @@ static int ocfs2_mknod(struct inode *dir,
 		goto leave;
 	}
 
+	inode = ocfs2_make_init_inode(dir, mode);
+	if (IS_ERR(inode)) {
+		status = PTR_ERR(inode);
+		mlog(ML_ERROR, "new_inode failed!\n");
+		goto leave;
+	}
+
 	/* Reserve a cluster if creating an extent based directory. */
 	if (S_ISDIR(mode) && !ocfs2_supports_inline_data(osb)) {
 		status = ocfs2_reserve_clusters(osb, 1, &data_ac);

@@ -271,7 +293,7 @@ static int ocfs2_mknod(struct inode *dir,
 	/* do the real work now. */
 	status = ocfs2_mknod_locked(osb, dir, dentry, mode, dev,
 				    &new_fe_bh, parent_fe_bh, handle,
-				    &inode, inode_ac);
+				    inode, inode_ac);
 	if (status < 0) {
 		mlog_errno(status);
 		goto leave;

@@ -353,7 +375,7 @@ static int ocfs2_mknod_locked(struct ocfs2_super *osb,
 			      struct buffer_head **new_fe_bh,
 			      struct buffer_head *parent_fe_bh,
 			      handle_t *handle,
-			      struct inode **ret_inode,
+			      struct inode *inode,
 			      struct ocfs2_alloc_context *inode_ac)
 {
 	int status = 0;

@@ -361,14 +383,12 @@ static int ocfs2_mknod_locked(struct ocfs2_super *osb,
 	struct ocfs2_extent_list *fel;
 	u64 fe_blkno = 0;
 	u16 suballoc_bit;
-	struct inode *inode = NULL;
 
 	mlog_entry("(0x%p, 0x%p, %d, %lu, '%.*s')\n", dir, dentry, mode,
 		   (unsigned long)dev, dentry->d_name.len,
 		   dentry->d_name.name);
 
 	*new_fe_bh = NULL;
-	*ret_inode = NULL;
 
 	status = ocfs2_claim_new_inode(osb, handle, inode_ac, &suballoc_bit,
 				       &fe_blkno);

@@ -377,13 +397,6 @@ static int ocfs2_mknod_locked(struct ocfs2_super *osb,
 		goto leave;
 	}
 
-	inode = new_inode(dir->i_sb);
-	if (!inode) {
-		status = -ENOMEM;
-		mlog(ML_ERROR, "new_inode failed!\n");
-		goto leave;
-	}
-
 	/* populate as many fields early on as possible - many of
 	 * these are used by the support functions here and in
 	 * callers. */

@@ -393,7 +406,6 @@ static int ocfs2_mknod_locked(struct ocfs2_super *osb,
 		inode->i_nlink = 2;
 	else
 		inode->i_nlink = 1;
-	inode->i_mode = mode;
 	spin_lock(&osb->osb_lock);
 	inode->i_generation = osb->s_next_generation++;
 	spin_unlock(&osb->osb_lock);

@@ -483,18 +495,12 @@ static int ocfs2_mknod_locked(struct ocfs2_super *osb,
 
 	status = 0; /* error in ocfs2_create_new_inode_locks is not
 		     * critical */
-
-	*ret_inode = inode;
 leave:
 	if (status < 0) {
 		if (*new_fe_bh) {
 			brelse(*new_fe_bh);
 			*new_fe_bh = NULL;
 		}
-		if (inode) {
-			clear_nlink(inode);
-			iput(inode);
-		}
 	}
 
 	mlog_exit(status);

@@ -1552,6 +1558,13 @@ static int ocfs2_symlink(struct inode *dir,
 		}
 	}
 
+	inode = ocfs2_make_init_inode(dir, (S_IFLNK | S_IRWXUGO));
+	if (IS_ERR(inode)) {
+		status = PTR_ERR(inode);
+		mlog(ML_ERROR, "new_inode failed!\n");
+		goto bail;
+	}
+
 	handle = ocfs2_start_trans(osb, credits);
 	if (IS_ERR(handle)) {
 		status = PTR_ERR(handle);

@@ -1563,7 +1576,7 @@ static int ocfs2_symlink(struct inode *dir,
 	status = ocfs2_mknod_locked(osb, dir, dentry,
 				    S_IFLNK | S_IRWXUGO, 0,
 				    &new_fe_bh, parent_fe_bh, handle,
-				    &inode, inode_ac);
+				    inode, inode_ac);
 	if (status < 0) {
 		mlog_errno(status);
 		goto bail;

Signature · 1 line

1.5.4.1

Mark Fasheh

2008-11-07 01:02 UTC #21 lore raw

PATCH 6/15 ocfs2: move new_inode out of the transaction

Quoted 30 lines · Tiger Yang

we need know the security xattr size before start transaction, so mov new_inode out of mknod_unlock and populate i_mode to get the exact security xattr size.

Signed-off-by Tiger Yang <[email protected]>

---
 fs/ocfs2/namei.c |   53 +++++++++++++++++++++++++++++++++--------------------
 1 files changed, 33 insertions(+), 20 deletions(-)

fs/ocfs2/namei.c +5 -1

index f4967e6..dd4ee52 100644
@@ -71,7 +71,7 @@ static int ocfs2_mknod_locked(struct ocfs2_super *osb,
 			      struct buffer_head **new_fe_bh,
 			      struct buffer_head *parent_fe_bh,
 			      handle_t *handle,
-			      struct inode **ret_inode,
+			      struct inode *inode,
 			      struct ocfs2_alloc_context *inode_ac);
 
 static int ocfs2_prepare_orphan_dir(struct ocfs2_super *osb,

@@ -186,6 +186,21 @@ bail:
 	return ret;
 }
 
+static inline struct inode *ocfs2_make_init_inode(struct inode *dir, int mode)
+{
+	struct inode *inode;
+

fs/ocfs2/namei.c bail: · line 202

Quoted 10 lines

fs/ocfs2/namei.c +10 -0

+	/*
+	 * move new_inode out of the transanction and populate i_mode
+	 * to get exact security xattr size.
+	 */
+	inode = new_inode(dir->i_sb);
+	if (!IS_ERR(inode))
+		inode->i_mode = mode;
+
+	return inode;
+}

Great, this is what I was hoping for. Check out this patch from Jan:

http://oss.oracle.com/pipermail/ocfs2-devel/2008-October/003266.html

(forward down to ocfs2_get_init_inode()).

Should we just copy that function in here and use that?

Btw, we probably want to do the minor changes to ocfs2_mknod_locked so that it doesn't fill i_mode (and whatever fields) a second time.

	--Mark

--
Mark Fasheh

Tiger Yang

author

2008-10-30 11:40 UTC #22 lore raw

PATCH 7/15 ocfs2: add ocfs2_xattr_set_handle

In reply to Tiger Yang

This function use to set xattr in a started transaction. It is similar to ocfs2_xattr_set.

Signed-off-by Tiger Yang <[email protected]>

---
 fs/ocfs2/xattr.c |   81 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/ocfs2/xattr.h |    4 ++
 2 files changed, 85 insertions(+), 0 deletions(-)

fs/ocfs2/xattr.c +81 -0

index 0e380c4..8c33a3f 100644
@@ -2319,6 +2319,87 @@ out:
 	return ret;
 }
 
+int ocfs2_xattr_set_handle(handle_t *handle,
+			   struct inode *inode,
+			   struct buffer_head *di_bh,
+			   int name_index,
+			   const char *name,
+			   const void *value,
+			   size_t value_len,
+			   int flags,
+			   struct ocfs2_alloc_context *meta_ac,
+			   struct ocfs2_alloc_context *data_ac)
+{
+	struct ocfs2_dinode *di;
+	int ret, credits;
+
+	struct ocfs2_xattr_info xi = {
+		.name_index = name_index,
+		.name = name,
+		.value = value,
+		.value_len = value_len,
+	};
+
+	struct ocfs2_xattr_search xis = {
+		.not_found = -ENODATA,
+	};
+
+	struct ocfs2_xattr_search xbs = {
+		.not_found = -ENODATA,
+	};
+
+	struct ocfs2_xattr_set_ctxt ctxt = {
+		.handle = handle,
+		.meta_ac = meta_ac,
+		.data_ac = data_ac,
+	};
+
+	if (!ocfs2_supports_xattr(OCFS2_SB(inode->i_sb)))
+		return -EOPNOTSUPP;
+
+	xbs.bucket = ocfs2_xattr_bucket_new(inode);
+	if (!xbs.bucket) {
+		mlog_errno(-ENOMEM);
+		return -ENOMEM;
+	}
+
+	xis.inode_bh = xbs.inode_bh = di_bh;
+	di = (struct ocfs2_dinode *)di_bh->b_data;
+
+	down_write(&OCFS2_I(inode)->ip_xattr_sem);
+
+	ret = ocfs2_xattr_ibody_find(inode, name_index, name, &xis);
+	if (ret)
+		goto cleanup;
+	if (xis.not_found) {
+		ret = ocfs2_xattr_block_find(inode, name_index, name, &xbs);
+		if (ret)
+			goto cleanup;
+	}
+
+	ret = ocfs2_calc_xattr_set_need(inode, di, &xi, &xis, &xbs,
+					NULL, NULL, &credits);
+	if (ret) {
+		mlog_errno(ret);
+		goto cleanup;
+	}
+
+	ret = ocfs2_extend_trans(handle, credits);
+	if (ret) {
+		mlog_errno(ret);
+		goto cleanup;
+	}
+
+	ret = __ocfs2_xattr_set_handle(inode, di, &xi, &xis, &xbs, &ctxt);
+
+cleanup:
+	up_write(&OCFS2_I(inode)->ip_xattr_sem);
+	brelse(xbs.xattr_bh);
+	ocfs2_xattr_bucket_free(xbs.bucket);
+
+	return ret;
+}
+
 /*
  * ocfs2_xattr_set()
  *

fs/ocfs2/xattr.h +4 -0

index 1d8314c..8fbdc16 100644
@@ -37,6 +37,10 @@ extern struct xattr_handler *ocfs2_xattr_handlers[];
 ssize_t ocfs2_listxattr(struct dentry *, char *, size_t);
 int ocfs2_xattr_set(struct inode *, int, const char *, const void *,
 		    size_t, int);
+int ocfs2_xattr_set_handle(handle_t *, struct inode *, struct buffer_head *,
+			   int, const char *, const void *, size_t, int,
+			   struct ocfs2_alloc_context *,
+			   struct ocfs2_alloc_context *);
 int ocfs2_xattr_remove(struct inode *, struct buffer_head *);
 
 #endif /* OCFS2_XATTR_H */

Signature · 1 line

1.5.4.1

Mark Fasheh

2008-11-07 01:03 UTC #23 lore raw

PATCH 7/15 ocfs2: add ocfs2_xattr_set_handle

Quoted 2 lines · Tiger Yang

This function use to set xattr in a started transaction. It is similar to ocfs2_xattr_set.

This looks good. Glad to see so much code re-use.

	--Mark

--
Mark Fasheh

Tiger Yang

author

2008-10-30 11:40 UTC #24 lore raw

PATCH 8/15 ocfs2: add security xattr API

In reply to Tiger Yang

This patch add security xattr set/get/list APIs to support security xattr in ocfs2.

Signed-off-by Tiger Yang <[email protected]>

---
 fs/ocfs2/xattr.c |   47 +++++++++++++++++++++++++++++++++++++++++++++++
 fs/ocfs2/xattr.h |    1 +
 2 files changed, 48 insertions(+), 0 deletions(-)

fs/ocfs2/xattr.c +47 -0

index 8c33a3f..93026dc 100644
@@ -35,6 +35,7 @@
 #include <linux/init.h>
 #include <linux/module.h>
 #include <linux/string.h>
+#include <linux/security.h>
 
 #define MLOG_MASK_PREFIX ML_XATTR
 #include <cluster/masklog.h>

@@ -88,12 +89,14 @@ static struct ocfs2_xattr_def_value_root def_xv = {
 struct xattr_handler *ocfs2_xattr_handlers[] = {
 	&ocfs2_xattr_user_handler,
 	&ocfs2_xattr_trusted_handler,
+	&ocfs2_xattr_security_handler,
 	NULL
 };
 
 static struct xattr_handler *ocfs2_xattr_handler_map[OCFS2_XATTR_MAX] = {
 	[OCFS2_XATTR_INDEX_USER]	= &ocfs2_xattr_user_handler,
 	[OCFS2_XATTR_INDEX_TRUSTED]	= &ocfs2_xattr_trusted_handler,
+	[OCFS2_XATTR_INDEX_SECURITY]	= &ocfs2_xattr_security_handler,
 };
 
 struct ocfs2_xattr_info {

@@ -4998,6 +5001,50 @@ out:
 }
 
 /*
+ * 'security' attributes support
+ */
+static size_t ocfs2_xattr_security_list(struct inode *inode, char *list,
+					size_t list_size, const char *name,
+					size_t name_len)
+{
+	const size_t prefix_len = XATTR_SECURITY_PREFIX_LEN;
+	const size_t total_len = prefix_len + name_len + 1;
+
+	if (list && total_len <= list_size) {
+		memcpy(list, XATTR_SECURITY_PREFIX, prefix_len);
+		memcpy(list + prefix_len, name, name_len);
+		list[prefix_len + name_len] = '\0';
+	}
+	return total_len;
+}
+
+static int ocfs2_xattr_security_get(struct inode *inode, const char *name,
+				    void *buffer, size_t size)
+{
+	if (strcmp(name, "") == 0)
+		return -EINVAL;
+	return ocfs2_xattr_get(inode, OCFS2_XATTR_INDEX_SECURITY, name,
+			       buffer, size);
+}
+
+static int ocfs2_xattr_security_set(struct inode *inode, const char *name,
+				    const void *value, size_t size, int flags)
+{
+	if (strcmp(name, "") == 0)
+		return -EINVAL;
+
+	return ocfs2_xattr_set(inode, OCFS2_XATTR_INDEX_SECURITY, name, value,
+			       size, flags);
+}
+
+struct xattr_handler ocfs2_xattr_security_handler = {
+	.prefix	= XATTR_SECURITY_PREFIX,
+	.list	= ocfs2_xattr_security_list,
+	.get	= ocfs2_xattr_security_get,
+	.set	= ocfs2_xattr_security_set,
+};
+
+/*
  * 'trusted' attributes support
  */
 static size_t ocfs2_xattr_trusted_list(struct inode *inode, char *list,

fs/ocfs2/xattr.h +1 -0

index 8fbdc16..55c5256 100644
@@ -32,6 +32,7 @@ enum ocfs2_xattr_type {
 
 extern struct xattr_handler ocfs2_xattr_user_handler;
 extern struct xattr_handler ocfs2_xattr_trusted_handler;
+extern struct xattr_handler ocfs2_xattr_security_handler;
 extern struct xattr_handler *ocfs2_xattr_handlers[];
 
 ssize_t ocfs2_listxattr(struct dentry *, char *, size_t);

Signature · 1 line

1.5.4.1

Mark Fasheh

2008-11-07 01:17 UTC #25 lore raw

PATCH 8/15 ocfs2: add security xattr API

Quoted 2 lines · Tiger Yang

This patch add security xattr set/get/list APIs to support security xattr in ocfs2.

This patch looks great! One question - should we be setting security attributes on symlinks too?

	--Mark

--
Mark Fasheh

Mark Fasheh

2008-11-07 01:17 UTC #26 lore raw

PATCH 8/15 ocfs2: add security xattr API

Quoted 6 lines · Mark Fasheh

Quoted 2 lines · Tiger Yang
This patch add security xattr set/get/list APIs to support security xattr in ocfs2.
This patch looks great! One question - should we be setting security attributes on symlinks too?

Sorry - this comment was meant for patch #9.

	--Mark

--
Mark Fasheh

Tiger Yang

author

2008-10-30 11:41 UTC #27 lore raw

PATCH 9/15 ocfs2: add ocfs2_init_security in mkno

In reply to Tiger Yang

security xattr must be set when creating a new inode. we do this in three steps. first, get security xattr's name and value by security_operation, then calculate and reserve the meta data and clusters needed by this security xattr before starting transaction, finally set it before add_entry.

Signed-off-by Tiger Yang <[email protected]>

---
 fs/ocfs2/namei.c |   56 +++++++++++++++++++++++++++++++++++----
 fs/ocfs2/xattr.c |   76 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/ocfs2/xattr.h |   10 +++++++
 3 files changed, 136 insertions(+), 6 deletions(-)

fs/ocfs2/namei.c +50 -6

index dd4ee52..6ec42ef 100644
@@ -215,7 +215,13 @@ static int ocfs2_mknod(struct inode *dir,
 	struct buffer_head *de_bh = NULL;
 	struct inode *inode = NULL;
 	struct ocfs2_alloc_context *inode_ac = NULL;
+	struct ocfs2_alloc_context *xattr_ac = NULL;
 	struct ocfs2_alloc_context *data_ac = NULL;
+	int want_clusters = 0;
+	int has_security = 1;
+	size_t security_len = 0;
+	void *security_value = NULL;
+	char *security_name = NULL;
 
 	mlog_entry("(0x%p, 0x%p, %d, %lu, '%.*s')\n", dir, dentry, mode,
 		   (unsigned long)dev, dentry->d_name.len,

@@ -272,16 +278,38 @@ static int ocfs2_mknod(struct inode *dir,
 		goto leave;
 	}
 
-	/* Reserve a cluster if creating an extent based directory. */
-	if (S_ISDIR(mode) && !ocfs2_supports_inline_data(osb)) {
-		status = ocfs2_reserve_clusters(osb, 1, &data_ac);
-		if (status < 0) {
-			if (status != -ENOSPC)
-				mlog_errno(status);
+	/* get security xattr */
+	status = ocfs2_init_security_get(inode, dir, &security_name,
+					 &security_value, &security_len);
+	if (status) {
+		if (status == -EOPNOTSUPP)
+			has_security = 0;
+		else {
+			mlog_errno(status);
 			goto leave;
 		}
 	}
 
+	/* calculate meta data/clusters for setting security xattr */
+	status = ocfs2_calc_xattr_init(dir, strlen(security_name),
+					security_len, has_security,
+					&want_clusters, &xattr_ac);
+	if (status < 0) {
+		mlog_errno(status);
+		goto leave;
+	}
+
+	/* Reserve a cluster if creating an extent based directory. */
+	if (S_ISDIR(mode) && !ocfs2_supports_inline_data(osb))
+		want_clusters += 1;
+
+	status = ocfs2_reserve_clusters(osb, want_clusters, &data_ac);
+	if (status < 0) {
+		if (status != -ENOSPC)
+			mlog_errno(status);
+		goto leave;
+	}
+
 	handle = ocfs2_start_trans(osb, OCFS2_MKNOD_CREDITS);
 	if (IS_ERR(handle)) {
 		status = PTR_ERR(handle);

@@ -322,6 +350,17 @@ static int ocfs2_mknod(struct inode *dir,
 		inc_nlink(dir);
 	}
 
+	if (has_security) {
+		status = ocfs2_init_security_set(handle, inode, new_fe_bh,
+						 security_name, security_value,
+						 security_len,
+						 xattr_ac, data_ac);
+		if (status < 0) {
+			mlog_errno(status);
+			goto leave;
+		}
+	}
+
 	status = ocfs2_add_entry(handle, dentry, inode,
 				 OCFS2_I(inode)->ip_blkno, parent_fe_bh,
 				 de_bh);

@@ -353,6 +392,8 @@ leave:
 	brelse(new_fe_bh);
 	brelse(de_bh);
 	brelse(parent_fe_bh);
+	kfree(security_value);
+	kfree(security_name);
 
 	if ((status < 0) && inode)
 		iput(inode);

@@ -360,6 +401,9 @@ leave:
 	if (inode_ac)
 		ocfs2_free_alloc_context(inode_ac);
 
+	if (xattr_ac)
+		ocfs2_free_alloc_context(xattr_ac);
+
 	if (data_ac)
 		ocfs2_free_alloc_context(data_ac);

fs/ocfs2/xattr.c +76 -0

94 lines not on this page · raw message

fs/ocfs2/xattr.h +10 -0

index 55c5256..c615ed9 100644
@@ -43,5 +43,15 @@ int ocfs2_xattr_set_handle(handle_t *, struct inode *, struct buffer_head *,
 			   struct ocfs2_alloc_context *,
 			   struct ocfs2_alloc_context *);
 int ocfs2_xattr_remove(struct inode *, struct buffer_head *);
+int ocfs2_init_security_get(struct inode *, struct inode *,
+			    char **, void **, size_t *);
+int ocfs2_init_security_set(handle_t *, struct inode *,
+			    struct buffer_head *,
+			    char *, void *, size_t,
+			    struct ocfs2_alloc_context *,
+			    struct ocfs2_alloc_context *);
+int ocfs2_calc_xattr_init(struct inode *, int, size_t,
+			  int, int *,
+			  struct ocfs2_alloc_context **);
 
 #endif /* OCFS2_XATTR_H */

Signature · 1 line

1.5.4.1

Tiger Yang

author

2008-10-30 11:41 UTC #28 lore raw

PATCH 10/15 ocfs2: add ocfs2_xattr_get_nolock

In reply to Tiger Yang

This function do the work of ocfs2_xattr_get under an opend lock.

Signed-off-by Tiger Yang <[email protected]>

---
 fs/ocfs2/xattr.c |   40 ++++++++++++++++++++++++++++------------
 fs/ocfs2/xattr.h |    2 ++
 2 files changed, 30 insertions(+), 12 deletions(-)

fs/ocfs2/xattr.c +28 ~~-12~~

index 3922b43..d926960 100644
@@ -921,12 +921,8 @@ cleanup:
 	return ret;
 }
 
-/* ocfs2_xattr_get()
- *
- * Copy an extended attribute into the buffer provided.
- * Buffer is NULL to compute the size of buffer required.
- */
-static int ocfs2_xattr_get(struct inode *inode,
+int ocfs2_xattr_get_nolock(struct inode *inode,
+			   struct buffer_head *di_bh,
 			   int name_index,
 			   const char *name,
 			   void *buffer,

@@ -934,7 +930,6 @@ static int ocfs2_xattr_get(struct inode *inode,
 {
 	int ret;
 	struct ocfs2_dinode *di = NULL;
-	struct buffer_head *di_bh = NULL;
 	struct ocfs2_inode_info *oi = OCFS2_I(inode);
 	struct ocfs2_xattr_search xis = {
 		.not_found = -ENODATA,

@@ -949,11 +944,6 @@ static int ocfs2_xattr_get(struct inode *inode,
 	if (!(oi->ip_dyn_features & OCFS2_HAS_XATTR_FL))
 		ret = -ENODATA;
 
-	ret = ocfs2_inode_lock(inode, &di_bh, 0);
-	if (ret < 0) {
-		mlog_errno(ret);
-		return ret;
-	}
 	xis.inode_bh = xbs.inode_bh = di_bh;
 	di = (struct ocfs2_dinode *)di_bh->b_data;

@@ -964,6 +954,32 @@ static int ocfs2_xattr_get(struct inode *inode,
 		ret = ocfs2_xattr_block_get(inode, name_index, name, buffer,
 					    buffer_size, &xbs);
 	up_read(&oi->ip_xattr_sem);
+
+	return ret;
+}
+
+/* ocfs2_xattr_get()
+ *
+ * Copy an extended attribute into the buffer provided.
+ * Buffer is NULL to compute the size of buffer required.
+ */
+static int ocfs2_xattr_get(struct inode *inode,
+			   int name_index,
+			   const char *name,
+			   void *buffer,
+			   size_t buffer_size)
+{
+	int ret;
+	struct buffer_head *di_bh = NULL;
+
+	ret = ocfs2_inode_lock(inode, &di_bh, 0);
+	if (ret < 0) {
+		mlog_errno(ret);
+		return ret;
+	}
+	ret = ocfs2_xattr_get_nolock(inode, di_bh, name_index,
+				     name, buffer, buffer_size);
+
 	ocfs2_inode_unlock(inode, 0);
 
 	brelse(di_bh);

fs/ocfs2/xattr.h +2 -0

index c615ed9..d9f3da9 100644
@@ -36,6 +36,8 @@ extern struct xattr_handler ocfs2_xattr_security_handler;
 extern struct xattr_handler *ocfs2_xattr_handlers[];
 
 ssize_t ocfs2_listxattr(struct dentry *, char *, size_t);
+int ocfs2_xattr_get_nolock(struct inode *, struct buffer_head *, int,
+			   const char *, void *, size_t);
 int ocfs2_xattr_set(struct inode *, int, const char *, const void *,
 		    size_t, int);
 int ocfs2_xattr_set_handle(handle_t *, struct inode *, struct buffer_head *,

Signature · 1 line

1.5.4.1

Tiger Yang

author

2008-10-30 11:41 UTC #29 lore raw

PATCH 11/15 ocfs2: add POSIX ACL API

In reply to Tiger Yang

This patch add POSIX ACL(access control lists) APIs in ocfs2. we convert struct posix_acl to many ocfs2_acl_entry and regard them as an extended attribute entry.

Signed-off-by Tiger Yang <[email protected]>

---
 fs/ocfs2/Makefile |    4 +
 fs/ocfs2/acl.c    |  421 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/ocfs2/acl.h    |   29 ++++
 fs/ocfs2/ocfs2.h  |    1 +
 fs/ocfs2/xattr.c  |   10 ++
 fs/ocfs2/xattr.h  |    4 +
 6 files changed, 469 insertions(+), 0 deletions(-)
 create mode 100644 fs/ocfs2/acl.c
 create mode 100644 fs/ocfs2/acl.h

fs/ocfs2/Makefile +4 -0

index 589dcdf..e9ef5d1 100644
@@ -37,6 +37,10 @@ ocfs2-objs := \
 	ver.o			\
 	xattr.o
 
+ifeq ($(CONFIG_OCFS2_FS_POSIX_ACL),y)
+ocfs2-objs += acl.o
+endif
+
 ocfs2_stackglue-objs := stackglue.o
 ocfs2_stack_o2cb-objs := stack_o2cb.o
 ocfs2_stack_user-objs := stack_user.o

fs/ocfs2/acl.c new file +421 -0

421 lines not on this page · raw message

fs/ocfs2/acl.h new file +29 -0

new file mode 100644
index 0000000..1b39f3e
@@ -0,0 +1,29 @@
+/* -*- mode: c; c-basic-offset: 8; -*-
+ * vim: noexpandtab sw=8 ts=8 sts=0:
+ *
+ * acl.h
+ *
+ * Copyright (C) 2004, 2008 Oracle.  All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ */
+
+#ifndef OCFS2_ACL_H
+#define OCFS2_ACL_H
+
+#include <linux/posix_acl_xattr.h>
+
+struct ocfs2_acl_entry {
+	__le16 e_tag;
+	__le16 e_perm;
+	__le32 e_id;
+};
+
+#endif /* OCFS2_ACL_H */

fs/ocfs2/ocfs2.h +1 -0

index fef7ece..1c1e966 100644
@@ -195,6 +195,7 @@ enum ocfs2_mount_options
 	OCFS2_MOUNT_LOCALFLOCKS = 1 << 5, /* No cluster aware user file locks */
 	OCFS2_MOUNT_NOUSERXATTR = 1 << 6, /* No user xattr */
 	OCFS2_MOUNT_INODE64 = 1 << 7,	/* Allow inode numbers > 2^32 */
+	OCFS2_MOUNT_POSIX_ACL = 1 << 8,	/* POSIX access control lists */
 };
 
 #define OCFS2_OSB_SOFT_RO	0x0001

fs/ocfs2/xattr.c +10 -0

index d926960..bcd4f1b 100644
@@ -91,6 +91,10 @@ static struct ocfs2_xattr_def_value_root def_xv = {
 
 struct xattr_handler *ocfs2_xattr_handlers[] = {
 	&ocfs2_xattr_user_handler,
+#ifdef CONFIG_OCFS2_FS_POSIX_ACL
+	&ocfs2_xattr_acl_access_handler,
+	&ocfs2_xattr_acl_default_handler,
+#endif
 	&ocfs2_xattr_trusted_handler,
 	&ocfs2_xattr_security_handler,
 	NULL

@@ -98,6 +102,12 @@ struct xattr_handler *ocfs2_xattr_handlers[] = {
 
 static struct xattr_handler *ocfs2_xattr_handler_map[OCFS2_XATTR_MAX] = {
 	[OCFS2_XATTR_INDEX_USER]	= &ocfs2_xattr_user_handler,
+#ifdef CONFIG_OCFS2_FS_POSIX_ACL
+	[OCFS2_XATTR_INDEX_POSIX_ACL_ACCESS]
+					= &ocfs2_xattr_acl_access_handler,
+	[OCFS2_XATTR_INDEX_POSIX_ACL_DEFAULT]
+					= &ocfs2_xattr_acl_default_handler,
+#endif
 	[OCFS2_XATTR_INDEX_TRUSTED]	= &ocfs2_xattr_trusted_handler,
 	[OCFS2_XATTR_INDEX_SECURITY]	= &ocfs2_xattr_security_handler,
 };

fs/ocfs2/xattr.h +4 -0

index d9f3da9..b8ab4dc 100644
@@ -33,6 +33,10 @@ enum ocfs2_xattr_type {
 extern struct xattr_handler ocfs2_xattr_user_handler;
 extern struct xattr_handler ocfs2_xattr_trusted_handler;
 extern struct xattr_handler ocfs2_xattr_security_handler;
+#ifdef CONFIG_OCFS2_FS_POSIX_ACL
+extern struct xattr_handler ocfs2_xattr_acl_access_handler;
+extern struct xattr_handler ocfs2_xattr_acl_default_handler;
+#endif
 extern struct xattr_handler *ocfs2_xattr_handlers[];
 
 ssize_t ocfs2_listxattr(struct dentry *, char *, size_t);

Signature · 1 line

1.5.4.1

Mark Fasheh

2008-11-07 01:26 UTC #30 lore raw

PATCH 11/15 ocfs2: add POSIX ACL API

Quoted 93 lines · Tiger Yang

fs/ocfs2/acl.c new file +93 -0

+static int ocfs2_set_acl_handle(handle_t *handle,
+				struct inode *inode,
+				struct buffer_head *di_bh,
+				int type,
+				struct posix_acl *acl,
+				struct ocfs2_alloc_context *meta_ac,
+				struct ocfs2_alloc_context *data_ac)
+{
+	int name_index;
+	void *value = NULL;
+	size_t size = 0;
+	int ret;
+
+	if (S_ISLNK(inode->i_mode))
+		return -EOPNOTSUPP;
+
+	switch (type) {
+	case ACL_TYPE_ACCESS:
+		name_index = OCFS2_XATTR_INDEX_POSIX_ACL_ACCESS;
+		if (acl) {
+			mode_t mode = inode->i_mode;
+			ret = posix_acl_equiv_mode(acl, &mode);
+			if (ret < 0)
+				return ret;
+			else {
+				inode->i_mode = mode;
+				if (ret == 0)
+					acl = NULL;
+			}
+		}
+		break;
+	case ACL_TYPE_DEFAULT:
+		name_index = OCFS2_XATTR_INDEX_POSIX_ACL_DEFAULT;
+		if (!S_ISDIR(inode->i_mode))
+			return acl ? -EACCES : 0;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (acl) {
+		value = ocfs2_acl_to_xattr(acl, &size);
+		if (IS_ERR(value))
+			return (int)PTR_ERR(value);
+	}
+
+	ret = ocfs2_xattr_set_handle(handle, inode, di_bh, name_index,
+				     "", value, size, 0, meta_ac, data_ac);
+
+	kfree(value);
+
+	return ret;
+}
+
+/*
+ * Set the access or default ACL of an inode.
+ */
+static int ocfs2_set_acl(struct inode *inode, int type, struct posix_acl *acl)
+{
+	int name_index;
+	void *value = NULL;
+	size_t size = 0;
+	int ret;
+
+	if (S_ISLNK(inode->i_mode))
+		return -EOPNOTSUPP;
+
+	switch (type) {
+	case ACL_TYPE_ACCESS:
+		name_index = OCFS2_XATTR_INDEX_POSIX_ACL_ACCESS;
+		if (acl) {
+			mode_t mode = inode->i_mode;
+			ret = posix_acl_equiv_mode(acl, &mode);
+			if (ret < 0)
+				return ret;
+			else {
+				inode->i_mode = mode;
+				if (ret == 0)
+					acl = NULL;
+			}
+		}
+		break;
+	case ACL_TYPE_DEFAULT:
+		name_index = OCFS2_XATTR_INDEX_POSIX_ACL_DEFAULT;
+		if (!S_ISDIR(inode->i_mode))
+			return acl ? -EACCES : 0;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (acl) {
+		value = ocfs2_acl_to_xattr(acl, &size);

fs/ocfs2/acl.c line 274

Quoted 10 lines

fs/ocfs2/acl.c new file +10 -0

+		if (IS_ERR(value))
+			return (int)PTR_ERR(value);
+	}
+
+	ret = ocfs2_xattr_set(inode, name_index, "", value, size, 0);
+
+	kfree(value);
+
+	return ret;
+}

Why don't you combine these two into one function which just calls ocfs2_xattr_set() when handle == NULL and otherwise calls ocfs2_xattr_set_handle()?

Otherwise, we could take the common part and make it it's own function which they both call.

	--Mark

--
Mark Fasheh

Tiger Yang

author

2008-10-30 11:41 UTC #31 lore raw

PATCH 12/15 ocfs2: add ocfs2_check_acl

In reply to Tiger Yang

This functions use to enhance permission checking with POSIX ACL.

Signed-off-by Tiger Yang <[email protected]>

---
 fs/ocfs2/acl.c  |   15 +++++++++++++++
 fs/ocfs2/acl.h  |   10 ++++++++++
 fs/ocfs2/file.c |    3 ++-
 3 files changed, 27 insertions(+), 1 deletions(-)

fs/ocfs2/acl.c +15 -0

index 1f20e73..584da84 100644
@@ -273,6 +273,21 @@ static int ocfs2_set_acl(struct inode *inode, int type, struct posix_acl *acl)
 	return ret;
 }
 
+int ocfs2_check_acl(struct inode *inode, int mask)
+{
+	struct posix_acl *acl = ocfs2_get_acl(inode, ACL_TYPE_ACCESS);
+
+	if (IS_ERR(acl))
+		return PTR_ERR(acl);
+	if (acl) {
+		int ret = posix_acl_permission(inode, acl, mask);
+		posix_acl_release(acl);
+		return ret;
+	}
+
+	return -EAGAIN;
+}
+
 static size_t ocfs2_xattr_list_acl_access(struct inode *inode,
 					  char *list,
 					  size_t list_len,

fs/ocfs2/acl.h +10 -0

index 1b39f3e..fef10f1 100644
@@ -26,4 +26,14 @@ struct ocfs2_acl_entry {
 	__le32 e_id;
 };
 
+#ifdef CONFIG_OCFS2_FS_POSIX_ACL
+
+extern int ocfs2_check_acl(struct inode *, int);
+
+#else /* CONFIG_OCFS2_FS_POSIX_ACL*/
+
+#define ocfs2_check_acl NULL
+
+#endif /* CONFIG_OCFS2_FS_POSIX_ACL*/
+
 #endif /* OCFS2_ACL_H */

fs/ocfs2/file.c +2 -1

index 1df4850..5a45b6b 100644
@@ -56,6 +56,7 @@
 #include "suballoc.h"
 #include "super.h"
 #include "xattr.h"
+#include "acl.h"
 
 #include "buffer_head_io.h"

@@ -1036,7 +1037,7 @@ int ocfs2_permission(struct inode *inode, int mask)
 		goto out;
 	}
 
-	ret = generic_permission(inode, mask, NULL);
+	ret = generic_permission(inode, mask, ocfs2_check_acl);
 
 	ocfs2_inode_unlock(inode, 0);
 out:

Signature · 1 line

1.5.4.1

Tiger Yang

author

2008-10-30 11:42 UTC #32 lore raw

PATCH 13/15 ocfs2: add ocfs2_acl_chmod

In reply to Tiger Yang

This function use to update acl xattrs when
file mode was changed.

Signed-off-by Tiger Yang <[email protected]>

---
 fs/ocfs2/acl.c  |   34 ++++++++++++++++++++++++++++++++++
 fs/ocfs2/acl.h  |    7 +++++++
 fs/ocfs2/file.c |    8 ++++++++
 3 files changed, 49 insertions(+), 0 deletions(-)

fs/ocfs2/acl.c +34 -0

index 584da84..7c7e4d4 100644
@@ -288,6 +288,40 @@ int ocfs2_check_acl(struct inode *inode, int mask)
 	return -EAGAIN;
 }
 
+int ocfs2_acl_chmod(handle_t *handle,
+		    struct inode *inode,
+		    struct buffer_head *di_bh)
+{
+	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
+	struct posix_acl *acl, *clone;
+	int ret;
+
+	if (S_ISLNK(inode->i_mode))
+		return -EOPNOTSUPP;
+
+	if (!(osb->s_mount_opt & OCFS2_MOUNT_POSIX_ACL))
+		return 0;
+
+	acl = ocfs2_get_acl_nolock(inode, ACL_TYPE_ACCESS, di_bh);
+	if (IS_ERR(acl) || !acl)
+		return PTR_ERR(acl);
+	clone = posix_acl_clone(acl, GFP_KERNEL);
+	posix_acl_release(acl);
+	if (!clone)
+		return -ENOMEM;
+	ret = posix_acl_chmod_masq(clone, inode->i_mode);
+	if (!ret)
+		/*
+		 * only replace same size acl here, so wouldn't
+		 * reserve metadata and clusters.
+		 */
+		ret = ocfs2_set_acl_handle(handle, inode, di_bh,
+					   ACL_TYPE_ACCESS, clone,
+					   NULL, NULL);
+	posix_acl_release(clone);
+	return ret;
+}
+
 static size_t ocfs2_xattr_list_acl_access(struct inode *inode,
 					  char *list,
 					  size_t list_len,

fs/ocfs2/acl.h +7 -0

index fef10f1..f7b1df7 100644
@@ -29,10 +29,17 @@ struct ocfs2_acl_entry {
 #ifdef CONFIG_OCFS2_FS_POSIX_ACL
 
 extern int ocfs2_check_acl(struct inode *, int);
+extern int ocfs2_acl_chmod(handle_t *, struct inode *, struct buffer_head *);
 
 #else /* CONFIG_OCFS2_FS_POSIX_ACL*/
 
 #define ocfs2_check_acl NULL
+static inline int ocfs2_acl_chmod(handle_t *handle,
+				  struct inode *inode,
+				  struct buffer_head *di_bh)
+{
+	return 0;
+}
 
 #endif /* CONFIG_OCFS2_FS_POSIX_ACL*/

fs/ocfs2/file.c +8 -0

index 5a45b6b..66d5395 100644
@@ -977,6 +977,14 @@ int ocfs2_setattr(struct dentry *dentry, struct iattr *attr)
 		goto bail_commit;
 	}
 
+	if (attr->ia_valid & ATTR_MODE) {
+		status = ocfs2_acl_chmod(handle, inode, bh);
+		if (status < 0) {
+			mlog_errno(status);
+			goto bail_commit;
+		}
+	}
+
 	status = ocfs2_mark_inode_dirty(handle, inode, bh);
 	if (status < 0)
 		mlog_errno(status);

Signature · 1 line

1.5.4.1

Mark Fasheh

2008-11-07 01:33 UTC #33 lore raw

PATCH 13/15 ocfs2: add ocfs2_acl_chmod

Quoted 86 lines · Tiger Yang

This function use to update acl xattrs when
file mode was changed.

Signed-off-by Tiger Yang <[email protected]>

---
 fs/ocfs2/acl.c  |   34 ++++++++++++++++++++++++++++++++++
 fs/ocfs2/acl.h  |    7 +++++++
 fs/ocfs2/file.c |    8 ++++++++
 3 files changed, 49 insertions(+), 0 deletions(-)

fs/ocfs2/acl.c +34 -0

index 584da84..7c7e4d4 100644
@@ -288,6 +288,40 @@ int ocfs2_check_acl(struct inode *inode, int mask)
 	return -EAGAIN;
 }
 
+int ocfs2_acl_chmod(handle_t *handle,
+		    struct inode *inode,
+		    struct buffer_head *di_bh)
+{
+	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
+	struct posix_acl *acl, *clone;
+	int ret;
+
+	if (S_ISLNK(inode->i_mode))
+		return -EOPNOTSUPP;
+
+	if (!(osb->s_mount_opt & OCFS2_MOUNT_POSIX_ACL))
+		return 0;
+
+	acl = ocfs2_get_acl_nolock(inode, ACL_TYPE_ACCESS, di_bh);
+	if (IS_ERR(acl) || !acl)
+		return PTR_ERR(acl);
+	clone = posix_acl_clone(acl, GFP_KERNEL);
+	posix_acl_release(acl);
+	if (!clone)
+		return -ENOMEM;
+	ret = posix_acl_chmod_masq(clone, inode->i_mode);
+	if (!ret)
+		/*
+		 * only replace same size acl here, so wouldn't
+		 * reserve metadata and clusters.
+		 */
+		ret = ocfs2_set_acl_handle(handle, inode, di_bh,
+					   ACL_TYPE_ACCESS, clone,
+					   NULL, NULL);
+	posix_acl_release(clone);
+	return ret;
+}
+
 static size_t ocfs2_xattr_list_acl_access(struct inode *inode,
 					  char *list,
 					  size_t list_len,

fs/ocfs2/acl.h +7 -0

index fef10f1..f7b1df7 100644
@@ -29,10 +29,17 @@ struct ocfs2_acl_entry {
 #ifdef CONFIG_OCFS2_FS_POSIX_ACL
 
 extern int ocfs2_check_acl(struct inode *, int);
+extern int ocfs2_acl_chmod(handle_t *, struct inode *, struct buffer_head *);
 
 #else /* CONFIG_OCFS2_FS_POSIX_ACL*/
 
 #define ocfs2_check_acl NULL
+static inline int ocfs2_acl_chmod(handle_t *handle,
+				  struct inode *inode,
+				  struct buffer_head *di_bh)
+{
+	return 0;
+}
 
 #endif /* CONFIG_OCFS2_FS_POSIX_ACL*/

fs/ocfs2/file.c +1 -0

index 5a45b6b..66d5395 100644
@@ -977,6 +977,14 @@ int ocfs2_setattr(struct dentry *dentry, struct iattr *attr)
 		goto bail_commit;
 	}
 
+	if (attr->ia_valid & ATTR_MODE) {

fs/ocfs2/file.c int ocfs2_setattr(struct dentry *dentry, struct iattr *attr) · line 990

Quoted 10 lines

fs/ocfs2/file.c +7 -0

+		status = ocfs2_acl_chmod(handle, inode, bh);
+		if (status < 0) {
+			mlog_errno(status);
+			goto bail_commit;
+		}
+	}
+
 	status = ocfs2_mark_inode_dirty(handle, inode, bh);
 	if (status < 0)
 		mlog_errno(status);

I think we need to increase the number of credits that the transaction started for this part of setattr uses.

This should be pretty easy though - the call to ocfs2_start_trans() is right above here, and it already takes OCFS2_INODE_UPDATE_CREDITS. I'd just use a 'credits' variable and increase it by however many is the maximum that might be required if the fs is mounted with acls turned on.

	--Mark

--
Mark Fasheh

Tiger Yang

author

2008-10-30 11:42 UTC #34 lore raw

PATCH 14/15 ocfs2: add ocfs2_init_acl in mknod

In reply to Tiger Yang

This function get the parent directory's acl, and let the child inherit it. We must calculate and reserve metadata and clusters for it need.

Signed-off-by Tiger Yang <[email protected]>

---
 fs/ocfs2/acl.c   |   61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/ocfs2/acl.h   |   14 ++++++++++++
 fs/ocfs2/namei.c |   13 +++++++++-
 fs/ocfs2/xattr.c |   48 ++++++++++++++++++++++++++++++++++++++---
 fs/ocfs2/xattr.h |    4 +-
 5 files changed, 132 insertions(+), 8 deletions(-)

fs/ocfs2/acl.c +61 -0

index 7c7e4d4..181ad1b 100644
@@ -322,6 +322,67 @@ int ocfs2_acl_chmod(handle_t *handle,
 	return ret;
 }
 
+/*
+ * Initialize the ACLs of a new inode. If parent directory has default ACL,
+ * then clone to new inode. Called from ocfs2_mknod.
+ */
+int ocfs2_init_acl(handle_t *handle,
+		   struct inode *inode,
+		   struct inode *dir,
+		   struct buffer_head *di_bh,
+		   struct buffer_head *dir_bh,
+		   struct ocfs2_alloc_context *meta_ac,
+		   struct ocfs2_alloc_context *data_ac)
+{
+	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
+	struct posix_acl *acl = NULL;
+	int ret = 0;
+
+	if (!S_ISLNK(inode->i_mode)) {
+		if (osb->s_mount_opt & OCFS2_MOUNT_POSIX_ACL) {
+			acl = ocfs2_get_acl_nolock(dir, ACL_TYPE_DEFAULT,
+						   dir_bh);
+			if (IS_ERR(acl))
+				return PTR_ERR(acl);
+		}
+		if (!acl)
+			inode->i_mode &= ~current->fs->umask;
+	}
+	if ((osb->s_mount_opt & OCFS2_MOUNT_POSIX_ACL) && acl) {
+		struct posix_acl *clone;
+		mode_t mode;
+
+		if (S_ISDIR(inode->i_mode)) {
+			ret = ocfs2_set_acl_handle(handle, inode, di_bh,
+						   ACL_TYPE_DEFAULT, acl,
+						   meta_ac, data_ac);
+			if (ret)
+				goto cleanup;
+		}
+		clone = posix_acl_clone(acl, GFP_NOFS);
+		ret = -ENOMEM;
+		if (!clone)
+			goto cleanup;
+
+		mode = inode->i_mode;
+		ret = posix_acl_create_masq(clone, &mode);
+		if (ret >= 0) {
+			inode->i_mode = mode;
+			if (ret > 0) {
+				ret = ocfs2_set_acl_handle(handle, inode,
+							   di_bh,
+							   ACL_TYPE_ACCESS,
+							   clone,
+							   meta_ac, data_ac);
+			}
+		}
+		posix_acl_release(clone);
+	}
+cleanup:
+	posix_acl_release(acl);
+	return ret;
+}
+
 static size_t ocfs2_xattr_list_acl_access(struct inode *inode,
 					  char *list,
 					  size_t list_len,

fs/ocfs2/acl.h +14 -0

index f7b1df7..bd48bf4 100644
@@ -30,6 +30,10 @@ struct ocfs2_acl_entry {
 
 extern int ocfs2_check_acl(struct inode *, int);
 extern int ocfs2_acl_chmod(handle_t *, struct inode *, struct buffer_head *);
+extern int ocfs2_init_acl(handle_t *, struct inode *, struct inode *,
+			  struct buffer_head *, struct buffer_head *,
+			  struct ocfs2_alloc_context *,
+			  struct ocfs2_alloc_context *);
 
 #else /* CONFIG_OCFS2_FS_POSIX_ACL*/

@@ -40,6 +44,16 @@ static inline int ocfs2_acl_chmod(handle_t *handle,
 {
 	return 0;
 }
+static inline int ocfs2_init_acl(handle_t *handle,
+				 struct inode *inode,
+				 struct inode *dir,
+				 struct buffer_head *di_bh,
+				 struct buffer_head *dir_bh,
+				 struct ocfs2_alloc_context *meta_ac,
+				 struct ocfs2_alloc_context *data_ac)
+{
+	return 0;
+}
 
 #endif /* CONFIG_OCFS2_FS_POSIX_ACL*/

fs/ocfs2/namei.c +11 -2

index 6ec42ef..c8781d5 100644
@@ -61,6 +61,7 @@
 #include "sysfile.h"
 #include "uptodate.h"
 #include "xattr.h"
+#include "acl.h"
 
 #include "buffer_head_io.h"

@@ -290,8 +291,9 @@ static int ocfs2_mknod(struct inode *dir,
 		}
 	}
 
-	/* calculate meta data/clusters for setting security xattr */
-	status = ocfs2_calc_xattr_init(dir, strlen(security_name),
+	/* calculate meta data/clusters for setting security and acl xattrs */
+	status = ocfs2_calc_xattr_init(dir, parent_fe_bh, mode,
+					strlen(security_name),
 					security_len, has_security,
 					&want_clusters, &xattr_ac);
 	if (status < 0) {

@@ -350,6 +352,13 @@ static int ocfs2_mknod(struct inode *dir,
 		inc_nlink(dir);
 	}
 
+	status = ocfs2_init_acl(handle, inode, dir, new_fe_bh, parent_fe_bh,
+				xattr_ac, data_ac);
+	if (status < 0) {
+		mlog_errno(status);
+		goto leave;
+	}
+
 	if (has_security) {
 		status = ocfs2_init_security_set(handle, inode, new_fe_bh,
 						 security_name, security_value,

fs/ocfs2/xattr.c +44 -4

84 lines not on this page · raw message

fs/ocfs2/xattr.h +2 -2

index b8ab4dc..de0f172 100644
@@ -56,8 +56,8 @@ int ocfs2_init_security_set(handle_t *, struct inode *,
 			    char *, void *, size_t,
 			    struct ocfs2_alloc_context *,
 			    struct ocfs2_alloc_context *);
-int ocfs2_calc_xattr_init(struct inode *, int, size_t,
-			  int, int *,
+int ocfs2_calc_xattr_init(struct inode *, struct buffer_head *,
+			  int, int, size_t, int, int *,
 			  struct ocfs2_alloc_context **);
 
 #endif /* OCFS2_XATTR_H */

Signature · 1 line

1.5.4.1

Tiger Yang

author

2008-10-30 11:42 UTC #35 lore raw

PATCH 15/15 ocfs2: add mount option and Kconfig option for acl

In reply to Tiger Yang

This patch add Kconfig option "CONFIG_OCFS2_FS_POSIX_ACL" and mount option "acl" for enable acl.

Signed-off-by Tiger Yang <[email protected]>

---
 Documentation/filesystems/ocfs2.txt |    4 ++--
 fs/Kconfig                          |    9 +++++++++
 fs/ocfs2/super.c                    |   33 +++++++++++++++++++++++++++++++++
 3 files changed, 44 insertions(+), 2 deletions(-)

Documentation/filesystems/ocfs2.txt +2 -2

index 4340cc8..578209c 100644
@@ -28,13 +28,11 @@ Manish Singh  <[email protected]>
 Caveats
 =======
 Features which OCFS2 does not support yet:
-	- extended attributes
 	- quotas
 	- cluster aware flock
 	- cluster aware lockf
 	- Directory change notification (F_NOTIFY)
 	- Distributed Caching (F_SETLEASE/F_GETLEASE/break_lease)
-	- POSIX ACLs
 
 Mount options
 =============

@@ -82,3 +80,5 @@ inode64			Indicates that Ocfs2 is allowed to create inodes at
 			bits of significance.
 user_xattr	(*)	Enables Extended User Attributes.
 nouser_xattr		Disables Extended User Attributes.
+acl			Enables POSIX Access Control Lists support.
+noacl		(*)	Disables POSIX Access Control Lists support.

fs/Kconfig +9 -0

index 522469a..3af6024 100644
@@ -268,6 +268,15 @@ config OCFS2_COMPAT_JBD
 	  is backwards compatible with JBD.  It is safe to say N here.
 	  However, if you really want to use the original JBD, say Y here.
 
+config OCFS2_FS_POSIX_ACL
+	bool "OCFS2 POSIX Access Control Lists"
+	depends on OCFS2_FS
+	select FS_POSIX_ACL
+	default n
+	help
+	  Posix Access Control Lists (ACLs) support permissions for users and
+	  groups beyond the owner/group/world scheme.
+
 endif # BLOCK
 
 config DNOTIFY

fs/ocfs2/super.c +33 -0

index 304b63a..9e7accc 100644
@@ -158,6 +158,8 @@ enum {
 	Opt_user_xattr,
 	Opt_nouser_xattr,
 	Opt_inode64,
+	Opt_acl,
+	Opt_noacl,
 	Opt_err,
 };

@@ -180,6 +182,8 @@ static const match_table_t tokens = {
 	{Opt_user_xattr, "user_xattr"},
 	{Opt_nouser_xattr, "nouser_xattr"},
 	{Opt_inode64, "inode64"},
+	{Opt_acl, "acl"},
+	{Opt_noacl, "noacl"},
 	{Opt_err, NULL}
 };

@@ -466,6 +470,8 @@ unlock_osb:
 	if (!ret) {
 		/* Only save off the new mount options in case of a successful
 		 * remount. */
+		if (!(osb->s_feature_incompat & OCFS2_FEATURE_INCOMPAT_XATTR))
+			parsed_options.mount_opt &= ~OCFS2_MOUNT_POSIX_ACL;
 		osb->s_mount_opt = parsed_options.mount_opt;
 		osb->s_atime_quantum = parsed_options.atime_quantum;
 		osb->preferred_slot = parsed_options.slot;

@@ -651,6 +657,10 @@ static int ocfs2_fill_super(struct super_block *sb, void *data, int silent)
 	}
 	brelse(bh);
 	bh = NULL;
+
+	if (!(osb->s_feature_incompat & OCFS2_FEATURE_INCOMPAT_XATTR))
+		parsed_options.mount_opt &= ~OCFS2_MOUNT_POSIX_ACL;
+
 	osb->s_mount_opt = parsed_options.mount_opt;
 	osb->s_atime_quantum = parsed_options.atime_quantum;
 	osb->preferred_slot = parsed_options.slot;

@@ -664,6 +674,9 @@ static int ocfs2_fill_super(struct super_block *sb, void *data, int silent)
 
 	sb->s_magic = OCFS2_SUPER_MAGIC;
 
+	sb->s_flags = (sb->s_flags & ~MS_POSIXACL) |
+		((osb->s_mount_opt & OCFS2_MOUNT_POSIX_ACL) ? MS_POSIXACL : 0);
+
 	/* Hard readonly mode only if: bdev_read_only, MS_RDONLY,
 	 * heartbeat=none */
 	if (bdev_read_only(sb->s_bdev)) {

@@ -945,6 +958,19 @@ static int ocfs2_parse_options(struct super_block *sb,
 		case Opt_inode64:
 			mopt->mount_opt |= OCFS2_MOUNT_INODE64;
 			break;
+#ifdef CONFIG_OCFS2_FS_POSIX_ACL
+		case Opt_acl:
+			mopt->mount_opt |= OCFS2_MOUNT_POSIX_ACL;
+			break;
+		case Opt_noacl:
+			mopt->mount_opt &= ~OCFS2_MOUNT_POSIX_ACL;
+			break;
+#else
+		case Opt_acl:
+		case Opt_noacl:
+			printk(KERN_INFO "ocfs2 (no)acl options not supported\n");
+			break;
+#endif
 		default:
 			mlog(ML_ERROR,
 			     "Unrecognized mount option \"%s\" "

@@ -1017,6 +1043,13 @@ static int ocfs2_show_options(struct seq_file *s, struct vfsmount *mnt)
 	if (opts & OCFS2_MOUNT_INODE64)
 		seq_printf(s, ",inode64");
 
+#ifdef CONFIG_OCFS2_FS_POSIX_ACL
+	if (opts & OCFS2_MOUNT_POSIX_ACL)
+		seq_printf(s, ",acl");
+	else
+		seq_printf(s, ",noacl");
+#endif
+
 	return 0;
 }

Signature · 1 line

1.5.4.1

Tao Ma

2008-10-30 13:45 UTC #36 lore raw

cover ocfs2: add security EA and ACL support V4

In reply to Tiger Yang

Quoted 6 lines · Tiger Yang

hi,
This is the version 4 patches for add security EA and ACL support in ocfs2. We have fixed many problems in version 3 and I split the previous big patch into some small patches. Them based on joel's xattr_buckets branch.

Quoted 7 lines

The first five patches from Tao Ma.
1: only extend xattr bucket in need
2: add cluster free in dealloc ctxt
3: move clusters free into dealloc
4: reserve meta data at the begining of ocfs2_xattr_set 5: merge xattr set transactions

One more thing. These patches are based on Joel's xattr-buckets branch. Joel, please review them. Thanks.

Regards,
Tao