From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from cuda.sgi.com (cuda3.sgi.com [192.48.176.15]) by oss.sgi.com (8.12.11.20060308/8.12.11/SuSE Linux 0.7) with ESMTP id mBJ4bqJ1015423 for ; Thu, 18 Dec 2008 22:37:53 -0600 Date: Fri, 19 Dec 2008 15:37:44 +1100 From: Dave Chinner Subject: Re: [PATCH v2] fs: Add new pre-allocation ioctls to vfs for compatibility with legacy xfs ioctls Message-ID: <20081219043744.GC17177@disturbed> References: <494ABEBC.8060101@ankitjain.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <494ABEBC.8060101@ankitjain.org> List-Id: XFS Filesystem from SGI List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Sender: xfs-bounces@oss.sgi.com Errors-To: xfs-bounces@oss.sgi.com To: Ankit Jain Cc: mfasheh@suse.com, joel.becker@oracle.com, linux-kernel@vger.kernel.org, Christoph Hellwig , xfs-masters@oss.sgi.com, Al Viro , linux-fsdevel@vger.kernel.org, xfs@oss.sgi.com, ocfs2-devel@oss.oracle.com T24gRnJpLCBEZWMgMTksIDIwMDggYXQgMDI6NTE6MDBBTSArMDUzMCwgQW5raXQgSmFpbiB3cm90 ZToKPiBUaGlzIHBhdGNoIGFkZHMgaW9jdGxzIHRvIHZmcyBmb3IgY29tcGF0aWJpbGl0eSB3aXRo IGxlZ2FjeSBYRlMKPiBwcmUtYWxsb2NhdGlvbiBpb2N0bHMgKFhGU19JT0NfKlJFU1ZQKikuIFRo ZSBpbXBsZW1lbnRhdGlvbgo+IGVmZmVjdGl2ZWx5IGludm9rZXMgc3lzX2ZhbGxvY2F0ZSBmb3Ig dGhlIG5ldyBpb2N0bHMuCj4gTm90ZTogVGhlc2UgbGVnYWN5IGlvY3RscyBhcmUgYWxzbyBpbXBs ZW1lbnRlZCBieSBPQ0ZTMi4KPiAKPiBDaGFuZ2VzIGluIHYyOgo+IC0gRHJvcHBlZCB0aGUgaW5j b3JyZWN0IGhhbmRsaW5nIG9mICpVTlJFU1ZTUCogaW9jdGwuCj4gLSBNYWRlIHRoZSBpb2N0bCBh bmQgYXJndW1lbnQgc3RydWN0dXJlIGtlcm5lbCBvbmx5IChfX0tFUk5FTF9fKQo+IAo+IFNpZ25l ZC1vZmYtYnk6IEFua2l0IEphaW4gPG1lQGFua2l0amFpbi5vcmc+Ci4uLi4uCj4gQEAgLTM2MSw2 ICszOTMsOSBAQCBzdGF0aWMgaW50IGZpbGVfaW9jdGwoc3RydWN0IGZpbGUgKmZpbHAsIHVuc2ln bmVkIGludCBjbWQsCj4gIAkJcmV0dXJuIHB1dF91c2VyKGlub2RlLT5pX3NiLT5zX2Jsb2Nrc2l6 ZSwgcCk7Cj4gIAljYXNlIEZJT05SRUFEOgo+ICAJCXJldHVybiBwdXRfdXNlcihpX3NpemVfcmVh ZChpbm9kZSkgLSBmaWxwLT5mX3BvcywgcCk7Cj4gKwljYXNlIEZfSU9DX1JFU1ZTUDoKPiArCWNh c2UgRl9JT0NfUkVTVlNQNjQ6Cj4gKwkJcmV0dXJuIGlvY3RsX3ByZWFsbG9jYXRlKGZpbHAsIGFy Zyk7Cj4gIAl9Cj4gIAo+ICAJcmV0dXJuIHZmc19pb2N0bChmaWxwLCBjbWQsIGFyZyk7CgpBZGRp bmcgdGhpcyBoZXJlIGJyZWFrcyBYRlNfSU9DX1JFU1ZTUCBpbiBzdWJ0bGUgYW5kIGludGVyZXN0 aW5nCndheXMuCgpYRlNfSU9DX1JFU1ZTUCBzdXBwb3J0cyBpbnZpc2libGUgSS9PLCBhbmQgdGhh dCBtZWFucwpYRlNfSU9DX1JFU1ZTUCBuZWVkcyB0byBiZSBwYXNzZWQgdGhyb3VnaCB0byB2ZnNf aW9jdGwoKQp0byB2ZWN0b3IgdG8gWEZTIHRvIGhhbmRsZS4KClRoaXMgaGFwcGVu0ZUgYmVjYXVz ZToKCj4gK3N0cnVjdCBzcGFjZV9yZXN2IHsKPiArCV9fczE2CQlsX3R5cGU7Cj4gKwlfX3MxNgkJ bF93aGVuY2U7Cj4gKwlfX3M2NAkJbF9zdGFydDsKPiArCV9fczY0CQlsX2xlbjsJCS8qIGxlbiA9 PSAwIG1lYW5zIHVudGlsIGVuZCBvZiBmaWxlICovCj4gKwlfX3MzMgkJbF9zeXNpZDsKPiArCV9f dTMyCQlsX3BpZDsKPiArCV9fczMyCQlsX3BhZFs0XTsJLyogcmVzZXJ2ZSBhcmVhCQkJICAgICov Cj4gK307Cj4gKwo+ICsjZGVmaW5lIEZfSU9DX1JFU1ZTUAkJX0lPVygnWCcsIDQwLCBzdHJ1Y3Qg c3BhY2VfcmVzdikKPiArI2RlZmluZSBGX0lPQ19SRVNWU1A2NAkJX0lPVygnWCcsIDQyLCBzdHJ1 Y3Qgc3BhY2VfcmVzdikKCklzIHRoZSBzYW1lIGFzOgoKI2RlZmluZSBYRlNfSU9DX1JFU1ZTUCAg ICAgICAgICBfSU9XICgnWCcsIDQwLCBzdHJ1Y3QgeGZzX2Zsb2NrNjQpCiNkZWZpbmUgWEZTX0lP Q19SRVNWU1A2NCAgICAgICAgX0lPVyAoJ1gnLCA0Miwgc3RydWN0IHhmc19mbG9jazY0KQoKYmVj YXVzZToKCnR5cGVkZWYgc3RydWN0IHhmc19mbG9jazY0IHsKICAgICAgICBfX3MxNiAgICAgICAg ICAgbF90eXBlOwogICAgICAgIF9fczE2ICAgICAgICAgICBsX3doZW5jZTsKICAgICAgICBfX3M2 NCAgICAgICAgICAgbF9zdGFydDsKICAgICAgICBfX3M2NCAgICAgICAgICAgbF9sZW47ICAgICAg ICAgIC8qIGxlbiA9PSAwIG1lYW5zIHVudGlsIGVuZCBvZiBmaWxlICovCiAgICAgICAgX19zMzIg ICAgICAgICAgIGxfc3lzaWQ7CiAgICAgICAgX191MzIgICAgICAgICAgIGxfcGlkOwogICAgICAg IF9fczMyICAgICAgICAgICBsX3BhZFs0XTsgICAgICAgLyogcmVzZXJ2ZSBhcmVhICAgICAgICAg ICAgICAgICAgICAgKi8KfSB4ZnNfZmxvY2s2NF90OwoKCmlzIHRoZSBzYW1lIHNpemUgYXMgc3Ry dWN0IHNwYWNlX3Jlc3YuIEhlbmNlIGV4aXN0aW5nIGNhbGxzIHRvClhGU19JT0NfUkVTVlNQIHdp bGwgbm93IHZlY3RvciBpbmNvcnJlY3RseSBkb3duIHRoZSBmYWxsb2NhdGUgcGF0aApyZXN1bHRp bmcgaW4gaW52aXNpYmxlIG9wZXJhdGlvbnMgd2lsbCBub3cgYmUgdmlzaWJsZS4uLi4KCkNoZWVy cywKCkRhdmUuCi0tIApEYXZlIENoaW5uZXIKZGF2aWRAZnJvbW9yYml0LmNvbQoKX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KeGZzIG1haWxpbmcgbGlzdAp4 ZnNAb3NzLnNnaS5jb20KaHR0cDovL29zcy5zZ2kuY29tL21haWxtYW4vbGlzdGluZm8veGZzCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754070AbYLSEiF (ORCPT ); Thu, 18 Dec 2008 23:38:05 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1752382AbYLSEhw (ORCPT ); Thu, 18 Dec 2008 23:37:52 -0500 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 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: ApoEACqESkl5LB1f/2dsb2JhbAC+UViQXYMG X-IronPort-AV: E=Sophos;i="4.36,247,1228051800"; d="scan'208";a="278977598" Date: Fri, 19 Dec 2008 15:37:44 +1100 From: Dave Chinner To: Ankit Jain 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 Subject: Re: [PATCH v2] fs: Add new pre-allocation ioctls to vfs for compatibility with legacy xfs ioctls Message-ID: <20081219043744.GC17177@disturbed> Mail-Followup-To: Ankit Jain , 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 References: <494ABEBC.8060101@ankitjain.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <494ABEBC.8060101@ankitjain.org> User-Agent: Mutt/1.5.18 (2008-05-17) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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. > > Changes in v2: > - Dropped the incorrect handling of *UNRESVSP* ioctl. > - Made the ioctl and argument structure kernel only (__KERNEL__) > > Signed-off-by: Ankit Jain ..... > @@ -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); > } > > 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ѕ because: > +struct space_resv { > + __s16 l_type; > + __s16 l_whence; > + __s64 l_start; > + __s64 l_len; /* len == 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 == 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. -- Dave Chinner david@fromorbit.com 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