All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ed L Cashin <ecashin@coraid.com>
To: linux-kernel@vger.kernel.org
Subject: Re: PATCH: fix block layer ioctl bug
Date: Fri, 01 Oct 2004 09:56:08 -0400	[thread overview]
Message-ID: <87lleqfsp3.fsf@coraid.com> (raw)
In-Reply-To: 1V9VL-3ty-5@gated-at.bofh.it

Alan Cox <alan@redhat.com> writes:

> The block layer checks for -EINVAL from block layer driver ioctls. This
> is wrong - ENOTTY is unknown and some drivers correctly use this. I suspect
> for an internal ioctl 2.7 should change to -ENOIOCTLCMD and bitch about
> old style returns
> 
> This is conservative fix for the 2.6 case, it keeps the bogus -EINVAL to
> avoid breaking stuff
> 
> 
> diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.6/drivers/block/ioctl.c linux-2.6.6/drivers/block/ioctl.c
> --- linux.vanilla-2.6.6/drivers/block/ioctl.c	2004-05-10 03:31:59.000000000 +0100
> +++ linux-2.6.6/drivers/block/ioctl.c	2004-05-11 20:05:09.000000000 +0100
> @@ -203,7 +203,8 @@
>  	case BLKROSET:
>  		if (disk->fops->ioctl) {
>  			ret = disk->fops->ioctl(inode, file, cmd, arg);
> -			if (ret != -EINVAL)
> +			/* -EINVAL to handle old uncorrected drivers */
> +			if (ret != -EINVAL && ret != -ENOTTY)
>  				return ret;
>  		}
>  		if (!capable(CAP_SYS_ADMIN))

There's a case just above BLKROSET that seems to be in need of a
similar change.  In 2.6.8.1, on a BLKFLSBUF ioctl, if a driver returns
-EINVAL, the block layer will call fsync_bdev, invalidate_bdev, and
return 0.  It only gets the default behavior if it's returning
-EINVAL, though.  

If returning -ENOTTY for unhandled ioctls is the correct thing for a
driver to do, shouldn't the BLKFLSBUF ioctl sync the block device when
the driver returns -ENOTTY?

If so, this patch follows the one above, supporting the drivers that
return -ENOTTY correctly as well as the ones that still return
-EINVAL, at least for now.

--- linux-2.6.8.1/drivers/block/ioctl.c.20041001	Fri Oct  1 08:31:52 2004
+++ linux-2.6.8.1/drivers/block/ioctl.c	Fri Oct  1 08:42:05 2004
@@ -192,11 +192,12 @@ int blkdev_ioctl(struct inode *inode, st
 	case BLKFLSBUF:
 		if (!capable(CAP_SYS_ADMIN))
 			return -EACCES;
 		if (disk->fops->ioctl) {
 			ret = disk->fops->ioctl(inode, file, cmd, arg);
-			if (ret != -EINVAL)
+			/* -EINVAL to handle old uncorrected drivers */
+			if (ret != -EINVAL && ret != -ENOTTY)
 				return ret;
 		}
 		fsync_bdev(bdev);
 		invalidate_bdev(bdev, 0);
 		return 0;


-- 
  Ed L Cashin <ecashin@coraid.com>


       reply	other threads:[~2004-10-01 14:10 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1V9VL-3ty-5@gated-at.bofh.it>
2004-10-01 13:56 ` Ed L Cashin [this message]
2004-05-12 21:40 PATCH: fix block layer ioctl bug Alan Cox

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87lleqfsp3.fsf@coraid.com \
    --to=ecashin@coraid.com \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.