Messages in this thread | | | Date | Tue, 7 Jul 2009 10:58:20 -0400 | From | Christoph Hellwig <> | Subject | Re: [rfc][patch 3/4] fs: new truncate sequence |
| |
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.
| |