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 from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
