Re: [PATCH 1/5] dasd: cleanup dasd_ioctl

[Christoph Hellwig <hch@lst.de>]

On Sun, Feb 12, 2006 at 07:03:08PM +0100, Bastian Blank wrote:
> On Sun, Feb 12, 2006 at 06:38:55PM +0100, Christoph Hellwig wrote:
> >  static int
> > -dasd_ioctl_api_version(struct block_device *bdev, int no, long args)
> > +dasd_ioctl_api_version(void __user *argp)
> >  {
> >  	int ver = DASD_API_VERSION;
> > -	return put_user(ver, (int __user *) args);
> > +	return put_user(ver, (int *)argp);
> >  }
> 
> Doesn't this need to be "int __user *"?

Yes.

> > +long
> > +dasd_compat_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
> >  {
> > -	int i;
> > +	int rval;
> >  
> > -	for (i = 0; dasd_ioctls[i].no != -1; i++)
> > -		dasd_ioctl_no_unregister(NULL, dasd_ioctls[i].no,
> > -					 dasd_ioctls[i].fn);
> > +	lock_kernel();
> > +	rval = dasd_ioctl(filp->f_dentry->d_inode, filp, cmd, arg);
> > +	unlock_kernel();
> 
> The lock_kernel looks spurious.

dasd_compat_ioctl just moved down unchanged to the end of the file so it
can call dasd_ioctl without a forward-prototype.  When I introduced this
function a while ago I added the lock_kernel because that the BKL is
held when dasd_ioctl is called directly and I wanted to avoid different
locks from different codepathes.  Once we can switch dasd to
->unlocked_ioctl it could probably go away.