2005-12-13 17:23:32

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 2/3] add ->compat_ioctl to dasd

Add a compat_ioctl method to the dasd driver so the last entries in
arch/s390/kernel/compat_ioctl.c can go away. Unlike the previous
attempt this one does not replace the ioctl method with an
unlocked_ioctl method so that the ioctl_by_bdev calls in s390 partition
code continue to work.


Signed-off-by: Christoph Hellwig <[email protected]>

Index: linux-2.6.15-rc5/drivers/s390/block/dasd.c
===================================================================
--- linux-2.6.15-rc5.orig/drivers/s390/block/dasd.c 2005-12-12 19:09:39.000000000 +0100
+++ linux-2.6.15-rc5/drivers/s390/block/dasd.c 2005-12-12 19:09:41.000000000 +0100
@@ -1751,6 +1751,7 @@
.open = dasd_open,
.release = dasd_release,
.ioctl = dasd_ioctl,
+ .compat_ioctl = dasd_compat_ioctl,
.getgeo = dasd_getgeo,
};

Index: linux-2.6.15-rc5/drivers/s390/block/dasd_int.h
===================================================================
--- linux-2.6.15-rc5.orig/drivers/s390/block/dasd_int.h 2005-12-12 19:09:39.000000000 +0100
+++ linux-2.6.15-rc5/drivers/s390/block/dasd_int.h 2005-12-12 19:09:41.000000000 +0100
@@ -527,6 +527,7 @@
int dasd_ioctl_no_register(struct module *, int, dasd_ioctl_fn_t);
int dasd_ioctl_no_unregister(struct module *, int, dasd_ioctl_fn_t);
int dasd_ioctl(struct inode *, struct file *, unsigned int, unsigned long);
+long dasd_compat_ioctl(struct file *, unsigned int, unsigned long);

/* externals in dasd_proc.c */
int dasd_proc_init(void);
Index: linux-2.6.15-rc5/drivers/s390/block/dasd_ioctl.c
===================================================================
--- linux-2.6.15-rc5.orig/drivers/s390/block/dasd_ioctl.c 2005-12-12 19:09:39.000000000 +0100
+++ linux-2.6.15-rc5/drivers/s390/block/dasd_ioctl.c 2005-12-12 19:09:41.000000000 +0100
@@ -118,6 +118,18 @@
return -EINVAL;
}

+long
+dasd_compat_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
+{
+ int rval;
+
+ lock_kernel();
+ rval = dasd_ioctl(filp->f_dentry->d_inode, filp, cmd, arg);
+ unlock_kernel();
+
+ return rval;
+}
+
static int
dasd_ioctl_api_version(struct block_device *bdev, int no, long args)
{
Index: linux-2.6.15-rc5/arch/s390/kernel/compat_ioctl.c
===================================================================
--- linux-2.6.15-rc5.orig/arch/s390/kernel/compat_ioctl.c 2005-12-12 19:09:39.000000000 +0100
+++ linux-2.6.15-rc5/arch/s390/kernel/compat_ioctl.c 2005-12-12 19:15:46.000000000 +0100
@@ -42,27 +42,6 @@
#include <linux/compat_ioctl.h>
#define DECLARES
#include "../../../fs/compat_ioctl.c"
-
-/* s390 only ioctls */
-COMPATIBLE_IOCTL(DASDAPIVER)
-COMPATIBLE_IOCTL(BIODASDDISABLE)
-COMPATIBLE_IOCTL(BIODASDENABLE)
-COMPATIBLE_IOCTL(BIODASDRSRV)
-COMPATIBLE_IOCTL(BIODASDRLSE)
-COMPATIBLE_IOCTL(BIODASDSLCK)
-COMPATIBLE_IOCTL(BIODASDINFO)
-COMPATIBLE_IOCTL(BIODASDINFO2)
-COMPATIBLE_IOCTL(BIODASDFMT)
-COMPATIBLE_IOCTL(BIODASDPRRST)
-COMPATIBLE_IOCTL(BIODASDQUIESCE)
-COMPATIBLE_IOCTL(BIODASDRESUME)
-COMPATIBLE_IOCTL(BIODASDPRRD)
-COMPATIBLE_IOCTL(BIODASDPSRD)
-COMPATIBLE_IOCTL(BIODASDGATTR)
-COMPATIBLE_IOCTL(BIODASDSATTR)
-COMPATIBLE_IOCTL(BIODASDCMFENABLE)
-COMPATIBLE_IOCTL(BIODASDCMFDISABLE)
-COMPATIBLE_IOCTL(BIODASDREADALLCMB)
};

int ioctl_table_size = ARRAY_SIZE(ioctl_start);


2005-12-14 12:07:17

by Martin Schwidefsky

[permalink] [raw]
Subject: Re: [PATCH 2/3] add ->compat_ioctl to dasd

On Tue, 2005-12-13 at 18:23 +0100, Christoph Hellwig wrote:
> Add a compat_ioctl method to the dasd driver so the last entries in
> arch/s390/kernel/compat_ioctl.c can go away. Unlike the previous
> attempt this one does not replace the ioctl method with an
> unlocked_ioctl method so that the ioctl_by_bdev calls in s390 partition
> code continue to work.

Looks better but still doesn't work. The dasd driver specific ioctls do
work but there are some generic ones that are only available on the
normal ioctl path, including BLKFLSBUF, BLKROSET and HDIO_GETGEO. That
makes e.g. the 32 bit version of fdasd fail with "IOCTL error".

--
blue skies,
Martin

Martin Schwidefsky
Linux for zSeries Development & Services
IBM Deutschland Entwicklung GmbH


2005-12-14 12:24:14

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 2/3] add ->compat_ioctl to dasd

On Wed, Dec 14, 2005 at 01:07:14PM +0100, Martin Schwidefsky wrote:
> On Tue, 2005-12-13 at 18:23 +0100, Christoph Hellwig wrote:
> > Add a compat_ioctl method to the dasd driver so the last entries in
> > arch/s390/kernel/compat_ioctl.c can go away. Unlike the previous
> > attempt this one does not replace the ioctl method with an
> > unlocked_ioctl method so that the ioctl_by_bdev calls in s390 partition
> > code continue to work.
>
> Looks better but still doesn't work. The dasd driver specific ioctls do
> work but there are some generic ones that are only available on the
> normal ioctl path, including BLKFLSBUF, BLKROSET and HDIO_GETGEO. That
> makes e.g. the 32 bit version of fdasd fail with "IOCTL error".

Sorry, that's the ENOIOCTLCMD thing again, I forgot it in the first
revision of the last patch aswell.

Here's the fix for that:


Index: linux-2.6.15-rc5/drivers/s390/block/dasd_ioctl.c
===================================================================
--- linux-2.6.15-rc5.orig/drivers/s390/block/dasd_ioctl.c 2005-12-13 18:25:34.000000000 +0100
+++ linux-2.6.15-rc5/drivers/s390/block/dasd_ioctl.c 2005-12-14 13:23:16.000000000 +0100
@@ -127,7 +127,7 @@
rval = dasd_ioctl(filp->f_dentry->d_inode, filp, cmd, arg);
unlock_kernel();

- return rval;
+ return (rval == -EINVAL) ? -ENOIOCTLCMD : rval;
}

static int

2005-12-14 12:39:46

by Martin Schwidefsky

[permalink] [raw]
Subject: Re: [PATCH 2/3] add ->compat_ioctl to dasd

On Wed, 2005-12-14 at 13:24 +0100, Christoph Hellwig wrote:
> On Wed, Dec 14, 2005 at 01:07:14PM +0100, Martin Schwidefsky wrote:
> > On Tue, 2005-12-13 at 18:23 +0100, Christoph Hellwig wrote:
> > > Add a compat_ioctl method to the dasd driver so the last entries in
> > > arch/s390/kernel/compat_ioctl.c can go away. Unlike the previous
> > > attempt this one does not replace the ioctl method with an
> > > unlocked_ioctl method so that the ioctl_by_bdev calls in s390 partition
> > > code continue to work.
> >
> > Looks better but still doesn't work. The dasd driver specific ioctls do
> > work but there are some generic ones that are only available on the
> > normal ioctl path, including BLKFLSBUF, BLKROSET and HDIO_GETGEO. That
> > makes e.g. the 32 bit version of fdasd fail with "IOCTL error".
>
> Sorry, that's the ENOIOCTLCMD thing again, I forgot it in the first
> revision of the last patch aswell.

Oh yes, I should have remember that fix. It works fine now.

--
blue skies,
Martin

Martin Schwidefsky
Linux for zSeries Development & Services
IBM Deutschland Entwicklung GmbH