On Fri, Apr 30, 2010 at 02:33:19PM -0700, Andrew Morton wrote: > > (Amit Arora <aarora@xxxxxxxxxx> wrote fallocate. cc added) Thanks for adding me to CC. > On Thu, 29 Apr 2010 10:14:06 +0530 > Nikanth Karthikesan <knikanth@xxxxxxx> wrote: > > > Here is an updated patch that takes the i_mutex and calls inode_newsize_ok() > > only for regular files. > > err, no. It's taking i_lock where it meant to take i_mutex. > > > Thanks > > Nikanth > > > > + if (S_ISREG(inode->i_mode)) { > > + spin_lock(&inode->i_lock); > > + ret = inode_newsize_ok(inode, (offset + len)); > > + spin_unlock(&inode->i_lock); > > + if (ret) > > + return ret; > > + } else if (S_ISDIR(inode->i_mode)) { > > + /* > > + * Let individual file system decide if it supports > > + * preallocation for directories or not. > > + */ > > + if (offset > inode->i_sb->s_maxbytes) > > + return -EFBIG; > > + } else > > + return -ENODEV; > > + > > if (!inode->i_op->fallocate) > > return -EOPNOTSUPP; > > Also, there doesn't seem to be much point in doing > > mutex_lock(i_mutex); > if (some_condition) > bale out > mutex_unlock(i_mutex); > > <stuff> > > because `some_condition' can now become true before or during the > execution of `stuff'. > > IOW, it's racy. Agreed. How about doing this check in the filesystem specific fallocate inode routines instead ? For example, in ext4 we could do : diff -Nuarp linux-2.6.org/fs/ext4/extents.c linux-2.6.new/fs/ext4/extents.c --- linux-2.6.org/fs/ext4/extents.c 2010-05-01 12:16:07.000000000 +0530 +++ linux-2.6.new/fs/ext4/extents.c 2010-05-01 12:17:37.000000000 +0530 @@ -3672,6 +3672,11 @@ long ext4_fallocate(struct inode *inode, */ credits = ext4_chunk_trans_blocks(inode, max_blocks); mutex_lock(&inode->i_mutex); + ret = inode_newsize_ok(inode, (offset + len)); + if (ret) { + mutex_unlock(&inode->i_mutex); + return ret; + } retry: while (ret >= 0 && ret < max_blocks) { block = block + ret; Similarly for ocfs2, btrfs and xfs.. -- Regards, Amit Arora -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html