From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dave Chinner Subject: Re: [PATCH v2] fs: Add new pre-allocation ioctls to vfs for compatibility with legacy xfs ioctls Date: Fri, 19 Dec 2008 15:37:44 +1100 Message-ID: <20081219043744.GC17177@disturbed> References: <494ABEBC.8060101@ankitjain.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: Al Viro , mfasheh@suse.com, linux-kernel@vger.kernel.org, joel.becker@oracle.com, Christoph Hellwig , xfs-masters@oss.sgi.com, linux-fsdevel@vger.kernel.org, xfs@oss.sgi.com, ocfs2-devel@oss.oracle.com To: Ankit Jain Return-path: Received: from ipmail05.adl2.internode.on.net ([203.16.214.145]:3783 "EHLO ipmail05.adl2.internode.on.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752290AbYLSEhv (ORCPT ); Thu, 18 Dec 2008 23:37:51 -0500 Content-Disposition: inline In-Reply-To: <494ABEBC.8060101@ankitjain.org> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Fri, Dec 19, 2008 at 02:51:00AM +0530, Ankit Jain wrote: > This patch adds ioctls to vfs for compatibility with legacy XFS > pre-allocation ioctls (XFS_IOC_*RESVP*). The implementation > effectively invokes sys_fallocate for the new ioctls. > Note: These legacy ioctls are also implemented by OCFS2. >=20 > Changes in v2: > - Dropped the incorrect handling of *UNRESVSP* ioctl. > - Made the ioctl and argument structure kernel only (__KERNEL__) >=20 > Signed-off-by: Ankit Jain =2E.... > @@ -361,6 +393,9 @@ static int file_ioctl(struct file *filp, unsigned= int cmd, > return put_user(inode->i_sb->s_blocksize, p); > case FIONREAD: > return put_user(i_size_read(inode) - filp->f_pos, p); > + case F_IOC_RESVSP: > + case F_IOC_RESVSP64: > + return ioctl_preallocate(filp, arg); > } > =20 > return vfs_ioctl(filp, cmd, arg); Adding this here breaks XFS_IOC_RESVSP in subtle and interesting ways. XFS_IOC_RESVSP supports invisible I/O, and that means XFS_IOC_RESVSP needs to be passed through to vfs_ioctl() to vector to XFS to handle. This happen=D1=95 because: > +struct space_resv { > + __s16 l_type; > + __s16 l_whence; > + __s64 l_start; > + __s64 l_len; /* len =3D=3D 0 means until end of file */ > + __s32 l_sysid; > + __u32 l_pid; > + __s32 l_pad[4]; /* reserve area */ > +}; > + > +#define F_IOC_RESVSP _IOW('X', 40, struct space_resv) > +#define F_IOC_RESVSP64 _IOW('X', 42, struct space_resv) Is the same as: #define XFS_IOC_RESVSP _IOW ('X', 40, struct xfs_flock64) #define XFS_IOC_RESVSP64 _IOW ('X', 42, struct xfs_flock64) because: typedef struct xfs_flock64 { __s16 l_type; __s16 l_whence; __s64 l_start; __s64 l_len; /* len =3D=3D 0 means until end= of file */ __s32 l_sysid; __u32 l_pid; __s32 l_pad[4]; /* reserve area = */ } xfs_flock64_t; is the same size as struct space_resv. Hence existing calls to XFS_IOC_RESVSP will now vector incorrectly down the fallocate path resulting in invisible operations will now be visible.... Cheers, Dave. --=20 Dave Chinner david@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel= " in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html