* [Ocfs2-devel] [PATCH] ocfs2: Use xs->bucket to set xattr value outside.
@ 2009-03-09 23:49 Tao Ma
2009-03-10 9:30 ` tristan.ye
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Tao Ma @ 2009-03-09 23:49 UTC (permalink / raw)
To: ocfs2-devel
Tristan,
could you please run your xattr test against it?
xs->base used to be allocated a 4K size and all the contents in the
bucket are copied to the it. So in ocfs2_xattr_bucket_set_value_outside,
we are safe to use xs->base + offset. Now we use ocfs2_xattr_bucket to
abstract xattr bucket and xs->base is initialized to the start of the
bu_bhs[0]. So xs->base + offset will overflow when the value root is
stored outside the first block.
Then why we can survive the xattr test by now? It is because we always
read the bucket contiguously now and kernel mm allocate continguous
memory for us. We are lucky, but we should fix it. So just get the
right value root as other callers do.
Signed-off-by: Tao Ma <tao.ma@oracle.com>
---
fs/ocfs2/xattr.c | 27 +++++++++++++++++++++------
1 files changed, 21 insertions(+), 6 deletions(-)
diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index f1b9af1..503a1d7 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -4795,19 +4795,34 @@ static int ocfs2_xattr_bucket_set_value_outside(struct inode *inode,
char *val,
int value_len)
{
- int offset;
+ int ret, offset, block_off;
struct ocfs2_xattr_value_root *xv;
struct ocfs2_xattr_entry *xe = xs->here;
+ struct ocfs2_xattr_bucket *bucket = xs->bucket;
+ struct ocfs2_xattr_header *xh = bucket_xh(bucket);
+ void *base;
BUG_ON(!xs->base || !xe || ocfs2_xattr_is_local(xe));
- offset = le16_to_cpu(xe->xe_name_offset) +
- OCFS2_XATTR_SIZE(xe->xe_name_len);
+ ret = ocfs2_xattr_bucket_get_name_value(inode, xh,
+ xe - xh->xh_entries,
+ &block_off,
+ &offset);
+ if (ret) {
+ mlog_errno(ret);
+ goto out;
+ }
- xv = (struct ocfs2_xattr_value_root *)(xs->base + offset);
+ base = bucket_block(xs->bucket, block_off);
+ xv = (struct ocfs2_xattr_value_root *)(base + offset +
+ OCFS2_XATTR_SIZE(xe->xe_name_len));
- return __ocfs2_xattr_set_value_outside(inode, handle,
- xv, val, value_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,
--
1.6.2.rc2.16.gf474c
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Ocfs2-devel] [PATCH] ocfs2: Use xs->bucket to set xattr value outside.
2009-03-09 23:49 [Ocfs2-devel] [PATCH] ocfs2: Use xs->bucket to set xattr value outside Tao Ma
@ 2009-03-10 9:30 ` tristan.ye
2009-03-12 2:28 ` tristan.ye
2009-03-12 9:29 ` Joel Becker
2 siblings, 0 replies; 6+ messages in thread
From: tristan.ye @ 2009-03-10 9:30 UTC (permalink / raw)
To: ocfs2-devel
On Tue, 2009-03-10 at 07:49 +0800, Tao Ma wrote:
> Tristan,
> could you please run your xattr test against it?
Sure, tests will be carried on soon:-)
Tristan.
>
> xs->base used to be allocated a 4K size and all the contents in the
> bucket are copied to the it. So in ocfs2_xattr_bucket_set_value_outside,
> we are safe to use xs->base + offset. Now we use ocfs2_xattr_bucket to
> abstract xattr bucket and xs->base is initialized to the start of the
> bu_bhs[0]. So xs->base + offset will overflow when the value root is
> stored outside the first block.
>
> Then why we can survive the xattr test by now? It is because we always
> read the bucket contiguously now and kernel mm allocate continguous
> memory for us. We are lucky, but we should fix it. So just get the
> right value root as other callers do.
>
> Signed-off-by: Tao Ma <tao.ma@oracle.com>
> ---
> fs/ocfs2/xattr.c | 27 +++++++++++++++++++++------
> 1 files changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
> index f1b9af1..503a1d7 100644
> --- a/fs/ocfs2/xattr.c
> +++ b/fs/ocfs2/xattr.c
> @@ -4795,19 +4795,34 @@ static int ocfs2_xattr_bucket_set_value_outside(struct inode *inode,
> char *val,
> int value_len)
> {
> - int offset;
> + int ret, offset, block_off;
> struct ocfs2_xattr_value_root *xv;
> struct ocfs2_xattr_entry *xe = xs->here;
> + struct ocfs2_xattr_bucket *bucket = xs->bucket;
> + struct ocfs2_xattr_header *xh = bucket_xh(bucket);
> + void *base;
>
> BUG_ON(!xs->base || !xe || ocfs2_xattr_is_local(xe));
>
> - offset = le16_to_cpu(xe->xe_name_offset) +
> - OCFS2_XATTR_SIZE(xe->xe_name_len);
> + ret = ocfs2_xattr_bucket_get_name_value(inode, xh,
> + xe - xh->xh_entries,
> + &block_off,
> + &offset);
> + if (ret) {
> + mlog_errno(ret);
> + goto out;
> + }
>
> - xv = (struct ocfs2_xattr_value_root *)(xs->base + offset);
> + base = bucket_block(xs->bucket, block_off);
> + xv = (struct ocfs2_xattr_value_root *)(base + offset +
> + OCFS2_XATTR_SIZE(xe->xe_name_len));
>
> - return __ocfs2_xattr_set_value_outside(inode, handle,
> - xv, val, value_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,
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Ocfs2-devel] [PATCH] ocfs2: Use xs->bucket to set xattr value outside.v2
2009-03-12 9:29 ` Joel Becker
@ 2009-03-12 0:37 ` Tao Ma
2009-03-12 10:18 ` Joel Becker
0 siblings, 1 reply; 6+ messages in thread
From: Tao Ma @ 2009-03-12 0:37 UTC (permalink / raw)
To: ocfs2-devel
V1 to V2:
Just remove the local variable bucket.
A long time ago, xs->base is allocated a 4K size and all the contents
in the bucket are copied to the it. Now we use ocfs2_xattr_bucket to
abstract xattr bucket and xs->base is initialized to the start of the
bu_bhs[0]. So xs->base + offset will overflow when the value root is
stored outside the first block.
Then why we can survive the xattr test by now? It is because we always
read the bucket contiguously now and kernel mm allocate continguous
memory for us. We are lucky, but we should fix it. So just get the
right value root as other callers do.
Signed-off-by: Tao Ma <tao.ma@oracle.com>
---
fs/ocfs2/xattr.c | 26 ++++++++++++++++++++------
1 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index f1b9af1..195120d 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -4795,19 +4795,33 @@ static int ocfs2_xattr_bucket_set_value_outside(struct inode *inode,
char *val,
int value_len)
{
- int offset;
+ 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));
- offset = le16_to_cpu(xe->xe_name_offset) +
- OCFS2_XATTR_SIZE(xe->xe_name_len);
+ ret = ocfs2_xattr_bucket_get_name_value(inode, xh,
+ xe - xh->xh_entries,
+ &block_off,
+ &offset);
+ if (ret) {
+ mlog_errno(ret);
+ goto out;
+ }
- xv = (struct ocfs2_xattr_value_root *)(xs->base + offset);
+ base = bucket_block(xs->bucket, block_off);
+ xv = (struct ocfs2_xattr_value_root *)(base + offset +
+ OCFS2_XATTR_SIZE(xe->xe_name_len));
- return __ocfs2_xattr_set_value_outside(inode, handle,
- xv, val, value_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,
--
1.6.2.rc2.16.gf474c
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Ocfs2-devel] [PATCH] ocfs2: Use xs->bucket to set xattr value outside.
2009-03-09 23:49 [Ocfs2-devel] [PATCH] ocfs2: Use xs->bucket to set xattr value outside Tao Ma
2009-03-10 9:30 ` tristan.ye
@ 2009-03-12 2:28 ` tristan.ye
2009-03-12 9:29 ` Joel Becker
2 siblings, 0 replies; 6+ messages in thread
From: tristan.ye @ 2009-03-12 2:28 UTC (permalink / raw)
To: ocfs2-devel
On Tue, 2009-03-10 at 07:49 +0800, Tao Ma wrote:
> Tristan,
> could you please run your xattr test against it?
Awesome! All single&multi-nodes xattr tests passed!
For detail testing info, refer to:
http://oss.oracle.com/osswiki/OCFS2/XattrTest
Regards,
Tristan
>
> xs->base used to be allocated a 4K size and all the contents in the
> bucket are copied to the it. So in ocfs2_xattr_bucket_set_value_outside,
> we are safe to use xs->base + offset. Now we use ocfs2_xattr_bucket to
> abstract xattr bucket and xs->base is initialized to the start of the
> bu_bhs[0]. So xs->base + offset will overflow when the value root is
> stored outside the first block.
>
> Then why we can survive the xattr test by now? It is because we always
> read the bucket contiguously now and kernel mm allocate continguous
> memory for us. We are lucky, but we should fix it. So just get the
> right value root as other callers do.
>
> Signed-off-by: Tao Ma <tao.ma@oracle.com>
> ---
> fs/ocfs2/xattr.c | 27 +++++++++++++++++++++------
> 1 files changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
> index f1b9af1..503a1d7 100644
> --- a/fs/ocfs2/xattr.c
> +++ b/fs/ocfs2/xattr.c
> @@ -4795,19 +4795,34 @@ static int ocfs2_xattr_bucket_set_value_outside(struct inode *inode,
> char *val,
> int value_len)
> {
> - int offset;
> + int ret, offset, block_off;
> struct ocfs2_xattr_value_root *xv;
> struct ocfs2_xattr_entry *xe = xs->here;
> + struct ocfs2_xattr_bucket *bucket = xs->bucket;
> + struct ocfs2_xattr_header *xh = bucket_xh(bucket);
> + void *base;
>
> BUG_ON(!xs->base || !xe || ocfs2_xattr_is_local(xe));
>
> - offset = le16_to_cpu(xe->xe_name_offset) +
> - OCFS2_XATTR_SIZE(xe->xe_name_len);
> + ret = ocfs2_xattr_bucket_get_name_value(inode, xh,
> + xe - xh->xh_entries,
> + &block_off,
> + &offset);
> + if (ret) {
> + mlog_errno(ret);
> + goto out;
> + }
>
> - xv = (struct ocfs2_xattr_value_root *)(xs->base + offset);
> + base = bucket_block(xs->bucket, block_off);
> + xv = (struct ocfs2_xattr_value_root *)(base + offset +
> + OCFS2_XATTR_SIZE(xe->xe_name_len));
>
> - return __ocfs2_xattr_set_value_outside(inode, handle,
> - xv, val, value_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,
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Ocfs2-devel] [PATCH] ocfs2: Use xs->bucket to set xattr value outside.
2009-03-09 23:49 [Ocfs2-devel] [PATCH] ocfs2: Use xs->bucket to set xattr value outside Tao Ma
2009-03-10 9:30 ` tristan.ye
2009-03-12 2:28 ` tristan.ye
@ 2009-03-12 9:29 ` Joel Becker
2009-03-12 0:37 ` [Ocfs2-devel] [PATCH] ocfs2: Use xs->bucket to set xattr value outside.v2 Tao Ma
2 siblings, 1 reply; 6+ messages in thread
From: Joel Becker @ 2009-03-12 9:29 UTC (permalink / raw)
To: ocfs2-devel
On Tue, Mar 10, 2009 at 07:49:33AM +0800, Tao Ma wrote:
> diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
> index f1b9af1..503a1d7 100644
> --- a/fs/ocfs2/xattr.c
> +++ b/fs/ocfs2/xattr.c
> @@ -4795,19 +4795,34 @@ static int ocfs2_xattr_bucket_set_value_outside(struct inode *inode,
> char *val,
> int value_len)
> {
> - int offset;
> + int ret, offset, block_off;
> struct ocfs2_xattr_value_root *xv;
> struct ocfs2_xattr_entry *xe = xs->here;
> + struct ocfs2_xattr_bucket *bucket = xs->bucket;
> + struct ocfs2_xattr_header *xh = bucket_xh(bucket);
You only use 'bucket' once, to initialize xh. I'd rather you
remove that variable and initialize xh with 'bucket_xh(xs->bucket)'. It
saves us some stack space.
Joel
--
Life's Little Instruction Book #99
"Think big thoughts, but relish small pleasures."
Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Ocfs2-devel] [PATCH] ocfs2: Use xs->bucket to set xattr value outside.v2
2009-03-12 0:37 ` [Ocfs2-devel] [PATCH] ocfs2: Use xs->bucket to set xattr value outside.v2 Tao Ma
@ 2009-03-12 10:18 ` Joel Becker
0 siblings, 0 replies; 6+ messages in thread
From: Joel Becker @ 2009-03-12 10:18 UTC (permalink / raw)
To: ocfs2-devel
On Thu, Mar 12, 2009 at 08:37:34AM +0800, Tao Ma wrote:
> V1 to V2:
> Just remove the local variable bucket.
>
> A long time ago, xs->base is allocated a 4K size and all the contents
> in the bucket are copied to the it. Now we use ocfs2_xattr_bucket to
> abstract xattr bucket and xs->base is initialized to the start of the
> bu_bhs[0]. So xs->base + offset will overflow when the value root is
> stored outside the first block.
>
> Then why we can survive the xattr test by now? It is because we always
> read the bucket contiguously now and kernel mm allocate continguous
> memory for us. We are lucky, but we should fix it. So just get the
> right value root as other callers do.
>
> Signed-off-by: Tao Ma <tao.ma@oracle.com>
Acked-by: Joel Becker <joel.becker@oracle.com>
> ---
> fs/ocfs2/xattr.c | 26 ++++++++++++++++++++------
> 1 files changed, 20 insertions(+), 6 deletions(-)
>
> diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
> index f1b9af1..195120d 100644
> --- a/fs/ocfs2/xattr.c
> +++ b/fs/ocfs2/xattr.c
> @@ -4795,19 +4795,33 @@ static int ocfs2_xattr_bucket_set_value_outside(struct inode *inode,
> char *val,
> int value_len)
> {
> - int offset;
> + 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));
>
> - offset = le16_to_cpu(xe->xe_name_offset) +
> - OCFS2_XATTR_SIZE(xe->xe_name_len);
> + ret = ocfs2_xattr_bucket_get_name_value(inode, xh,
> + xe - xh->xh_entries,
> + &block_off,
> + &offset);
> + if (ret) {
> + mlog_errno(ret);
> + goto out;
> + }
>
> - xv = (struct ocfs2_xattr_value_root *)(xs->base + offset);
> + base = bucket_block(xs->bucket, block_off);
> + xv = (struct ocfs2_xattr_value_root *)(base + offset +
> + OCFS2_XATTR_SIZE(xe->xe_name_len));
>
> - return __ocfs2_xattr_set_value_outside(inode, handle,
> - xv, val, value_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,
> --
> 1.6.2.rc2.16.gf474c
>
>
> _______________________________________________
> Ocfs2-devel mailing list
> Ocfs2-devel at oss.oracle.com
> http://oss.oracle.com/mailman/listinfo/ocfs2-devel
--
Life's Little Instruction Book #356
"Be there when people need you."
Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-03-12 10:18 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-03-09 23:49 [Ocfs2-devel] [PATCH] ocfs2: Use xs->bucket to set xattr value outside Tao Ma
2009-03-10 9:30 ` tristan.ye
2009-03-12 2:28 ` tristan.ye
2009-03-12 9:29 ` Joel Becker
2009-03-12 0:37 ` [Ocfs2-devel] [PATCH] ocfs2: Use xs->bucket to set xattr value outside.v2 Tao Ma
2009-03-12 10:18 ` Joel Becker
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.