* [Ocfs2-devel] [PATCH 01/14] ocfs2: Introduce ocfs2_xa_loc
2009-08-19 19:54 [Ocfs2-devel] [PATCH 0/14] ocfs2: Unify the setting of extended attributes Joel Becker
@ 2009-08-19 19:54 ` Joel Becker
2009-08-27 7:48 ` Tao Ma
2009-08-19 19:54 ` [Ocfs2-devel] [PATCH 02/14] ocfs2: Remove xattrs via ocfs2_xa_loc Joel Becker
` (13 subsequent siblings)
14 siblings, 1 reply; 33+ messages in thread
From: Joel Becker @ 2009-08-19 19:54 UTC (permalink / raw)
To: ocfs2-devel
The ocfs2 extended attribute (xattr) code is very flexible. It can
store xattrs in the inode itself, in an external block, or in a tree of
data structures. This allows the number of xattrs to be bounded by the
filesystem size.
However, the code that manages each possible storage location is
different. Maintaining the ocfs2 xattr code requires changing each hunk
separately.
This patch is the start of a series introducing the ocfs2_xa_loc
structure. This structure wraps the on-disk details of an xattr
entry. The goal is that the generic xattr routines can use
ocfs2_xa_loc without knowing the underlying storage location.
This first pass merely implements the basic structure, initializing it,
and wiping the name+value pair of the entry.
Signed-off-by: Joel Becker <joel.becker@oracle.com>
---
fs/ocfs2/xattr.c | 243 ++++++++++++++++++++++++++++++++++++++++++++++++++----
1 files changed, 228 insertions(+), 15 deletions(-)
diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index d1a27cd..953cf32 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -140,6 +140,51 @@ struct ocfs2_xattr_search {
int not_found;
};
+/* Operations on struct ocfs2_xa_entry */
+struct ocfs2_xa_loc;
+struct ocfs2_xa_loc_operations {
+ /*
+ * Return a pointer to the appropriate buffer in loc->xl_storage
+ * at the given offset from loc->xl_header.
+ */
+ void *(*xlo_offset_pointer)(struct ocfs2_xa_loc *loc, int offset);
+
+ /*
+ * Remove the name+value at this location. Do whatever is
+ * appropriate with the remaining name+value pairs.
+ */
+ void (*xlo_wipe_namevalue)(struct ocfs2_xa_loc *loc);
+};
+
+/*
+ * Describes an xattr entry location. This is a memory structure
+ * tracking the on-disk structure.
+ */
+struct ocfs2_xa_loc {
+ /* The ocfs2_xattr_header inside the on-disk storage. Not NULL. */
+ struct ocfs2_xattr_header *xl_header;
+
+ /* Bytes from xl_header to the end of the storage */
+ int xl_size;
+
+ /*
+ * The ocfs2_xattr_entry this location describes. If this is
+ * NULL, this location describes the on-disk structure where it
+ * would have been.
+ */
+ struct ocfs2_xattr_entry *xl_entry;
+
+ /*
+ * Internal housekeeping
+ */
+
+ /* Buffer(s) containing this entry */
+ void *xl_storage;
+
+ /* Operations on the storage backing this location */
+ const struct ocfs2_xa_loc_operations *xl_ops;
+};
+
static int ocfs2_xattr_bucket_get_name_value(struct inode *inode,
struct ocfs2_xattr_header *xh,
int index,
@@ -1364,6 +1409,171 @@ static int ocfs2_xattr_set_value_outside(struct inode *inode,
}
/*
+ * Wipe the name+value pair and allow the storage to reclaim it. This
+ * must be followed by either removal of the entry or a call to
+ * ocfs2_xa_add_namevalue().
+ */
+static void ocfs2_xa_wipe_namevalue(struct ocfs2_xa_loc *loc)
+{
+ loc->xl_ops->xlo_wipe_namevalue(loc);
+}
+
+static void *ocfs2_xa_block_offset_pointer(struct ocfs2_xa_loc *loc,
+ int offset)
+{
+ struct buffer_head *bh = loc->xl_storage;
+
+ BUG_ON(offset >= bh->b_size);
+ return bh->b_data + offset;
+}
+
+/*
+ * Block storage for xattrs keeps the name+value pairs compacted. When
+ * we remove one, we have to shift any that preceded it towards the end.
+ */
+static void ocfs2_xa_block_wipe_namevalue(struct ocfs2_xa_loc *loc)
+{
+ int i, offset;
+ int namevalue_offset, first_namevalue_offset, namevalue_size;
+ struct ocfs2_xattr_entry *entry = loc->xl_entry;
+ struct ocfs2_xattr_header *xh = loc->xl_header;
+ u64 value_size = le64_to_cpu(entry->xe_value_size);
+ int count = le16_to_cpu(xh->xh_count);
+
+ namevalue_offset = le16_to_cpu(entry->xe_name_offset);
+ namevalue_size = OCFS2_XATTR_SIZE(entry->xe_name_len);
+ if (value_size > OCFS2_XATTR_INLINE_SIZE)
+ namevalue_size += OCFS2_XATTR_ROOT_SIZE;
+ else
+ namevalue_size += OCFS2_XATTR_SIZE(value_size);
+
+ for (i = 0, first_namevalue_offset = loc->xl_size;
+ i < count; i++) {
+ offset = le16_to_cpu(xh->xh_entries[i].xe_name_offset);
+ if (offset < first_namevalue_offset)
+ first_namevalue_offset = offset;
+ }
+
+ /* Shift the name+value pairs */
+ memmove((char *)xh + first_namevalue_offset + namevalue_size,
+ (char *)xh + first_namevalue_offset,
+ namevalue_offset - first_namevalue_offset);
+ memset((char *)xh + first_namevalue_offset, 0, namevalue_size);
+
+ /* Now tell xh->xh_entries about it */
+ for (i = 0; i < count; i++) {
+ offset = le16_to_cpu(xh->xh_entries[i].xe_name_offset);
+ if (offset < namevalue_offset)
+ le16_add_cpu(&xh->xh_entries[i].xe_name_offset,
+ namevalue_size);
+ }
+
+ /*
+ * Note that we don't update xh_free_start or xh_name_value_len
+ * because they're not used in block-stored xattrs.
+ */
+}
+
+/*
+ * Operations for xattrs stored in blocks. This includes inline inode
+ * storage and unindexed ocfs2_xattr_blocks.
+ */
+static const struct ocfs2_xa_loc_operations ocfs2_xa_block_loc_ops = {
+ .xlo_offset_pointer = ocfs2_xa_block_offset_pointer,
+ .xlo_wipe_namevalue = ocfs2_xa_block_wipe_namevalue,
+};
+
+static void *ocfs2_xa_bucket_offset_pointer(struct ocfs2_xa_loc *loc,
+ int offset)
+{
+ struct ocfs2_xattr_bucket *bucket = loc->xl_storage;
+ int block, block_offset;
+
+ BUG_ON(offset >= OCFS2_XATTR_BUCKET_SIZE);
+
+ block = offset >> bucket->bu_inode->i_sb->s_blocksize_bits;
+ block_offset = offset % bucket->bu_inode->i_sb->s_blocksize;
+
+ return bucket_block(bucket, block) + block_offset;
+}
+
+static void ocfs2_xa_bucket_wipe_namevalue(struct ocfs2_xa_loc *loc)
+{
+ int namevalue_size;
+ struct ocfs2_xattr_entry *entry = loc->xl_entry;
+ u64 value_size = le64_to_cpu(entry->xe_value_size);
+
+ namevalue_size = OCFS2_XATTR_SIZE(entry->xe_name_len);
+ if (value_size > OCFS2_XATTR_INLINE_SIZE)
+ namevalue_size += OCFS2_XATTR_ROOT_SIZE;
+ else
+ namevalue_size += OCFS2_XATTR_SIZE(value_size);
+
+ le16_add_cpu(&loc->xl_header->xh_name_value_len, -namevalue_size);
+}
+
+/* Operations for xattrs stored in buckets. */
+static const struct ocfs2_xa_loc_operations ocfs2_xa_bucket_loc_ops = {
+ .xlo_offset_pointer = ocfs2_xa_bucket_offset_pointer,
+ .xlo_wipe_namevalue = ocfs2_xa_bucket_wipe_namevalue,
+};
+
+static void ocfs2_xa_remove_entry(struct ocfs2_xa_loc *loc)
+{
+ ocfs2_xa_wipe_namevalue(loc);
+}
+
+static void ocfs2_init_dinode_xa_loc(struct ocfs2_xa_loc *loc,
+ struct inode *inode,
+ struct buffer_head *bh,
+ struct ocfs2_xattr_entry *entry)
+{
+ struct ocfs2_dinode *di = (struct ocfs2_dinode *)bh->b_data;
+
+ loc->xl_ops = &ocfs2_xa_block_loc_ops;
+ loc->xl_storage = bh;
+ loc->xl_entry = entry;
+
+ if (OCFS2_I(inode)->ip_dyn_features & OCFS2_INLINE_XATTR_FL)
+ loc->xl_size = le16_to_cpu(di->i_xattr_inline_size);
+ else {
+ BUG_ON(entry);
+ loc->xl_size = OCFS2_SB(inode->i_sb)->s_xattr_inline_size;
+ }
+ loc->xl_header =
+ (struct ocfs2_xattr_header *)(bh->b_data + bh->b_size -
+ loc->xl_size);
+}
+
+static void ocfs2_init_xattr_block_xa_loc(struct ocfs2_xa_loc *loc,
+ struct buffer_head *bh,
+ struct ocfs2_xattr_entry *entry)
+{
+ struct ocfs2_xattr_block *xb =
+ (struct ocfs2_xattr_block *)bh->b_data;
+
+ BUG_ON(le16_to_cpu(xb->xb_flags) & OCFS2_XATTR_INDEXED);
+
+ loc->xl_ops = &ocfs2_xa_block_loc_ops;
+ loc->xl_storage = bh;
+ loc->xl_header = &(xb->xb_attrs.xb_header);
+ loc->xl_entry = entry;
+ loc->xl_size = bh->b_size - offsetof(struct ocfs2_xattr_block,
+ xb_attrs.xb_header);
+}
+
+static void ocfs2_init_xattr_bucket_xa_loc(struct ocfs2_xa_loc *loc,
+ struct ocfs2_xattr_bucket *bucket,
+ struct ocfs2_xattr_entry *entry)
+{
+ loc->xl_ops = &ocfs2_xa_bucket_loc_ops;
+ loc->xl_storage = bucket;
+ loc->xl_header = bucket_xh(bucket);
+ loc->xl_entry = entry;
+ loc->xl_size = OCFS2_XATTR_BUCKET_SIZE;
+}
+
+/*
* ocfs2_xattr_set_entry_local()
*
* Set, replace or remove extended attribute in local.
@@ -1376,7 +1586,14 @@ static void ocfs2_xattr_set_entry_local(struct inode *inode,
{
size_t name_len = strlen(xi->name);
int i;
+ struct ocfs2_xa_loc loc;
+ if (xs->xattr_bh == xs->inode_bh)
+ ocfs2_init_dinode_xa_loc(&loc, inode, xs->inode_bh,
+ xs->not_found ? NULL : xs->here);
+ else
+ ocfs2_init_xattr_block_xa_loc(&loc, xs->xattr_bh,
+ xs->not_found ? NULL : xs->here);
if (xi->value && xs->not_found) {
/* Insert the new xattr entry. */
le16_add_cpu(&xs->header->xh_count, 1);
@@ -1415,9 +1632,9 @@ static void ocfs2_xattr_set_entry_local(struct inode *inode,
xi->value_len);
return;
}
+
/* Remove the old name+value. */
- memmove(first_val + size, first_val, val - first_val);
- memset(first_val, 0, size);
+ ocfs2_xa_wipe_namevalue(&loc);
xs->here->xe_name_hash = 0;
xs->here->xe_name_offset = 0;
ocfs2_xattr_set_local(xs->here, 1);
@@ -1425,23 +1642,16 @@ static void ocfs2_xattr_set_entry_local(struct inode *inode,
min_offs += size;
- /* Adjust all value offsets. */
- last = xs->header->xh_entries;
- for (i = 0 ; i < le16_to_cpu(xs->header->xh_count); i++) {
- size_t o = le16_to_cpu(last->xe_name_offset);
-
- if (o < offs)
- last->xe_name_offset = cpu_to_le16(o + size);
- last += 1;
- }
-
if (!xi->value) {
/* Remove the old entry. */
- last -= 1;
+ i = le16_to_cpu(xs->header->xh_count);
+ i--;
+ last = &xs->header->xh_entries[i - 1];
+ xs->header->xh_count = cpu_to_le16(i);
+
memmove(xs->here, xs->here + 1,
(void *)last - (void *)xs->here);
memset(last, 0, sizeof(struct ocfs2_xattr_entry));
- le16_add_cpu(&xs->header->xh_count, -1);
}
}
if (xi->value) {
@@ -4527,7 +4737,10 @@ static void ocfs2_xattr_set_entry_normal(struct inode *inode,
size_t blocksize = inode->i_sb->s_blocksize;
char *val;
size_t offs, size, new_size;
+ struct ocfs2_xa_loc loc;
+ ocfs2_init_xattr_bucket_xa_loc(&loc, xs->bucket,
+ xs->not_found ? NULL : xs->here);
last = &xh->xh_entries[count];
if (!xs->not_found) {
xe = xs->here;
@@ -4548,7 +4761,7 @@ static void ocfs2_xattr_set_entry_normal(struct inode *inode,
new_size = OCFS2_XATTR_SIZE(name_len) +
OCFS2_XATTR_SIZE(xi->value_len);
- le16_add_cpu(&xh->xh_name_value_len, -size);
+ ocfs2_xa_wipe_namevalue(&loc);
if (xi->value) {
if (new_size > size)
goto set_new_name_value;
--
1.6.3.3
^ permalink raw reply related [flat|nested] 33+ messages in thread* [Ocfs2-devel] [PATCH 01/14] ocfs2: Introduce ocfs2_xa_loc
2009-08-19 19:54 ` [Ocfs2-devel] [PATCH 01/14] ocfs2: Introduce ocfs2_xa_loc Joel Becker
@ 2009-08-27 7:48 ` Tao Ma
2009-08-27 9:28 ` Joel Becker
0 siblings, 1 reply; 33+ messages in thread
From: Tao Ma @ 2009-08-27 7:48 UTC (permalink / raw)
To: ocfs2-devel
Hi Joel,
Joel Becker wrote:
> The ocfs2 extended attribute (xattr) code is very flexible. It can
> store xattrs in the inode itself, in an external block, or in a tree of
> data structures. This allows the number of xattrs to be bounded by the
> filesystem size.
>
> However, the code that manages each possible storage location is
> different. Maintaining the ocfs2 xattr code requires changing each hunk
> separately.
>
> This patch is the start of a series introducing the ocfs2_xa_loc
> structure. This structure wraps the on-disk details of an xattr
> entry. The goal is that the generic xattr routines can use
> ocfs2_xa_loc without knowing the underlying storage location.
>
> This first pass merely implements the basic structure, initializing it,
> and wiping the name+value pair of the entry.
>
> Signed-off-by: Joel Becker <joel.becker@oracle.com>
> ---
> fs/ocfs2/xattr.c | 243 ++++++++++++++++++++++++++++++++++++++++++++++++++----
> 1 files changed, 228 insertions(+), 15 deletions(-)
>
> diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
> index d1a27cd..953cf32 100644
> --- a/fs/ocfs2/xattr.c
> +++ b/fs/ocfs2/xattr.c
> @@ -140,6 +140,51 @@ struct ocfs2_xattr_search {
> int not_found;
> };
<snip>
> +static void ocfs2_xa_bucket_wipe_namevalue(struct ocfs2_xa_loc *loc)
> +{
> + int namevalue_size;
> + struct ocfs2_xattr_entry *entry = loc->xl_entry;
> + u64 value_size = le64_to_cpu(entry->xe_value_size);
> +
> + namevalue_size = OCFS2_XATTR_SIZE(entry->xe_name_len);
> + if (value_size > OCFS2_XATTR_INLINE_SIZE)
> + namevalue_size += OCFS2_XATTR_ROOT_SIZE;
> + else
> + namevalue_size += OCFS2_XATTR_SIZE(value_size);
A minor issue. I see a similar part in the function in
ocfs2_xa_block_wipe_namevalue, so can we create a inline function for
it? maybe ocfs2_xe_name_value_size?
> + le16_add_cpu(&loc->xl_header->xh_name_value_len, -namevalue_size);
> +}
> +
> +/* Operations for xattrs stored in buckets. */
> +static const struct ocfs2_xa_loc_operations ocfs2_xa_bucket_loc_ops = {
> + .xlo_offset_pointer = ocfs2_xa_bucket_offset_pointer,
> + .xlo_wipe_namevalue = ocfs2_xa_bucket_wipe_namevalue,
> +};
> +
<snip>
> @@ -1376,7 +1586,14 @@ static void ocfs2_xattr_set_entry_local(struct inode *inode,
> {
> size_t name_len = strlen(xi->name);
> int i;
> + struct ocfs2_xa_loc loc;
>
> + if (xs->xattr_bh == xs->inode_bh)
> + ocfs2_init_dinode_xa_loc(&loc, inode, xs->inode_bh,
> + xs->not_found ? NULL : xs->here);
> + else
> + ocfs2_init_xattr_block_xa_loc(&loc, xs->xattr_bh,
> + xs->not_found ? NULL : xs->here);
> if (xi->value && xs->not_found) {
> /* Insert the new xattr entry. */
> le16_add_cpu(&xs->header->xh_count, 1);
> @@ -1415,9 +1632,9 @@ static void ocfs2_xattr_set_entry_local(struct inode *inode,
> xi->value_len);
> return;
> }
> +
> /* Remove the old name+value. */
> - memmove(first_val + size, first_val, val - first_val);
> - memset(first_val, 0, size);
> + ocfs2_xa_wipe_namevalue(&loc);
> xs->here->xe_name_hash = 0;
> xs->here->xe_name_offset = 0;
> ocfs2_xattr_set_local(xs->here, 1);
> @@ -1425,23 +1642,16 @@ static void ocfs2_xattr_set_entry_local(struct inode *inode,
>
> min_offs += size;
>
> - /* Adjust all value offsets. */
> - last = xs->header->xh_entries;
> - for (i = 0 ; i < le16_to_cpu(xs->header->xh_count); i++) {
> - size_t o = le16_to_cpu(last->xe_name_offset);
> -
> - if (o < offs)
> - last->xe_name_offset = cpu_to_le16(o + size);
> - last += 1;
> - }
> -
> if (!xi->value) {
> /* Remove the old entry. */
> - last -= 1;
> + i = le16_to_cpu(xs->header->xh_count);
> + i--;
any reason why not "i = le16_to_cpu(xs->header->xh_count) - 1"?
> + last = &xs->header->xh_entries[i - 1];
here is a bug. last should be "&xs->header->xh_entries[i]". In the old
implementation, the above "for" loop ends with last = xh_entries[xh_count].
> + xs->header->xh_count = cpu_to_le16(i);
> +
> memmove(xs->here, xs->here + 1,
> (void *)last - (void *)xs->here);
> memset(last, 0, sizeof(struct ocfs2_xattr_entry));
> - le16_add_cpu(&xs->header->xh_count, -1);
> }
> }
> if (xi->value) {
Regards,
Tao
^ permalink raw reply [flat|nested] 33+ messages in thread* [Ocfs2-devel] [PATCH 01/14] ocfs2: Introduce ocfs2_xa_loc
2009-08-27 7:48 ` Tao Ma
@ 2009-08-27 9:28 ` Joel Becker
0 siblings, 0 replies; 33+ messages in thread
From: Joel Becker @ 2009-08-27 9:28 UTC (permalink / raw)
To: ocfs2-devel
On Thu, Aug 27, 2009 at 03:48:19PM +0800, Tao Ma wrote:
> Joel Becker wrote:
> >+ u64 value_size = le64_to_cpu(entry->xe_value_size);
> >+
> >+ namevalue_size = OCFS2_XATTR_SIZE(entry->xe_name_len);
> >+ if (value_size > OCFS2_XATTR_INLINE_SIZE)
> >+ namevalue_size += OCFS2_XATTR_ROOT_SIZE;
> >+ else
> >+ namevalue_size += OCFS2_XATTR_SIZE(value_size);
> A minor issue. I see a similar part in the function in
> ocfs2_xa_block_wipe_namevalue, so can we create a inline function
> for it? maybe ocfs2_xe_name_value_size?
See patch 5 :-)
> >@@ -1376,7 +1586,14 @@ static void ocfs2_xattr_set_entry_local(struct inode *inode,
> > {
> > size_t name_len = strlen(xi->name);
> > int i;
> >+ struct ocfs2_xa_loc loc;
> >+ if (xs->xattr_bh == xs->inode_bh)
> >+ ocfs2_init_dinode_xa_loc(&loc, inode, xs->inode_bh,
> >+ xs->not_found ? NULL : xs->here);
> >+ else
> >+ ocfs2_init_xattr_block_xa_loc(&loc, xs->xattr_bh,
> >+ xs->not_found ? NULL : xs->here);
> > if (xi->value && xs->not_found) {
> > /* Insert the new xattr entry. */
> > le16_add_cpu(&xs->header->xh_count, 1);
> >@@ -1415,9 +1632,9 @@ static void ocfs2_xattr_set_entry_local(struct inode *inode,
> > xi->value_len);
> > return;
> > }
> >+
> > /* Remove the old name+value. */
> >- memmove(first_val + size, first_val, val - first_val);
> >- memset(first_val, 0, size);
> >+ ocfs2_xa_wipe_namevalue(&loc);
> > xs->here->xe_name_hash = 0;
> > xs->here->xe_name_offset = 0;
> > ocfs2_xattr_set_local(xs->here, 1);
> >@@ -1425,23 +1642,16 @@ static void ocfs2_xattr_set_entry_local(struct inode *inode,
> > min_offs += size;
> >- /* Adjust all value offsets. */
> >- last = xs->header->xh_entries;
> >- for (i = 0 ; i < le16_to_cpu(xs->header->xh_count); i++) {
> >- size_t o = le16_to_cpu(last->xe_name_offset);
> >-
> >- if (o < offs)
> >- last->xe_name_offset = cpu_to_le16(o + size);
> >- last += 1;
> >- }
> >-
> > if (!xi->value) {
> > /* Remove the old entry. */
> >- last -= 1;
> >+ i = le16_to_cpu(xs->header->xh_count);
> >+ i--;
> any reason why not "i = le16_to_cpu(xs->header->xh_count) - 1"?
No good one. I moved these couple lines around a bit, so I
probably was just adjusting.
> >+ last = &xs->header->xh_entries[i - 1];
> here is a bug. last should be "&xs->header->xh_entries[i]". In the
> old implementation, the above "for" loop ends with last =
> xh_entries[xh_count].
My brain wants to protest, but every time I walk the logic you
come out right. Maybe this used to be above the i-- or something.
Good catch.
Joel
--
"Always give your best, never get discouraged, never be petty; always
remember, others may hate you. Those who hate you don't win unless
you hate them. And then you destroy yourself."
- Richard M. Nixon
Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127
^ permalink raw reply [flat|nested] 33+ messages in thread
* [Ocfs2-devel] [PATCH 02/14] ocfs2: Remove xattrs via ocfs2_xa_loc
2009-08-19 19:54 [Ocfs2-devel] [PATCH 0/14] ocfs2: Unify the setting of extended attributes Joel Becker
2009-08-19 19:54 ` [Ocfs2-devel] [PATCH 01/14] ocfs2: Introduce ocfs2_xa_loc Joel Becker
@ 2009-08-19 19:54 ` Joel Becker
2009-08-19 19:54 ` [Ocfs2-devel] [PATCH 03/14] ocfs2: Prefix the member fields of struct ocfs2_xattr_info Joel Becker
` (12 subsequent siblings)
14 siblings, 0 replies; 33+ messages in thread
From: Joel Becker @ 2009-08-19 19:54 UTC (permalink / raw)
To: ocfs2-devel
Add ocfs2_xa_remove_entry(), which will remove an xattr entry from its
storage via the ocfs2_xa_loc descriptor.
Signed-off-by: Joel Becker <joel.becker@oracle.com>
---
fs/ocfs2/xattr.c | 62 ++++++++++++++++++++++++-----------------------------
1 files changed, 28 insertions(+), 34 deletions(-)
diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index 953cf32..f42edb1 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -1520,7 +1520,28 @@ static const struct ocfs2_xa_loc_operations ocfs2_xa_bucket_loc_ops = {
static void ocfs2_xa_remove_entry(struct ocfs2_xa_loc *loc)
{
+ int index, count;
+ struct ocfs2_xattr_header *xh = loc->xl_header;
+ struct ocfs2_xattr_entry *entry = loc->xl_entry;
+
ocfs2_xa_wipe_namevalue(loc);
+
+ le16_add_cpu(&xh->xh_count, -1);
+ count = le16_to_cpu(xh->xh_count);
+
+ /*
+ * Only zero out the entry if there are more remaining. This is
+ * important for an empty bucket, as it keeps track of the
+ * bucket's hash value. It doesn't hurt empty block storage.
+ */
+ if (count) {
+ index = ((char *)entry - (char *)&xh->xh_entries) /
+ sizeof(struct ocfs2_xattr_entry);
+ memmove(&xh->xh_entries[index], &xh->xh_entries[index + 1],
+ (count - index) * sizeof(struct ocfs2_xattr_entry));
+ memset(&xh->xh_entries[count], 0,
+ sizeof(struct ocfs2_xattr_entry));
+ }
}
static void ocfs2_init_dinode_xa_loc(struct ocfs2_xa_loc *loc,
@@ -1585,7 +1606,6 @@ static void ocfs2_xattr_set_entry_local(struct inode *inode,
size_t min_offs)
{
size_t name_len = strlen(xi->name);
- int i;
struct ocfs2_xa_loc loc;
if (xs->xattr_bh == xs->inode_bh)
@@ -1633,26 +1653,12 @@ static void ocfs2_xattr_set_entry_local(struct inode *inode,
return;
}
- /* Remove the old name+value. */
- ocfs2_xa_wipe_namevalue(&loc);
- xs->here->xe_name_hash = 0;
- xs->here->xe_name_offset = 0;
- ocfs2_xattr_set_local(xs->here, 1);
- xs->here->xe_value_size = 0;
+ if (!xi->value)
+ ocfs2_xa_remove_entry(&loc);
+ else
+ ocfs2_xa_wipe_namevalue(&loc);
min_offs += size;
-
- if (!xi->value) {
- /* Remove the old entry. */
- i = le16_to_cpu(xs->header->xh_count);
- i--;
- last = &xs->header->xh_entries[i - 1];
- xs->header->xh_count = cpu_to_le16(i);
-
- memmove(xs->here, xs->here + 1,
- (void *)last - (void *)xs->here);
- memset(last, 0, sizeof(struct ocfs2_xattr_entry));
- }
}
if (xi->value) {
/* Insert the new name+value. */
@@ -4761,8 +4767,8 @@ static void ocfs2_xattr_set_entry_normal(struct inode *inode,
new_size = OCFS2_XATTR_SIZE(name_len) +
OCFS2_XATTR_SIZE(xi->value_len);
- ocfs2_xa_wipe_namevalue(&loc);
if (xi->value) {
+ ocfs2_xa_wipe_namevalue(&loc);
if (new_size > size)
goto set_new_name_value;
@@ -4784,20 +4790,8 @@ static void ocfs2_xattr_set_entry_normal(struct inode *inode,
ocfs2_xattr_set_local(xe, local);
return;
} else {
- /*
- * Remove the old entry if there is more than one.
- * We don't remove the last entry so that we can
- * use it to indicate the hash value of the empty
- * bucket.
- */
- last -= 1;
- le16_add_cpu(&xh->xh_count, -1);
- if (xh->xh_count) {
- memmove(xe, xe + 1,
- (void *)last - (void *)xe);
- memset(last, 0,
- sizeof(struct ocfs2_xattr_entry));
- } else
+ ocfs2_xa_remove_entry(&loc);
+ if (!xh->xh_count)
xh->xh_free_start =
cpu_to_le16(OCFS2_XATTR_BUCKET_SIZE);
--
1.6.3.3
^ permalink raw reply related [flat|nested] 33+ messages in thread* [Ocfs2-devel] [PATCH 03/14] ocfs2: Prefix the member fields of struct ocfs2_xattr_info.
2009-08-19 19:54 [Ocfs2-devel] [PATCH 0/14] ocfs2: Unify the setting of extended attributes Joel Becker
2009-08-19 19:54 ` [Ocfs2-devel] [PATCH 01/14] ocfs2: Introduce ocfs2_xa_loc Joel Becker
2009-08-19 19:54 ` [Ocfs2-devel] [PATCH 02/14] ocfs2: Remove xattrs via ocfs2_xa_loc Joel Becker
@ 2009-08-19 19:54 ` Joel Becker
2009-08-19 19:54 ` [Ocfs2-devel] [PATCH 04/14] ocfs2: Add a name_len field to ocfs2_xattr_info Joel Becker
` (11 subsequent siblings)
14 siblings, 0 replies; 33+ messages in thread
From: Joel Becker @ 2009-08-19 19:54 UTC (permalink / raw)
To: ocfs2-devel
struct ocfs2_xattr_info is a useful structure describing an xattr
you'd like to set. Let's put prefixes on the member fields so it's
easier to read and use.
Signed-off-by: Joel Becker <joel.becker@oracle.com>
---
fs/ocfs2/xattr.c | 210 +++++++++++++++++++++++++++--------------------------
1 files changed, 107 insertions(+), 103 deletions(-)
diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index f42edb1..606235a 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -119,10 +119,10 @@ static struct xattr_handler *ocfs2_xattr_handler_map[OCFS2_XATTR_MAX] = {
};
struct ocfs2_xattr_info {
- int name_index;
- const char *name;
- const void *value;
- size_t value_len;
+ int xi_name_index;
+ const char *xi_name;
+ const void *xi_value;
+ size_t xi_value_len;
};
struct ocfs2_xattr_search {
@@ -1307,7 +1307,7 @@ static int ocfs2_xattr_cleanup(struct inode *inode,
size_t offs)
{
int ret = 0;
- size_t name_len = strlen(xi->name);
+ size_t name_len = strlen(xi->xi_name);
void *val = xs->base + offs;
size_t size = OCFS2_XATTR_SIZE(name_len) + OCFS2_XATTR_ROOT_SIZE;
@@ -1347,8 +1347,8 @@ static int ocfs2_xattr_update_entry(struct inode *inode,
}
xs->here->xe_name_offset = cpu_to_le16(offs);
- xs->here->xe_value_size = cpu_to_le64(xi->value_len);
- if (xi->value_len <= OCFS2_XATTR_INLINE_SIZE)
+ xs->here->xe_value_size = cpu_to_le64(xi->xi_value_len);
+ if (xi->xi_value_len <= OCFS2_XATTR_INLINE_SIZE)
ocfs2_xattr_set_local(xs->here, 1);
else
ocfs2_xattr_set_local(xs->here, 0);
@@ -1373,14 +1373,14 @@ static int ocfs2_xattr_set_value_outside(struct inode *inode,
struct ocfs2_xattr_value_buf *vb,
size_t offs)
{
- size_t name_len = strlen(xi->name);
+ size_t name_len = strlen(xi->xi_name);
void *val = xs->base + offs;
struct ocfs2_xattr_value_root *xv = NULL;
size_t size = OCFS2_XATTR_SIZE(name_len) + OCFS2_XATTR_ROOT_SIZE;
int ret = 0;
memset(val, 0, size);
- memcpy(val, xi->name, name_len);
+ memcpy(val, xi->xi_name, name_len);
xv = (struct ocfs2_xattr_value_root *)
(val + OCFS2_XATTR_SIZE(name_len));
xv->xr_clusters = 0;
@@ -1390,7 +1390,7 @@ static int ocfs2_xattr_set_value_outside(struct inode *inode,
xv->xr_list.l_next_free_rec = 0;
vb->vb_xv = xv;
- ret = ocfs2_xattr_value_truncate(inode, vb, xi->value_len, ctxt);
+ ret = ocfs2_xattr_value_truncate(inode, vb, xi->xi_value_len, ctxt);
if (ret < 0) {
mlog_errno(ret);
return ret;
@@ -1401,7 +1401,7 @@ static int ocfs2_xattr_set_value_outside(struct inode *inode,
return ret;
}
ret = __ocfs2_xattr_set_value_outside(inode, ctxt->handle, vb->vb_xv,
- xi->value, xi->value_len);
+ xi->xi_value, xi->xi_value_len);
if (ret < 0)
mlog_errno(ret);
@@ -1605,7 +1605,7 @@ static void ocfs2_xattr_set_entry_local(struct inode *inode,
struct ocfs2_xattr_entry *last,
size_t min_offs)
{
- size_t name_len = strlen(xi->name);
+ size_t name_len = strlen(xi->xi_name);
struct ocfs2_xa_loc loc;
if (xs->xattr_bh == xs->inode_bh)
@@ -1614,10 +1614,10 @@ static void ocfs2_xattr_set_entry_local(struct inode *inode,
else
ocfs2_init_xattr_block_xa_loc(&loc, xs->xattr_bh,
xs->not_found ? NULL : xs->here);
- if (xi->value && xs->not_found) {
+ if (xi->xi_value && xs->not_found) {
/* Insert the new xattr entry. */
le16_add_cpu(&xs->header->xh_count, 1);
- ocfs2_xattr_set_type(last, xi->name_index);
+ ocfs2_xattr_set_type(last, xi->xi_name_index);
ocfs2_xattr_set_local(last, 1);
last->xe_name_len = name_len;
} else {
@@ -1637,42 +1637,42 @@ static void ocfs2_xattr_set_entry_local(struct inode *inode,
size = OCFS2_XATTR_SIZE(name_len) +
OCFS2_XATTR_SIZE(le64_to_cpu(xs->here->xe_value_size));
- if (xi->value && size == OCFS2_XATTR_SIZE(name_len) +
- OCFS2_XATTR_SIZE(xi->value_len)) {
+ if (xi->xi_value && size == OCFS2_XATTR_SIZE(name_len) +
+ OCFS2_XATTR_SIZE(xi->xi_value_len)) {
/* The old and the new value have the
same size. Just replace the value. */
ocfs2_xattr_set_local(xs->here, 1);
- xs->here->xe_value_size = cpu_to_le64(xi->value_len);
+ xs->here->xe_value_size = cpu_to_le64(xi->xi_value_len);
/* Clear value bytes. */
memset(val + OCFS2_XATTR_SIZE(name_len),
0,
- OCFS2_XATTR_SIZE(xi->value_len));
+ OCFS2_XATTR_SIZE(xi->xi_value_len));
memcpy(val + OCFS2_XATTR_SIZE(name_len),
- xi->value,
- xi->value_len);
+ xi->xi_value,
+ xi->xi_value_len);
return;
}
- if (!xi->value)
+ if (!xi->xi_value)
ocfs2_xa_remove_entry(&loc);
else
ocfs2_xa_wipe_namevalue(&loc);
min_offs += size;
}
- if (xi->value) {
+ if (xi->xi_value) {
/* Insert the new name+value. */
size_t size = OCFS2_XATTR_SIZE(name_len) +
- OCFS2_XATTR_SIZE(xi->value_len);
+ OCFS2_XATTR_SIZE(xi->xi_value_len);
void *val = xs->base + min_offs - size;
xs->here->xe_name_offset = cpu_to_le16(min_offs - size);
memset(val, 0, size);
- memcpy(val, xi->name, name_len);
+ memcpy(val, xi->xi_name, name_len);
memcpy(val + OCFS2_XATTR_SIZE(name_len),
- xi->value,
- xi->value_len);
- xs->here->xe_value_size = cpu_to_le64(xi->value_len);
+ xi->xi_value,
+ xi->xi_value_len);
+ xs->here->xe_value_size = cpu_to_le64(xi->xi_value_len);
ocfs2_xattr_set_local(xs->here, 1);
ocfs2_xattr_hash_entry(inode, xs->header, xs->here);
}
@@ -1698,15 +1698,15 @@ static int ocfs2_xattr_set_entry(struct inode *inode,
struct ocfs2_xattr_entry *last;
struct ocfs2_inode_info *oi = OCFS2_I(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 min_offs = xs->end - xs->base, name_len = strlen(xi->xi_name);
size_t size_l = 0;
handle_t *handle = ctxt->handle;
int free, i, ret;
struct ocfs2_xattr_info xi_l = {
- .name_index = xi->name_index,
- .name = xi->name,
- .value = xi->value,
- .value_len = xi->value_len,
+ .xi_name_index = xi->xi_name_index,
+ .xi_name = xi->xi_name,
+ .xi_value = xi->xi_value,
+ .xi_value_len = xi->xi_value_len,
};
struct ocfs2_xattr_value_buf vb = {
.vb_bh = xs->xattr_bh,
@@ -1744,7 +1744,7 @@ static int ocfs2_xattr_set_entry(struct inode *inode,
free += (size + sizeof(struct ocfs2_xattr_entry));
}
/* Check free space in inode or block */
- if (xi->value && xi->value_len > OCFS2_XATTR_INLINE_SIZE) {
+ if (xi->xi_value && xi->xi_value_len > OCFS2_XATTR_INLINE_SIZE) {
if (free < sizeof(struct ocfs2_xattr_entry) +
OCFS2_XATTR_SIZE(name_len) +
OCFS2_XATTR_ROOT_SIZE) {
@@ -1752,12 +1752,12 @@ static int ocfs2_xattr_set_entry(struct inode *inode,
goto out;
}
size_l = OCFS2_XATTR_SIZE(name_len) + OCFS2_XATTR_ROOT_SIZE;
- xi_l.value = (void *)&def_xv;
- xi_l.value_len = OCFS2_XATTR_ROOT_SIZE;
- } else if (xi->value) {
+ xi_l.xi_value = (void *)&def_xv;
+ xi_l.xi_value_len = OCFS2_XATTR_ROOT_SIZE;
+ } else if (xi->xi_value) {
if (free < sizeof(struct ocfs2_xattr_entry) +
OCFS2_XATTR_SIZE(name_len) +
- OCFS2_XATTR_SIZE(xi->value_len)) {
+ OCFS2_XATTR_SIZE(xi->xi_value_len)) {
ret = -ENOSPC;
goto out;
}
@@ -1782,16 +1782,16 @@ static int ocfs2_xattr_set_entry(struct inode *inode,
vb.vb_xv = (struct ocfs2_xattr_value_root *)
(val + OCFS2_XATTR_SIZE(name_len));
- if (xi->value_len > OCFS2_XATTR_INLINE_SIZE) {
+ if (xi->xi_value_len > OCFS2_XATTR_INLINE_SIZE) {
/*
* If new value need set outside also,
* first truncate old value to new value,
* then set new value with set_value_outside().
*/
ret = ocfs2_xattr_value_truncate(inode,
- &vb,
- xi->value_len,
- ctxt);
+ &vb,
+ xi->xi_value_len,
+ ctxt);
if (ret < 0) {
mlog_errno(ret);
goto out;
@@ -1809,10 +1809,10 @@ static int ocfs2_xattr_set_entry(struct inode *inode,
}
ret = __ocfs2_xattr_set_value_outside(inode,
- handle,
- vb.vb_xv,
- xi->value,
- xi->value_len);
+ handle,
+ vb.vb_xv,
+ xi->xi_value,
+ xi->xi_value_len);
if (ret < 0)
mlog_errno(ret);
goto out;
@@ -1890,7 +1890,7 @@ static int ocfs2_xattr_set_entry(struct inode *inode,
if (ret < 0)
mlog_errno(ret);
- if (!ret && xi->value_len > OCFS2_XATTR_INLINE_SIZE) {
+ if (!ret && xi->xi_value_len > OCFS2_XATTR_INLINE_SIZE) {
/*
* Set value outside in B tree.
* This is the second step for value size > INLINE_SIZE.
@@ -2418,13 +2418,13 @@ static int ocfs2_xattr_can_be_in_inode(struct inode *inode,
BUG_ON(!xs->not_found);
- if (xi->value_len > OCFS2_XATTR_INLINE_SIZE)
+ if (xi->xi_value_len > OCFS2_XATTR_INLINE_SIZE)
value_size = OCFS2_XATTR_ROOT_SIZE;
else
- value_size = OCFS2_XATTR_SIZE(xi->value_len);
+ value_size = OCFS2_XATTR_SIZE(xi->xi_value_len);
if (free >= sizeof(struct ocfs2_xattr_entry) +
- OCFS2_XATTR_SIZE(strlen(xi->name)) + value_size)
+ OCFS2_XATTR_SIZE(strlen(xi->xi_name)) + value_size)
return 1;
return 0;
@@ -2448,7 +2448,7 @@ static int ocfs2_calc_xattr_set_need(struct inode *inode,
char *base = NULL;
int name_offset, name_len = 0;
u32 new_clusters = ocfs2_clusters_for_bytes(inode->i_sb,
- xi->value_len);
+ xi->xi_value_len);
u64 value_size;
/*
@@ -2456,14 +2456,14 @@ static int ocfs2_calc_xattr_set_need(struct inode *inode,
* No matter whether we replace an old one or add a new one,
* we need this for writing.
*/
- if (xi->value_len > OCFS2_XATTR_INLINE_SIZE)
+ if (xi->xi_value_len > OCFS2_XATTR_INLINE_SIZE)
credits += new_clusters *
ocfs2_clusters_to_blocks(inode->i_sb, 1);
if (xis->not_found && xbs->not_found) {
credits += ocfs2_blocks_per_xattr_bucket(inode->i_sb);
- if (xi->value_len > OCFS2_XATTR_INLINE_SIZE) {
+ if (xi->xi_value_len > OCFS2_XATTR_INLINE_SIZE) {
clusters_add += new_clusters;
credits += ocfs2_calc_extend_credits(inode->i_sb,
&def_xv.xv.xr_list,
@@ -2508,7 +2508,7 @@ static int ocfs2_calc_xattr_set_need(struct inode *inode,
* The credits for removing the value tree will be extended
* by ocfs2_remove_extent itself.
*/
- if (!xi->value) {
+ if (!xi->xi_value) {
if (!ocfs2_xattr_is_local(xe))
credits += ocfs2_remove_extent_credits(inode->i_sb);
@@ -2538,7 +2538,7 @@ static int ocfs2_calc_xattr_set_need(struct inode *inode,
}
}
- if (xi->value_len > OCFS2_XATTR_INLINE_SIZE) {
+ if (xi->xi_value_len > OCFS2_XATTR_INLINE_SIZE) {
/* the new values will be stored outside. */
u32 old_clusters = 0;
@@ -2571,9 +2571,10 @@ static int ocfs2_calc_xattr_set_need(struct inode *inode,
* 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) ||
+ if ((ocfs2_xattr_is_local(xe) &&
+ (value_size >= xi->xi_value_len)) ||
(!ocfs2_xattr_is_local(xe) &&
- OCFS2_XATTR_ROOT_SIZE >= xi->value_len))
+ OCFS2_XATTR_ROOT_SIZE >= xi->xi_value_len))
goto out;
}
@@ -2661,7 +2662,7 @@ static int ocfs2_init_xattr_set_ctxt(struct inode *inode,
}
mlog(0, "Set xattr %s, reserve meta blocks = %d, clusters = %d, "
- "credits = %d\n", xi->name, meta_add, clusters_add, *credits);
+ "credits = %d\n", xi->xi_name, meta_add, clusters_add, *credits);
if (meta_add) {
ret = ocfs2_reserve_new_metadata_blocks(osb, meta_add,
@@ -2701,7 +2702,7 @@ static int __ocfs2_xattr_set_handle(struct inode *inode,
{
int ret = 0, credits, old_found;
- if (!xi->value) {
+ if (!xi->xi_value) {
/* Remove existing extended attribute */
if (!xis->not_found)
ret = ocfs2_xattr_ibody_set(inode, xi, xis, ctxt);
@@ -2715,8 +2716,8 @@ static int __ocfs2_xattr_set_handle(struct inode *inode,
* If succeed and that extended attribute existing in
* external block, then we will remove it.
*/
- xi->value = NULL;
- xi->value_len = 0;
+ xi->xi_value = NULL;
+ xi->xi_value_len = 0;
old_found = xis->not_found;
xis->not_found = -ENODATA;
@@ -2744,8 +2745,8 @@ static int __ocfs2_xattr_set_handle(struct inode *inode,
} else if (ret == -ENOSPC) {
if (di->i_xattr_loc && !xbs->xattr_bh) {
ret = ocfs2_xattr_block_find(inode,
- xi->name_index,
- xi->name, xbs);
+ xi->xi_name_index,
+ xi->xi_name, xbs);
if (ret)
goto out;
@@ -2784,8 +2785,8 @@ static int __ocfs2_xattr_set_handle(struct inode *inode,
* If succeed and that extended attribute
* existing in inode, we will remove it.
*/
- xi->value = NULL;
- xi->value_len = 0;
+ xi->xi_value = NULL;
+ xi->xi_value_len = 0;
xbs->not_found = -ENODATA;
ret = ocfs2_calc_xattr_set_need(inode,
di,
@@ -2851,10 +2852,10 @@ int ocfs2_xattr_set_handle(handle_t *handle,
int ret;
struct ocfs2_xattr_info xi = {
- .name_index = name_index,
- .name = name,
- .value = value,
- .value_len = value_len,
+ .xi_name_index = name_index,
+ .xi_name = name,
+ .xi_value = value,
+ .xi_value_len = value_len,
};
struct ocfs2_xattr_search xis = {
@@ -2933,10 +2934,10 @@ int ocfs2_xattr_set(struct inode *inode,
struct ocfs2_xattr_set_ctxt ctxt = { NULL, NULL, };
struct ocfs2_xattr_info xi = {
- .name_index = name_index,
- .name = name,
- .value = value,
- .value_len = value_len,
+ .xi_name_index = name_index,
+ .xi_name = name,
+ .xi_value = value,
+ .xi_value_len = value_len,
};
struct ocfs2_xattr_search xis = {
@@ -4737,7 +4738,7 @@ static void ocfs2_xattr_set_entry_normal(struct inode *inode,
int local)
{
struct ocfs2_xattr_entry *last, *xe;
- int name_len = strlen(xi->name);
+ int name_len = strlen(xi->xi_name);
struct ocfs2_xattr_header *xh = xs->header;
u16 count = le16_to_cpu(xh->xh_count), start;
size_t blocksize = inode->i_sb->s_blocksize;
@@ -4759,22 +4760,24 @@ static void ocfs2_xattr_set_entry_normal(struct inode *inode,
OCFS2_XATTR_SIZE(OCFS2_XATTR_ROOT_SIZE);
/*
- * If the new value will be stored outside, xi->value has been
- * initalized as an empty ocfs2_xattr_value_root, and the same
- * goes with xi->value_len, so we can set new_size safely here.
+ * If the new value will be stored outside, xi->xi_value has
+ * been initalized as an empty ocfs2_xattr_value_root, and
+ * the same goes with xi->xi_value_len, so we can set
+ * new_size safely here.
* See ocfs2_xattr_set_in_bucket.
*/
new_size = OCFS2_XATTR_SIZE(name_len) +
- OCFS2_XATTR_SIZE(xi->value_len);
+ OCFS2_XATTR_SIZE(xi->xi_value_len);
- if (xi->value) {
+ if (xi->xi_value) {
ocfs2_xa_wipe_namevalue(&loc);
if (new_size > size)
goto set_new_name_value;
/* Now replace the old value with new one. */
if (local)
- xe->xe_value_size = cpu_to_le64(xi->value_len);
+ xe->xe_value_size =
+ cpu_to_le64(xi->xi_value_len);
else
xe->xe_value_size = 0;
@@ -4782,9 +4785,9 @@ static void ocfs2_xattr_set_entry_normal(struct inode *inode,
xs->bucket, offs);
memset(val + OCFS2_XATTR_SIZE(name_len), 0,
size - OCFS2_XATTR_SIZE(name_len));
- if (OCFS2_XATTR_SIZE(xi->value_len) > 0)
+ if (OCFS2_XATTR_SIZE(xi->xi_value_len) > 0)
memcpy(val + OCFS2_XATTR_SIZE(name_len),
- xi->value, xi->value_len);
+ xi->xi_value, xi->xi_value_len);
le16_add_cpu(&xh->xh_name_value_len, new_size);
ocfs2_xattr_set_local(xe, local);
@@ -4825,12 +4828,12 @@ static void ocfs2_xattr_set_entry_normal(struct inode *inode,
memset(xe, 0, sizeof(struct ocfs2_xattr_entry));
xe->xe_name_hash = cpu_to_le32(name_hash);
xe->xe_name_len = name_len;
- ocfs2_xattr_set_type(xe, xi->name_index);
+ ocfs2_xattr_set_type(xe, xi->xi_name_index);
}
set_new_name_value:
/* Insert the new name+value. */
- size = OCFS2_XATTR_SIZE(name_len) + OCFS2_XATTR_SIZE(xi->value_len);
+ size = OCFS2_XATTR_SIZE(name_len) + OCFS2_XATTR_SIZE(xi->xi_value_len);
/*
* We must make sure that the name/value pair
@@ -4849,10 +4852,11 @@ set_new_name_value:
xe->xe_name_offset = cpu_to_le16(offs - size);
memset(val, 0, size);
- memcpy(val, xi->name, name_len);
- memcpy(val + OCFS2_XATTR_SIZE(name_len), xi->value, xi->value_len);
+ memcpy(val, xi->xi_name, name_len);
+ memcpy(val + OCFS2_XATTR_SIZE(name_len), xi->xi_value,
+ xi->xi_value_len);
- xe->xe_value_size = cpu_to_le64(xi->value_len);
+ xe->xe_value_size = cpu_to_le64(xi->xi_value_len);
ocfs2_xattr_set_local(xe, local);
xs->here = xe;
le16_add_cpu(&xh->xh_free_start, -size);
@@ -4877,7 +4881,7 @@ static int ocfs2_xattr_set_entry_in_bucket(struct inode *inode,
u64 blkno;
mlog(0, "Set xattr entry len = %lu index = %d in bucket %llu\n",
- (unsigned long)xi->value_len, xi->name_index,
+ (unsigned long)xi->xi_value_len, xi->xi_name_index,
(unsigned long long)bucket_blkno(xs->bucket));
if (!xs->bucket->bu_bhs[1]) {
@@ -5161,10 +5165,10 @@ static int ocfs2_xattr_set_in_bucket(struct inode *inode,
{
int ret, local = 1;
size_t value_len;
- char *val = (char *)xi->value;
+ char *val = (char *)xi->xi_value;
struct ocfs2_xattr_entry *xe = xs->here;
- u32 name_hash = ocfs2_xattr_name_hash(inode, xi->name,
- strlen(xi->name));
+ u32 name_hash = ocfs2_xattr_name_hash(inode, xi->xi_name,
+ strlen(xi->xi_name));
if (!xs->not_found && !ocfs2_xattr_is_local(xe)) {
/*
@@ -5179,8 +5183,8 @@ static int ocfs2_xattr_set_in_bucket(struct inode *inode,
* the modification to the xattr block will be done
* by following steps.
*/
- if (xi->value_len > OCFS2_XATTR_INLINE_SIZE)
- value_len = xi->value_len;
+ if (xi->xi_value_len > OCFS2_XATTR_INLINE_SIZE)
+ value_len = xi->xi_value_len;
else
value_len = 0;
@@ -5194,7 +5198,7 @@ static int ocfs2_xattr_set_in_bucket(struct inode *inode,
goto set_value_outside;
}
- value_len = xi->value_len;
+ value_len = xi->xi_value_len;
/* So we have to handle the inside block change now. */
if (value_len > OCFS2_XATTR_INLINE_SIZE) {
/*
@@ -5202,8 +5206,8 @@ static int ocfs2_xattr_set_in_bucket(struct inode *inode,
* initalize a new empty value root and insert it first.
*/
local = 0;
- xi->value = &def_xv;
- xi->value_len = OCFS2_XATTR_ROOT_SIZE;
+ xi->xi_value = &def_xv;
+ xi->xi_value_len = OCFS2_XATTR_ROOT_SIZE;
}
ret = ocfs2_xattr_set_entry_in_bucket(inode, ctxt->handle, xi, xs,
@@ -5277,11 +5281,11 @@ static int ocfs2_xattr_set_entry_index_block(struct inode *inode,
struct ocfs2_xattr_entry *xe;
u16 count, header_size, xh_free_start;
int free, max_free, need, old;
- size_t value_size = 0, name_len = strlen(xi->name);
+ size_t value_size = 0, name_len = strlen(xi->xi_name);
size_t blocksize = inode->i_sb->s_blocksize;
int ret, allocation = 0;
- mlog_entry("Set xattr %s in xattr index block\n", xi->name);
+ mlog_entry("Set xattr %s in xattr index block\n", xi->xi_name);
try_again:
xh = xs->header;
@@ -5297,10 +5301,10 @@ try_again:
(unsigned long long)bucket_blkno(xs->bucket),
header_size);
- if (xi->value && xi->value_len > OCFS2_XATTR_INLINE_SIZE)
+ if (xi->xi_value && xi->xi_value_len > OCFS2_XATTR_INLINE_SIZE)
value_size = OCFS2_XATTR_ROOT_SIZE;
- else if (xi->value)
- value_size = OCFS2_XATTR_SIZE(xi->value_len);
+ else if (xi->xi_value)
+ value_size = OCFS2_XATTR_SIZE(xi->xi_value_len);
if (xs->not_found)
need = sizeof(struct ocfs2_xattr_entry) +
@@ -5383,7 +5387,7 @@ try_again:
*/
ret = ocfs2_check_xattr_bucket_collision(inode,
xs->bucket,
- xi->name);
+ xi->xi_name);
if (ret) {
mlog_errno(ret);
goto out;
@@ -5407,8 +5411,8 @@ try_again:
*/
ocfs2_xattr_bucket_relse(xs->bucket);
ret = ocfs2_xattr_index_block_find(inode, xs->xattr_bh,
- xi->name_index,
- xi->name, xs);
+ xi->xi_name_index,
+ xi->xi_name, xs);
if (ret && ret != -ENODATA)
goto out;
xs->not_found = ret;
--
1.6.3.3
^ permalink raw reply related [flat|nested] 33+ messages in thread* [Ocfs2-devel] [PATCH 04/14] ocfs2: Add a name_len field to ocfs2_xattr_info.
2009-08-19 19:54 [Ocfs2-devel] [PATCH 0/14] ocfs2: Unify the setting of extended attributes Joel Becker
` (2 preceding siblings ...)
2009-08-19 19:54 ` [Ocfs2-devel] [PATCH 03/14] ocfs2: Prefix the member fields of struct ocfs2_xattr_info Joel Becker
@ 2009-08-19 19:54 ` Joel Becker
2009-08-19 19:54 ` [Ocfs2-devel] [PATCH 05/14] ocfs2: Wrap calculation of name+value pair size Joel Becker
` (10 subsequent siblings)
14 siblings, 0 replies; 33+ messages in thread
From: Joel Becker @ 2009-08-19 19:54 UTC (permalink / raw)
To: ocfs2-devel
Rather than calculating strlen all over the place, let's store the
name length directly on ocfs2_xattr_info.
Signed-off-by: Joel Becker <joel.becker@oracle.com>
---
fs/ocfs2/xattr.c | 84 ++++++++++++++++++++++++++++-------------------------
1 files changed, 44 insertions(+), 40 deletions(-)
diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index 606235a..53d04b2 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -121,6 +121,7 @@ static struct xattr_handler *ocfs2_xattr_handler_map[OCFS2_XATTR_MAX] = {
struct ocfs2_xattr_info {
int xi_name_index;
const char *xi_name;
+ int xi_name_len;
const void *xi_value;
size_t xi_value_len;
};
@@ -1307,9 +1308,9 @@ static int ocfs2_xattr_cleanup(struct inode *inode,
size_t offs)
{
int ret = 0;
- size_t name_len = strlen(xi->xi_name);
void *val = xs->base + offs;
- size_t size = OCFS2_XATTR_SIZE(name_len) + OCFS2_XATTR_ROOT_SIZE;
+ size_t size = OCFS2_XATTR_SIZE(xi->xi_name_len) +
+ OCFS2_XATTR_ROOT_SIZE;
ret = vb->vb_access(handle, inode, vb->vb_bh,
OCFS2_JOURNAL_ACCESS_WRITE);
@@ -1373,16 +1374,16 @@ static int ocfs2_xattr_set_value_outside(struct inode *inode,
struct ocfs2_xattr_value_buf *vb,
size_t offs)
{
- size_t name_len = strlen(xi->xi_name);
void *val = xs->base + offs;
struct ocfs2_xattr_value_root *xv = NULL;
- size_t size = OCFS2_XATTR_SIZE(name_len) + OCFS2_XATTR_ROOT_SIZE;
+ size_t size = OCFS2_XATTR_SIZE(xi->xi_name_len) +
+ OCFS2_XATTR_ROOT_SIZE;
int ret = 0;
memset(val, 0, size);
- memcpy(val, xi->xi_name, name_len);
+ memcpy(val, xi->xi_name, xi->xi_name_len);
xv = (struct ocfs2_xattr_value_root *)
- (val + OCFS2_XATTR_SIZE(name_len));
+ (val + OCFS2_XATTR_SIZE(xi->xi_name_len));
xv->xr_clusters = 0;
xv->xr_last_eb_blk = 0;
xv->xr_list.l_tree_depth = 0;
@@ -1605,7 +1606,6 @@ static void ocfs2_xattr_set_entry_local(struct inode *inode,
struct ocfs2_xattr_entry *last,
size_t min_offs)
{
- size_t name_len = strlen(xi->xi_name);
struct ocfs2_xa_loc loc;
if (xs->xattr_bh == xs->inode_bh)
@@ -1619,7 +1619,7 @@ static void ocfs2_xattr_set_entry_local(struct inode *inode,
le16_add_cpu(&xs->header->xh_count, 1);
ocfs2_xattr_set_type(last, xi->xi_name_index);
ocfs2_xattr_set_local(last, 1);
- last->xe_name_len = name_len;
+ last->xe_name_len = xi->xi_name_len;
} else {
void *first_val;
void *val;
@@ -1631,23 +1631,23 @@ static void ocfs2_xattr_set_entry_local(struct inode *inode,
if (le64_to_cpu(xs->here->xe_value_size) >
OCFS2_XATTR_INLINE_SIZE)
- size = OCFS2_XATTR_SIZE(name_len) +
+ size = OCFS2_XATTR_SIZE(xi->xi_name_len) +
OCFS2_XATTR_ROOT_SIZE;
else
- size = OCFS2_XATTR_SIZE(name_len) +
+ size = OCFS2_XATTR_SIZE(xi->xi_name_len) +
OCFS2_XATTR_SIZE(le64_to_cpu(xs->here->xe_value_size));
- if (xi->xi_value && size == OCFS2_XATTR_SIZE(name_len) +
+ if (xi->xi_value && size == OCFS2_XATTR_SIZE(xi->xi_name_len) +
OCFS2_XATTR_SIZE(xi->xi_value_len)) {
/* The old and the new value have the
same size. Just replace the value. */
ocfs2_xattr_set_local(xs->here, 1);
xs->here->xe_value_size = cpu_to_le64(xi->xi_value_len);
/* Clear value bytes. */
- memset(val + OCFS2_XATTR_SIZE(name_len),
+ memset(val + OCFS2_XATTR_SIZE(xi->xi_name_len),
0,
OCFS2_XATTR_SIZE(xi->xi_value_len));
- memcpy(val + OCFS2_XATTR_SIZE(name_len),
+ memcpy(val + OCFS2_XATTR_SIZE(xi->xi_name_len),
xi->xi_value,
xi->xi_value_len);
return;
@@ -1662,14 +1662,14 @@ static void ocfs2_xattr_set_entry_local(struct inode *inode,
}
if (xi->xi_value) {
/* Insert the new name+value. */
- size_t size = OCFS2_XATTR_SIZE(name_len) +
+ size_t size = OCFS2_XATTR_SIZE(xi->xi_name_len) +
OCFS2_XATTR_SIZE(xi->xi_value_len);
void *val = xs->base + min_offs - size;
xs->here->xe_name_offset = cpu_to_le16(min_offs - size);
memset(val, 0, size);
- memcpy(val, xi->xi_name, name_len);
- memcpy(val + OCFS2_XATTR_SIZE(name_len),
+ memcpy(val, xi->xi_name, xi->xi_name_len);
+ memcpy(val + OCFS2_XATTR_SIZE(xi->xi_name_len),
xi->xi_value,
xi->xi_value_len);
xs->here->xe_value_size = cpu_to_le64(xi->xi_value_len);
@@ -1698,13 +1698,14 @@ static int ocfs2_xattr_set_entry(struct inode *inode,
struct ocfs2_xattr_entry *last;
struct ocfs2_inode_info *oi = OCFS2_I(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->xi_name);
+ size_t min_offs = xs->end - xs->base;
size_t size_l = 0;
handle_t *handle = ctxt->handle;
int free, i, ret;
struct ocfs2_xattr_info xi_l = {
.xi_name_index = xi->xi_name_index,
.xi_name = xi->xi_name,
+ .xi_name_len = xi->xi_name_len,
.xi_value = xi->xi_value,
.xi_value_len = xi->xi_value_len,
};
@@ -1736,27 +1737,28 @@ static int ocfs2_xattr_set_entry(struct inode *inode,
if (!xs->not_found) {
size_t size = 0;
if (ocfs2_xattr_is_local(xs->here))
- size = OCFS2_XATTR_SIZE(name_len) +
+ size = OCFS2_XATTR_SIZE(xi->xi_name_len) +
OCFS2_XATTR_SIZE(le64_to_cpu(xs->here->xe_value_size));
else
- size = OCFS2_XATTR_SIZE(name_len) +
+ size = OCFS2_XATTR_SIZE(xi->xi_name_len) +
OCFS2_XATTR_ROOT_SIZE;
free += (size + sizeof(struct ocfs2_xattr_entry));
}
/* Check free space in inode or block */
if (xi->xi_value && xi->xi_value_len > OCFS2_XATTR_INLINE_SIZE) {
if (free < sizeof(struct ocfs2_xattr_entry) +
- OCFS2_XATTR_SIZE(name_len) +
+ OCFS2_XATTR_SIZE(xi->xi_name_len) +
OCFS2_XATTR_ROOT_SIZE) {
ret = -ENOSPC;
goto out;
}
- size_l = OCFS2_XATTR_SIZE(name_len) + OCFS2_XATTR_ROOT_SIZE;
+ size_l = OCFS2_XATTR_SIZE(xi->xi_name_len) +
+ OCFS2_XATTR_ROOT_SIZE;
xi_l.xi_value = (void *)&def_xv;
xi_l.xi_value_len = OCFS2_XATTR_ROOT_SIZE;
} else if (xi->xi_value) {
if (free < sizeof(struct ocfs2_xattr_entry) +
- OCFS2_XATTR_SIZE(name_len) +
+ OCFS2_XATTR_SIZE(xi->xi_name_len) +
OCFS2_XATTR_SIZE(xi->xi_value_len)) {
ret = -ENOSPC;
goto out;
@@ -1765,7 +1767,7 @@ static int ocfs2_xattr_set_entry(struct inode *inode,
if (!xs->not_found) {
/* For existing extended attribute */
- size_t size = OCFS2_XATTR_SIZE(name_len) +
+ size_t size = OCFS2_XATTR_SIZE(xi->xi_name_len) +
OCFS2_XATTR_SIZE(le64_to_cpu(xs->here->xe_value_size));
size_t offs = le16_to_cpu(xs->here->xe_name_offset);
void *val = xs->base + offs;
@@ -1780,7 +1782,7 @@ static int ocfs2_xattr_set_entry(struct inode *inode,
} else if (!ocfs2_xattr_is_local(xs->here)) {
/* For existing xattr which has value outside */
vb.vb_xv = (struct ocfs2_xattr_value_root *)
- (val + OCFS2_XATTR_SIZE(name_len));
+ (val + OCFS2_XATTR_SIZE(xi->xi_name_len));
if (xi->xi_value_len > OCFS2_XATTR_INLINE_SIZE) {
/*
@@ -2424,7 +2426,7 @@ static int ocfs2_xattr_can_be_in_inode(struct inode *inode,
value_size = OCFS2_XATTR_SIZE(xi->xi_value_len);
if (free >= sizeof(struct ocfs2_xattr_entry) +
- OCFS2_XATTR_SIZE(strlen(xi->xi_name)) + value_size)
+ OCFS2_XATTR_SIZE(xi->xi_name_len) + value_size)
return 1;
return 0;
@@ -2854,6 +2856,7 @@ int ocfs2_xattr_set_handle(handle_t *handle,
struct ocfs2_xattr_info xi = {
.xi_name_index = name_index,
.xi_name = name,
+ .xi_name_len = strlen(name),
.xi_value = value,
.xi_value_len = value_len,
};
@@ -2936,6 +2939,7 @@ int ocfs2_xattr_set(struct inode *inode,
struct ocfs2_xattr_info xi = {
.xi_name_index = name_index,
.xi_name = name,
+ .xi_name_len = strlen(name),
.xi_value = value,
.xi_value_len = value_len,
};
@@ -4738,7 +4742,6 @@ static void ocfs2_xattr_set_entry_normal(struct inode *inode,
int local)
{
struct ocfs2_xattr_entry *last, *xe;
- int name_len = strlen(xi->xi_name);
struct ocfs2_xattr_header *xh = xs->header;
u16 count = le16_to_cpu(xh->xh_count), start;
size_t blocksize = inode->i_sb->s_blocksize;
@@ -4753,10 +4756,10 @@ static void ocfs2_xattr_set_entry_normal(struct inode *inode,
xe = xs->here;
offs = le16_to_cpu(xe->xe_name_offset);
if (ocfs2_xattr_is_local(xe))
- size = OCFS2_XATTR_SIZE(name_len) +
+ size = OCFS2_XATTR_SIZE(xi->xi_name_len) +
OCFS2_XATTR_SIZE(le64_to_cpu(xe->xe_value_size));
else
- size = OCFS2_XATTR_SIZE(name_len) +
+ size = OCFS2_XATTR_SIZE(xi->xi_name_len) +
OCFS2_XATTR_SIZE(OCFS2_XATTR_ROOT_SIZE);
/*
@@ -4766,7 +4769,7 @@ static void ocfs2_xattr_set_entry_normal(struct inode *inode,
* new_size safely here.
* See ocfs2_xattr_set_in_bucket.
*/
- new_size = OCFS2_XATTR_SIZE(name_len) +
+ new_size = OCFS2_XATTR_SIZE(xi->xi_name_len) +
OCFS2_XATTR_SIZE(xi->xi_value_len);
if (xi->xi_value) {
@@ -4783,10 +4786,10 @@ static void ocfs2_xattr_set_entry_normal(struct inode *inode,
val = ocfs2_xattr_bucket_get_val(inode,
xs->bucket, offs);
- memset(val + OCFS2_XATTR_SIZE(name_len), 0,
- size - OCFS2_XATTR_SIZE(name_len));
+ memset(val + OCFS2_XATTR_SIZE(xi->xi_name_len), 0,
+ size - OCFS2_XATTR_SIZE(xi->xi_name_len));
if (OCFS2_XATTR_SIZE(xi->xi_value_len) > 0)
- memcpy(val + OCFS2_XATTR_SIZE(name_len),
+ memcpy(val + OCFS2_XATTR_SIZE(xi->xi_name_len),
xi->xi_value, xi->xi_value_len);
le16_add_cpu(&xh->xh_name_value_len, new_size);
@@ -4827,13 +4830,14 @@ static void ocfs2_xattr_set_entry_normal(struct inode *inode,
le16_add_cpu(&xh->xh_count, 1);
memset(xe, 0, sizeof(struct ocfs2_xattr_entry));
xe->xe_name_hash = cpu_to_le32(name_hash);
- xe->xe_name_len = name_len;
+ xe->xe_name_len = xi->xi_name_len;
ocfs2_xattr_set_type(xe, xi->xi_name_index);
}
set_new_name_value:
/* Insert the new name+value. */
- size = OCFS2_XATTR_SIZE(name_len) + OCFS2_XATTR_SIZE(xi->xi_value_len);
+ size = OCFS2_XATTR_SIZE(xi->xi_name_len) +
+ OCFS2_XATTR_SIZE(xi->xi_value_len);
/*
* We must make sure that the name/value pair
@@ -4852,8 +4856,8 @@ set_new_name_value:
xe->xe_name_offset = cpu_to_le16(offs - size);
memset(val, 0, size);
- memcpy(val, xi->xi_name, name_len);
- memcpy(val + OCFS2_XATTR_SIZE(name_len), xi->xi_value,
+ memcpy(val, xi->xi_name, xi->xi_name_len);
+ memcpy(val + OCFS2_XATTR_SIZE(xi->xi_name_len), xi->xi_value,
xi->xi_value_len);
xe->xe_value_size = cpu_to_le64(xi->xi_value_len);
@@ -5168,7 +5172,7 @@ static int ocfs2_xattr_set_in_bucket(struct inode *inode,
char *val = (char *)xi->xi_value;
struct ocfs2_xattr_entry *xe = xs->here;
u32 name_hash = ocfs2_xattr_name_hash(inode, xi->xi_name,
- strlen(xi->xi_name));
+ xi->xi_name_len);
if (!xs->not_found && !ocfs2_xattr_is_local(xe)) {
/*
@@ -5281,7 +5285,7 @@ static int ocfs2_xattr_set_entry_index_block(struct inode *inode,
struct ocfs2_xattr_entry *xe;
u16 count, header_size, xh_free_start;
int free, max_free, need, old;
- size_t value_size = 0, name_len = strlen(xi->xi_name);
+ size_t value_size = 0;
size_t blocksize = inode->i_sb->s_blocksize;
int ret, allocation = 0;
@@ -5308,9 +5312,9 @@ try_again:
if (xs->not_found)
need = sizeof(struct ocfs2_xattr_entry) +
- OCFS2_XATTR_SIZE(name_len) + value_size;
+ OCFS2_XATTR_SIZE(xi->xi_name_len) + value_size;
else {
- need = value_size + OCFS2_XATTR_SIZE(name_len);
+ need = value_size + OCFS2_XATTR_SIZE(xi->xi_name_len);
/*
* We only replace the old value if the new length is smaller
--
1.6.3.3
^ permalink raw reply related [flat|nested] 33+ messages in thread* [Ocfs2-devel] [PATCH 05/14] ocfs2: Wrap calculation of name+value pair size.
2009-08-19 19:54 [Ocfs2-devel] [PATCH 0/14] ocfs2: Unify the setting of extended attributes Joel Becker
` (3 preceding siblings ...)
2009-08-19 19:54 ` [Ocfs2-devel] [PATCH 04/14] ocfs2: Add a name_len field to ocfs2_xattr_info Joel Becker
@ 2009-08-19 19:54 ` Joel Becker
2009-08-19 19:54 ` [Ocfs2-devel] [PATCH 06/14] ocfs2: Set the xattr name+value pair in one place Joel Becker
` (9 subsequent siblings)
14 siblings, 0 replies; 33+ messages in thread
From: Joel Becker @ 2009-08-19 19:54 UTC (permalink / raw)
To: ocfs2-devel
An ocfs2 xattr entry stores the text name and value as a pair in the
storage area. Obviously names and values can be variable-sized. If a
value is too large for the entry storage, a tree root is stored instead.
The name+value pair is also padded.
Because of this, there are a million places in the code that do:
if (needs_external_tree(value_size)
namevalue_size = pad(name_size) + tree_root_size;
else
namevalue_size = pad(name_size) + pad(value_size);
Let's create some convenience functions to make the code more readable.
There are three forms. The first takes the raw sizes. The second takes
an ocfs2_xattr_info structure. The third takes an existing
ocfs2_xattr_entry.
Signed-off-by: Joel Becker <joel.becker@oracle.com>
---
fs/ocfs2/xattr.c | 170 +++++++++++++++++++++---------------------------------
1 files changed, 65 insertions(+), 105 deletions(-)
diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index 53d04b2..ad010ec 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -186,6 +186,33 @@ struct ocfs2_xa_loc {
const struct ocfs2_xa_loc_operations *xl_ops;
};
+/*
+ * Convenience functions to calculate how much space is needed for a
+ * given name+value pair
+ */
+static int namevalue_size(int name_len, uint64_t value_len)
+{
+ if (value_len > OCFS2_XATTR_INLINE_SIZE)
+ return OCFS2_XATTR_SIZE(name_len) + OCFS2_XATTR_ROOT_SIZE;
+ else
+ return OCFS2_XATTR_SIZE(name_len) + OCFS2_XATTR_SIZE(value_len);
+}
+
+static int namevalue_size_xi(struct ocfs2_xattr_info *xi)
+{
+ return namevalue_size(xi->xi_name_len, xi->xi_value_len);
+}
+
+static int namevalue_size_xe(struct ocfs2_xattr_entry *xe)
+{
+ u64 value_len = le64_to_cpu(xe->xe_value_size);
+
+ BUG_ON((value_len > OCFS2_XATTR_INLINE_SIZE) &&
+ ocfs2_xattr_is_local(xe));
+ return namevalue_size(xe->xe_name_len, value_len);
+}
+
+
static int ocfs2_xattr_bucket_get_name_value(struct inode *inode,
struct ocfs2_xattr_header *xh,
int index,
@@ -503,15 +530,20 @@ static void ocfs2_xattr_hash_entry(struct inode *inode,
static int ocfs2_xattr_entry_real_size(int name_len, size_t value_len)
{
- int size = 0;
+ return namevalue_size(name_len, value_len) +
+ sizeof(struct ocfs2_xattr_entry);
+}
- if (value_len <= OCFS2_XATTR_INLINE_SIZE)
- size = OCFS2_XATTR_SIZE(name_len) + OCFS2_XATTR_SIZE(value_len);
- else
- size = OCFS2_XATTR_SIZE(name_len) + OCFS2_XATTR_ROOT_SIZE;
- size += sizeof(struct ocfs2_xattr_entry);
+static int ocfs2_xi_entry_usage(struct ocfs2_xattr_info *xi)
+{
+ return namevalue_size_xi(xi) +
+ sizeof(struct ocfs2_xattr_entry);
+}
- return size;
+static int ocfs2_xe_entry_usage(struct ocfs2_xattr_entry *xe)
+{
+ return namevalue_size_xe(xe) +
+ sizeof(struct ocfs2_xattr_entry);
}
int ocfs2_calc_security_init(struct inode *dir,
@@ -1309,8 +1341,7 @@ static int ocfs2_xattr_cleanup(struct inode *inode,
{
int ret = 0;
void *val = xs->base + offs;
- size_t size = OCFS2_XATTR_SIZE(xi->xi_name_len) +
- OCFS2_XATTR_ROOT_SIZE;
+ size_t size = namevalue_size_xi(xi);
ret = vb->vb_access(handle, inode, vb->vb_bh,
OCFS2_JOURNAL_ACCESS_WRITE);
@@ -1376,8 +1407,7 @@ static int ocfs2_xattr_set_value_outside(struct inode *inode,
{
void *val = xs->base + offs;
struct ocfs2_xattr_value_root *xv = NULL;
- size_t size = OCFS2_XATTR_SIZE(xi->xi_name_len) +
- OCFS2_XATTR_ROOT_SIZE;
+ size_t size = namevalue_size_xi(xi);
int ret = 0;
memset(val, 0, size);
@@ -1438,15 +1468,10 @@ static void ocfs2_xa_block_wipe_namevalue(struct ocfs2_xa_loc *loc)
int namevalue_offset, first_namevalue_offset, namevalue_size;
struct ocfs2_xattr_entry *entry = loc->xl_entry;
struct ocfs2_xattr_header *xh = loc->xl_header;
- u64 value_size = le64_to_cpu(entry->xe_value_size);
int count = le16_to_cpu(xh->xh_count);
namevalue_offset = le16_to_cpu(entry->xe_name_offset);
- namevalue_size = OCFS2_XATTR_SIZE(entry->xe_name_len);
- if (value_size > OCFS2_XATTR_INLINE_SIZE)
- namevalue_size += OCFS2_XATTR_ROOT_SIZE;
- else
- namevalue_size += OCFS2_XATTR_SIZE(value_size);
+ namevalue_size = namevalue_size_xe(entry);
for (i = 0, first_namevalue_offset = loc->xl_size;
i < count; i++) {
@@ -1500,17 +1525,8 @@ static void *ocfs2_xa_bucket_offset_pointer(struct ocfs2_xa_loc *loc,
static void ocfs2_xa_bucket_wipe_namevalue(struct ocfs2_xa_loc *loc)
{
- int namevalue_size;
- struct ocfs2_xattr_entry *entry = loc->xl_entry;
- u64 value_size = le64_to_cpu(entry->xe_value_size);
-
- namevalue_size = OCFS2_XATTR_SIZE(entry->xe_name_len);
- if (value_size > OCFS2_XATTR_INLINE_SIZE)
- namevalue_size += OCFS2_XATTR_ROOT_SIZE;
- else
- namevalue_size += OCFS2_XATTR_SIZE(value_size);
-
- le16_add_cpu(&loc->xl_header->xh_name_value_len, -namevalue_size);
+ le16_add_cpu(&loc->xl_header->xh_name_value_len,
+ -namevalue_size_xe(loc->xl_entry));
}
/* Operations for xattrs stored in buckets. */
@@ -1629,16 +1645,8 @@ static void ocfs2_xattr_set_entry_local(struct inode *inode,
offs = le16_to_cpu(xs->here->xe_name_offset);
val = xs->base + offs;
- if (le64_to_cpu(xs->here->xe_value_size) >
- OCFS2_XATTR_INLINE_SIZE)
- size = OCFS2_XATTR_SIZE(xi->xi_name_len) +
- OCFS2_XATTR_ROOT_SIZE;
- else
- size = OCFS2_XATTR_SIZE(xi->xi_name_len) +
- OCFS2_XATTR_SIZE(le64_to_cpu(xs->here->xe_value_size));
-
- if (xi->xi_value && size == OCFS2_XATTR_SIZE(xi->xi_name_len) +
- OCFS2_XATTR_SIZE(xi->xi_value_len)) {
+ size = namevalue_size_xe(xs->here);
+ if (xi->xi_value && (size == namevalue_size_xi(xi))) {
/* The old and the new value have the
same size. Just replace the value. */
ocfs2_xattr_set_local(xs->here, 1);
@@ -1662,8 +1670,7 @@ static void ocfs2_xattr_set_entry_local(struct inode *inode,
}
if (xi->xi_value) {
/* Insert the new name+value. */
- size_t size = OCFS2_XATTR_SIZE(xi->xi_name_len) +
- OCFS2_XATTR_SIZE(xi->xi_value_len);
+ size_t size = namevalue_size_xi(xi);
void *val = xs->base + min_offs - size;
xs->here->xe_name_offset = cpu_to_le16(min_offs - size);
@@ -1734,41 +1741,25 @@ static int ocfs2_xattr_set_entry(struct inode *inode,
if (free < 0)
return -EIO;
- if (!xs->not_found) {
- size_t size = 0;
- if (ocfs2_xattr_is_local(xs->here))
- size = OCFS2_XATTR_SIZE(xi->xi_name_len) +
- OCFS2_XATTR_SIZE(le64_to_cpu(xs->here->xe_value_size));
- else
- size = OCFS2_XATTR_SIZE(xi->xi_name_len) +
- OCFS2_XATTR_ROOT_SIZE;
- free += (size + sizeof(struct ocfs2_xattr_entry));
- }
+ if (!xs->not_found)
+ free += ocfs2_xe_entry_usage(xs->here);
+
/* Check free space in inode or block */
- if (xi->xi_value && xi->xi_value_len > OCFS2_XATTR_INLINE_SIZE) {
- if (free < sizeof(struct ocfs2_xattr_entry) +
- OCFS2_XATTR_SIZE(xi->xi_name_len) +
- OCFS2_XATTR_ROOT_SIZE) {
+ if (xi->xi_value) {
+ if (free < ocfs2_xi_entry_usage(xi)) {
ret = -ENOSPC;
goto out;
}
- size_l = OCFS2_XATTR_SIZE(xi->xi_name_len) +
- OCFS2_XATTR_ROOT_SIZE;
- xi_l.xi_value = (void *)&def_xv;
- xi_l.xi_value_len = OCFS2_XATTR_ROOT_SIZE;
- } else if (xi->xi_value) {
- if (free < sizeof(struct ocfs2_xattr_entry) +
- OCFS2_XATTR_SIZE(xi->xi_name_len) +
- OCFS2_XATTR_SIZE(xi->xi_value_len)) {
- ret = -ENOSPC;
- goto out;
+ if (xi->xi_value_len > OCFS2_XATTR_INLINE_SIZE) {
+ size_l = namevalue_size_xi(xi);
+ xi_l.xi_value = (void *)&def_xv;
+ xi_l.xi_value_len = OCFS2_XATTR_ROOT_SIZE;
}
}
if (!xs->not_found) {
/* For existing extended attribute */
- size_t size = OCFS2_XATTR_SIZE(xi->xi_name_len) +
- OCFS2_XATTR_SIZE(le64_to_cpu(xs->here->xe_value_size));
+ size_t size = namevalue_size_xe(xs->here);
size_t offs = le16_to_cpu(xs->here->xe_name_offset);
void *val = xs->base + offs;
@@ -2397,7 +2388,6 @@ 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;
@@ -2420,13 +2410,7 @@ static int ocfs2_xattr_can_be_in_inode(struct inode *inode,
BUG_ON(!xs->not_found);
- if (xi->xi_value_len > OCFS2_XATTR_INLINE_SIZE)
- value_size = OCFS2_XATTR_ROOT_SIZE;
- else
- value_size = OCFS2_XATTR_SIZE(xi->xi_value_len);
-
- if (free >= sizeof(struct ocfs2_xattr_entry) +
- OCFS2_XATTR_SIZE(xi->xi_name_len) + value_size)
+ if (free >= (sizeof(struct ocfs2_xattr_entry) + namevalue_size_xi(xi)))
return 1;
return 0;
@@ -3738,7 +3722,7 @@ static int ocfs2_defrag_xattr_bucket(struct inode *inode,
struct ocfs2_xattr_bucket *bucket)
{
int ret, i;
- size_t end, offset, len, value_len;
+ size_t end, offset, len;
struct ocfs2_xattr_header *xh;
char *entries, *buf, *bucket_buf = NULL;
u64 blkno = bucket_blkno(bucket);
@@ -3792,12 +3776,7 @@ static int ocfs2_defrag_xattr_bucket(struct inode *inode,
end = OCFS2_XATTR_BUCKET_SIZE;
for (i = 0; i < le16_to_cpu(xh->xh_count); i++, xe++) {
offset = le16_to_cpu(xe->xe_name_offset);
- if (ocfs2_xattr_is_local(xe))
- value_len = OCFS2_XATTR_SIZE(
- le64_to_cpu(xe->xe_value_size));
- else
- value_len = OCFS2_XATTR_ROOT_SIZE;
- len = OCFS2_XATTR_SIZE(xe->xe_name_len) + value_len;
+ len = namevalue_size_xe(xe);
/*
* We must make sure that the name/value pair
@@ -3986,7 +3965,7 @@ static int ocfs2_divide_xattr_bucket(struct inode *inode,
int new_bucket_head)
{
int ret, i;
- int count, start, len, name_value_len = 0, xe_len, name_offset = 0;
+ int count, start, len, name_value_len = 0, name_offset = 0;
struct ocfs2_xattr_bucket *s_bucket = NULL, *t_bucket = NULL;
struct ocfs2_xattr_header *xh;
struct ocfs2_xattr_entry *xe;
@@ -4077,13 +4056,7 @@ static int ocfs2_divide_xattr_bucket(struct inode *inode,
name_value_len = 0;
for (i = 0; i < start; i++) {
xe = &xh->xh_entries[i];
- xe_len = OCFS2_XATTR_SIZE(xe->xe_name_len);
- if (ocfs2_xattr_is_local(xe))
- xe_len +=
- OCFS2_XATTR_SIZE(le64_to_cpu(xe->xe_value_size));
- else
- xe_len += OCFS2_XATTR_ROOT_SIZE;
- name_value_len += xe_len;
+ name_value_len += namevalue_size_xe(xe);
if (le16_to_cpu(xe->xe_name_offset) < name_offset)
name_offset = le16_to_cpu(xe->xe_name_offset);
}
@@ -4113,12 +4086,6 @@ static int ocfs2_divide_xattr_bucket(struct inode *inode,
xh->xh_free_start = cpu_to_le16(OCFS2_XATTR_BUCKET_SIZE);
for (i = 0; i < le16_to_cpu(xh->xh_count); i++) {
xe = &xh->xh_entries[i];
- xe_len = OCFS2_XATTR_SIZE(xe->xe_name_len);
- if (ocfs2_xattr_is_local(xe))
- xe_len +=
- OCFS2_XATTR_SIZE(le64_to_cpu(xe->xe_value_size));
- else
- xe_len += OCFS2_XATTR_ROOT_SIZE;
if (le16_to_cpu(xe->xe_name_offset) <
le16_to_cpu(xh->xh_free_start))
xh->xh_free_start = xe->xe_name_offset;
@@ -4755,12 +4722,7 @@ static void ocfs2_xattr_set_entry_normal(struct inode *inode,
if (!xs->not_found) {
xe = xs->here;
offs = le16_to_cpu(xe->xe_name_offset);
- if (ocfs2_xattr_is_local(xe))
- size = OCFS2_XATTR_SIZE(xi->xi_name_len) +
- OCFS2_XATTR_SIZE(le64_to_cpu(xe->xe_value_size));
- else
- size = OCFS2_XATTR_SIZE(xi->xi_name_len) +
- OCFS2_XATTR_SIZE(OCFS2_XATTR_ROOT_SIZE);
+ size = namevalue_size_xe(xe);
/*
* If the new value will be stored outside, xi->xi_value has
@@ -4769,8 +4731,7 @@ static void ocfs2_xattr_set_entry_normal(struct inode *inode,
* new_size safely here.
* See ocfs2_xattr_set_in_bucket.
*/
- new_size = OCFS2_XATTR_SIZE(xi->xi_name_len) +
- OCFS2_XATTR_SIZE(xi->xi_value_len);
+ new_size = namevalue_size_xi(xi);
if (xi->xi_value) {
ocfs2_xa_wipe_namevalue(&loc);
@@ -4836,8 +4797,7 @@ static void ocfs2_xattr_set_entry_normal(struct inode *inode,
set_new_name_value:
/* Insert the new name+value. */
- size = OCFS2_XATTR_SIZE(xi->xi_name_len) +
- OCFS2_XATTR_SIZE(xi->xi_value_len);
+ size = namevalue_size_xi(xi);
/*
* We must make sure that the name/value pair
--
1.6.3.3
^ permalink raw reply related [flat|nested] 33+ messages in thread* [Ocfs2-devel] [PATCH 06/14] ocfs2: Set the xattr name+value pair in one place
2009-08-19 19:54 [Ocfs2-devel] [PATCH 0/14] ocfs2: Unify the setting of extended attributes Joel Becker
` (4 preceding siblings ...)
2009-08-19 19:54 ` [Ocfs2-devel] [PATCH 05/14] ocfs2: Wrap calculation of name+value pair size Joel Becker
@ 2009-08-19 19:54 ` Joel Becker
2009-09-02 9:34 ` Tiger Yang
2009-08-19 19:54 ` [Ocfs2-devel] [PATCH 07/14] ocfs2: Handle value tree roots in ocfs2_xa_set_inline_value() Joel Becker
` (8 subsequent siblings)
14 siblings, 1 reply; 33+ messages in thread
From: Joel Becker @ 2009-08-19 19:54 UTC (permalink / raw)
To: ocfs2-devel
We create two new functions on ocfs2_xa_loc, ocfs2_xa_prepare_entry()
and ocfs2_xa_store_inline_value().
ocfs2_xa_prepare_entry() makes sure that the xl_entry field of
ocfs2_xa_loc is ready to receive an xattr. The entry will point to an
appropriately sized name+value region in storage. If an existing entry
can be reused, it will be. If no entry already exists, it will be
allocated. If there isn't space to allocate it, -ENOSPC will be
returned.
ocfs2_xa_store_inline_value() stores the data that goes into the 'value'
part of the name+value pair. For values that don't fit directly, this
stores the value tree root.
A number of operations are added to ocfs2_xa_loc_operations to support
these functions. This reflects the disparate behaviors of xattr blocks
and buckets.
With these functions, the overlapping ocfs2_xattr_set_entry_local() and
ocfs2_xattr_set_entry_normal() can be replaced with a single call
scheme.
Signed-off-by: Joel Becker <joel.becker@oracle.com>
---
fs/ocfs2/xattr.c | 606 ++++++++++++++++++++++++++++++++++--------------------
1 files changed, 386 insertions(+), 220 deletions(-)
diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index ad010ec..a69bdaf 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -150,11 +150,31 @@ struct ocfs2_xa_loc_operations {
*/
void *(*xlo_offset_pointer)(struct ocfs2_xa_loc *loc, int offset);
+ /* Can we reuse the existing entry for the new value? */
+ int (*xlo_can_reuse)(struct ocfs2_xa_loc *loc,
+ struct ocfs2_xattr_info *xi);
+
+ /* How much space is needed for the new value? */
+ int (*xlo_has_space)(struct ocfs2_xa_loc *loc,
+ struct ocfs2_xattr_info *xi);
+
+ /*
+ * Return the offset of the first name+value pair. This is
+ * the start of our downward-filling free space.
+ */
+ int (*xlo_get_free_start)(struct ocfs2_xa_loc *loc);
+
/*
* Remove the name+value at this location. Do whatever is
* appropriate with the remaining name+value pairs.
*/
void (*xlo_wipe_namevalue)(struct ocfs2_xa_loc *loc);
+
+ /* Fill xl_entry with a new entry */
+ void (*xlo_add_entry)(struct ocfs2_xa_loc *loc, u32 name_hash);
+
+ /* Add name+value storage to an entry */
+ void (*xlo_add_namevalue)(struct ocfs2_xa_loc *loc, int size);
};
/*
@@ -1439,6 +1459,29 @@ static int ocfs2_xattr_set_value_outside(struct inode *inode,
return ret;
}
+static int ocfs2_xa_has_space_helper(int needed_space, int free_start,
+ int num_entries)
+{
+ int free_space;
+
+ free_space = free_start -
+ sizeof(struct ocfs2_xattr_header) -
+ (num_entries * sizeof(struct ocfs2_xattr_entry)) -
+ OCFS2_XATTR_HEADER_GAP;
+ if (free_space < 0)
+ return -EIO;
+ if (free_space < needed_space)
+ return -ENOSPC;
+
+ return 0;
+}
+
+/* Give a pointer into the storage for the given offset */
+static void *ocfs2_xa_offset_pointer(struct ocfs2_xa_loc *loc, int offset)
+{
+ return loc->xl_ops->xlo_offset_pointer(loc, offset);
+}
+
/*
* Wipe the name+value pair and allow the storage to reclaim it. This
* must be followed by either removal of the entry or a call to
@@ -1449,6 +1492,60 @@ static void ocfs2_xa_wipe_namevalue(struct ocfs2_xa_loc *loc)
loc->xl_ops->xlo_wipe_namevalue(loc);
}
+/*
+ * Find lowest offset to a name+value pair. This is the start of our
+ * downward-growing free space.
+ */
+static int ocfs2_xa_get_free_start(struct ocfs2_xa_loc *loc)
+{
+ return loc->xl_ops->xlo_get_free_start(loc);
+}
+
+/* Can we reuse loc->xl_entry for xi? */
+static int ocfs2_xa_can_reuse_entry(struct ocfs2_xa_loc *loc,
+ struct ocfs2_xattr_info *xi)
+{
+ return loc->xl_ops->xlo_can_reuse(loc, xi);
+}
+
+/* How much free space is needed to set the new value */
+static int ocfs2_xa_has_space(struct ocfs2_xa_loc *loc,
+ struct ocfs2_xattr_info *xi)
+{
+ return loc->xl_ops->xlo_has_space(loc, xi);
+}
+
+static void ocfs2_xa_add_entry(struct ocfs2_xa_loc *loc, u32 name_hash)
+{
+ loc->xl_ops->xlo_add_entry(loc, name_hash);
+ loc->xl_entry->xe_name_hash = cpu_to_le32(name_hash);
+ /*
+ * We can't leave the new entry's xe_name_offset at zero or
+ * add_namevalue() will go nuts. We set it to the size of our
+ * storage so that it can never be less than any other entry.
+ */
+ loc->xl_entry->xe_name_offset = cpu_to_le16(loc->xl_size);
+}
+
+static void ocfs2_xa_add_namevalue(struct ocfs2_xa_loc *loc,
+ struct ocfs2_xattr_info *xi)
+{
+ int size = namevalue_size_xi(xi);
+ int nameval_offset;
+ char *nameval_buf;
+
+ loc->xl_ops->xlo_add_namevalue(loc, size);
+ loc->xl_entry->xe_value_size = cpu_to_le64(xi->xi_value_len);
+ ocfs2_xattr_set_type(loc->xl_entry, xi->xi_name_index);
+ ocfs2_xattr_set_local(loc->xl_entry,
+ xi->xi_value_len <= OCFS2_XATTR_INLINE_SIZE);
+
+ nameval_offset = le16_to_cpu(loc->xl_entry->xe_name_offset);
+ nameval_buf = ocfs2_xa_offset_pointer(loc, nameval_offset);
+ memset(nameval_buf, 0, size);
+ memcpy(nameval_buf, xi->xi_name, xi->xi_name_len);
+}
+
static void *ocfs2_xa_block_offset_pointer(struct ocfs2_xa_loc *loc,
int offset)
{
@@ -1458,6 +1555,56 @@ static void *ocfs2_xa_block_offset_pointer(struct ocfs2_xa_loc *loc,
return bh->b_data + offset;
}
+static int ocfs2_xa_block_can_reuse(struct ocfs2_xa_loc *loc,
+ struct ocfs2_xattr_info *xi)
+{
+ /*
+ * Block storage is strict. If the sizes aren't exact, we will
+ * remove the old one and reinsert the new.
+ */
+ return namevalue_size_xe(loc->xl_entry) ==
+ namevalue_size_xi(xi);
+}
+
+static int ocfs2_xa_block_get_free_start(struct ocfs2_xa_loc *loc)
+{
+ struct ocfs2_xattr_header *xh = loc->xl_header;
+ int i, count = le16_to_cpu(xh->xh_count);
+ int offset, free_start = loc->xl_size;
+
+ for (i = 0; i < count; i++) {
+ offset = le16_to_cpu(xh->xh_entries[i].xe_name_offset);
+ if (offset < free_start)
+ free_start = offset;
+ }
+
+ return free_start;
+}
+
+static int ocfs2_xa_block_has_space(struct ocfs2_xa_loc *loc,
+ struct ocfs2_xattr_info *xi)
+{
+ int count = le16_to_cpu(loc->xl_header->xh_count);
+ int free_start = ocfs2_xa_get_free_start(loc);
+ int needed_space = ocfs2_xi_entry_usage(xi);
+
+ /*
+ * Block storage will reclaim the original entry before inserting
+ * the new value, so we only need the difference. If the new
+ * entry is smaller than the old one, we don't need anything.
+ */
+ if (loc->xl_entry) {
+ /* Don't need space if we're reusing! */
+ if (ocfs2_xa_can_reuse_entry(loc, xi))
+ needed_space = 0;
+ else
+ needed_space -= ocfs2_xe_entry_usage(loc->xl_entry);
+ }
+ if (needed_space < 0)
+ needed_space = 0;
+ return ocfs2_xa_has_space_helper(needed_space, free_start, count);
+}
+
/*
* Block storage for xattrs keeps the name+value pairs compacted. When
* we remove one, we have to shift any that preceded it towards the end.
@@ -1472,13 +1619,7 @@ static void ocfs2_xa_block_wipe_namevalue(struct ocfs2_xa_loc *loc)
namevalue_offset = le16_to_cpu(entry->xe_name_offset);
namevalue_size = namevalue_size_xe(entry);
-
- for (i = 0, first_namevalue_offset = loc->xl_size;
- i < count; i++) {
- offset = le16_to_cpu(xh->xh_entries[i].xe_name_offset);
- if (offset < first_namevalue_offset)
- first_namevalue_offset = offset;
- }
+ first_namevalue_offset = ocfs2_xa_get_free_start(loc);
/* Shift the name+value pairs */
memmove((char *)xh + first_namevalue_offset + namevalue_size,
@@ -1500,13 +1641,33 @@ static void ocfs2_xa_block_wipe_namevalue(struct ocfs2_xa_loc *loc)
*/
}
+static void ocfs2_xa_block_add_entry(struct ocfs2_xa_loc *loc, u32 name_hash)
+{
+ int count = le16_to_cpu(loc->xl_header->xh_count);
+ loc->xl_entry = &(loc->xl_header->xh_entries[count]);
+ le16_add_cpu(&loc->xl_header->xh_count, 1);
+ memset(loc->xl_entry, 0, sizeof(struct ocfs2_xattr_entry));
+}
+
+static void ocfs2_xa_block_add_namevalue(struct ocfs2_xa_loc *loc, int size)
+{
+ int free_start = ocfs2_xa_get_free_start(loc);
+
+ loc->xl_entry->xe_name_offset = cpu_to_le16(free_start - size);
+}
+
/*
* Operations for xattrs stored in blocks. This includes inline inode
* storage and unindexed ocfs2_xattr_blocks.
*/
static const struct ocfs2_xa_loc_operations ocfs2_xa_block_loc_ops = {
.xlo_offset_pointer = ocfs2_xa_block_offset_pointer,
+ .xlo_has_space = ocfs2_xa_block_has_space,
+ .xlo_can_reuse = ocfs2_xa_block_can_reuse,
+ .xlo_get_free_start = ocfs2_xa_block_get_free_start,
.xlo_wipe_namevalue = ocfs2_xa_block_wipe_namevalue,
+ .xlo_add_entry = ocfs2_xa_block_add_entry,
+ .xlo_add_namevalue = ocfs2_xa_block_add_namevalue,
};
static void *ocfs2_xa_bucket_offset_pointer(struct ocfs2_xa_loc *loc,
@@ -1523,16 +1684,128 @@ static void *ocfs2_xa_bucket_offset_pointer(struct ocfs2_xa_loc *loc,
return bucket_block(bucket, block) + block_offset;
}
+static int ocfs2_xa_bucket_can_reuse(struct ocfs2_xa_loc *loc,
+ struct ocfs2_xattr_info *xi)
+{
+ return namevalue_size_xe(loc->xl_entry) >=
+ namevalue_size_xi(xi);
+}
+
+static int ocfs2_xa_bucket_get_free_start(struct ocfs2_xa_loc *loc)
+{
+ struct ocfs2_xattr_bucket *bucket = loc->xl_storage;
+ return le16_to_cpu(bucket_xh(bucket)->xh_free_start);
+}
+
+static int ocfs2_bucket_align_free_start(struct super_block *sb,
+ int free_start, int size)
+{
+ /*
+ * We need to make sure that the name+value pair fits within
+ * one block.
+ */
+ if (((free_start - size) >> sb->s_blocksize_bits) !=
+ ((free_start - 1) >> sb->s_blocksize_bits))
+ free_start -= free_start % sb->s_blocksize;
+
+ return free_start;
+}
+
+static int ocfs2_xa_bucket_has_space(struct ocfs2_xa_loc *loc,
+ struct ocfs2_xattr_info *xi)
+{
+ int count = le16_to_cpu(loc->xl_header->xh_count);
+ int free_start = ocfs2_xa_get_free_start(loc);
+ int needed_space = ocfs2_xi_entry_usage(xi);
+ int size = namevalue_size_xi(xi);
+ struct ocfs2_xattr_bucket *bucket = loc->xl_storage;
+ struct super_block *sb = bucket->bu_inode->i_sb;
+
+ /*
+ * Bucket storage does not reclaim name+value pairs it cannot
+ * reuse. They live as holes until the bucket fills, and then
+ * the bucket is defragmented. However, the bucket can reclaim
+ * the ocfs2_xattr_entry.
+ */
+ if (loc->xl_entry) {
+ /* Don't need space if we're reusing! */
+ if (ocfs2_xa_can_reuse_entry(loc, xi))
+ needed_space = 0;
+ else
+ needed_space -= sizeof(struct ocfs2_xattr_entry);
+ }
+ BUG_ON(needed_space < 0);
+
+ free_start = ocfs2_bucket_align_free_start(sb, free_start, size);
+ return ocfs2_xa_has_space_helper(needed_space, free_start, count);
+}
+
static void ocfs2_xa_bucket_wipe_namevalue(struct ocfs2_xa_loc *loc)
{
le16_add_cpu(&loc->xl_header->xh_name_value_len,
-namevalue_size_xe(loc->xl_entry));
}
+static void ocfs2_xa_bucket_add_entry(struct ocfs2_xa_loc *loc, u32 name_hash)
+{
+ struct ocfs2_xattr_header *xh = loc->xl_header;
+ int count = le16_to_cpu(xh->xh_count);
+ int low = 0, high = count - 1, tmp;
+ struct ocfs2_xattr_entry *tmp_xe;
+
+ /*
+ * We keep buckets sorted by name_hash, so we need to find
+ * our insert place.
+ */
+ while (low <= high && count) {
+ tmp = (low + high) / 2;
+ tmp_xe = &xh->xh_entries[tmp];
+
+ if (name_hash > le32_to_cpu(tmp_xe->xe_name_hash))
+ low = tmp + 1;
+ else if (name_hash < le32_to_cpu(tmp_xe->xe_name_hash))
+ high = tmp - 1;
+ else {
+ low = tmp;
+ break;
+ }
+ }
+
+ if (low != count)
+ memmove(&xh->xh_entries[low + 1],
+ &xh->xh_entries[low],
+ ((count - low) * sizeof(struct ocfs2_xattr_entry)));
+
+ le16_add_cpu(&xh->xh_count, 1);
+ loc->xl_entry = &xh->xh_entries[low];
+ memset(loc->xl_entry, 0, sizeof(struct ocfs2_xattr_entry));
+}
+
+static void ocfs2_xa_bucket_add_namevalue(struct ocfs2_xa_loc *loc, int size)
+{
+ int free_start = ocfs2_xa_get_free_start(loc);
+ struct ocfs2_xattr_header *xh = loc->xl_header;
+ struct ocfs2_xattr_bucket *bucket = loc->xl_storage;
+ struct super_block *sb = bucket->bu_inode->i_sb;
+ int nameval_offset;
+
+ free_start = ocfs2_bucket_align_free_start(sb, free_start, size);
+ nameval_offset = free_start - size;
+ loc->xl_entry->xe_name_offset = cpu_to_le16(nameval_offset);
+ xh->xh_free_start = cpu_to_le16(nameval_offset);
+ le16_add_cpu(&xh->xh_name_value_len, size);
+
+}
+
/* Operations for xattrs stored in buckets. */
static const struct ocfs2_xa_loc_operations ocfs2_xa_bucket_loc_ops = {
.xlo_offset_pointer = ocfs2_xa_bucket_offset_pointer,
+ .xlo_has_space = ocfs2_xa_bucket_has_space,
+ .xlo_can_reuse = ocfs2_xa_bucket_can_reuse,
+ .xlo_get_free_start = ocfs2_xa_bucket_get_free_start,
.xlo_wipe_namevalue = ocfs2_xa_bucket_wipe_namevalue,
+ .xlo_add_entry = ocfs2_xa_bucket_add_entry,
+ .xlo_add_namevalue = ocfs2_xa_bucket_add_namevalue,
};
static void ocfs2_xa_remove_entry(struct ocfs2_xa_loc *loc)
@@ -1561,6 +1834,74 @@ static void ocfs2_xa_remove_entry(struct ocfs2_xa_loc *loc)
}
}
+/*
+ * Prepares loc->xl_entry to receive the new xattr. This includes
+ * properly setting up the name+value pair region. If loc->xl_entry
+ * already exists, it will take care of modifying it appropriately.
+ * This also includes deleting entries, but don't call this to remove
+ * a non-existant entry. That's just a bug.
+ *
+ * Note that this modifies the data. You did journal_access already,
+ * right?
+ */
+static int ocfs2_xa_prepare_entry(struct ocfs2_xa_loc *loc,
+ struct ocfs2_xattr_info *xi,
+ u32 name_hash)
+{
+ int rc = 0;
+ int name_size = OCFS2_XATTR_SIZE(xi->xi_name_len);
+ char *nameval_buf;
+
+ if (!xi->xi_value) {
+ ocfs2_xa_remove_entry(loc);
+ goto out;
+ }
+
+ rc = ocfs2_xa_has_space(loc, xi);
+ if (rc)
+ goto out;
+
+ if (loc->xl_entry) {
+ if (ocfs2_xa_can_reuse_entry(loc, xi)) {
+ nameval_buf = ocfs2_xa_offset_pointer(loc,
+ le16_to_cpu(loc->xl_entry->xe_name_offset));
+ memset(nameval_buf + name_size, 0,
+ namevalue_size_xe(loc->xl_entry) - name_size);
+ loc->xl_entry->xe_value_size =
+ cpu_to_le64(xi->xi_value_len);
+ goto out;
+ }
+
+ ocfs2_xa_wipe_namevalue(loc);
+ } else
+ ocfs2_xa_add_entry(loc, name_hash);
+
+ /*
+ * If we get here, we have a blank entry. Fill it. We grow our
+ * name+value pair back from the end.
+ */
+ ocfs2_xa_add_namevalue(loc, xi);
+
+out:
+ return rc;
+}
+
+/*
+ * Store the value portion of the name+value pair. This is either an
+ * inline value or the tree root of an external value.
+ */
+static void ocfs2_xa_store_inline_value(struct ocfs2_xa_loc *loc,
+ struct ocfs2_xattr_info *xi)
+{
+ int nameval_offset = le16_to_cpu(loc->xl_entry->xe_name_offset);
+ int name_size = OCFS2_XATTR_SIZE(xi->xi_name_len);
+ int size = namevalue_size_xi(xi);
+ char *nameval_buf;
+
+ nameval_buf = ocfs2_xa_offset_pointer(loc, nameval_offset);
+ memcpy(nameval_buf + name_size, xi->xi_value, size - name_size);
+}
+
static void ocfs2_init_dinode_xa_loc(struct ocfs2_xa_loc *loc,
struct inode *inode,
struct buffer_head *bh,
@@ -1611,81 +1952,6 @@ static void ocfs2_init_xattr_bucket_xa_loc(struct ocfs2_xa_loc *loc,
loc->xl_size = OCFS2_XATTR_BUCKET_SIZE;
}
-/*
- * ocfs2_xattr_set_entry_local()
- *
- * Set, replace or remove extended attribute in local.
- */
-static void ocfs2_xattr_set_entry_local(struct inode *inode,
- struct ocfs2_xattr_info *xi,
- struct ocfs2_xattr_search *xs,
- struct ocfs2_xattr_entry *last,
- size_t min_offs)
-{
- struct ocfs2_xa_loc loc;
-
- if (xs->xattr_bh == xs->inode_bh)
- ocfs2_init_dinode_xa_loc(&loc, inode, xs->inode_bh,
- xs->not_found ? NULL : xs->here);
- else
- ocfs2_init_xattr_block_xa_loc(&loc, xs->xattr_bh,
- xs->not_found ? NULL : xs->here);
- if (xi->xi_value && xs->not_found) {
- /* Insert the new xattr entry. */
- le16_add_cpu(&xs->header->xh_count, 1);
- ocfs2_xattr_set_type(last, xi->xi_name_index);
- ocfs2_xattr_set_local(last, 1);
- last->xe_name_len = xi->xi_name_len;
- } else {
- void *first_val;
- void *val;
- size_t offs, size;
-
- first_val = xs->base + min_offs;
- offs = le16_to_cpu(xs->here->xe_name_offset);
- val = xs->base + offs;
-
- size = namevalue_size_xe(xs->here);
- if (xi->xi_value && (size == namevalue_size_xi(xi))) {
- /* The old and the new value have the
- same size. Just replace the value. */
- ocfs2_xattr_set_local(xs->here, 1);
- xs->here->xe_value_size = cpu_to_le64(xi->xi_value_len);
- /* Clear value bytes. */
- memset(val + OCFS2_XATTR_SIZE(xi->xi_name_len),
- 0,
- OCFS2_XATTR_SIZE(xi->xi_value_len));
- memcpy(val + OCFS2_XATTR_SIZE(xi->xi_name_len),
- xi->xi_value,
- xi->xi_value_len);
- return;
- }
-
- if (!xi->xi_value)
- ocfs2_xa_remove_entry(&loc);
- else
- ocfs2_xa_wipe_namevalue(&loc);
-
- min_offs += size;
- }
- if (xi->xi_value) {
- /* Insert the new name+value. */
- size_t size = namevalue_size_xi(xi);
- void *val = xs->base + min_offs - size;
-
- xs->here->xe_name_offset = cpu_to_le16(min_offs - size);
- memset(val, 0, size);
- memcpy(val, xi->xi_name, xi->xi_name_len);
- memcpy(val + OCFS2_XATTR_SIZE(xi->xi_name_len),
- xi->xi_value,
- xi->xi_value_len);
- xs->here->xe_value_size = cpu_to_le64(xi->xi_value_len);
- ocfs2_xattr_set_local(xs->here, 1);
- ocfs2_xattr_hash_entry(inode, xs->header, xs->here);
- }
-
- return;
-}
/*
* ocfs2_xattr_set_entry()
@@ -1693,7 +1959,7 @@ static void ocfs2_xattr_set_entry_local(struct inode *inode,
* Set extended attribute entry into inode or block.
*
* If extended attribute value size > OCFS2_XATTR_INLINE_SIZE,
- * We first insert tree root(ocfs2_xattr_value_root) with set_entry_local(),
+ * We first insert tree root(ocfs2_xattr_value_root) like a normal value,
* then set value in B tree with set_value_outside().
*/
static int ocfs2_xattr_set_entry(struct inode *inode,
@@ -1709,6 +1975,9 @@ static int ocfs2_xattr_set_entry(struct inode *inode,
size_t size_l = 0;
handle_t *handle = ctxt->handle;
int free, i, ret;
+ u32 name_hash = ocfs2_xattr_name_hash(inode, xi->xi_name,
+ xi->xi_name_len);
+ struct ocfs2_xa_loc loc;
struct ocfs2_xattr_info xi_l = {
.xi_name_index = xi->xi_name_index,
.xi_name = xi->xi_name,
@@ -1840,11 +2109,28 @@ static int ocfs2_xattr_set_entry(struct inode *inode,
}
}
+ if (xs->xattr_bh == xs->inode_bh)
+ ocfs2_init_dinode_xa_loc(&loc, inode, xs->inode_bh,
+ xs->not_found ? NULL : xs->here);
+ else
+ ocfs2_init_xattr_block_xa_loc(&loc, xs->xattr_bh,
+ xs->not_found ? NULL : xs->here);
+
/*
- * Set value in local, include set tree root in local.
- * This is the first step for value size >INLINE_SIZE.
+ * Prepare our entry and insert the inline value. This will
+ * be a value tree root for values that are larger than
+ * OCFS2_XATTR_INLINE_SIZE.
*/
- ocfs2_xattr_set_entry_local(inode, &xi_l, xs, last, min_offs);
+ ret = ocfs2_xa_prepare_entry(&loc, xi, name_hash);
+ if (ret) {
+ if (ret != -ENOSPC)
+ mlog_errno(ret);
+ goto out;
+ }
+ /* XXX For now, until we make ocfs2_xa_prepare_entry() primary */
+ BUG_ON(ret == -ENOSPC);
+ ocfs2_xa_store_inline_value(&loc, xi);
+ xs->here = loc.xl_entry;
if (!(flag & OCFS2_INLINE_XATTR_FL)) {
ret = ocfs2_journal_dirty(handle, xs->xattr_bh);
@@ -4697,139 +4983,6 @@ static inline char *ocfs2_xattr_bucket_get_val(struct inode *inode,
}
/*
- * Handle the normal xattr set, including replace, delete and new.
- *
- * Note: "local" indicates the real data's locality. So we can't
- * just its bucket locality by its length.
- */
-static void ocfs2_xattr_set_entry_normal(struct inode *inode,
- struct ocfs2_xattr_info *xi,
- struct ocfs2_xattr_search *xs,
- u32 name_hash,
- int local)
-{
- struct ocfs2_xattr_entry *last, *xe;
- struct ocfs2_xattr_header *xh = xs->header;
- u16 count = le16_to_cpu(xh->xh_count), start;
- size_t blocksize = inode->i_sb->s_blocksize;
- char *val;
- size_t offs, size, new_size;
- struct ocfs2_xa_loc loc;
-
- ocfs2_init_xattr_bucket_xa_loc(&loc, xs->bucket,
- xs->not_found ? NULL : xs->here);
- last = &xh->xh_entries[count];
- if (!xs->not_found) {
- xe = xs->here;
- offs = le16_to_cpu(xe->xe_name_offset);
- size = namevalue_size_xe(xe);
-
- /*
- * If the new value will be stored outside, xi->xi_value has
- * been initalized as an empty ocfs2_xattr_value_root, and
- * the same goes with xi->xi_value_len, so we can set
- * new_size safely here.
- * See ocfs2_xattr_set_in_bucket.
- */
- new_size = namevalue_size_xi(xi);
-
- if (xi->xi_value) {
- ocfs2_xa_wipe_namevalue(&loc);
- if (new_size > size)
- goto set_new_name_value;
-
- /* Now replace the old value with new one. */
- if (local)
- xe->xe_value_size =
- cpu_to_le64(xi->xi_value_len);
- else
- xe->xe_value_size = 0;
-
- val = ocfs2_xattr_bucket_get_val(inode,
- xs->bucket, offs);
- memset(val + OCFS2_XATTR_SIZE(xi->xi_name_len), 0,
- size - OCFS2_XATTR_SIZE(xi->xi_name_len));
- if (OCFS2_XATTR_SIZE(xi->xi_value_len) > 0)
- memcpy(val + OCFS2_XATTR_SIZE(xi->xi_name_len),
- xi->xi_value, xi->xi_value_len);
-
- le16_add_cpu(&xh->xh_name_value_len, new_size);
- ocfs2_xattr_set_local(xe, local);
- return;
- } else {
- ocfs2_xa_remove_entry(&loc);
- if (!xh->xh_count)
- xh->xh_free_start =
- cpu_to_le16(OCFS2_XATTR_BUCKET_SIZE);
-
- return;
- }
- } else {
- /* find a new entry for insert. */
- int low = 0, high = count - 1, tmp;
- struct ocfs2_xattr_entry *tmp_xe;
-
- while (low <= high && count) {
- tmp = (low + high) / 2;
- tmp_xe = &xh->xh_entries[tmp];
-
- if (name_hash > le32_to_cpu(tmp_xe->xe_name_hash))
- low = tmp + 1;
- else if (name_hash <
- le32_to_cpu(tmp_xe->xe_name_hash))
- high = tmp - 1;
- else {
- low = tmp;
- break;
- }
- }
-
- xe = &xh->xh_entries[low];
- if (low != count)
- memmove(xe + 1, xe, (void *)last - (void *)xe);
-
- le16_add_cpu(&xh->xh_count, 1);
- memset(xe, 0, sizeof(struct ocfs2_xattr_entry));
- xe->xe_name_hash = cpu_to_le32(name_hash);
- xe->xe_name_len = xi->xi_name_len;
- ocfs2_xattr_set_type(xe, xi->xi_name_index);
- }
-
-set_new_name_value:
- /* Insert the new name+value. */
- size = namevalue_size_xi(xi);
-
- /*
- * We must make sure that the name/value pair
- * exists in the same block.
- */
- offs = le16_to_cpu(xh->xh_free_start);
- start = offs - size;
-
- if (start >> inode->i_sb->s_blocksize_bits !=
- (offs - 1) >> inode->i_sb->s_blocksize_bits) {
- offs = offs - offs % blocksize;
- xh->xh_free_start = cpu_to_le16(offs);
- }
-
- val = ocfs2_xattr_bucket_get_val(inode, xs->bucket, offs - size);
- xe->xe_name_offset = cpu_to_le16(offs - size);
-
- memset(val, 0, size);
- memcpy(val, xi->xi_name, xi->xi_name_len);
- memcpy(val + OCFS2_XATTR_SIZE(xi->xi_name_len), xi->xi_value,
- xi->xi_value_len);
-
- xe->xe_value_size = cpu_to_le64(xi->xi_value_len);
- ocfs2_xattr_set_local(xe, local);
- xs->here = xe;
- le16_add_cpu(&xh->xh_free_start, -size);
- le16_add_cpu(&xh->xh_name_value_len, size);
-
- return;
-}
-
-/*
* Set the xattr entry in the specified bucket.
* The bucket is indicated by xs->bucket and it should have the enough
* space for the xattr insertion.
@@ -4843,6 +4996,7 @@ static int ocfs2_xattr_set_entry_in_bucket(struct inode *inode,
{
int ret;
u64 blkno;
+ struct ocfs2_xa_loc loc;
mlog(0, "Set xattr entry len = %lu index = %d in bucket %llu\n",
(unsigned long)xi->xi_value_len, xi->xi_name_index,
@@ -4865,7 +5019,19 @@ static int ocfs2_xattr_set_entry_in_bucket(struct inode *inode,
goto out;
}
- ocfs2_xattr_set_entry_normal(inode, xi, xs, name_hash, local);
+ ocfs2_init_xattr_bucket_xa_loc(&loc, xs->bucket,
+ xs->not_found ? NULL : xs->here);
+ ret = ocfs2_xa_prepare_entry(&loc, xi, name_hash);
+ if (ret) {
+ if (ret != -ENOSPC)
+ mlog_errno(ret);
+ goto out;
+ }
+ /* XXX For now, until we make ocfs2_xa_prepare_entry() primary */
+ BUG_ON(ret == -ENOSPC);
+ ocfs2_xa_store_inline_value(&loc, xi);
+ xs->here = loc.xl_entry;
+
ocfs2_xattr_bucket_journal_dirty(handle, xs->bucket);
out:
--
1.6.3.3
^ permalink raw reply related [flat|nested] 33+ messages in thread* [Ocfs2-devel] [PATCH 06/14] ocfs2: Set the xattr name+value pair in one place
2009-08-19 19:54 ` [Ocfs2-devel] [PATCH 06/14] ocfs2: Set the xattr name+value pair in one place Joel Becker
@ 2009-09-02 9:34 ` Tiger Yang
2009-09-02 10:30 ` Joel Becker
0 siblings, 1 reply; 33+ messages in thread
From: Tiger Yang @ 2009-09-02 9:34 UTC (permalink / raw)
To: ocfs2-devel
Joel Becker wrote:
> +
> +static void ocfs2_xa_add_namevalue(struct ocfs2_xa_loc *loc,
> + struct ocfs2_xattr_info *xi)
> +{
> + int size = namevalue_size_xi(xi);
> + int nameval_offset;
> + char *nameval_buf;
> +
> + loc->xl_ops->xlo_add_namevalue(loc, size);
> + loc->xl_entry->xe_value_size = cpu_to_le64(xi->xi_value_len);
> + ocfs2_xattr_set_type(loc->xl_entry, xi->xi_name_index);
> + ocfs2_xattr_set_local(loc->xl_entry,
> + xi->xi_value_len <= OCFS2_XATTR_INLINE_SIZE);
> +
> + nameval_offset = le16_to_cpu(loc->xl_entry->xe_name_offset);
> + nameval_buf = ocfs2_xa_offset_pointer(loc, nameval_offset);
> + memset(nameval_buf, 0, size);
> + memcpy(nameval_buf, xi->xi_name, xi->xi_name_len);
> +}
In add namevalue, we should update xl_entry->xe_name_len as well. In
here we only have a blank entry, so we need to fill this one.
Maybe we should update xe_name_len somewhere else.
thanks,
tiger
^ permalink raw reply [flat|nested] 33+ messages in thread* [Ocfs2-devel] [PATCH 06/14] ocfs2: Set the xattr name+value pair in one place
2009-09-02 9:34 ` Tiger Yang
@ 2009-09-02 10:30 ` Joel Becker
0 siblings, 0 replies; 33+ messages in thread
From: Joel Becker @ 2009-09-02 10:30 UTC (permalink / raw)
To: ocfs2-devel
On Wed, Sep 02, 2009 at 05:34:16PM +0800, Tiger Yang wrote:
> Joel Becker wrote:
> >+
> >+static void ocfs2_xa_add_namevalue(struct ocfs2_xa_loc *loc,
> >+ struct ocfs2_xattr_info *xi)
> >+{
> >+ int size = namevalue_size_xi(xi);
> >+ int nameval_offset;
> >+ char *nameval_buf;
> >+
> >+ loc->xl_ops->xlo_add_namevalue(loc, size);
> >+ loc->xl_entry->xe_value_size = cpu_to_le64(xi->xi_value_len);
> >+ ocfs2_xattr_set_type(loc->xl_entry, xi->xi_name_index);
> >+ ocfs2_xattr_set_local(loc->xl_entry,
> >+ xi->xi_value_len <= OCFS2_XATTR_INLINE_SIZE);
> >+
> >+ nameval_offset = le16_to_cpu(loc->xl_entry->xe_name_offset);
> >+ nameval_buf = ocfs2_xa_offset_pointer(loc, nameval_offset);
> >+ memset(nameval_buf, 0, size);
> >+ memcpy(nameval_buf, xi->xi_name, xi->xi_name_len);
> >+}
> In add namevalue, we should update xl_entry->xe_name_len as well. In
> here we only have a blank entry, so we need to fill this one.
> Maybe we should update xe_name_len somewhere else.
Good catch. It belongs right here. Fixed version pushed.
Joel
--
"Can any of you seriously say the Bill of Rights could get through
Congress today? It wouldn't even get out of committee."
- F. Lee Bailey
Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127
^ permalink raw reply [flat|nested] 33+ messages in thread
* [Ocfs2-devel] [PATCH 07/14] ocfs2: Handle value tree roots in ocfs2_xa_set_inline_value()
2009-08-19 19:54 [Ocfs2-devel] [PATCH 0/14] ocfs2: Unify the setting of extended attributes Joel Becker
` (5 preceding siblings ...)
2009-08-19 19:54 ` [Ocfs2-devel] [PATCH 06/14] ocfs2: Set the xattr name+value pair in one place Joel Becker
@ 2009-08-19 19:54 ` Joel Becker
2009-08-19 19:54 ` [Ocfs2-devel] [PATCH 08/14] ocfs2: Provide ocfs2_xa_fill_value_buf() for external value processing Joel Becker
` (7 subsequent siblings)
14 siblings, 0 replies; 33+ messages in thread
From: Joel Becker @ 2009-08-19 19:54 UTC (permalink / raw)
To: ocfs2-devel
Previously the xattr code would send in a fake value, containing a tree
root, to the function that installed name+value pairs. Instead, we pass
the real value to ocfs2_xa_set_inline_value(), and it notices that the
value cannot fit. Thus, it installs a tree root.
Signed-off-by: Joel Becker <joel.becker@oracle.com>
---
fs/ocfs2/xattr.c | 54 ++++++++++++++++--------------------------------------
1 files changed, 16 insertions(+), 38 deletions(-)
diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index a69bdaf..0f24e0a 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -1895,11 +1895,16 @@ static void ocfs2_xa_store_inline_value(struct ocfs2_xa_loc *loc,
{
int nameval_offset = le16_to_cpu(loc->xl_entry->xe_name_offset);
int name_size = OCFS2_XATTR_SIZE(xi->xi_name_len);
- int size = namevalue_size_xi(xi);
+ int inline_value_size = namevalue_size_xi(xi) - name_size;
+ const void *value = xi->xi_value;
char *nameval_buf;
+ if (xi->xi_value_len > OCFS2_XATTR_INLINE_SIZE) {
+ value = &def_xv;
+ inline_value_size = OCFS2_XATTR_ROOT_SIZE;
+ }
nameval_buf = ocfs2_xa_offset_pointer(loc, nameval_offset);
- memcpy(nameval_buf + name_size, xi->xi_value, size - name_size);
+ memcpy(nameval_buf + name_size, value, inline_value_size);
}
static void ocfs2_init_dinode_xa_loc(struct ocfs2_xa_loc *loc,
@@ -1978,13 +1983,6 @@ static int ocfs2_xattr_set_entry(struct inode *inode,
u32 name_hash = ocfs2_xattr_name_hash(inode, xi->xi_name,
xi->xi_name_len);
struct ocfs2_xa_loc loc;
- struct ocfs2_xattr_info xi_l = {
- .xi_name_index = xi->xi_name_index,
- .xi_name = xi->xi_name,
- .xi_name_len = xi->xi_name_len,
- .xi_value = xi->xi_value,
- .xi_value_len = xi->xi_value_len,
- };
struct ocfs2_xattr_value_buf vb = {
.vb_bh = xs->xattr_bh,
.vb_access = ocfs2_journal_access_di,
@@ -2014,16 +2012,9 @@ static int ocfs2_xattr_set_entry(struct inode *inode,
free += ocfs2_xe_entry_usage(xs->here);
/* Check free space in inode or block */
- if (xi->xi_value) {
- if (free < ocfs2_xi_entry_usage(xi)) {
- ret = -ENOSPC;
- goto out;
- }
- if (xi->xi_value_len > OCFS2_XATTR_INLINE_SIZE) {
- size_l = namevalue_size_xi(xi);
- xi_l.xi_value = (void *)&def_xv;
- xi_l.xi_value_len = OCFS2_XATTR_ROOT_SIZE;
- }
+ if (xi->xi_value && (free < ocfs2_xi_entry_usage(xi))) {
+ ret = -ENOSPC;
+ goto out;
}
if (!xs->not_found) {
@@ -4991,8 +4982,7 @@ 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)
+ u32 name_hash)
{
int ret;
u64 blkno;
@@ -5293,13 +5283,14 @@ static int ocfs2_xattr_set_in_bucket(struct inode *inode,
struct ocfs2_xattr_search *xs,
struct ocfs2_xattr_set_ctxt *ctxt)
{
- int ret, local = 1;
+ int ret;
size_t value_len;
char *val = (char *)xi->xi_value;
struct ocfs2_xattr_entry *xe = xs->here;
u32 name_hash = ocfs2_xattr_name_hash(inode, xi->xi_name,
xi->xi_name_len);
+ value_len = xi->xi_value_len;
if (!xs->not_found && !ocfs2_xattr_is_local(xe)) {
/*
* We need to truncate the xattr storage first.
@@ -5313,9 +5304,7 @@ static int ocfs2_xattr_set_in_bucket(struct inode *inode,
* the modification to the xattr block will be done
* by following steps.
*/
- if (xi->xi_value_len > OCFS2_XATTR_INLINE_SIZE)
- value_len = xi->xi_value_len;
- else
+ if (xi->xi_value_len <= OCFS2_XATTR_INLINE_SIZE)
value_len = 0;
ret = ocfs2_xattr_bucket_value_truncate_xs(inode, xs,
@@ -5328,26 +5317,15 @@ static int ocfs2_xattr_set_in_bucket(struct inode *inode,
goto set_value_outside;
}
- value_len = xi->xi_value_len;
/* So we have to handle the inside block change now. */
- if (value_len > OCFS2_XATTR_INLINE_SIZE) {
- /*
- * If the new value will be stored outside of block,
- * initalize a new empty value root and insert it first.
- */
- local = 0;
- xi->xi_value = &def_xv;
- xi->xi_value_len = OCFS2_XATTR_ROOT_SIZE;
- }
-
ret = ocfs2_xattr_set_entry_in_bucket(inode, ctxt->handle, xi, xs,
- name_hash, local);
+ name_hash);
if (ret) {
mlog_errno(ret);
goto out;
}
- if (value_len <= OCFS2_XATTR_INLINE_SIZE)
+ if (xi->xi_value_len <= OCFS2_XATTR_INLINE_SIZE)
goto out;
/* allocate the space now for the outside block storage. */
--
1.6.3.3
^ permalink raw reply related [flat|nested] 33+ messages in thread* [Ocfs2-devel] [PATCH 08/14] ocfs2: Provide ocfs2_xa_fill_value_buf() for external value processing
2009-08-19 19:54 [Ocfs2-devel] [PATCH 0/14] ocfs2: Unify the setting of extended attributes Joel Becker
` (6 preceding siblings ...)
2009-08-19 19:54 ` [Ocfs2-devel] [PATCH 07/14] ocfs2: Handle value tree roots in ocfs2_xa_set_inline_value() Joel Becker
@ 2009-08-19 19:54 ` Joel Becker
2009-08-19 19:54 ` [Ocfs2-devel] [PATCH 09/14] ocfs2: Teach ocfs2_xa_loc how to do its own journal work Joel Becker
` (6 subsequent siblings)
14 siblings, 0 replies; 33+ messages in thread
From: Joel Becker @ 2009-08-19 19:54 UTC (permalink / raw)
To: ocfs2-devel
We use the ocfs2_xattr_value_buf structure to manage external values.
It lets the value tree code do its work regardless of the containing
storage. ocfs2_xa_fill_value_buf() initializes a value buf from an
ocfs2_xa_loc entry.
Signed-off-by: Joel Becker <joel.becker@oracle.com>
---
fs/ocfs2/xattr.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 59 insertions(+), 0 deletions(-)
diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index 0f24e0a..b867aa0 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -175,6 +175,13 @@ struct ocfs2_xa_loc_operations {
/* Add name+value storage to an entry */
void (*xlo_add_namevalue)(struct ocfs2_xa_loc *loc, int size);
+
+ /*
+ * Initialize the value buf's access and bh fields for this entry.
+ * ocfs2_xa_fill_value_buf() will handle the xv pointer.
+ */
+ void (*xlo_fill_value_buf)(struct ocfs2_xa_loc *loc,
+ struct ocfs2_xattr_value_buf *vb);
};
/*
@@ -1546,6 +1553,23 @@ static void ocfs2_xa_add_namevalue(struct ocfs2_xa_loc *loc,
memcpy(nameval_buf, xi->xi_name, xi->xi_name_len);
}
+static void ocfs2_xa_fill_value_buf(struct ocfs2_xa_loc *loc,
+ struct ocfs2_xattr_value_buf *vb)
+{
+ int nameval_offset = le16_to_cpu(loc->xl_entry->xe_name_offset);
+ int name_size = OCFS2_XATTR_SIZE(loc->xl_entry->xe_name_len);
+
+ /* Value bufs are for value trees */
+ BUG_ON(namevalue_size_xe(loc->xl_entry) !=
+ (name_size + OCFS2_XATTR_ROOT_SIZE));
+
+ loc->xl_ops->xlo_fill_value_buf(loc, vb);
+ vb->vb_xv =
+ (struct ocfs2_xattr_value_root *)ocfs2_xa_offset_pointer(loc,
+ nameval_offset +
+ name_size);
+}
+
static void *ocfs2_xa_block_offset_pointer(struct ocfs2_xa_loc *loc,
int offset)
{
@@ -1656,6 +1680,20 @@ static void ocfs2_xa_block_add_namevalue(struct ocfs2_xa_loc *loc, int size)
loc->xl_entry->xe_name_offset = cpu_to_le16(free_start - size);
}
+static void ocfs2_xa_block_fill_value_buf(struct ocfs2_xa_loc *loc,
+ struct ocfs2_xattr_value_buf *vb)
+{
+ struct buffer_head *bh = loc->xl_storage;
+
+ if (loc->xl_size == (bh->b_size -
+ offsetof(struct ocfs2_xattr_block,
+ xb_attrs.xb_header)))
+ vb->vb_access = ocfs2_journal_access_xb;
+ else
+ vb->vb_access = ocfs2_journal_access_di;
+ vb->vb_bh = bh;
+}
+
/*
* Operations for xattrs stored in blocks. This includes inline inode
* storage and unindexed ocfs2_xattr_blocks.
@@ -1668,6 +1706,7 @@ static const struct ocfs2_xa_loc_operations ocfs2_xa_block_loc_ops = {
.xlo_wipe_namevalue = ocfs2_xa_block_wipe_namevalue,
.xlo_add_entry = ocfs2_xa_block_add_entry,
.xlo_add_namevalue = ocfs2_xa_block_add_namevalue,
+ .xlo_fill_value_buf = ocfs2_xa_block_fill_value_buf,
};
static void *ocfs2_xa_bucket_offset_pointer(struct ocfs2_xa_loc *loc,
@@ -1797,6 +1836,25 @@ static void ocfs2_xa_bucket_add_namevalue(struct ocfs2_xa_loc *loc, int size)
}
+static void ocfs2_xa_bucket_fill_value_buf(struct ocfs2_xa_loc *loc,
+ struct ocfs2_xattr_value_buf *vb)
+{
+ struct ocfs2_xattr_bucket *bucket = loc->xl_storage;
+ struct super_block *sb = bucket->bu_inode->i_sb;
+ int nameval_offset = le16_to_cpu(loc->xl_entry->xe_name_offset);
+ int size = namevalue_size_xe(loc->xl_entry);
+ int block_offset = nameval_offset >> sb->s_blocksize_bits;
+
+ /* Values are not allowed to straddle block boundaries */
+ BUG_ON(block_offset !=
+ ((nameval_offset + size - 1) >> sb->s_blocksize_bits));
+ /* We expect the bucket to be filled in */
+ BUG_ON(!bucket->bu_bhs[block_offset]);
+
+ vb->vb_access = ocfs2_journal_access;
+ vb->vb_bh = bucket->bu_bhs[block_offset];
+}
+
/* Operations for xattrs stored in buckets. */
static const struct ocfs2_xa_loc_operations ocfs2_xa_bucket_loc_ops = {
.xlo_offset_pointer = ocfs2_xa_bucket_offset_pointer,
@@ -1806,6 +1864,7 @@ static const struct ocfs2_xa_loc_operations ocfs2_xa_bucket_loc_ops = {
.xlo_wipe_namevalue = ocfs2_xa_bucket_wipe_namevalue,
.xlo_add_entry = ocfs2_xa_bucket_add_entry,
.xlo_add_namevalue = ocfs2_xa_bucket_add_namevalue,
+ .xlo_fill_value_buf = ocfs2_xa_bucket_fill_value_buf,
};
static void ocfs2_xa_remove_entry(struct ocfs2_xa_loc *loc)
--
1.6.3.3
^ permalink raw reply related [flat|nested] 33+ messages in thread* [Ocfs2-devel] [PATCH 09/14] ocfs2: Teach ocfs2_xa_loc how to do its own journal work
2009-08-19 19:54 [Ocfs2-devel] [PATCH 0/14] ocfs2: Unify the setting of extended attributes Joel Becker
` (7 preceding siblings ...)
2009-08-19 19:54 ` [Ocfs2-devel] [PATCH 08/14] ocfs2: Provide ocfs2_xa_fill_value_buf() for external value processing Joel Becker
@ 2009-08-19 19:54 ` Joel Becker
2009-08-19 19:54 ` [Ocfs2-devel] [PATCH 10/14] ocfs2: Allocation in ocfs2_xa_prepare_entry() values in ocfs2_xa_store_value() Joel Becker
` (5 subsequent siblings)
14 siblings, 0 replies; 33+ messages in thread
From: Joel Becker @ 2009-08-19 19:54 UTC (permalink / raw)
To: ocfs2-devel
We're going to want to make sure our buffers get accessed and dirtied
correctly. So have the xa_loc do the work. This includes storing the
inode on ocfs2_xa_loc.
Signed-off-by: Joel Becker <joel.becker@oracle.com>
---
fs/ocfs2/xattr.c | 115 ++++++++++++++++++++++++++++++++++++++++--------------
1 files changed, 86 insertions(+), 29 deletions(-)
diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index b867aa0..7c8edbe 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -145,6 +145,13 @@ struct ocfs2_xattr_search {
struct ocfs2_xa_loc;
struct ocfs2_xa_loc_operations {
/*
+ * Journal functions
+ */
+ int (*xlo_journal_access)(handle_t *handle, struct ocfs2_xa_loc *loc,
+ int type);
+ void (*xlo_journal_dirty)(handle_t *handle, struct ocfs2_xa_loc *loc);
+
+ /*
* Return a pointer to the appropriate buffer in loc->xl_storage
* at the given offset from loc->xl_header.
*/
@@ -189,6 +196,9 @@ struct ocfs2_xa_loc_operations {
* tracking the on-disk structure.
*/
struct ocfs2_xa_loc {
+ /* This xattr belongs to this inode */
+ struct inode *xl_inode;
+
/* The ocfs2_xattr_header inside the on-disk storage. Not NULL. */
struct ocfs2_xattr_header *xl_header;
@@ -1483,6 +1493,17 @@ static int ocfs2_xa_has_space_helper(int needed_space, int free_start,
return 0;
}
+static int ocfs2_xa_journal_access(handle_t *handle, struct ocfs2_xa_loc *loc,
+ int type)
+{
+ return loc->xl_ops->xlo_journal_access(handle, loc, type);
+}
+
+static void ocfs2_xa_journal_dirty(handle_t *handle, struct ocfs2_xa_loc *loc)
+{
+ loc->xl_ops->xlo_journal_dirty(handle, loc);
+}
+
/* Give a pointer into the storage for the given offset */
static void *ocfs2_xa_offset_pointer(struct ocfs2_xa_loc *loc, int offset)
{
@@ -1570,6 +1591,29 @@ static void ocfs2_xa_fill_value_buf(struct ocfs2_xa_loc *loc,
name_size);
}
+static int ocfs2_xa_block_journal_access(handle_t *handle,
+ struct ocfs2_xa_loc *loc, int type)
+{
+ struct buffer_head *bh = loc->xl_storage;
+ ocfs2_journal_access_func access;
+
+ if (loc->xl_size == (bh->b_size -
+ offsetof(struct ocfs2_xattr_block,
+ xb_attrs.xb_header)))
+ access = ocfs2_journal_access_xb;
+ else
+ access = ocfs2_journal_access_di;
+ return access(handle, loc->xl_inode, bh, type);
+}
+
+static void ocfs2_xa_block_journal_dirty(handle_t *handle,
+ struct ocfs2_xa_loc *loc)
+{
+ struct buffer_head *bh = loc->xl_storage;
+
+ ocfs2_journal_dirty(handle, bh);
+}
+
static void *ocfs2_xa_block_offset_pointer(struct ocfs2_xa_loc *loc,
int offset)
{
@@ -1699,6 +1743,8 @@ static void ocfs2_xa_block_fill_value_buf(struct ocfs2_xa_loc *loc,
* storage and unindexed ocfs2_xattr_blocks.
*/
static const struct ocfs2_xa_loc_operations ocfs2_xa_block_loc_ops = {
+ .xlo_journal_access = ocfs2_xa_block_journal_access,
+ .xlo_journal_dirty = ocfs2_xa_block_journal_dirty,
.xlo_offset_pointer = ocfs2_xa_block_offset_pointer,
.xlo_has_space = ocfs2_xa_block_has_space,
.xlo_can_reuse = ocfs2_xa_block_can_reuse,
@@ -1709,6 +1755,22 @@ static const struct ocfs2_xa_loc_operations ocfs2_xa_block_loc_ops = {
.xlo_fill_value_buf = ocfs2_xa_block_fill_value_buf,
};
+static int ocfs2_xa_bucket_journal_access(handle_t *handle,
+ struct ocfs2_xa_loc *loc, int type)
+{
+ struct ocfs2_xattr_bucket *bucket = loc->xl_storage;
+
+ return ocfs2_xattr_bucket_journal_access(handle, bucket, type);
+}
+
+static void ocfs2_xa_bucket_journal_dirty(handle_t *handle,
+ struct ocfs2_xa_loc *loc)
+{
+ struct ocfs2_xattr_bucket *bucket = loc->xl_storage;
+
+ ocfs2_xattr_bucket_journal_dirty(handle, bucket);
+}
+
static void *ocfs2_xa_bucket_offset_pointer(struct ocfs2_xa_loc *loc,
int offset)
{
@@ -1717,8 +1779,8 @@ static void *ocfs2_xa_bucket_offset_pointer(struct ocfs2_xa_loc *loc,
BUG_ON(offset >= OCFS2_XATTR_BUCKET_SIZE);
- block = offset >> bucket->bu_inode->i_sb->s_blocksize_bits;
- block_offset = offset % bucket->bu_inode->i_sb->s_blocksize;
+ block = offset >> loc->xl_inode->i_sb->s_blocksize_bits;
+ block_offset = offset % loc->xl_inode->i_sb->s_blocksize;
return bucket_block(bucket, block) + block_offset;
}
@@ -1757,8 +1819,7 @@ static int ocfs2_xa_bucket_has_space(struct ocfs2_xa_loc *loc,
int free_start = ocfs2_xa_get_free_start(loc);
int needed_space = ocfs2_xi_entry_usage(xi);
int size = namevalue_size_xi(xi);
- struct ocfs2_xattr_bucket *bucket = loc->xl_storage;
- struct super_block *sb = bucket->bu_inode->i_sb;
+ struct super_block *sb = loc->xl_inode->i_sb;
/*
* Bucket storage does not reclaim name+value pairs it cannot
@@ -1824,8 +1885,7 @@ static void ocfs2_xa_bucket_add_namevalue(struct ocfs2_xa_loc *loc, int size)
{
int free_start = ocfs2_xa_get_free_start(loc);
struct ocfs2_xattr_header *xh = loc->xl_header;
- struct ocfs2_xattr_bucket *bucket = loc->xl_storage;
- struct super_block *sb = bucket->bu_inode->i_sb;
+ struct super_block *sb = loc->xl_inode->i_sb;
int nameval_offset;
free_start = ocfs2_bucket_align_free_start(sb, free_start, size);
@@ -1840,7 +1900,7 @@ static void ocfs2_xa_bucket_fill_value_buf(struct ocfs2_xa_loc *loc,
struct ocfs2_xattr_value_buf *vb)
{
struct ocfs2_xattr_bucket *bucket = loc->xl_storage;
- struct super_block *sb = bucket->bu_inode->i_sb;
+ struct super_block *sb = loc->xl_inode->i_sb;
int nameval_offset = le16_to_cpu(loc->xl_entry->xe_name_offset);
int size = namevalue_size_xe(loc->xl_entry);
int block_offset = nameval_offset >> sb->s_blocksize_bits;
@@ -1857,6 +1917,8 @@ static void ocfs2_xa_bucket_fill_value_buf(struct ocfs2_xa_loc *loc,
/* Operations for xattrs stored in buckets. */
static const struct ocfs2_xa_loc_operations ocfs2_xa_bucket_loc_ops = {
+ .xlo_journal_access = ocfs2_xa_bucket_journal_access,
+ .xlo_journal_dirty = ocfs2_xa_bucket_journal_dirty,
.xlo_offset_pointer = ocfs2_xa_bucket_offset_pointer,
.xlo_has_space = ocfs2_xa_bucket_has_space,
.xlo_can_reuse = ocfs2_xa_bucket_can_reuse,
@@ -1973,6 +2035,7 @@ static void ocfs2_init_dinode_xa_loc(struct ocfs2_xa_loc *loc,
{
struct ocfs2_dinode *di = (struct ocfs2_dinode *)bh->b_data;
+ loc->xl_inode = inode;
loc->xl_ops = &ocfs2_xa_block_loc_ops;
loc->xl_storage = bh;
loc->xl_entry = entry;
@@ -1989,6 +2052,7 @@ static void ocfs2_init_dinode_xa_loc(struct ocfs2_xa_loc *loc,
}
static void ocfs2_init_xattr_block_xa_loc(struct ocfs2_xa_loc *loc,
+ struct inode *inode,
struct buffer_head *bh,
struct ocfs2_xattr_entry *entry)
{
@@ -1997,6 +2061,7 @@ static void ocfs2_init_xattr_block_xa_loc(struct ocfs2_xa_loc *loc,
BUG_ON(le16_to_cpu(xb->xb_flags) & OCFS2_XATTR_INDEXED);
+ loc->xl_inode = inode;
loc->xl_ops = &ocfs2_xa_block_loc_ops;
loc->xl_storage = bh;
loc->xl_header = &(xb->xb_attrs.xb_header);
@@ -2009,6 +2074,7 @@ static void ocfs2_init_xattr_bucket_xa_loc(struct ocfs2_xa_loc *loc,
struct ocfs2_xattr_bucket *bucket,
struct ocfs2_xattr_entry *entry)
{
+ loc->xl_inode = bucket->bu_inode;
loc->xl_ops = &ocfs2_xa_bucket_loc_ops;
loc->xl_storage = bucket;
loc->xl_header = bucket_xh(bucket);
@@ -2150,21 +2216,18 @@ static int ocfs2_xattr_set_entry(struct inode *inode,
goto out;
}
- if (!(flag & OCFS2_INLINE_XATTR_FL)) {
- ret = vb.vb_access(handle, inode, vb.vb_bh,
- OCFS2_JOURNAL_ACCESS_WRITE);
- if (ret) {
- mlog_errno(ret);
- goto out;
- }
- }
-
if (xs->xattr_bh == xs->inode_bh)
ocfs2_init_dinode_xa_loc(&loc, inode, xs->inode_bh,
xs->not_found ? NULL : xs->here);
else
- ocfs2_init_xattr_block_xa_loc(&loc, xs->xattr_bh,
+ ocfs2_init_xattr_block_xa_loc(&loc, inode, xs->xattr_bh,
xs->not_found ? NULL : xs->here);
+ ret = ocfs2_xa_journal_access(handle, &loc,
+ OCFS2_JOURNAL_ACCESS_WRITE);
+ if (ret) {
+ mlog_errno(ret);
+ goto out;
+ }
/*
* Prepare our entry and insert the inline value. This will
@@ -2182,13 +2245,7 @@ static int ocfs2_xattr_set_entry(struct inode *inode,
ocfs2_xa_store_inline_value(&loc, xi);
xs->here = loc.xl_entry;
- if (!(flag & OCFS2_INLINE_XATTR_FL)) {
- ret = ocfs2_journal_dirty(handle, xs->xattr_bh);
- if (ret < 0) {
- mlog_errno(ret);
- goto out;
- }
- }
+ ocfs2_xa_journal_dirty(handle, &loc);
if (!(oi->ip_dyn_features & OCFS2_INLINE_XATTR_FL) &&
(flag & OCFS2_INLINE_XATTR_FL)) {
@@ -5061,15 +5118,15 @@ static int ocfs2_xattr_set_entry_in_bucket(struct inode *inode,
}
}
- ret = ocfs2_xattr_bucket_journal_access(handle, xs->bucket,
- OCFS2_JOURNAL_ACCESS_WRITE);
+ ocfs2_init_xattr_bucket_xa_loc(&loc, xs->bucket,
+ xs->not_found ? NULL : xs->here);
+ ret = ocfs2_xa_journal_access(handle, &loc,
+ OCFS2_JOURNAL_ACCESS_WRITE);
if (ret < 0) {
mlog_errno(ret);
goto out;
}
- ocfs2_init_xattr_bucket_xa_loc(&loc, xs->bucket,
- xs->not_found ? NULL : xs->here);
ret = ocfs2_xa_prepare_entry(&loc, xi, name_hash);
if (ret) {
if (ret != -ENOSPC)
@@ -5081,7 +5138,7 @@ static int ocfs2_xattr_set_entry_in_bucket(struct inode *inode,
ocfs2_xa_store_inline_value(&loc, xi);
xs->here = loc.xl_entry;
- ocfs2_xattr_bucket_journal_dirty(handle, xs->bucket);
+ ocfs2_xa_journal_dirty(handle, &loc);
out:
return ret;
--
1.6.3.3
^ permalink raw reply related [flat|nested] 33+ messages in thread* [Ocfs2-devel] [PATCH 10/14] ocfs2: Allocation in ocfs2_xa_prepare_entry() values in ocfs2_xa_store_value()
2009-08-19 19:54 [Ocfs2-devel] [PATCH 0/14] ocfs2: Unify the setting of extended attributes Joel Becker
` (8 preceding siblings ...)
2009-08-19 19:54 ` [Ocfs2-devel] [PATCH 09/14] ocfs2: Teach ocfs2_xa_loc how to do its own journal work Joel Becker
@ 2009-08-19 19:54 ` Joel Becker
2009-08-19 19:54 ` [Ocfs2-devel] [PATCH 11/14] ocfs2: Gell into ocfs2_xa_set() Joel Becker
` (4 subsequent siblings)
14 siblings, 0 replies; 33+ messages in thread
From: Joel Becker @ 2009-08-19 19:54 UTC (permalink / raw)
To: ocfs2-devel
ocfs2_xa_prepare_entry() gets all the logic to add, remove, or modify
external value trees. Now, when it exits, the entry is ready to receive
a value of any size.
ocfs2_xa_store_inline_value() becomes ocfs2_xa_store_value(). It can
store any value.
ocfs2_xattr_set_entry() loses all the allocation logic and just uses
these functions. ocfs2_xattr_set_value_outside() disappears.
ocfs2_xattr_set_in_bucket() uses these functions and makes
ocfs2_xattr_set_entry_in_bucket() obsolete. That goes away, as does
ocfs2_xattr_bucket_set_value_outside() and
ocfs2_xattr_bucket_value_truncate().
Signed-off-by: Joel Becker <joel.becker@oracle.com>
---
fs/ocfs2/xattr.c | 607 +++++++++++++-----------------------------------------
1 files changed, 139 insertions(+), 468 deletions(-)
diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index 7c8edbe..a4ee086 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -547,24 +547,6 @@ static u32 ocfs2_xattr_name_hash(struct inode *inode,
return hash;
}
-/*
- * ocfs2_xattr_hash_entry()
- *
- * Compute the hash of an extended attribute.
- */
-static void ocfs2_xattr_hash_entry(struct inode *inode,
- struct ocfs2_xattr_header *header,
- struct ocfs2_xattr_entry *entry)
-{
- u32 hash = 0;
- char *name = (char *)header + le16_to_cpu(entry->xe_name_offset);
-
- hash = ocfs2_xattr_name_hash(inode, name, entry->xe_name_len);
- entry->xe_name_hash = cpu_to_le32(hash);
-
- return;
-}
-
static int ocfs2_xattr_entry_real_size(int name_len, size_t value_len)
{
return namevalue_size(name_len, value_len) +
@@ -1369,113 +1351,6 @@ out:
return ret;
}
-static int ocfs2_xattr_cleanup(struct inode *inode,
- handle_t *handle,
- struct ocfs2_xattr_info *xi,
- struct ocfs2_xattr_search *xs,
- struct ocfs2_xattr_value_buf *vb,
- size_t offs)
-{
- int ret = 0;
- void *val = xs->base + offs;
- size_t size = namevalue_size_xi(xi);
-
- ret = vb->vb_access(handle, inode, vb->vb_bh,
- OCFS2_JOURNAL_ACCESS_WRITE);
- if (ret) {
- mlog_errno(ret);
- goto out;
- }
- /* Decrease xattr count */
- le16_add_cpu(&xs->header->xh_count, -1);
- /* Remove the xattr entry and tree root which has already be set*/
- memset((void *)xs->here, 0, sizeof(struct ocfs2_xattr_entry));
- memset(val, 0, size);
-
- ret = ocfs2_journal_dirty(handle, vb->vb_bh);
- if (ret < 0)
- mlog_errno(ret);
-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,
- struct ocfs2_xattr_value_buf *vb,
- size_t offs)
-{
- int ret;
-
- ret = vb->vb_access(handle, inode, vb->vb_bh,
- OCFS2_JOURNAL_ACCESS_WRITE);
- if (ret) {
- mlog_errno(ret);
- goto out;
- }
-
- xs->here->xe_name_offset = cpu_to_le16(offs);
- xs->here->xe_value_size = cpu_to_le64(xi->xi_value_len);
- if (xi->xi_value_len <= OCFS2_XATTR_INLINE_SIZE)
- ocfs2_xattr_set_local(xs->here, 1);
- else
- ocfs2_xattr_set_local(xs->here, 0);
- ocfs2_xattr_hash_entry(inode, xs->header, xs->here);
-
- ret = ocfs2_journal_dirty(handle, vb->vb_bh);
- if (ret < 0)
- mlog_errno(ret);
-out:
- return ret;
-}
-
-/*
- * ocfs2_xattr_set_value_outside()
- *
- * Set large size value in B tree.
- */
-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,
- struct ocfs2_xattr_value_buf *vb,
- size_t offs)
-{
- void *val = xs->base + offs;
- struct ocfs2_xattr_value_root *xv = NULL;
- size_t size = namevalue_size_xi(xi);
- int ret = 0;
-
- memset(val, 0, size);
- memcpy(val, xi->xi_name, xi->xi_name_len);
- xv = (struct ocfs2_xattr_value_root *)
- (val + OCFS2_XATTR_SIZE(xi->xi_name_len));
- xv->xr_clusters = 0;
- xv->xr_last_eb_blk = 0;
- xv->xr_list.l_tree_depth = 0;
- xv->xr_list.l_count = cpu_to_le16(1);
- xv->xr_list.l_next_free_rec = 0;
- vb->vb_xv = xv;
-
- ret = ocfs2_xattr_value_truncate(inode, vb, xi->xi_value_len, ctxt);
- if (ret < 0) {
- mlog_errno(ret);
- return ret;
- }
- ret = ocfs2_xattr_update_entry(inode, ctxt->handle, xi, xs, vb, offs);
- if (ret < 0) {
- mlog_errno(ret);
- return ret;
- }
- ret = __ocfs2_xattr_set_value_outside(inode, ctxt->handle, vb->vb_xv,
- xi->xi_value, xi->xi_value_len);
- if (ret < 0)
- mlog_errno(ret);
-
- return ret;
-}
-
static int ocfs2_xa_has_space_helper(int needed_space, int free_start,
int num_entries)
{
@@ -1955,6 +1830,78 @@ static void ocfs2_xa_remove_entry(struct ocfs2_xa_loc *loc)
}
}
+static int ocfs2_xa_value_truncate(struct ocfs2_xa_loc *loc, u64 bytes,
+ struct ocfs2_xattr_set_ctxt *ctxt)
+{
+ struct ocfs2_xattr_value_buf vb;
+
+ ocfs2_xa_fill_value_buf(loc, &vb);
+ return ocfs2_xattr_value_truncate(loc->xl_inode, &vb, bytes, ctxt);
+}
+
+static void ocfs2_xa_install_value_root(struct ocfs2_xa_loc *loc)
+{
+ int name_size = OCFS2_XATTR_SIZE(loc->xl_entry->xe_name_len);
+ char *nameval_buf;
+
+ nameval_buf = ocfs2_xa_offset_pointer(loc,
+ le16_to_cpu(loc->xl_entry->xe_name_offset));
+ memcpy(nameval_buf + name_size, &def_xv, OCFS2_XATTR_ROOT_SIZE);
+}
+
+/*
+ * Take an existing entry and make it ready for the new value. This
+ * won't allocate space, but it may free space. It should be ready for
+ * ocfs2_xa_prepare_entry() to finish the work.
+ */
+static int ocfs2_xa_reuse_entry(struct ocfs2_xa_loc *loc,
+ struct ocfs2_xattr_info *xi,
+ struct ocfs2_xattr_set_ctxt *ctxt)
+{
+ int rc = 0;
+ int name_size = OCFS2_XATTR_SIZE(xi->xi_name_len);
+ char *nameval_buf;
+ int xe_local = ocfs2_xattr_is_local(loc->xl_entry);
+ int xi_local = xi->xi_value_len <= OCFS2_XATTR_INLINE_SIZE;
+
+ BUG_ON(OCFS2_XATTR_SIZE(loc->xl_entry->xe_name_len) !=
+ name_size);
+
+ nameval_buf = ocfs2_xa_offset_pointer(loc,
+ le16_to_cpu(loc->xl_entry->xe_name_offset));
+ if (xe_local) {
+ memset(nameval_buf + name_size, 0,
+ namevalue_size_xe(loc->xl_entry) - name_size);
+ if (!xi_local)
+ ocfs2_xa_install_value_root(loc);
+ } else {
+ if (xi_local) {
+ rc = ocfs2_xa_value_truncate(loc, 0, ctxt);
+ if (rc < 0) {
+ mlog_errno(rc);
+ goto out;
+ }
+ memset(nameval_buf + name_size, 0,
+ namevalue_size_xe(loc->xl_entry) -
+ name_size);
+ } else if (le64_to_cpu(loc->xl_entry->xe_value_size) >
+ xi->xi_value_len) {
+ rc = ocfs2_xa_value_truncate(loc, xi->xi_value_len,
+ ctxt);
+ if (rc < 0) {
+ mlog_errno(rc);
+ goto out;
+ }
+ }
+ }
+
+ loc->xl_entry->xe_value_size = cpu_to_le64(xi->xi_value_len);
+ ocfs2_xattr_set_local(loc->xl_entry, xi_local);
+
+out:
+ return rc;
+}
+
/*
* Prepares loc->xl_entry to receive the new xattr. This includes
* properly setting up the name+value pair region. If loc->xl_entry
@@ -1967,11 +1914,10 @@ static void ocfs2_xa_remove_entry(struct ocfs2_xa_loc *loc)
*/
static int ocfs2_xa_prepare_entry(struct ocfs2_xa_loc *loc,
struct ocfs2_xattr_info *xi,
- u32 name_hash)
+ u32 name_hash,
+ struct ocfs2_xattr_set_ctxt *ctxt)
{
int rc = 0;
- int name_size = OCFS2_XATTR_SIZE(xi->xi_name_len);
- char *nameval_buf;
if (!xi->xi_value) {
ocfs2_xa_remove_entry(loc);
@@ -1984,13 +1930,10 @@ static int ocfs2_xa_prepare_entry(struct ocfs2_xa_loc *loc,
if (loc->xl_entry) {
if (ocfs2_xa_can_reuse_entry(loc, xi)) {
- nameval_buf = ocfs2_xa_offset_pointer(loc,
- le16_to_cpu(loc->xl_entry->xe_name_offset));
- memset(nameval_buf + name_size, 0,
- namevalue_size_xe(loc->xl_entry) - name_size);
- loc->xl_entry->xe_value_size =
- cpu_to_le64(xi->xi_value_len);
- goto out;
+ rc = ocfs2_xa_reuse_entry(loc, xi, ctxt);
+ if (rc)
+ goto out;
+ goto alloc_value;
}
ocfs2_xa_wipe_namevalue(loc);
@@ -2002,30 +1945,47 @@ static int ocfs2_xa_prepare_entry(struct ocfs2_xa_loc *loc,
* name+value pair back from the end.
*/
ocfs2_xa_add_namevalue(loc, xi);
+ if (xi->xi_value_len > OCFS2_XATTR_INLINE_SIZE)
+ ocfs2_xa_install_value_root(loc);
+
+alloc_value:
+ if (xi->xi_value_len > OCFS2_XATTR_INLINE_SIZE) {
+ rc = ocfs2_xa_value_truncate(loc, xi->xi_value_len, ctxt);
+ if (rc < 0)
+ mlog_errno(rc);
+ }
out:
return rc;
}
/*
- * Store the value portion of the name+value pair. This is either an
- * inline value or the tree root of an external value.
+ * Store the value portion of the name+value pair. This will skip
+ * values that are stored externally. Their tree roots were set up
+ * by ocfs2_xa_prepare_entry().
*/
-static void ocfs2_xa_store_inline_value(struct ocfs2_xa_loc *loc,
- struct ocfs2_xattr_info *xi)
+static int ocfs2_xa_store_value(struct ocfs2_xa_loc *loc,
+ struct ocfs2_xattr_info *xi,
+ struct ocfs2_xattr_set_ctxt *ctxt)
{
+ int rc = 0;
int nameval_offset = le16_to_cpu(loc->xl_entry->xe_name_offset);
int name_size = OCFS2_XATTR_SIZE(xi->xi_name_len);
- int inline_value_size = namevalue_size_xi(xi) - name_size;
- const void *value = xi->xi_value;
char *nameval_buf;
+ struct ocfs2_xattr_value_root *xv;
- if (xi->xi_value_len > OCFS2_XATTR_INLINE_SIZE) {
- value = &def_xv;
- inline_value_size = OCFS2_XATTR_ROOT_SIZE;
- }
nameval_buf = ocfs2_xa_offset_pointer(loc, nameval_offset);
- memcpy(nameval_buf + name_size, value, inline_value_size);
+ if (xi->xi_value_len > OCFS2_XATTR_INLINE_SIZE) {
+ xv = (struct ocfs2_xattr_value_root *)nameval_buf +
+ name_size;
+ rc = __ocfs2_xattr_set_value_outside(loc->xl_inode,
+ ctxt->handle, xv,
+ xi->xi_value,
+ xi->xi_value_len);
+ } else
+ memcpy(nameval_buf + name_size, xi->xi_value, xi->xi_value_len);
+
+ return rc;
}
static void ocfs2_init_dinode_xa_loc(struct ocfs2_xa_loc *loc,
@@ -2098,117 +2058,19 @@ static int ocfs2_xattr_set_entry(struct inode *inode,
struct ocfs2_xattr_set_ctxt *ctxt,
int flag)
{
- struct ocfs2_xattr_entry *last;
struct ocfs2_inode_info *oi = OCFS2_I(inode);
struct ocfs2_dinode *di = (struct ocfs2_dinode *)xs->inode_bh->b_data;
- size_t min_offs = xs->end - xs->base;
- size_t size_l = 0;
handle_t *handle = ctxt->handle;
- int free, i, ret;
+ int ret;
u32 name_hash = ocfs2_xattr_name_hash(inode, xi->xi_name,
xi->xi_name_len);
struct ocfs2_xa_loc loc;
- struct ocfs2_xattr_value_buf vb = {
- .vb_bh = xs->xattr_bh,
- .vb_access = ocfs2_journal_access_di,
- };
- if (!(flag & OCFS2_INLINE_XATTR_FL)) {
+ if (!(flag & OCFS2_INLINE_XATTR_FL))
BUG_ON(xs->xattr_bh == xs->inode_bh);
- vb.vb_access = ocfs2_journal_access_xb;
- } else
+ else
BUG_ON(xs->xattr_bh != xs->inode_bh);
- /* Compute min_offs, last and free space. */
- 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) - OCFS2_XATTR_HEADER_GAP;
- if (free < 0)
- return -EIO;
-
- if (!xs->not_found)
- free += ocfs2_xe_entry_usage(xs->here);
-
- /* Check free space in inode or block */
- if (xi->xi_value && (free < ocfs2_xi_entry_usage(xi))) {
- ret = -ENOSPC;
- goto out;
- }
-
- if (!xs->not_found) {
- /* For existing extended attribute */
- size_t size = namevalue_size_xe(xs->here);
- size_t offs = le16_to_cpu(xs->here->xe_name_offset);
- void *val = xs->base + offs;
-
- 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,
- ctxt, &vb, offs);
- if (ret < 0)
- mlog_errno(ret);
- goto out;
- } else if (!ocfs2_xattr_is_local(xs->here)) {
- /* For existing xattr which has value outside */
- vb.vb_xv = (struct ocfs2_xattr_value_root *)
- (val + OCFS2_XATTR_SIZE(xi->xi_name_len));
-
- if (xi->xi_value_len > OCFS2_XATTR_INLINE_SIZE) {
- /*
- * If new value need set outside also,
- * first truncate old value to new value,
- * then set new value with set_value_outside().
- */
- ret = ocfs2_xattr_value_truncate(inode,
- &vb,
- xi->xi_value_len,
- ctxt);
- if (ret < 0) {
- mlog_errno(ret);
- goto out;
- }
-
- ret = ocfs2_xattr_update_entry(inode,
- handle,
- xi,
- xs,
- &vb,
- offs);
- if (ret < 0) {
- mlog_errno(ret);
- goto out;
- }
-
- ret = __ocfs2_xattr_set_value_outside(inode,
- handle,
- vb.vb_xv,
- xi->xi_value,
- xi->xi_value_len);
- if (ret < 0)
- mlog_errno(ret);
- goto out;
- } else {
- /*
- * If new value need set in local,
- * just trucate old value to zero.
- */
- ret = ocfs2_xattr_value_truncate(inode,
- &vb,
- 0,
- ctxt);
- if (ret < 0)
- mlog_errno(ret);
- }
- }
- }
-
ret = ocfs2_journal_access_di(handle, inode, xs->inode_bh,
OCFS2_JOURNAL_ACCESS_WRITE);
if (ret) {
@@ -2229,22 +2091,20 @@ static int ocfs2_xattr_set_entry(struct inode *inode,
goto out;
}
- /*
- * Prepare our entry and insert the inline value. This will
- * be a value tree root for values that are larger than
- * OCFS2_XATTR_INLINE_SIZE.
- */
- ret = ocfs2_xa_prepare_entry(&loc, xi, name_hash);
+ ret = ocfs2_xa_prepare_entry(&loc, xi, name_hash, ctxt);
if (ret) {
if (ret != -ENOSPC)
mlog_errno(ret);
goto out;
}
- /* XXX For now, until we make ocfs2_xa_prepare_entry() primary */
- BUG_ON(ret == -ENOSPC);
- ocfs2_xa_store_inline_value(&loc, xi);
xs->here = loc.xl_entry;
+ ret = ocfs2_xa_store_value(&loc, xi, ctxt);
+ if (ret) {
+ mlog_errno(ret);
+ goto out;
+ }
+
ocfs2_xa_journal_dirty(handle, &loc);
if (!(oi->ip_dyn_features & OCFS2_INLINE_XATTR_FL) &&
@@ -2276,28 +2136,6 @@ static int ocfs2_xattr_set_entry(struct inode *inode,
if (ret < 0)
mlog_errno(ret);
- if (!ret && xi->xi_value_len > OCFS2_XATTR_INLINE_SIZE) {
- /*
- * Set value outside in B tree.
- * 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, ctxt,
- &vb, offs);
- if (ret < 0) {
- int ret2;
-
- mlog_errno(ret);
- /*
- * If set value outside failed, we have to clean
- * the junk tree root we have already set in local.
- */
- ret2 = ocfs2_xattr_cleanup(inode, ctxt->handle,
- xi, xs, &vb, offs);
- if (ret2 < 0)
- mlog_errno(ret2);
- }
- }
out:
return ret;
}
@@ -5090,61 +4928,6 @@ static inline char *ocfs2_xattr_bucket_get_val(struct inode *inode,
}
/*
- * Set the xattr entry in the specified bucket.
- * The bucket is indicated by xs->bucket and it should have the enough
- * 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 ret;
- u64 blkno;
- struct ocfs2_xa_loc loc;
-
- mlog(0, "Set xattr entry len = %lu index = %d in bucket %llu\n",
- (unsigned long)xi->xi_value_len, xi->xi_name_index,
- (unsigned long long)bucket_blkno(xs->bucket));
-
- if (!xs->bucket->bu_bhs[1]) {
- blkno = bucket_blkno(xs->bucket);
- ocfs2_xattr_bucket_relse(xs->bucket);
- ret = ocfs2_read_xattr_bucket(xs->bucket, blkno);
- if (ret) {
- mlog_errno(ret);
- goto out;
- }
- }
-
- ocfs2_init_xattr_bucket_xa_loc(&loc, xs->bucket,
- xs->not_found ? NULL : xs->here);
- ret = ocfs2_xa_journal_access(handle, &loc,
- OCFS2_JOURNAL_ACCESS_WRITE);
- if (ret < 0) {
- mlog_errno(ret);
- goto out;
- }
-
- ret = ocfs2_xa_prepare_entry(&loc, xi, name_hash);
- if (ret) {
- if (ret != -ENOSPC)
- mlog_errno(ret);
- goto out;
- }
- /* XXX For now, until we make ocfs2_xa_prepare_entry() primary */
- BUG_ON(ret == -ENOSPC);
- ocfs2_xa_store_inline_value(&loc, xi);
- xs->here = loc.xl_entry;
-
- ocfs2_xa_journal_dirty(handle, &loc);
-
-out:
- return ret;
-}
-
-/*
* Truncate the specified xe_off entry in xattr bucket.
* bucket is indicated by header_bh and len is the new length.
* Both the ocfs2_xattr_value_root and the entry will be updated here.
@@ -5214,61 +4997,6 @@ out:
return ret;
}
-static int ocfs2_xattr_bucket_value_truncate_xs(struct inode *inode,
- struct ocfs2_xattr_search *xs,
- int len,
- struct ocfs2_xattr_set_ctxt *ctxt)
-{
- int ret, offset;
- struct ocfs2_xattr_entry *xe = xs->here;
- struct ocfs2_xattr_header *xh = (struct ocfs2_xattr_header *)xs->base;
-
- BUG_ON(!xs->bucket->bu_bhs[0] || !xe || ocfs2_xattr_is_local(xe));
-
- offset = xe - xh->xh_entries;
- ret = ocfs2_xattr_bucket_value_truncate(inode, xs->bucket,
- offset, len, ctxt);
- if (ret)
- mlog_errno(ret);
-
- return ret;
-}
-
-static int ocfs2_xattr_bucket_set_value_outside(struct inode *inode,
- handle_t *handle,
- struct ocfs2_xattr_search *xs,
- char *val,
- int value_len)
-{
- int ret, offset, block_off;
- struct ocfs2_xattr_value_root *xv;
- struct ocfs2_xattr_entry *xe = xs->here;
- struct ocfs2_xattr_header *xh = bucket_xh(xs->bucket);
- void *base;
-
- BUG_ON(!xs->base || !xe || ocfs2_xattr_is_local(xe));
-
- ret = ocfs2_xattr_bucket_get_name_value(inode, xh,
- xe - xh->xh_entries,
- &block_off,
- &offset);
- if (ret) {
- mlog_errno(ret);
- goto out;
- }
-
- base = bucket_block(xs->bucket, block_off);
- xv = (struct ocfs2_xattr_value_root *)(base + offset +
- OCFS2_XATTR_SIZE(xe->xe_name_len));
-
- ret = __ocfs2_xattr_set_value_outside(inode, handle,
- xv, val, value_len);
- if (ret)
- mlog_errno(ret);
-out:
- return ret;
-}
-
static int ocfs2_rm_xattr_cluster(struct inode *inode,
struct buffer_head *root_bh,
u64 blkno,
@@ -5358,41 +5086,8 @@ out:
return ret;
}
-static void ocfs2_xattr_bucket_remove_xs(struct inode *inode,
- handle_t *handle,
- struct ocfs2_xattr_search *xs)
-{
- 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;
-
- ret = ocfs2_xattr_bucket_journal_access(handle, xs->bucket,
- OCFS2_JOURNAL_ACCESS_WRITE);
- if (ret) {
- mlog_errno(ret);
- return;
- }
-
- /* Remove the old entry. */
- memmove(xs->here, xs->here + 1,
- (void *)last - (void *)xs->here);
- memset(last, 0, sizeof(struct ocfs2_xattr_entry));
- le16_add_cpu(&xh->xh_count, -1);
-
- ocfs2_xattr_bucket_journal_dirty(handle, xs->bucket);
-}
-
/*
* Set the xattr name/value in the bucket specified in xs.
- *
- * As the new value in xi may be stored in the bucket or in an outside cluster,
- * we divide the whole process into 3 steps:
- * 1. insert name/value in the bucket(ocfs2_xattr_set_entry_in_bucket)
- * 2. truncate of the outside cluster(ocfs2_xattr_bucket_value_truncate_xs)
- * 3. Set the value to the outside cluster(ocfs2_xattr_bucket_set_value_outside)
- * 4. If the clusters for the new outside value can't be allocated, we need
- * to free the xattr we allocated in set.
*/
static int ocfs2_xattr_set_in_bucket(struct inode *inode,
struct ocfs2_xattr_info *xi,
@@ -5400,70 +5095,46 @@ static int ocfs2_xattr_set_in_bucket(struct inode *inode,
struct ocfs2_xattr_set_ctxt *ctxt)
{
int ret;
- size_t value_len;
- char *val = (char *)xi->xi_value;
- struct ocfs2_xattr_entry *xe = xs->here;
+ u64 blkno;
+ struct ocfs2_xa_loc loc;
u32 name_hash = ocfs2_xattr_name_hash(inode, xi->xi_name,
xi->xi_name_len);
- value_len = xi->xi_value_len;
- if (!xs->not_found && !ocfs2_xattr_is_local(xe)) {
- /*
- * We need to truncate the xattr storage first.
- *
- * If both the old and new value are stored to
- * outside block, we only need to truncate
- * the storage and then set the value outside.
- *
- * If the new value should be stored within block,
- * we should free all the outside block first and
- * the modification to the xattr block will be done
- * by following steps.
- */
- if (xi->xi_value_len <= OCFS2_XATTR_INLINE_SIZE)
- value_len = 0;
-
- ret = ocfs2_xattr_bucket_value_truncate_xs(inode, xs,
- value_len,
- ctxt);
- if (ret)
+ if (!xs->bucket->bu_bhs[1]) {
+ blkno = bucket_blkno(xs->bucket);
+ ocfs2_xattr_bucket_relse(xs->bucket);
+ ret = ocfs2_read_xattr_bucket(xs->bucket, blkno);
+ if (ret) {
+ mlog_errno(ret);
goto out;
-
- if (value_len)
- goto set_value_outside;
+ }
}
- /* So we have to handle the inside block change now. */
- ret = ocfs2_xattr_set_entry_in_bucket(inode, ctxt->handle, xi, xs,
- name_hash);
- if (ret) {
+ ocfs2_init_xattr_bucket_xa_loc(&loc, xs->bucket,
+ xs->not_found ? NULL : xs->here);
+ ret = ocfs2_xa_journal_access(ctxt->handle, &loc,
+ OCFS2_JOURNAL_ACCESS_WRITE);
+ if (ret < 0) {
mlog_errno(ret);
goto out;
}
- if (xi->xi_value_len <= OCFS2_XATTR_INLINE_SIZE)
+ ret = ocfs2_xa_prepare_entry(&loc, xi, name_hash, ctxt);
+ if (ret) {
+ if (ret != -ENOSPC)
+ mlog_errno(ret);
goto out;
+ }
+ xs->here = loc.xl_entry;
- /* allocate the space now for the outside block storage. */
- ret = ocfs2_xattr_bucket_value_truncate_xs(inode, xs,
- value_len, ctxt);
+ ret = ocfs2_xa_store_value(&loc, xi, ctxt);
if (ret) {
mlog_errno(ret);
-
- if (xs->not_found) {
- /*
- * We can't allocate enough clusters for outside
- * storage and we have allocated xattr already,
- * so need to remove it.
- */
- ocfs2_xattr_bucket_remove_xs(inode, ctxt->handle, xs);
- }
goto out;
}
-set_value_outside:
- ret = ocfs2_xattr_bucket_set_value_outside(inode, ctxt->handle,
- xs, val, value_len);
+ ocfs2_xa_journal_dirty(ctxt->handle, &loc);
+
out:
return ret;
}
--
1.6.3.3
^ permalink raw reply related [flat|nested] 33+ messages in thread* [Ocfs2-devel] [PATCH 11/14] ocfs2: Gell into ocfs2_xa_set()
2009-08-19 19:54 [Ocfs2-devel] [PATCH 0/14] ocfs2: Unify the setting of extended attributes Joel Becker
` (9 preceding siblings ...)
2009-08-19 19:54 ` [Ocfs2-devel] [PATCH 10/14] ocfs2: Allocation in ocfs2_xa_prepare_entry() values in ocfs2_xa_store_value() Joel Becker
@ 2009-08-19 19:54 ` Joel Becker
2009-08-19 19:54 ` [Ocfs2-devel] [PATCH 12/14] ocfs2: Let ocfs2_xa_prepare_entry() do space checks Joel Becker
` (3 subsequent siblings)
14 siblings, 0 replies; 33+ messages in thread
From: Joel Becker @ 2009-08-19 19:54 UTC (permalink / raw)
To: ocfs2-devel
ocfs2_xa_set() wraps the ocfs2_xa_prepare_entry()/ocfs2_xa_store_value()
logic. Both callers can now use the same routine.
Signed-off-by: Joel Becker <joel.becker@oracle.com>
---
fs/ocfs2/xattr.c | 71 +++++++++++++++++++++++++++--------------------------
1 files changed, 36 insertions(+), 35 deletions(-)
diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index a4ee086..7527546 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -1988,6 +1988,40 @@ static int ocfs2_xa_store_value(struct ocfs2_xa_loc *loc,
return rc;
}
+static int ocfs2_xa_set(struct ocfs2_xa_loc *loc,
+ struct ocfs2_xattr_info *xi,
+ struct ocfs2_xattr_set_ctxt *ctxt)
+{
+ int ret;
+ u32 name_hash = ocfs2_xattr_name_hash(loc->xl_inode, xi->xi_name,
+ xi->xi_name_len);
+
+ ret = ocfs2_xa_journal_access(ctxt->handle, loc,
+ OCFS2_JOURNAL_ACCESS_WRITE);
+ if (ret) {
+ mlog_errno(ret);
+ goto out;
+ }
+
+ ret = ocfs2_xa_prepare_entry(loc, xi, name_hash, ctxt);
+ if (ret) {
+ if (ret != -ENOSPC)
+ mlog_errno(ret);
+ goto out;
+ }
+
+ ret = ocfs2_xa_store_value(loc, xi, ctxt);
+ if (ret) {
+ mlog_errno(ret);
+ goto out;
+ }
+
+ ocfs2_xa_journal_dirty(ctxt->handle, loc);
+
+out:
+ return ret;
+}
+
static void ocfs2_init_dinode_xa_loc(struct ocfs2_xa_loc *loc,
struct inode *inode,
struct buffer_head *bh,
@@ -2062,8 +2096,6 @@ static int ocfs2_xattr_set_entry(struct inode *inode,
struct ocfs2_dinode *di = (struct ocfs2_dinode *)xs->inode_bh->b_data;
handle_t *handle = ctxt->handle;
int ret;
- u32 name_hash = ocfs2_xattr_name_hash(inode, xi->xi_name,
- xi->xi_name_len);
struct ocfs2_xa_loc loc;
if (!(flag & OCFS2_INLINE_XATTR_FL))
@@ -2084,14 +2116,8 @@ static int ocfs2_xattr_set_entry(struct inode *inode,
else
ocfs2_init_xattr_block_xa_loc(&loc, inode, xs->xattr_bh,
xs->not_found ? NULL : xs->here);
- ret = ocfs2_xa_journal_access(handle, &loc,
- OCFS2_JOURNAL_ACCESS_WRITE);
- if (ret) {
- mlog_errno(ret);
- goto out;
- }
- ret = ocfs2_xa_prepare_entry(&loc, xi, name_hash, ctxt);
+ ret = ocfs2_xa_set(&loc, xi, ctxt);
if (ret) {
if (ret != -ENOSPC)
mlog_errno(ret);
@@ -2099,14 +2125,6 @@ static int ocfs2_xattr_set_entry(struct inode *inode,
}
xs->here = loc.xl_entry;
- ret = ocfs2_xa_store_value(&loc, xi, ctxt);
- if (ret) {
- mlog_errno(ret);
- goto out;
- }
-
- ocfs2_xa_journal_dirty(handle, &loc);
-
if (!(oi->ip_dyn_features & OCFS2_INLINE_XATTR_FL) &&
(flag & OCFS2_INLINE_XATTR_FL)) {
struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
@@ -5097,8 +5115,6 @@ static int ocfs2_xattr_set_in_bucket(struct inode *inode,
int ret;
u64 blkno;
struct ocfs2_xa_loc loc;
- u32 name_hash = ocfs2_xattr_name_hash(inode, xi->xi_name,
- xi->xi_name_len);
if (!xs->bucket->bu_bhs[1]) {
blkno = bucket_blkno(xs->bucket);
@@ -5112,14 +5128,7 @@ static int ocfs2_xattr_set_in_bucket(struct inode *inode,
ocfs2_init_xattr_bucket_xa_loc(&loc, xs->bucket,
xs->not_found ? NULL : xs->here);
- ret = ocfs2_xa_journal_access(ctxt->handle, &loc,
- OCFS2_JOURNAL_ACCESS_WRITE);
- if (ret < 0) {
- mlog_errno(ret);
- goto out;
- }
-
- ret = ocfs2_xa_prepare_entry(&loc, xi, name_hash, ctxt);
+ ret = ocfs2_xa_set(&loc, xi, ctxt);
if (ret) {
if (ret != -ENOSPC)
mlog_errno(ret);
@@ -5127,14 +5136,6 @@ static int ocfs2_xattr_set_in_bucket(struct inode *inode,
}
xs->here = loc.xl_entry;
- ret = ocfs2_xa_store_value(&loc, xi, ctxt);
- if (ret) {
- mlog_errno(ret);
- goto out;
- }
-
- ocfs2_xa_journal_dirty(ctxt->handle, &loc);
-
out:
return ret;
}
--
1.6.3.3
^ permalink raw reply related [flat|nested] 33+ messages in thread* [Ocfs2-devel] [PATCH 12/14] ocfs2: Let ocfs2_xa_prepare_entry() do space checks.
2009-08-19 19:54 [Ocfs2-devel] [PATCH 0/14] ocfs2: Unify the setting of extended attributes Joel Becker
` (10 preceding siblings ...)
2009-08-19 19:54 ` [Ocfs2-devel] [PATCH 11/14] ocfs2: Gell into ocfs2_xa_set() Joel Becker
@ 2009-08-19 19:54 ` Joel Becker
2009-08-19 19:54 ` [Ocfs2-devel] [PATCH 13/14] ocfs2: Set xattr block entries with ocfs2_xa_set() Joel Becker
` (2 subsequent siblings)
14 siblings, 0 replies; 33+ messages in thread
From: Joel Becker @ 2009-08-19 19:54 UTC (permalink / raw)
To: ocfs2-devel
ocfs2_xattr_set_in_bucket() doesn't need to do its own hacky space
checking. Let's let ocfs2_xa_prepare_entry() (via ocfs2_xa_set()) do
the more accurate work. Whenever it doesn't have space,
ocfs2_xattr_set_in_bucket() can try to get more space.
Signed-off-by: Joel Becker <joel.becker@oracle.com>
---
fs/ocfs2/xattr.c | 239 ++++++++++++++++-------------------------------------
1 files changed, 72 insertions(+), 167 deletions(-)
diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index 7527546..f0205e6 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -5105,42 +5105,6 @@ out:
}
/*
- * Set the xattr name/value in the bucket specified in xs.
- */
-static int ocfs2_xattr_set_in_bucket(struct inode *inode,
- struct ocfs2_xattr_info *xi,
- struct ocfs2_xattr_search *xs,
- struct ocfs2_xattr_set_ctxt *ctxt)
-{
- int ret;
- u64 blkno;
- struct ocfs2_xa_loc loc;
-
- if (!xs->bucket->bu_bhs[1]) {
- blkno = bucket_blkno(xs->bucket);
- ocfs2_xattr_bucket_relse(xs->bucket);
- ret = ocfs2_read_xattr_bucket(xs->bucket, blkno);
- if (ret) {
- mlog_errno(ret);
- goto out;
- }
- }
-
- ocfs2_init_xattr_bucket_xa_loc(&loc, xs->bucket,
- xs->not_found ? NULL : xs->here);
- ret = ocfs2_xa_set(&loc, xi, ctxt);
- if (ret) {
- if (ret != -ENOSPC)
- mlog_errno(ret);
- goto out;
- }
- xs->here = loc.xl_entry;
-
-out:
- return ret;
-}
-
-/*
* check whether the xattr bucket is filled up with the same hash value.
* If we want to insert the xattr with the same hash, return -ENOSPC.
* If we want to insert a xattr with different hash value, go ahead
@@ -5173,151 +5137,92 @@ static int ocfs2_xattr_set_entry_index_block(struct inode *inode,
struct ocfs2_xattr_search *xs,
struct ocfs2_xattr_set_ctxt *ctxt)
{
- struct ocfs2_xattr_header *xh;
- struct ocfs2_xattr_entry *xe;
- u16 count, header_size, xh_free_start;
- int free, max_free, need, old;
- size_t value_size = 0;
- size_t blocksize = inode->i_sb->s_blocksize;
- int ret, allocation = 0;
+ int ret;
+ struct ocfs2_xa_loc loc;
mlog_entry("Set xattr %s in xattr index block\n", xi->xi_name);
-try_again:
- xh = xs->header;
- count = le16_to_cpu(xh->xh_count);
- xh_free_start = le16_to_cpu(xh->xh_free_start);
- header_size = sizeof(struct ocfs2_xattr_header) +
- count * sizeof(struct ocfs2_xattr_entry);
- max_free = OCFS2_XATTR_BUCKET_SIZE - header_size -
- le16_to_cpu(xh->xh_name_value_len) - OCFS2_XATTR_HEADER_GAP;
-
- mlog_bug_on_msg(header_size > blocksize, "bucket %llu has header size "
- "of %u which exceed block size\n",
- (unsigned long long)bucket_blkno(xs->bucket),
- header_size);
-
- if (xi->xi_value && xi->xi_value_len > OCFS2_XATTR_INLINE_SIZE)
- value_size = OCFS2_XATTR_ROOT_SIZE;
- else if (xi->xi_value)
- value_size = OCFS2_XATTR_SIZE(xi->xi_value_len);
+ BUG_ON(!xs->bucket->bu_bhs[1]);
- if (xs->not_found)
- need = sizeof(struct ocfs2_xattr_entry) +
- OCFS2_XATTR_SIZE(xi->xi_name_len) + value_size;
- else {
- need = value_size + OCFS2_XATTR_SIZE(xi->xi_name_len);
+ ocfs2_init_xattr_bucket_xa_loc(&loc, xs->bucket,
+ xs->not_found ? NULL : xs->here);
+ ret = ocfs2_xa_set(&loc, xi, ctxt);
+ if (!ret) {
+ xs->here = loc.xl_entry;
+ goto out;
+ }
+ if (ret != -ENOSPC) {
+ mlog_errno(ret);
+ goto out;
+ }
- /*
- * We only replace the old value if the new length is smaller
- * than the old one. Otherwise we will allocate new space in the
- * bucket to store it.
- */
- xe = xs->here;
- if (ocfs2_xattr_is_local(xe))
- old = OCFS2_XATTR_SIZE(le64_to_cpu(xe->xe_value_size));
- else
- old = OCFS2_XATTR_SIZE(OCFS2_XATTR_ROOT_SIZE);
+ /* Ok, we need space. Let's try defragmenting the bucket. */
+ ret = ocfs2_defrag_xattr_bucket(inode, ctxt->handle,
+ xs->bucket);
+ if (ret) {
+ mlog_errno(ret);
+ goto out;
+ }
- if (old >= value_size)
- need = 0;
+ ret = ocfs2_xa_set(&loc, xi, ctxt);
+ if (!ret) {
+ xs->here = loc.xl_entry;
+ goto out;
}
+ if (ret != -ENOSPC) {
+ mlog_errno(ret);
+ goto out;
+ }
+
+ /* Ack, need more space. Let's try to get another bucket! */
- free = xh_free_start - header_size - OCFS2_XATTR_HEADER_GAP;
/*
- * We need to make sure the new name/value pair
- * can exist in the same block.
+ * We do not allow for overlapping ranges between buckets. And
+ * the maximum number of collisions we will allow for then is
+ * one bucket's worth, so check it here whether we need to
+ * add a new bucket for the insert.
*/
- if (xh_free_start % blocksize < need)
- free -= xh_free_start % blocksize;
-
- mlog(0, "xs->not_found = %d, in xattr bucket %llu: free = %d, "
- "need = %d, max_free = %d, xh_free_start = %u, xh_name_value_len ="
- " %u\n", xs->not_found,
- (unsigned long long)bucket_blkno(xs->bucket),
- free, need, max_free, le16_to_cpu(xh->xh_free_start),
- le16_to_cpu(xh->xh_name_value_len));
-
- 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)) {
- /*
- * We can create the space by defragment. Since only the
- * name/value will be moved, the xe shouldn't be changed
- * in xs.
- */
- ret = ocfs2_defrag_xattr_bucket(inode, ctxt->handle,
- xs->bucket);
- if (ret) {
- mlog_errno(ret);
- goto out;
- }
-
- xh_free_start = le16_to_cpu(xh->xh_free_start);
- free = xh_free_start - header_size
- - OCFS2_XATTR_HEADER_GAP;
- if (xh_free_start % blocksize < need)
- free -= xh_free_start % blocksize;
-
- if (free >= need)
- goto xattr_set;
-
- mlog(0, "Can't get enough space for xattr insert by "
- "defragment. Need %u bytes, but we have %d, so "
- "allocate new bucket for it.\n", need, free);
- }
-
- /*
- * We have to add new buckets or clusters and one
- * allocation should leave us enough space for insert.
- */
- BUG_ON(allocation);
-
- /*
- * We do not allow for overlapping ranges between buckets. And
- * the maximum number of collisions we will allow for then is
- * one bucket's worth, so check it here whether we need to
- * add a new bucket for the insert.
- */
- ret = ocfs2_check_xattr_bucket_collision(inode,
- xs->bucket,
- xi->xi_name);
- if (ret) {
- mlog_errno(ret);
- goto out;
- }
-
- ret = ocfs2_add_new_xattr_bucket(inode,
- xs->xattr_bh,
+ ret = ocfs2_check_xattr_bucket_collision(inode,
xs->bucket,
- ctxt);
- if (ret) {
- mlog_errno(ret);
- goto out;
- }
+ xi->xi_name);
+ if (ret) {
+ mlog_errno(ret);
+ goto out;
+ }
- /*
- * ocfs2_add_new_xattr_bucket() will have updated
- * xs->bucket if it moved, but it will not have updated
- * any of the other search fields. Thus, we drop it and
- * re-search. Everything should be cached, so it'll be
- * quick.
- */
- ocfs2_xattr_bucket_relse(xs->bucket);
- ret = ocfs2_xattr_index_block_find(inode, xs->xattr_bh,
- xi->xi_name_index,
- xi->xi_name, xs);
- if (ret && ret != -ENODATA)
- goto out;
- xs->not_found = ret;
- allocation = 1;
- goto try_again;
+ ret = ocfs2_add_new_xattr_bucket(inode,
+ xs->xattr_bh,
+ xs->bucket,
+ ctxt);
+ if (ret) {
+ mlog_errno(ret);
+ goto out;
}
-xattr_set:
- ret = ocfs2_xattr_set_in_bucket(inode, xi, xs, ctxt);
+ /*
+ * ocfs2_add_new_xattr_bucket() will have updated
+ * xs->bucket if it moved, but it will not have updated
+ * any of the other search fields. Thus, we drop it and
+ * re-search. Everything should be cached, so it'll be
+ * quick.
+ */
+ ocfs2_xattr_bucket_relse(xs->bucket);
+ ret = ocfs2_xattr_index_block_find(inode, xs->xattr_bh,
+ xi->xi_name_index,
+ xi->xi_name, xs);
+ if (ret && ret != -ENODATA)
+ goto out;
+ xs->not_found = ret;
+
+ /* We also need to re-init our ocfs2_xa_loc */
+ ocfs2_init_xattr_bucket_xa_loc(&loc, xs->bucket,
+ xs->not_found ? NULL : xs->here);
+ ret = ocfs2_xa_set(&loc, xi, ctxt);
+ if (!ret)
+ xs->here = loc.xl_entry;
+ else if (ret != -ENOSPC)
+ mlog_errno(ret);
+
out:
mlog_exit(ret);
return ret;
--
1.6.3.3
^ permalink raw reply related [flat|nested] 33+ messages in thread* [Ocfs2-devel] [PATCH 13/14] ocfs2: Set xattr block entries with ocfs2_xa_set()
2009-08-19 19:54 [Ocfs2-devel] [PATCH 0/14] ocfs2: Unify the setting of extended attributes Joel Becker
` (11 preceding siblings ...)
2009-08-19 19:54 ` [Ocfs2-devel] [PATCH 12/14] ocfs2: Let ocfs2_xa_prepare_entry() do space checks Joel Becker
@ 2009-08-19 19:54 ` Joel Becker
2009-08-19 19:54 ` [Ocfs2-devel] [PATCH 14/14] ocfs2: Set inline xattr " Joel Becker
2009-08-20 2:03 ` [Ocfs2-devel] [PATCH 0/14] ocfs2: Unify the setting of extended attributes Tao Ma
14 siblings, 0 replies; 33+ messages in thread
From: Joel Becker @ 2009-08-19 19:54 UTC (permalink / raw)
To: ocfs2-devel
ocfs2_xattr_block_set() calls into ocfs2_xattr_set_entry() with just the
HAS_XATTR flag. Most of the machinery of ocfs2_xattr_set_entry() is
skipped. All that really happens other than the call to ocfs2_xa_set()
is making sure the HAS_XATTR flag is set on the inode.
But HAS_XATTR should be set when we also set di->i_xattr_loc. And
that's done in ocfs2_xattr_block_set(). So let's move it there, and
then ocfs2_xattr_block_set() can just call ocfs2_xa_set().
While we're there, ocfs2_xattr_block_set() is kind of busy. Split the
creation of a new xattr block to ocfs2_xattr_block_new(). Now
ocfs2_xattr_block_set() is quite the readable function.
Signed-off-by: Joel Becker <joel.becker@oracle.com>
---
fs/ocfs2/xattr.c | 157 +++++++++++++++++++++++++++++++-----------------------
1 files changed, 90 insertions(+), 67 deletions(-)
diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index f0205e6..0dd691f 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -2098,10 +2098,8 @@ static int ocfs2_xattr_set_entry(struct inode *inode,
int ret;
struct ocfs2_xa_loc loc;
- if (!(flag & OCFS2_INLINE_XATTR_FL))
- BUG_ON(xs->xattr_bh == xs->inode_bh);
- else
- BUG_ON(xs->xattr_bh != xs->inode_bh);
+ BUG_ON(!(flag & OCFS2_INLINE_XATTR_FL));
+ BUG_ON(xs->xattr_bh != xs->inode_bh);
ret = ocfs2_journal_access_di(handle, inode, xs->inode_bh,
OCFS2_JOURNAL_ACCESS_WRITE);
@@ -2110,13 +2108,8 @@ static int ocfs2_xattr_set_entry(struct inode *inode,
goto out;
}
- if (xs->xattr_bh == xs->inode_bh)
- ocfs2_init_dinode_xa_loc(&loc, inode, xs->inode_bh,
- xs->not_found ? NULL : xs->here);
- else
- ocfs2_init_xattr_block_xa_loc(&loc, inode, xs->xattr_bh,
- xs->not_found ? NULL : xs->here);
-
+ ocfs2_init_dinode_xa_loc(&loc, inode, xs->inode_bh,
+ xs->not_found ? NULL : xs->here);
ret = ocfs2_xa_set(&loc, xi, ctxt);
if (ret) {
if (ret != -ENOSPC)
@@ -2125,8 +2118,7 @@ static int ocfs2_xattr_set_entry(struct inode *inode,
}
xs->here = loc.xl_entry;
- if (!(oi->ip_dyn_features & OCFS2_INLINE_XATTR_FL) &&
- (flag & OCFS2_INLINE_XATTR_FL)) {
+ if (!(oi->ip_dyn_features & OCFS2_INLINE_XATTR_FL)) {
struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
unsigned int xattrsize = osb->s_xattr_inline_size;
@@ -2146,7 +2138,7 @@ static int ocfs2_xattr_set_entry(struct inode *inode,
}
/* Update xattr flag */
spin_lock(&oi->ip_lock);
- oi->ip_dyn_features |= flag;
+ oi->ip_dyn_features |= OCFS2_INLINE_XATTR_FL;
di->i_dyn_features = cpu_to_le16(oi->ip_dyn_features);
spin_unlock(&oi->ip_lock);
@@ -2541,6 +2533,72 @@ cleanup:
return ret;
}
+static int ocfs2_xattr_block_new(struct inode *inode,
+ struct buffer_head *di_bh,
+ struct ocfs2_xattr_set_ctxt *ctxt,
+ struct buffer_head **xb_bh)
+{
+ int ret;
+ struct buffer_head *new_bh = NULL;
+ struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
+ struct ocfs2_dinode *di = (struct ocfs2_dinode *)di_bh->b_data;
+ u16 suballoc_bit_start;
+ u32 num_got;
+ u64 first_blkno;
+ struct ocfs2_xattr_block *xb;
+
+ ret = ocfs2_journal_access_di(ctxt->handle, inode, di_bh,
+ OCFS2_JOURNAL_ACCESS_WRITE);
+ if (ret < 0) {
+ mlog_errno(ret);
+ goto end;
+ }
+
+ ret = ocfs2_claim_metadata(osb, ctxt->handle, ctxt->meta_ac, 1,
+ &suballoc_bit_start, &num_got,
+ &first_blkno);
+ if (ret < 0) {
+ mlog_errno(ret);
+ goto end;
+ }
+
+ new_bh = sb_getblk(inode->i_sb, first_blkno);
+ ocfs2_set_new_buffer_uptodate(inode, new_bh);
+ ret = ocfs2_journal_access_xb(ctxt->handle, inode, new_bh,
+ OCFS2_JOURNAL_ACCESS_CREATE);
+ if (ret < 0) {
+ mlog_errno(ret);
+ goto end;
+ }
+
+ /* Initialize ocfs2_xattr_block */
+ xb = (struct ocfs2_xattr_block *)new_bh->b_data;
+ memset(xb, 0, inode->i_sb->s_blocksize);
+ strcpy((void *)xb, OCFS2_XATTR_BLOCK_SIGNATURE);
+ xb->xb_suballoc_slot = cpu_to_le16(osb->slot_num);
+ xb->xb_suballoc_bit = cpu_to_le16(suballoc_bit_start);
+ xb->xb_fs_generation = cpu_to_le32(osb->fs_generation);
+ xb->xb_blkno = cpu_to_le64(first_blkno);
+ ocfs2_journal_dirty(ctxt->handle, new_bh);
+
+ /* Add it to the inode */
+ di->i_xattr_loc = cpu_to_le64(first_blkno);
+
+ spin_lock(&OCFS2_I(inode)->ip_lock);
+ OCFS2_I(inode)->ip_dyn_features |= OCFS2_HAS_XATTR_FL;
+ di->i_dyn_features = cpu_to_le16(OCFS2_I(inode)->ip_dyn_features);
+ spin_unlock(&OCFS2_I(inode)->ip_lock);
+
+ ocfs2_journal_dirty(ctxt->handle, di_bh);
+
+ *xb_bh = new_bh;
+ new_bh = NULL;
+
+end:
+ brelse(new_bh);
+ return ret;
+}
+
/*
* ocfs2_xattr_block_set()
*
@@ -2553,82 +2611,47 @@ static int ocfs2_xattr_block_set(struct inode *inode,
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;
- handle_t *handle = ctxt->handle;
struct ocfs2_xattr_block *xblk = NULL;
- u16 suballoc_bit_start;
- u32 num_got;
- u64 first_blkno;
int ret;
+ struct ocfs2_xa_loc loc;
if (!xs->xattr_bh) {
- ret = ocfs2_journal_access_di(handle, inode, xs->inode_bh,
- OCFS2_JOURNAL_ACCESS_CREATE);
- if (ret < 0) {
- mlog_errno(ret);
- goto end;
- }
-
- ret = ocfs2_claim_metadata(osb, handle, ctxt->meta_ac, 1,
- &suballoc_bit_start, &num_got,
- &first_blkno);
- if (ret < 0) {
- mlog_errno(ret);
- goto end;
- }
-
- new_bh = sb_getblk(inode->i_sb, first_blkno);
- ocfs2_set_new_buffer_uptodate(inode, new_bh);
-
- ret = ocfs2_journal_access_xb(handle, inode, new_bh,
- OCFS2_JOURNAL_ACCESS_CREATE);
- if (ret < 0) {
+ ret = ocfs2_xattr_block_new(inode, xs->inode_bh, ctxt,
+ &new_bh);
+ if (ret) {
mlog_errno(ret);
goto end;
}
- /* Initialize ocfs2_xattr_block */
xs->xattr_bh = new_bh;
- xblk = (struct ocfs2_xattr_block *)new_bh->b_data;
- memset(xblk, 0, inode->i_sb->s_blocksize);
- strcpy((void *)xblk, OCFS2_XATTR_BLOCK_SIGNATURE);
- xblk->xb_suballoc_slot = cpu_to_le16(osb->slot_num);
- xblk->xb_suballoc_bit = cpu_to_le16(suballoc_bit_start);
- xblk->xb_fs_generation = cpu_to_le32(osb->fs_generation);
- xblk->xb_blkno = cpu_to_le64(first_blkno);
-
+ xblk = (struct ocfs2_xattr_block *)xs->xattr_bh->b_data;
xs->header = &xblk->xb_attrs.xb_header;
xs->base = (void *)xs->header;
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);
- goto end;
- }
- di->i_xattr_loc = cpu_to_le64(first_blkno);
- ocfs2_journal_dirty(handle, xs->inode_bh);
} else
xblk = (struct ocfs2_xattr_block *)xs->xattr_bh->b_data;
if (!(le16_to_cpu(xblk->xb_flags) & OCFS2_XATTR_INDEXED)) {
- /* Set extended attribute into external block */
- ret = ocfs2_xattr_set_entry(inode, xi, xs, ctxt,
- OCFS2_HAS_XATTR_FL);
- if (!ret || ret != -ENOSPC)
- goto end;
+ ocfs2_init_xattr_block_xa_loc(&loc, inode, xs->xattr_bh,
+ xs->not_found ? NULL : xs->here);
- ret = ocfs2_xattr_create_index_block(inode, xs, ctxt);
- if (ret)
+ ret = ocfs2_xa_set(&loc, xi, ctxt);
+ if (!ret)
+ xs->here = loc.xl_entry;
+ else if (ret != -ENOSPC)
goto end;
+ else {
+ ret = ocfs2_xattr_create_index_block(inode, xs, ctxt);
+ if (ret)
+ goto end;
+ }
}
- ret = ocfs2_xattr_set_entry_index_block(inode, xi, xs, ctxt);
+ if (le16_to_cpu(xblk->xb_flags) & OCFS2_XATTR_INDEXED)
+ ret = ocfs2_xattr_set_entry_index_block(inode, xi, xs, ctxt);
end:
-
return ret;
}
--
1.6.3.3
^ permalink raw reply related [flat|nested] 33+ messages in thread* [Ocfs2-devel] [PATCH 14/14] ocfs2: Set inline xattr entries with ocfs2_xa_set()
2009-08-19 19:54 [Ocfs2-devel] [PATCH 0/14] ocfs2: Unify the setting of extended attributes Joel Becker
` (12 preceding siblings ...)
2009-08-19 19:54 ` [Ocfs2-devel] [PATCH 13/14] ocfs2: Set xattr block entries with ocfs2_xa_set() Joel Becker
@ 2009-08-19 19:54 ` Joel Becker
2009-08-20 2:03 ` [Ocfs2-devel] [PATCH 0/14] ocfs2: Unify the setting of extended attributes Tao Ma
14 siblings, 0 replies; 33+ messages in thread
From: Joel Becker @ 2009-08-19 19:54 UTC (permalink / raw)
To: ocfs2-devel
ocfs2_xattr_ibody_set() is the only remaining user of
ocfs2_xattr_set_entry(). ocfs2_xattr_set_entry() actually does two
things: it calls ocfs2_xa_set(), and it initializes the inline xattrs.
Initializing the inline space really belongs in its own call.
We lift the initialization to ocfs2_xattr_ibody_init(), called from
ocfs2_xattr_ibody_set() only when necessary. Now
ocfs2_xattr_ibody_set() can call ocfs2_xa_set() directly.
ocfs2_xattr_set_entry() goes away.
Another nice fact is that ocfs2_init_dinode_xa_loc() can trust
i_xattr_inline_size.
Signed-off-by: Joel Becker <joel.becker@oracle.com>
---
fs/ocfs2/xattr.c | 156 +++++++++++++++++++++++++-----------------------------
1 files changed, 72 insertions(+), 84 deletions(-)
diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index 0dd691f..95b639d 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -2029,17 +2029,13 @@ static void ocfs2_init_dinode_xa_loc(struct ocfs2_xa_loc *loc,
{
struct ocfs2_dinode *di = (struct ocfs2_dinode *)bh->b_data;
+ BUG_ON(!(OCFS2_I(inode)->ip_dyn_features & OCFS2_INLINE_XATTR_FL));
+
loc->xl_inode = inode;
loc->xl_ops = &ocfs2_xa_block_loc_ops;
loc->xl_storage = bh;
loc->xl_entry = entry;
-
- if (OCFS2_I(inode)->ip_dyn_features & OCFS2_INLINE_XATTR_FL)
- loc->xl_size = le16_to_cpu(di->i_xattr_inline_size);
- else {
- BUG_ON(entry);
- loc->xl_size = OCFS2_SB(inode->i_sb)->s_xattr_inline_size;
- }
+ loc->xl_size = le16_to_cpu(di->i_xattr_inline_size);
loc->xl_header =
(struct ocfs2_xattr_header *)(bh->b_data + bh->b_size -
loc->xl_size);
@@ -2076,80 +2072,6 @@ static void ocfs2_init_xattr_bucket_xa_loc(struct ocfs2_xa_loc *loc,
loc->xl_size = OCFS2_XATTR_BUCKET_SIZE;
}
-
-/*
- * ocfs2_xattr_set_entry()
- *
- * Set extended attribute entry into inode or block.
- *
- * If extended attribute value size > OCFS2_XATTR_INLINE_SIZE,
- * We first insert tree root(ocfs2_xattr_value_root) like a normal value,
- * then set value in B tree with set_value_outside().
- */
-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_inode_info *oi = OCFS2_I(inode);
- struct ocfs2_dinode *di = (struct ocfs2_dinode *)xs->inode_bh->b_data;
- handle_t *handle = ctxt->handle;
- int ret;
- struct ocfs2_xa_loc loc;
-
- BUG_ON(!(flag & OCFS2_INLINE_XATTR_FL));
- BUG_ON(xs->xattr_bh != xs->inode_bh);
-
- ret = ocfs2_journal_access_di(handle, inode, xs->inode_bh,
- OCFS2_JOURNAL_ACCESS_WRITE);
- if (ret) {
- mlog_errno(ret);
- goto out;
- }
-
- ocfs2_init_dinode_xa_loc(&loc, inode, xs->inode_bh,
- xs->not_found ? NULL : xs->here);
- ret = ocfs2_xa_set(&loc, xi, ctxt);
- if (ret) {
- if (ret != -ENOSPC)
- mlog_errno(ret);
- goto out;
- }
- xs->here = loc.xl_entry;
-
- if (!(oi->ip_dyn_features & OCFS2_INLINE_XATTR_FL)) {
- struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
- unsigned int xattrsize = osb->s_xattr_inline_size;
-
- /*
- * Adjust extent record count or inline data size
- * to reserve space for extended attribute.
- */
- if (oi->ip_dyn_features & OCFS2_INLINE_DATA_FL) {
- struct ocfs2_inline_data *idata = &di->id2.i_data;
- le16_add_cpu(&idata->id_count, -xattrsize);
- } else if (!(ocfs2_inode_is_fast_symlink(inode))) {
- struct ocfs2_extent_list *el = &di->id2.i_list;
- le16_add_cpu(&el->l_count, -(xattrsize /
- sizeof(struct ocfs2_extent_rec)));
- }
- di->i_xattr_inline_size = cpu_to_le16(xattrsize);
- }
- /* Update xattr flag */
- spin_lock(&oi->ip_lock);
- oi->ip_dyn_features |= OCFS2_INLINE_XATTR_FL;
- di->i_dyn_features = cpu_to_le16(oi->ip_dyn_features);
- spin_unlock(&oi->ip_lock);
-
- ret = ocfs2_journal_dirty(handle, xs->inode_bh);
- if (ret < 0)
- mlog_errno(ret);
-
-out:
- return ret;
-}
-
static int ocfs2_remove_value_outside(struct inode*inode,
struct ocfs2_xattr_value_buf *vb,
struct ocfs2_xattr_header *header)
@@ -2446,6 +2368,55 @@ static int ocfs2_xattr_ibody_find(struct inode *inode,
return 0;
}
+static int ocfs2_xattr_ibody_init(struct inode *inode,
+ struct buffer_head *di_bh,
+ struct ocfs2_xattr_set_ctxt *ctxt)
+{
+ int ret;
+ struct ocfs2_inode_info *oi = OCFS2_I(inode);
+ struct ocfs2_dinode *di = (struct ocfs2_dinode *)di_bh->b_data;
+ struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
+ unsigned int xattrsize = osb->s_xattr_inline_size;
+
+ if (!ocfs2_xattr_has_space_inline(inode, di)) {
+ ret = -ENOSPC;
+ goto out;
+ }
+
+ ret = ocfs2_journal_access_di(ctxt->handle, inode, di_bh,
+ OCFS2_JOURNAL_ACCESS_WRITE);
+ if (ret) {
+ mlog_errno(ret);
+ goto out;
+ }
+
+ /*
+ * Adjust extent record count or inline data size
+ * to reserve space for extended attribute.
+ */
+ if (oi->ip_dyn_features & OCFS2_INLINE_DATA_FL) {
+ struct ocfs2_inline_data *idata = &di->id2.i_data;
+ le16_add_cpu(&idata->id_count, -xattrsize);
+ } else if (!(ocfs2_inode_is_fast_symlink(inode))) {
+ struct ocfs2_extent_list *el = &di->id2.i_list;
+ le16_add_cpu(&el->l_count, -(xattrsize /
+ sizeof(struct ocfs2_extent_rec)));
+ }
+ di->i_xattr_inline_size = cpu_to_le16(xattrsize);
+
+ spin_lock(&oi->ip_lock);
+ oi->ip_dyn_features |= OCFS2_INLINE_XATTR_FL|OCFS2_HAS_XATTR_FL;
+ di->i_dyn_features = cpu_to_le16(oi->ip_dyn_features);
+ spin_unlock(&oi->ip_lock);
+
+ ret = ocfs2_journal_dirty(ctxt->handle, di_bh);
+ if (ret < 0)
+ mlog_errno(ret);
+
+out:
+ return ret;
+}
+
/*
* ocfs2_xattr_ibody_set()
*
@@ -2457,9 +2428,10 @@ static int ocfs2_xattr_ibody_set(struct inode *inode,
struct ocfs2_xattr_search *xs,
struct ocfs2_xattr_set_ctxt *ctxt)
{
+ int ret;
struct ocfs2_inode_info *oi = OCFS2_I(inode);
struct ocfs2_dinode *di = (struct ocfs2_dinode *)xs->inode_bh->b_data;
- int ret;
+ struct ocfs2_xa_loc loc;
if (inode->i_sb->s_blocksize == OCFS2_MIN_BLOCKSIZE)
return -ENOSPC;
@@ -2472,8 +2444,24 @@ static int ocfs2_xattr_ibody_set(struct inode *inode,
}
}
- ret = ocfs2_xattr_set_entry(inode, xi, xs, ctxt,
- (OCFS2_INLINE_XATTR_FL | OCFS2_HAS_XATTR_FL));
+ if (!(oi->ip_dyn_features & OCFS2_INLINE_XATTR_FL)) {
+ ret = ocfs2_xattr_ibody_init(inode, xs->inode_bh, ctxt);
+ if (ret) {
+ mlog_errno(ret);
+ goto out;
+ }
+ }
+
+ ocfs2_init_dinode_xa_loc(&loc, inode, xs->inode_bh,
+ xs->not_found ? NULL : xs->here);
+ ret = ocfs2_xa_set(&loc, xi, ctxt);
+ if (ret) {
+ if (ret != -ENOSPC)
+ mlog_errno(ret);
+ goto out;
+ }
+ xs->here = loc.xl_entry;
+
out:
up_write(&oi->ip_alloc_sem);
--
1.6.3.3
^ permalink raw reply related [flat|nested] 33+ messages in thread* [Ocfs2-devel] [PATCH 0/14] ocfs2: Unify the setting of extended attributes
2009-08-19 19:54 [Ocfs2-devel] [PATCH 0/14] ocfs2: Unify the setting of extended attributes Joel Becker
` (13 preceding siblings ...)
2009-08-19 19:54 ` [Ocfs2-devel] [PATCH 14/14] ocfs2: Set inline xattr " Joel Becker
@ 2009-08-20 2:03 ` Tao Ma
14 siblings, 0 replies; 33+ messages in thread
From: Tao Ma @ 2009-08-20 2:03 UTC (permalink / raw)
To: ocfs2-devel
Hi Joel,
Joel Becker wrote:
> ocfs2 can store extended attributes in many ways. They can live inside
> the inode's inline data area, they can be stored in a single external
> block, or they can be in a "bucket" hanging off of a lookup tree. There
> are differences in how each storage type manages the attributes, and the
> current ocfs2 code reflects this.
>
> There are two entirely separate code paths for setting extended
> attributes. The first code path handles "block" storage - the inline
> inode or the external block. The second handles "bucket" storage. They
> do very similar things, but they go about them in very different ways.
> This makes reading and understanding the ocfs2 xattr code harder than it
> needs to be. Worse, updating the xattr code requires understanding and
> modifying both paths.
>
> This patch series unifies the xattr set code such that inodes, block,
> and buckets make the same call. It removes most of the redundant code,
> like the repeated implementations of truncating externally stored values.
Due to some historical reasons, the setting of xattrs has been divided
into 2 parts. I am so sorry for that and to be frank, I want to change
it for a very long time. So thanks for doing this.
>
> The core of the implementation is the ocfs2_xa_loc structure. This is
> an in-memory representation of an ocfs2_xattr_entry. It has operations
> that allow blocks and buckets to behave differently during the xattr
> modification process. Blocks and buckets are different enough that
> ocfs2_xa_loc_operations has ten different ops! But with these
> differences encapsulated, we can have a single code path to modify an
> entry.
>
> The changes don't reduce the size of the source code perceptibly (~10
> lines of actual code without comments or blanks), but to my mind they
> greatly improve the readability and maintainability.
>
> The current series is based on the 'fixes' branch; I'll need to rebase
> it atop cachme before it can go upstream. I wanted to send it out as it
> stands now so that the concept can be reviewed while I work on
> merge-window.
>
> Tao and Tiger, I'd really appreciate you going over the changes with a
> find-toothed comb. Make sure I have my ideas about block and bucket
> stuff correct, etc. Let me know if you think this is a stupid change,
> too :-)
the idea is cool and it is really helpful and I will review it carefully.
Regards,
Tao
^ permalink raw reply [flat|nested] 33+ messages in thread