Skip to contentSkip to conversation

procfs: stop using vmtruncate

PATCH In review Christoph Hellwig 2 participants 3 messages Jun 20, 2011 – Jun 21, 2011

Subsystems (3)
VFS Proc filesystem Proc sysctl
lore
Share & tools
lore

Christoph Hellwig

author
#0 lore raw

PATCH 1/1 procfs: stop using vmtruncate

Procfs doens't have any ->truncate instances, so all vmtruncate calls can be replaced with truncate_setsize, given that we've just done the inode_newsize_ok check as part of inode_change_ok just before.

Signed-off-by Christoph Hellwig <[email protected]>
fs/proc/base.c +2 -6
Index: linux-2.6/fs/proc/base.c
===================================================================
@@ -615,12 +615,8 @@ int proc_setattr(struct dentry *dentry,
 	if (error)
 		return error;
 
-	if ((attr->ia_valid & ATTR_SIZE) &&
-	    attr->ia_size != i_size_read(inode)) {
-		error = vmtruncate(inode, attr->ia_size);
-		if (error)
-			return error;
-	}
+	if ((attr->ia_valid & ATTR_SIZE) && attr->ia_size != i_size_read(inode))
+		truncate_setsize(inode, attr->ia_size);
 
 	setattr_copy(inode, attr);
 	mark_inode_dirty(inode);
fs/proc/generic.c +5 -8
Index: linux-2.6/fs/proc/generic.c
===================================================================
@@ -251,7 +251,7 @@ static const struct file_operations proc
 	.write		= proc_file_write,
 };
 
-static int proc_notify_change(struct dentry *dentry, struct iattr *iattr)
+static int proc_file_setattr(struct dentry *dentry, struct iattr *iattr)
 {
 	struct inode *inode = dentry->d_inode;
 	struct proc_dir_entry *de = PDE(inode);

@@ -262,11 +262,8 @@ static int proc_notify_change(struct den
 		return error;
 
 	if ((iattr->ia_valid & ATTR_SIZE) &&
-	    iattr->ia_size != i_size_read(inode)) {
-		error = vmtruncate(inode, iattr->ia_size);
-		if (error)
-			return error;
-	}
+	    iattr->ia_size != i_size_read(inode))
+		truncate_setsize(inode, iattr->ia_size);
 
 	setattr_copy(inode, iattr);
 	mark_inode_dirty(inode);

@@ -290,7 +287,7 @@ static int proc_getattr(struct vfsmount
 }
 
 static const struct inode_operations proc_file_inode_operations = {
-	.setattr	= proc_notify_change,
+	.setattr	= proc_file_setattr,
 };
 
 /*

@@ -549,7 +546,7 @@ static const struct file_operations proc
 static const struct inode_operations proc_dir_inode_operations = {
 	.lookup		= proc_lookup,
 	.getattr	= proc_getattr,
-	.setattr	= proc_notify_change,
+	.setattr	= proc_file_setattr,
 };
 
 static int proc_register(struct proc_dir_entry * dir, struct proc_dir_entry * dp)
fs/proc/proc_sysctl.c +2 -6
Index: linux-2.6/fs/proc/proc_sysctl.c
===================================================================
@@ -337,12 +337,8 @@ static int proc_sys_setattr(struct dentr
 	if (error)
 		return error;
 
-	if ((attr->ia_valid & ATTR_SIZE) &&
-	    attr->ia_size != i_size_read(inode)) {
-		error = vmtruncate(inode, attr->ia_size);
-		if (error)
-			return error;
-	}
+	if ((attr->ia_valid & ATTR_SIZE) && attr->ia_size != i_size_read(inode))
+		truncate_setsize(inode, attr->ia_size);
 
 	setattr_copy(inode, attr);
 	mark_inode_dirty(inode);
maintainer
#1 lore raw
Quoted 3 lines · Christoph Hellwig

Procfs doens't have any ->truncate instances, so all vmtruncate calls can be replaced with truncate_setsize, given that we've just done the inode_newsize_ok check as part of inode_change_ok just before.

What files on procfs allow meaningful truncate anyway? I agree that vmtruncate() there is bogus, but AFAICT we have the following picture:
* there are files with non-zero i_size (/proc/kcore and
/proc/bus/pci/*/*); for any of those ATTR_SIZE should fail and I don't think it should fail silently.
* the rest has size 0 and for those ATTR_SIZE to non-0 length
should definitely fail with an error.

author
#2 lore raw
Quoted 5 lines · Al Viro
Quoted 3 lines · Christoph Hellwig

Procfs doens't have any ->truncate instances, so all vmtruncate calls can be replaced with truncate_setsize, given that we've just done the inode_newsize_ok check as part of inode_change_ok just before.

Quoted 7 lines

What files on procfs allow meaningful truncate anyway? I agree that vmtruncate() there is bogus, but AFAICT we have the following picture:
* there are files with non-zero i_size (/proc/kcore and
/proc/bus/pci/*/*); for any of those ATTR_SIZE should fail and I don't think it should fail silently.
* the rest has size 0 and for those ATTR_SIZE to non-0 length
should definitely fail with an error.

True. We probably should simply refuse any ATTR_SIZE calls.