[PATCH 06/21] ocfs2: drop vmtruncate

[PATCH 06/21] ocfs2: drop vmtruncate

[Marco Stornelli]

Removed vmtruncate.

Signed-off-by: Marco Stornelli <marco.stornelli@gmail.com>
---
 fs/ocfs2/file.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
index 46a1f6d..dd95798 100644
--- a/fs/ocfs2/file.c
+++ b/fs/ocfs2/file.c
@@ -1231,11 +1231,12 @@ int ocfs2_setattr(struct dentry *dentry, struct iattr *attr)
 	 */
 	if ((attr->ia_valid & ATTR_SIZE) &&
 	    attr->ia_size != i_size_read(inode)) {
-		status = vmtruncate(inode, attr->ia_size);
+		status = inode_newsize_ok(inode, attr->ia_size);
 		if (status) {
 			mlog_errno(status);
 			goto bail_commit;
 		}
+		truncate_setsize(inode, attr->ia_size);
 	}
 
 	setattr_copy(inode, attr);
-- 
1.7.3.4

[PATCH 06/21] ocfs2: drop vmtruncate

[Marco Stornelli]

Removed vmtruncate

Signed-off-by: Marco Stornelli <marco.stornelli@gmail.com>
---
 fs/ocfs2/file.c |   18 ------------------
 1 files changed, 0 insertions(+), 18 deletions(-)

diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
index dda0898..9366d32 100644
--- a/fs/ocfs2/file.c
+++ b/fs/ocfs2/file.c
@@ -1218,24 +1218,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

[PATCH 06/21] ocfs2: drop vmtruncate

[Marco Stornelli]

Removed vmtruncate

Signed-off-by: Marco Stornelli <marco.stornelli@gmail.com>
---
 fs/ocfs2/file.c |   18 ------------------
 1 files changed, 0 insertions(+), 18 deletions(-)

diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
index 5a4ee77..03f0d08 100644
--- a/fs/ocfs2/file.c
+++ b/fs/ocfs2/file.c
@@ -1218,24 +1218,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

Re: [PATCH 06/21] ocfs2: drop vmtruncate

[Marco Stornelli]

2012/10/25 Joel Becker <jlbec@evilplan.org>:
> On Tue, Oct 23, 2012 at 02:48:38PM +0200, Marco Stornelli wrote:
>> 2012/10/23 Joel Becker <jlbec@evilplan.org>:
>> > On Tue, Oct 23, 2012 at 10:58:42AM +0200, Marco Stornelli wrote:
>> >> 2012/10/23 Joel Becker <jlbec@evilplan.org>:
>> >> > On Sat, Oct 20, 2012 at 02:19:00PM +0200, Marco Stornelli wrote:
>> >> >> Removed vmtruncate
>> >> >>
>> >> >> Signed-off-by: Marco Stornelli <marco.stornelli@gmail.com>
>> >> >
>> >> > Acked-by: Joel Becker <jlbec@evilplan.org>
>> >> >
>> >> > 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

Re: [PATCH 06/21] ocfs2: drop vmtruncate

[Joel Becker]

On Tue, Oct 23, 2012 at 02:48:38PM +0200, Marco Stornelli wrote:
> 2012/10/23 Joel Becker <jlbec@evilplan.org>:
> > On Tue, Oct 23, 2012 at 10:58:42AM +0200, Marco Stornelli wrote:
> >> 2012/10/23 Joel Becker <jlbec@evilplan.org>:
> >> > On Sat, Oct 20, 2012 at 02:19:00PM +0200, Marco Stornelli wrote:
> >> >> Removed vmtruncate
> >> >>
> >> >> Signed-off-by: Marco Stornelli <marco.stornelli@gmail.com>
> >> >
> >> > Acked-by: Joel Becker <jlbec@evilplan.org>
> >> >
> >> > 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 majordomo@vger.kernel.org
> 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/
			jlbec@evilplan.org

Re: [PATCH 06/21] ocfs2: drop vmtruncate

[Joel Becker]

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 <marco.stornelli@gmail.com>
> > 
> > Acked-by: Joel Becker <jlbec@evilplan.org>
> > 
> > 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/
			jlbec@evilplan.org

Re: [PATCH 06/21] ocfs2: drop vmtruncate

[Christoph Hellwig]

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 <marco.stornelli@gmail.com>
> 
> Acked-by: Joel Becker <jlbec@evilplan.org>
> 
> 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.

Re: [PATCH 06/21] ocfs2: drop vmtruncate

[Marco Stornelli]

2012/10/23 Joel Becker <jlbec@evilplan.org>:
> On Tue, Oct 23, 2012 at 10:58:42AM +0200, Marco Stornelli wrote:
>> 2012/10/23 Joel Becker <jlbec@evilplan.org>:
>> > On Sat, Oct 20, 2012 at 02:19:00PM +0200, Marco Stornelli wrote:
>> >> Removed vmtruncate
>> >>
>> >> Signed-off-by: Marco Stornelli <marco.stornelli@gmail.com>
>> >
>> > Acked-by: Joel Becker <jlbec@evilplan.org>
>> >
>> > 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

Re: [PATCH 06/21] ocfs2: drop vmtruncate

[Joel Becker]

On Tue, Oct 23, 2012 at 10:58:42AM +0200, Marco Stornelli wrote:
> 2012/10/23 Joel Becker <jlbec@evilplan.org>:
> > On Sat, Oct 20, 2012 at 02:19:00PM +0200, Marco Stornelli wrote:
> >> Removed vmtruncate
> >>
> >> Signed-off-by: Marco Stornelli <marco.stornelli@gmail.com>
> >
> > Acked-by: Joel Becker <jlbec@evilplan.org>
> >
> > 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 majordomo@vger.kernel.org
> 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/
			jlbec@evilplan.org

Re: [PATCH 06/21] ocfs2: drop vmtruncate

[Marco Stornelli]

2012/10/23 Joel Becker <jlbec@evilplan.org>:
> On Sat, Oct 20, 2012 at 02:19:00PM +0200, Marco Stornelli wrote:
>> Removed vmtruncate
>>
>> Signed-off-by: Marco Stornelli <marco.stornelli@gmail.com>
>
> Acked-by: Joel Becker <jlbec@evilplan.org>
>
> 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

Re: [PATCH 06/21] ocfs2: drop vmtruncate

[Joel Becker]

On Sat, Oct 20, 2012 at 02:19:00PM +0200, Marco Stornelli wrote:
> Removed vmtruncate
> 
> Signed-off-by: Marco Stornelli <marco.stornelli@gmail.com>

Acked-by: Joel Becker <jlbec@evilplan.org>

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/
			jlbec@evilplan.org

[PATCH 06/21] ocfs2: drop vmtruncate

[Marco Stornelli]

Removed vmtruncate

Signed-off-by: Marco Stornelli <marco.stornelli@gmail.com>
---
 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