ocfs2: drop vmtruncate

PATCH In review Marco Stornelli 3 participants 9 messages Oct 20, 2012 – Oct 25, 2012

Subsystems (2)

VFS OCFS2

lore

Share & tools

lore

Marco Stornelli

author

2012-10-20 12:19 UTC #0 lore raw

PATCH 6/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(-)

fs/ocfs2/file.c +1 ~~-18~~

index 5a4ee77..eb16e44 100644
@@ -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);

Signature · 1 line

1.7.3.4

Joel Becker

maintainer Acked-by on v1 6/21

2012-10-23 08:54 UTC #1 lore raw

Quoted 3 lines · Marco Stornelli

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

Quoted 43 lines

---
 fs/ocfs2/file.c |   19 +------------------
 1 files changed, 1 insertions(+), 18 deletions(-)

fs/ocfs2/file.c +1 ~~-18~~

index 5a4ee77..eb16e44 100644
@@ -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);

Signature · 1 line

1.7.3.4

Signature · 6 lines

"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]

Marco Stornelli

author

2012-10-23 08:58 UTC #2 lore raw

2012/10/23 Joel Becker <[email protected]>:

Quoted 11 lines · Marco Stornelli

Quoted 3 lines
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

Joel Becker

maintainer

2012-10-23 09:02 UTC #3 lore raw

Quoted 6 lines · Marco Stornelli

2012/10/23 Joel Becker <[email protected]>:
Quoted 5 lines · Marco Stornelli
Quoted 3 lines
Removed vmtruncate
Signed-off-by Marco Stornelli <[email protected]>

Quoted 8 lines

Quoted 6 lines
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

Quoted 7 lines

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/

Signature · 5 lines

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

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

Marco Stornelli

author

2012-10-23 12:48 UTC #4 lore raw

2012/10/23 Joel Becker <[email protected]>:

Quoted 11 lines · Marco Stornelli

Quoted 10 lines
2012/10/23 Joel Becker <[email protected]>:
Quoted 9 lines · Marco Stornelli
Quoted 3 lines
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?

Quoted 9 lines

Quoted 4 lines
Quoted 2 lines
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

Christoph Hellwig

2012-10-23 13:02 UTC #7 lore raw

In reply to Joel Becker

Quoted 8 lines · Joel Becker

Quoted 3 lines · Marco Stornelli
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.

Joel Becker

maintainer

2012-10-25 00:17 UTC #8 lore raw

Quoted 4 lines · Christoph Hellwig

Quoted 3 lines · Joel Becker
Quoted 2 lines · Marco Stornelli
Removed vmtruncate

Quoted 8 lines

Quoted 5 lines
Quoted 1 line
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

Signature · 5 lines

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

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

Joel Becker

maintainer

2012-10-25 00:18 UTC #5 lore raw

In reply to Marco Stornelli

Quoted 19 lines · Marco Stornelli

2012/10/23 Joel Becker <[email protected]>:
Quoted 18 lines · Marco Stornelli
Quoted 6 lines
2012/10/23 Joel Becker <[email protected]>:
Quoted 5 lines · Marco Stornelli
Quoted 3 lines
Removed vmtruncate
Signed-off-by Marco Stornelli <[email protected]>
Quoted 8 lines
Quoted 6 lines
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!

Quoted 8 lines

Quoted 2 lines
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

Quoted 8 lines

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/

Signature · 6 lines

"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]

Marco Stornelli

author

2012-10-25 06:37 UTC #6 lore raw

2012/10/25 Joel Becker <[email protected]>:

Quoted 29 lines · Marco Stornelli

Quoted 19 lines
2012/10/23 Joel Becker <[email protected]>:
Quoted 18 lines · Marco Stornelli
Quoted 6 lines
2012/10/23 Joel Becker <[email protected]>:
Quoted 5 lines · Marco Stornelli
Quoted 3 lines
Removed vmtruncate
Signed-off-by Marco Stornelli <[email protected]>
Quoted 8 lines
Quoted 6 lines
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!
Quoted 8 lines
Quoted 2 lines
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?

Quoted 5 lines

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