Skip to contentSkip to conversation

ocfs2: drop vmtruncate

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

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 6/21 ocfs2: drop vmtruncate

Removed vmtruncate

---
 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
maintainer Acked-by on v1 6/21
#1 lore raw
Quoted 3 lines · Marco Stornelli

Removed vmtruncate

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]
maintainer
#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

Quoted 8 lines
Quoted 6 lines

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

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

maintainer
#5 lore raw
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

Quoted 8 lines
Quoted 6 lines

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

Quoted 8 lines
Quoted 6 lines

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

maintainer
#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

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]