On Fri, Aug 16, 2019 at 7:32 PM Andreas Gruenbacher <agruenba@xxxxxxxxxx> wrote: > > On Wed, Aug 14, 2019 at 10:45 PM Arnd Bergmann <arnd@xxxxxxxx> wrote: > > + /* These are just misnamed, they actually get/put from/to user an int */ > > + switch(cmd) { > > + case FS_IOC32_GETFLAGS: > > + cmd = FS_IOC_GETFLAGS; > > + break; > > + case FS_IOC32_SETFLAGS: > > + cmd = FS_IOC_SETFLAGS; > > + break; > > I'd like the code to be more explicit here: > > case FITRIM: > case FS_IOC_GETFSLABEL: > break; > default: > return -ENOIOCTLCMD; I've looked at it again: if we do this, the function actually becomes longer than the native gfs2_ioctl(). Should we just make a full copy then? static long gfs2_compat_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) { switch(cmd) { case FS_IOC32_GETFLAGS: return gfs2_get_flags(filp, (u32 __user *)arg); case FS_IOC32_SETFLAGS: return gfs2_set_flags(filp, (u32 __user *)arg); case FITRIM: return gfs2_fitrim(filp, (void __user *)arg); case FS_IOC_GETFSLABEL: return gfs2_getlabel(filp, (char __user *)arg); } return -ENOTTY; } > Should we feed this through the gfs2 tree? A later patch that removes the FITRIM handling from fs/compat_ioctl.c depends on it, so I'd like to keep everything together. Arnd