On Tue, Jul 07, 2009 at 04:48:23PM +0200, Nick Piggin wrote:

Don't know whether it is a good idea to move i_alloc_sem into implementation.
Maybe it is better to leave it in the caller in this patchset?

Generally moving locks without changing prototypes can be a very subtle
way to break things.  A good option is to move the locking in a separate
patch set in a patch series or at least release if it's otherwise to
complicated.

+int simple_setsize(struct dentry *dentry, loff_t newsize,
+			unsigned flags, struct file *file)

This one could probably also use kerneldoc comment.

+{
+	struct inode *inode = dentry->d_inode;
+	loff_t oldsize;
+	int error;
+
+	error = inode_newsize_ok(inode, newsize);
+	if (error)
+		return error;
+
+	oldsize = inode->i_size;
+	i_size_write(inode, newsize);
+	truncate_pagecache(inode, oldsize, newsize);
+
+	return error;
+}

+	if (ia_valid & ATTR_SIZE) {
+		if (inode->i_op && inode->i_op->setsize) {

inode->i_op is mandatory these days.

+			unsigned int flags = 0;
+			struct file *file = NULL;
+
+			if (ia_valid & ATTR_FILE) {
+				flags |= SETSIZE_FILE;
+				file = attr->ia_file;
+			}
+			if (ia_valid & ATTR_OPEN)
+				flags |= SETSIZE_OPEN;
+			error = inode->i_op->setsize(dentry, attr->ia_size,
+							flags, file);
+			if (error)
+				return error;

So you still pass down to ->setattr if ->setsize succeeded?  That's a
very confusing calling convention.  It also means we first do the
truncation and any following time/mode updates are in a separate
transaction which is a no-go.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>