2012-10-20 12:25:40

by Marco Stornelli

[permalink] [raw]
Subject: [PATCH 06/21] ocfs2: drop vmtruncate

Removed vmtruncate

Signed-off-by: Marco Stornelli <[email protected]>
---
fs/ocfs2/file.c | 19 +------------------
1 files changed, 1 insertions(+), 18 deletions(-)

diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
index 5a4ee77..eb16e44 100644
--- a/fs/ocfs2/file.c
+++ b/fs/ocfs2/file.c
@@ -1172,6 +1172,7 @@ int ocfs2_setattr(struct dentry *dentry, struct iattr *attr)
status = -ENOSPC;
goto bail_unlock;
}
+ truncate_setsize(inode, attr->ia_size);
}

if ((attr->ia_valid & ATTR_UID && attr->ia_uid != inode->i_uid) ||
@@ -1218,24 +1219,6 @@ int ocfs2_setattr(struct dentry *dentry, struct iattr *attr)
}
}

- /*
- * This will intentionally not wind up calling truncate_setsize(),
- * since all the work for a size change has been done above.
- * Otherwise, we could get into problems with truncate as
- * ip_alloc_sem is used there to protect against i_size
- * changes.
- *
- * XXX: this means the conditional below can probably be removed.
- */
- if ((attr->ia_valid & ATTR_SIZE) &&
- attr->ia_size != i_size_read(inode)) {
- status = vmtruncate(inode, attr->ia_size);
- if (status) {
- mlog_errno(status);
- goto bail_commit;
- }
- }
-
setattr_copy(inode, attr);
mark_inode_dirty(inode);

--
1.7.3.4


2012-10-23 08:54:56

by Joel Becker

[permalink] [raw]
Subject: Re: [PATCH 06/21] ocfs2: drop vmtruncate

On Sat, Oct 20, 2012 at 02:19:00PM +0200, Marco Stornelli wrote:
> Removed vmtruncate
>
> Signed-off-by: Marco Stornelli <[email protected]>

Acked-by: Joel Becker <[email protected]>

Do you want me to pull this, or are you going to send it with your set?

Joel

> ---
> fs/ocfs2/file.c | 19 +------------------
> 1 files changed, 1 insertions(+), 18 deletions(-)
>
> diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
> index 5a4ee77..eb16e44 100644
> --- a/fs/ocfs2/file.c
> +++ b/fs/ocfs2/file.c
> @@ -1172,6 +1172,7 @@ int ocfs2_setattr(struct dentry *dentry, struct iattr *attr)
> status = -ENOSPC;
> goto bail_unlock;
> }
> + truncate_setsize(inode, attr->ia_size);
> }
>
> if ((attr->ia_valid & ATTR_UID && attr->ia_uid != inode->i_uid) ||
> @@ -1218,24 +1219,6 @@ int ocfs2_setattr(struct dentry *dentry, struct iattr *attr)
> }
> }
>
> - /*
> - * This will intentionally not wind up calling truncate_setsize(),
> - * since all the work for a size change has been done above.
> - * Otherwise, we could get into problems with truncate as
> - * ip_alloc_sem is used there to protect against i_size
> - * changes.
> - *
> - * XXX: this means the conditional below can probably be removed.
> - */
> - if ((attr->ia_valid & ATTR_SIZE) &&
> - attr->ia_size != i_size_read(inode)) {
> - status = vmtruncate(inode, attr->ia_size);
> - if (status) {
> - mlog_errno(status);
> - goto bail_commit;
> - }
> - }
> -
> setattr_copy(inode, attr);
> mark_inode_dirty(inode);
>
> --
> 1.7.3.4

--

"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

http://www.jlbec.org/
[email protected]

2012-10-23 08:58:45

by Marco Stornelli

[permalink] [raw]
Subject: Re: [PATCH 06/21] ocfs2: drop vmtruncate

2012/10/23 Joel Becker <[email protected]>:
> On Sat, Oct 20, 2012 at 02:19:00PM +0200, Marco Stornelli wrote:
>> Removed vmtruncate
>>
>> Signed-off-by: Marco Stornelli <[email protected]>
>
> Acked-by: Joel Becker <[email protected]>
>
> Do you want me to pull this, or are you going to send it with your set?
>
> Joel
>

I'd prefer to push all via Al's tree since there is a VFS change.

Marco

2012-10-23 09:02:39

by Joel Becker

[permalink] [raw]
Subject: Re: [PATCH 06/21] ocfs2: drop vmtruncate

On Tue, Oct 23, 2012 at 10:58:42AM +0200, Marco Stornelli wrote:
> 2012/10/23 Joel Becker <[email protected]>:
> > On Sat, Oct 20, 2012 at 02:19:00PM +0200, Marco Stornelli wrote:
> >> Removed vmtruncate
> >>
> >> Signed-off-by: Marco Stornelli <[email protected]>
> >
> > Acked-by: Joel Becker <[email protected]>
> >
> > Do you want me to pull this, or are you going to send it with your set?
> >
> > Joel
> >
>
> I'd prefer to push all via Al's tree since there is a VFS change.

SGTM. Thanks!

Joel

>
> Marco
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

--

"In the long run...we'll all be dead."
-Unknown

http://www.jlbec.org/
[email protected]

2012-10-23 12:48:41

by Marco Stornelli

[permalink] [raw]
Subject: Re: [PATCH 06/21] ocfs2: drop vmtruncate

2012/10/23 Joel Becker <[email protected]>:
> On Tue, Oct 23, 2012 at 10:58:42AM +0200, Marco Stornelli wrote:
>> 2012/10/23 Joel Becker <[email protected]>:
>> > On Sat, Oct 20, 2012 at 02:19:00PM +0200, Marco Stornelli wrote:
>> >> Removed vmtruncate
>> >>
>> >> Signed-off-by: Marco Stornelli <[email protected]>
>> >
>> > Acked-by: Joel Becker <[email protected]>
>> >
>> > Do you want me to pull this, or are you going to send it with your set?
>> >
>> > Joel
>> >
>>
>> I'd prefer to push all via Al's tree since there is a VFS change.
>
> SGTM. Thanks!
>
> Joel
>

I've got a doubt and I ask to ocfs2 expert :) It seems i_size_write()
and truncate_inode_pages() and so on, they are already called in each
path of setattr for a size change, so maybe we can remove
truncate_setsize() and simply to remove the vmtuncate code, can you
give me your opinion?
Thanks.

Marco

2012-10-23 13:02:41

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 06/21] ocfs2: drop vmtruncate

On Tue, Oct 23, 2012 at 01:54:48AM -0700, Joel Becker wrote:
> On Sat, Oct 20, 2012 at 02:19:00PM +0200, Marco Stornelli wrote:
> > Removed vmtruncate
> >
> > Signed-off-by: Marco Stornelli <[email protected]>
>
> Acked-by: Joel Becker <[email protected]>
>
> Do you want me to pull this, or are you going to send it with your set?

Is this really correct? It now updates i_size before starting a
transaction.

2012-10-25 00:17:40

by Joel Becker

[permalink] [raw]
Subject: Re: [PATCH 06/21] ocfs2: drop vmtruncate

On Tue, Oct 23, 2012 at 09:02:38AM -0400, Christoph Hellwig wrote:
> On Tue, Oct 23, 2012 at 01:54:48AM -0700, Joel Becker wrote:
> > On Sat, Oct 20, 2012 at 02:19:00PM +0200, Marco Stornelli wrote:
> > > Removed vmtruncate
> > >
> > > Signed-off-by: Marco Stornelli <[email protected]>
> >
> > Acked-by: Joel Becker <[email protected]>
> >
> > Do you want me to pull this, or are you going to send it with your set?
>
> Is this really correct? It now updates i_size before starting a
> transaction.

Ok, I missed that, and I need to re-check.

Joel

--

"If at first you don't succeed, cover all traces that you tried."
-Unknown

http://www.jlbec.org/
[email protected]

2012-10-25 00:18:30

by Joel Becker

[permalink] [raw]
Subject: Re: [PATCH 06/21] ocfs2: drop vmtruncate

On Tue, Oct 23, 2012 at 02:48:38PM +0200, Marco Stornelli wrote:
> 2012/10/23 Joel Becker <[email protected]>:
> > On Tue, Oct 23, 2012 at 10:58:42AM +0200, Marco Stornelli wrote:
> >> 2012/10/23 Joel Becker <[email protected]>:
> >> > On Sat, Oct 20, 2012 at 02:19:00PM +0200, Marco Stornelli wrote:
> >> >> Removed vmtruncate
> >> >>
> >> >> Signed-off-by: Marco Stornelli <[email protected]>
> >> >
> >> > Acked-by: Joel Becker <[email protected]>
> >> >
> >> > Do you want me to pull this, or are you going to send it with your set?
> >> >
> >> > Joel
> >> >
> >>
> >> I'd prefer to push all via Al's tree since there is a VFS change.
> >
> > SGTM. Thanks!
> >
> > Joel
> >
>
> I've got a doubt and I ask to ocfs2 expert :) It seems i_size_write()
> and truncate_inode_pages() and so on, they are already called in each
> path of setattr for a size change, so maybe we can remove
> truncate_setsize() and simply to remove the vmtuncate code, can you
> give me your opinion?

Actually, I have to revisit this, because Chrisoph points out
your adjusting i_size outside of a transaction.

Joel

> Thanks.
>
> Marco
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

--

"Practice random acts of kindness and senseless acts of beauty."

Oh, and don't forget where your towel is.

http://www.jlbec.org/
[email protected]

2012-10-25 06:37:49

by Marco Stornelli

[permalink] [raw]
Subject: Re: [PATCH 06/21] ocfs2: drop vmtruncate

2012/10/25 Joel Becker <[email protected]>:
> On Tue, Oct 23, 2012 at 02:48:38PM +0200, Marco Stornelli wrote:
>> 2012/10/23 Joel Becker <[email protected]>:
>> > On Tue, Oct 23, 2012 at 10:58:42AM +0200, Marco Stornelli wrote:
>> >> 2012/10/23 Joel Becker <[email protected]>:
>> >> > On Sat, Oct 20, 2012 at 02:19:00PM +0200, Marco Stornelli wrote:
>> >> >> Removed vmtruncate
>> >> >>
>> >> >> Signed-off-by: Marco Stornelli <[email protected]>
>> >> >
>> >> > Acked-by: Joel Becker <[email protected]>
>> >> >
>> >> > Do you want me to pull this, or are you going to send it with your set?
>> >> >
>> >> > Joel
>> >> >
>> >>
>> >> I'd prefer to push all via Al's tree since there is a VFS change.
>> >
>> > SGTM. Thanks!
>> >
>> > Joel
>> >
>>
>> I've got a doubt and I ask to ocfs2 expert :) It seems i_size_write()
>> and truncate_inode_pages() and so on, they are already called in each
>> path of setattr for a size change, so maybe we can remove
>> truncate_setsize() and simply to remove the vmtuncate code, can you
>> give me your opinion?
>
> Actually, I have to revisit this, because Chrisoph points out
> your adjusting i_size outside of a transaction.
>
> Joel
>

Actually i_size should be changed in a transaction previous, the code
is not changed from this point of view. Indeed the call of
truncate_setsize() should be a nop.

Marco