* [Ocfs2-devel] [PATCH 1/2] ocfs2: xattr: fix inlined xattr reflink
@ 2013-06-20 6:17 Junxiao Bi
2013-06-20 6:17 ` [Ocfs2-devel] [PATCH 2/2] ocfs2: xattr: remove useless free space checking Junxiao Bi
2013-06-20 9:01 ` [Ocfs2-devel] [PATCH 1/2] ocfs2: xattr: fix inlined xattr reflink Jeff Liu
0 siblings, 2 replies; 6+ messages in thread
From: Junxiao Bi @ 2013-06-20 6:17 UTC (permalink / raw)
To: ocfs2-devel
Inlined xattr shared free space of inode block with inlined data
or data extent record, so the size of the later two should be
adjusted when inlined xattr is enabled. See ocfs2_xattr_ibody_init().
But this isn't done well when reflink. For inode with inlined data,
its max inlined data size is adjusted in ocfs2_duplicate_inline_data(),
no problem. But for inode with data extent record, its record count isn't
adjusted. Fix it, or data extent record and inlined xattr may overwrite
each other, then cause data corruption or xattr failure.
Cc: <stable@kernel.org>
Signed-off-by: Junxiao Bi <junxiao.bi@oracle.com>
---
fs/ocfs2/xattr.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index 2e3ea30..3bb0708 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -6499,6 +6499,16 @@ static int ocfs2_reflink_xattr_inline(struct ocfs2_xattr_reflink *args)
}
new_oi = OCFS2_I(args->new_inode);
+ /*
+ * Adjust extent record count to reserve space for extended attribute.
+ * Inline data count had been adjusted in ocfs2_duplicate_inline_data().
+ */
+ if (!(new_oi->ip_dyn_features & OCFS2_INLINE_DATA_FL)
+ && !(ocfs2_inode_is_fast_symlink(new_inode))) {
+ struct ocfs2_extent_list *el = &new_di->id2.i_list;
+ le16_add_cpu(&el->l_count, -(inline_size /
+ sizeof(struct ocfs2_extent_rec)));
+ }
spin_lock(&new_oi->ip_lock);
new_oi->ip_dyn_features |= OCFS2_HAS_XATTR_FL | OCFS2_INLINE_XATTR_FL;
new_di->i_dyn_features = cpu_to_le16(new_oi->ip_dyn_features);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Ocfs2-devel] [PATCH 2/2] ocfs2: xattr: remove useless free space checking
2013-06-20 6:17 [Ocfs2-devel] [PATCH 1/2] ocfs2: xattr: fix inlined xattr reflink Junxiao Bi
@ 2013-06-20 6:17 ` Junxiao Bi
2013-06-20 9:01 ` Jeff Liu
2013-06-20 9:01 ` [Ocfs2-devel] [PATCH 1/2] ocfs2: xattr: fix inlined xattr reflink Jeff Liu
1 sibling, 1 reply; 6+ messages in thread
From: Junxiao Bi @ 2013-06-20 6:17 UTC (permalink / raw)
To: ocfs2-devel
free space checking will be done in ocfs2_xattr_ibody_init().
So remove here.
Signed-off-by: Junxiao Bi <junxiao.bi@oracle.com>
---
fs/ocfs2/xattr.c | 7 -------
1 file changed, 7 deletions(-)
diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index 3bb0708..7de1a78 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -2759,13 +2759,6 @@ static int ocfs2_xattr_ibody_set(struct inode *inode,
down_write(&oi->ip_alloc_sem);
if (!(oi->ip_dyn_features & OCFS2_INLINE_XATTR_FL)) {
- if (!ocfs2_xattr_has_space_inline(inode, di)) {
- ret = -ENOSPC;
- goto out;
- }
- }
-
- if (!(oi->ip_dyn_features & OCFS2_INLINE_XATTR_FL)) {
ret = ocfs2_xattr_ibody_init(inode, xs->inode_bh, ctxt);
if (ret) {
if (ret != -ENOSPC)
--
1.7.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Ocfs2-devel] [PATCH 2/2] ocfs2: xattr: remove useless free space checking
2013-06-20 6:17 ` [Ocfs2-devel] [PATCH 2/2] ocfs2: xattr: remove useless free space checking Junxiao Bi
@ 2013-06-20 9:01 ` Jeff Liu
2013-07-01 22:44 ` Joel Becker
0 siblings, 1 reply; 6+ messages in thread
From: Jeff Liu @ 2013-06-20 9:01 UTC (permalink / raw)
To: ocfs2-devel
On 06/20/2013 02:17 PM, Junxiao Bi wrote:
> free space checking will be done in ocfs2_xattr_ibody_init().
> So remove here.
>
> Signed-off-by: Junxiao Bi <junxiao.bi@oracle.com>
Reviewed-by: Jie Liu <jeff.liu@oracle.com>
> ---
> fs/ocfs2/xattr.c | 7 -------
> 1 file changed, 7 deletions(-)
>
> diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
> index 3bb0708..7de1a78 100644
> --- a/fs/ocfs2/xattr.c
> +++ b/fs/ocfs2/xattr.c
> @@ -2759,13 +2759,6 @@ static int ocfs2_xattr_ibody_set(struct inode *inode,
>
> down_write(&oi->ip_alloc_sem);
> if (!(oi->ip_dyn_features & OCFS2_INLINE_XATTR_FL)) {
> - if (!ocfs2_xattr_has_space_inline(inode, di)) {
> - ret = -ENOSPC;
> - goto out;
> - }
> - }
> -
> - if (!(oi->ip_dyn_features & OCFS2_INLINE_XATTR_FL)) {
> ret = ocfs2_xattr_ibody_init(inode, xs->inode_bh, ctxt);
> if (ret) {
> if (ret != -ENOSPC)
^ permalink raw reply [flat|nested] 6+ messages in thread* [Ocfs2-devel] [PATCH 2/2] ocfs2: xattr: remove useless free space checking
2013-06-20 9:01 ` Jeff Liu
@ 2013-07-01 22:44 ` Joel Becker
0 siblings, 0 replies; 6+ messages in thread
From: Joel Becker @ 2013-07-01 22:44 UTC (permalink / raw)
To: ocfs2-devel
On Thu, Jun 20, 2013 at 05:01:25PM +0800, Jeff Liu wrote:
> On 06/20/2013 02:17 PM, Junxiao Bi wrote:
>
> > free space checking will be done in ocfs2_xattr_ibody_init().
> > So remove here.
> >
> > Signed-off-by: Junxiao Bi <junxiao.bi@oracle.com>
>
> Reviewed-by: Jie Liu <jeff.liu@oracle.com>
Acked-by: Joel Becker <jlbec@evilplan.org>
>
> > ---
> > fs/ocfs2/xattr.c | 7 -------
> > 1 file changed, 7 deletions(-)
> >
> > diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
> > index 3bb0708..7de1a78 100644
> > --- a/fs/ocfs2/xattr.c
> > +++ b/fs/ocfs2/xattr.c
> > @@ -2759,13 +2759,6 @@ static int ocfs2_xattr_ibody_set(struct inode *inode,
> >
> > down_write(&oi->ip_alloc_sem);
> > if (!(oi->ip_dyn_features & OCFS2_INLINE_XATTR_FL)) {
> > - if (!ocfs2_xattr_has_space_inline(inode, di)) {
> > - ret = -ENOSPC;
> > - goto out;
> > - }
> > - }
> > -
> > - if (!(oi->ip_dyn_features & OCFS2_INLINE_XATTR_FL)) {
> > ret = ocfs2_xattr_ibody_init(inode, xs->inode_bh, ctxt);
> > if (ret) {
> > if (ret != -ENOSPC)
>
>
>
> _______________________________________________
> Ocfs2-devel mailing list
> Ocfs2-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/ocfs2-devel
--
Brain: I shall pollute the water supply with this DNAdefibuliser,
turning everyone into mindless slaves.
Pinky: What about the people who drink bottled water?
Brain: Pinky, people who pay 5 dollars for a bottle of water are
already mindless slaves.
http://www.jlbec.org/
jlbec at evilplan.org
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Ocfs2-devel] [PATCH 1/2] ocfs2: xattr: fix inlined xattr reflink
2013-06-20 6:17 [Ocfs2-devel] [PATCH 1/2] ocfs2: xattr: fix inlined xattr reflink Junxiao Bi
2013-06-20 6:17 ` [Ocfs2-devel] [PATCH 2/2] ocfs2: xattr: remove useless free space checking Junxiao Bi
@ 2013-06-20 9:01 ` Jeff Liu
2013-06-20 9:15 ` Junxiao Bi
1 sibling, 1 reply; 6+ messages in thread
From: Jeff Liu @ 2013-06-20 9:01 UTC (permalink / raw)
To: ocfs2-devel
Hi Junxiao,
On 06/20/2013 02:17 PM, Junxiao Bi wrote:
> Inlined xattr shared free space of inode block with inlined data
> or data extent record, so the size of the later two should be
> adjusted when inlined xattr is enabled. See ocfs2_xattr_ibody_init().
> But this isn't done well when reflink. For inode with inlined data,
> its max inlined data size is adjusted in ocfs2_duplicate_inline_data(),
> no problem. But for inode with data extent record, its record count isn't
> adjusted. Fix it, or data extent record and inlined xattr may overwrite
> each other, then cause data corruption or xattr failure.
>
> Cc: <stable@kernel.org>
> Signed-off-by: Junxiao Bi <junxiao.bi@oracle.com>
> ---
> fs/ocfs2/xattr.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
> index 2e3ea30..3bb0708 100644
> --- a/fs/ocfs2/xattr.c
> +++ b/fs/ocfs2/xattr.c
> @@ -6499,6 +6499,16 @@ static int ocfs2_reflink_xattr_inline(struct ocfs2_xattr_reflink *args)
> }
>
> new_oi = OCFS2_I(args->new_inode);
> + /*
> + * Adjust extent record count to reserve space for extended attribute.
> + * Inline data count had been adjusted in ocfs2_duplicate_inline_data().
> + */
> + if (!(new_oi->ip_dyn_features & OCFS2_INLINE_DATA_FL)
> + && !(ocfs2_inode_is_fast_symlink(new_inode))) {
A minor code style adjustment would be better as:
if (!(new_oi->ip_dyn_features & OCFS2_INLINE_DATA_FL) &&
!(ocfs2_inode_.....)) {
Otherwise this fix looks fine.
> + struct ocfs2_extent_list *el = &new_di->id2.i_list;
> + le16_add_cpu(&el->l_count, -(inline_size /
> + sizeof(struct ocfs2_extent_rec)));
> + }
> spin_lock(&new_oi->ip_lock);
> new_oi->ip_dyn_features |= OCFS2_HAS_XATTR_FL | OCFS2_INLINE_XATTR_FL;
> new_di->i_dyn_features = cpu_to_le16(new_oi->ip_dyn_features);
Thanks,
-Jeff
^ permalink raw reply [flat|nested] 6+ messages in thread* [Ocfs2-devel] [PATCH 1/2] ocfs2: xattr: fix inlined xattr reflink
2013-06-20 9:01 ` [Ocfs2-devel] [PATCH 1/2] ocfs2: xattr: fix inlined xattr reflink Jeff Liu
@ 2013-06-20 9:15 ` Junxiao Bi
0 siblings, 0 replies; 6+ messages in thread
From: Junxiao Bi @ 2013-06-20 9:15 UTC (permalink / raw)
To: ocfs2-devel
Hi Jeff,
On 06/20/2013 05:01 PM, Jeff Liu wrote:
> Hi Junxiao,
>
> On 06/20/2013 02:17 PM, Junxiao Bi wrote:
>
>> Inlined xattr shared free space of inode block with inlined data
>> or data extent record, so the size of the later two should be
>> adjusted when inlined xattr is enabled. See ocfs2_xattr_ibody_init().
>> But this isn't done well when reflink. For inode with inlined data,
>> its max inlined data size is adjusted in ocfs2_duplicate_inline_data(),
>> no problem. But for inode with data extent record, its record count isn't
>> adjusted. Fix it, or data extent record and inlined xattr may overwrite
>> each other, then cause data corruption or xattr failure.
>>
>> Cc: <stable@kernel.org>
>> Signed-off-by: Junxiao Bi <junxiao.bi@oracle.com>
>> ---
>> fs/ocfs2/xattr.c | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
>> index 2e3ea30..3bb0708 100644
>> --- a/fs/ocfs2/xattr.c
>> +++ b/fs/ocfs2/xattr.c
>> @@ -6499,6 +6499,16 @@ static int ocfs2_reflink_xattr_inline(struct ocfs2_xattr_reflink *args)
>> }
>>
>> new_oi = OCFS2_I(args->new_inode);
>> + /*
>> + * Adjust extent record count to reserve space for extended attribute.
>> + * Inline data count had been adjusted in ocfs2_duplicate_inline_data().
>> + */
>> + if (!(new_oi->ip_dyn_features & OCFS2_INLINE_DATA_FL)
>> + && !(ocfs2_inode_is_fast_symlink(new_inode))) {
> A minor code style adjustment would be better as:
> if (!(new_oi->ip_dyn_features & OCFS2_INLINE_DATA_FL) &&
> !(ocfs2_inode_.....)) {
Thanks for review it, will make the change in V2.
>
> Otherwise this fix looks fine.
>
>> + struct ocfs2_extent_list *el = &new_di->id2.i_list;
>> + le16_add_cpu(&el->l_count, -(inline_size /
>> + sizeof(struct ocfs2_extent_rec)));
>> + }
>> spin_lock(&new_oi->ip_lock);
>> new_oi->ip_dyn_features |= OCFS2_HAS_XATTR_FL | OCFS2_INLINE_XATTR_FL;
>> new_di->i_dyn_features = cpu_to_le16(new_oi->ip_dyn_features);
>
> Thanks,
> -Jeff
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-07-01 22:44 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-06-20 6:17 [Ocfs2-devel] [PATCH 1/2] ocfs2: xattr: fix inlined xattr reflink Junxiao Bi
2013-06-20 6:17 ` [Ocfs2-devel] [PATCH 2/2] ocfs2: xattr: remove useless free space checking Junxiao Bi
2013-06-20 9:01 ` Jeff Liu
2013-07-01 22:44 ` Joel Becker
2013-06-20 9:01 ` [Ocfs2-devel] [PATCH 1/2] ocfs2: xattr: fix inlined xattr reflink Jeff Liu
2013-06-20 9:15 ` Junxiao Bi
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.