From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752292AbXGRVHp (ORCPT ); Wed, 18 Jul 2007 17:07:45 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1756562AbXGRVHP (ORCPT ); Wed, 18 Jul 2007 17:07:15 -0400 Received: from ug-out-1314.google.com ([66.249.92.169]:49140 "EHLO ug-out-1314.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752292AbXGRVHK (ORCPT ); Wed, 18 Jul 2007 17:07:10 -0400 DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=beta; h=received:from:to:subject:date:user-agent:cc:references:in-reply-to:mime-version:content-disposition:message-id:content-type:content-transfer-encoding; b=Iz4u6CXnJCVsqci9VcY0KR7P/K4Vs6vc7OXjJGCFgAw+CMhT01vNnkBzlEkO2DYT+RSxtY8WsCaZ5ZNfj+DrRxFdEnxKMzhyEuTQUQKFLzxTJj8rz1P/sV2vQbwo04kRSgtl9oVGza8APoesmI/Cn3zHbMtme8ESjBP3iA/PQnQ= From: Bartlomiej Zolnierkiewicz To: James Bottomley Subject: Re: block/bsg.c Date: Wed, 18 Jul 2007 22:32:58 +0200 User-Agent: KMail/1.9.6 Cc: Jeff Garzik , Andrew Morton , Jens Axboe , FUJITA Tomonori , linux-kernel@vger.kernel.org, linux-scsi@vger.kernel.org, linux-ide@vger.kernel.org References: <20070716165706.348f6bbf.akpm@linux-foundation.org> <200707180243.13368.bzolnier@gmail.com> <1184767917.3464.13.camel@localhost.localdomain> In-Reply-To: <1184767917.3464.13.camel@localhost.localdomain> MIME-Version: 1.0 Content-Disposition: inline Message-Id: <200707182232.58961.bzolnier@gmail.com> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org Hi, On Wednesday 18 July 2007, James Bottomley wrote: > On Wed, 2007-07-18 at 02:43 +0200, Bartlomiej Zolnierkiewicz wrote: > > [ James, please remeber to cc: linux-ide on IDE patches, thanks. ] > > Blame Andrew ... I assumed he'd be reporting the problem to the relevant > lists, so I just did a reply all ... No need to blame anybody, especially since it seems that one more person forgot about adding linux-ide ML. ;) > > On Wednesday 18 July 2007, Jeff Garzik wrote: > > > James Bottomley wrote: > > > > @@ -1052,9 +1054,10 @@ int generic_ide_ioctl(ide_drive_t *drive, struct file *file, struct block_device > > > > int err, (*setfunc)(ide_drive_t *, int); > > > > u8 *val; > > > > > > > > - err = scsi_cmd_ioctl(file, bdev->bd_disk->queue, bdev->bd_disk, cmd, p); > > > > - if (err != -ENOTTY) > > > > - return err; > > > > + switch (cmd) { > > > > + case SG_IO: > > > > + return scsi_cmd_ioctl(file, bdev->bd_disk->queue, bdev->bd_disk, cmd, p); > > > > + } > > > > > > > > switch (cmd) { > > > > case HDIO_GET_32BIT: val = &drive->io_32bit; goto read_val; > > > > > > > > > At that point you might as well use an 'if'. > > > > > > But overall -- agreed. ACK. > > > > James/Jeff thanks for following the issue but NAK. ;) > > > > Causes regression wrt ide-floppy CDROMEJECT/CDROMCLOSETRAY support when > > compared to 2.6.22 and SG_IO is not supported by ide-{disk,scsi,tape}. > > Well ... that was why I put the case statement in ... I was sure there > would be other ioctls I missed. The thing is that ide-{disk,scsi,tape} really don't support REQ_TYPE_BLOCK_PC type requests so ide.c::generic_ide_ioctl() is not the best place to add handling of SG_IO. Patch attached. > > Luckily Linus has already fixed the issue properly. > > Actually, no, that's just a reversion. I think we need the attached to > complete all of this. > > > BTW cmd == 1 IOCTL is not defined/used by IDE driver. > > Andrew's trace clearly shows that something is sending cmd == 1 down, so I still haven't got the trace. Andrew, please (re)send it in PM, thanks. > I'm nearly positive at one time this was a legitimate ioctl for IDE. Could be... > Anyway it's probably time to kill this deprecated ioctl in SCSI. Fully agreed. Thanks, Bart [PATCH] ide: add support for SCSI ioctls to ide-floppy Now that ide-floppy supports SG_IO we can add support for SCSI ioctls (except deprecated SCSI_IOCTL_SEND_COMMAND and legacy CDROM_SEND_PACKET ones - we can add them later iff really needed). While at it remove handling of CDROMEJECT and CDROMCLOSETRAY ioctls from generic_ide_ioctl(): - This prevents ide-{disk,tape,scsi} device drivers from obtaining REQ_TYPE_BLOCK_PC type requests which are currently unsupported by these drivers and which are potentially harmful (as reported by Andrew). - There is no functionality loss since aforementioned ioctls will now be handled by idefloppy_ioctl()->scsi_cmd_ioctl() (for devices using ide-floppy driver) and by idecd_ioctl->cdrom_ioctl()->scsi_cmd_ioctl() (for devices using ide-cd driver). Signed-off-by: Bartlomiej Zolnierkiewicz Cc: Jens Axboe Cc: FUJITA Tomonori Cc: James Bottomley Cc: Jeff Garzik Cc: Andrew Morton --- drivers/ide/ide-floppy.c | 18 +++++++++++++++++- drivers/ide/ide.c | 4 ---- 2 files changed, 17 insertions(+), 5 deletions(-) Index: b/drivers/ide/ide-floppy.c =================================================================== --- a/drivers/ide/ide-floppy.c +++ b/drivers/ide/ide-floppy.c @@ -99,6 +99,8 @@ #include #include +#include + #include #include #include @@ -2099,7 +2101,21 @@ static int idefloppy_ioctl(struct inode case IDEFLOPPY_IOCTL_FORMAT_GET_PROGRESS: return idefloppy_get_format_progress(drive, argp); } - return generic_ide_ioctl(drive, file, bdev, cmd, arg); + + /* + * skip SCSI_IOCTL_SEND_COMMAND (deprecated) + * and CDROM_SEND_PACKET (legacy) ioctls + */ + if (cmd != CDROM_SEND_PACKET && cmd != SCSI_IOCTL_SEND_COMMAND) + err = scsi_cmd_ioctl(file, bdev->bd_disk->queue, + bdev->bd_disk, cmd, argp); + else + err = -ENOTTY; + + if (err == -ENOTTY) + err = generic_ide_ioctl(drive, file, bdev, cmd, arg); + + return err; } static int idefloppy_media_changed(struct gendisk *disk) Index: b/drivers/ide/ide.c =================================================================== --- a/drivers/ide/ide.c +++ b/drivers/ide/ide.c @@ -1171,10 +1171,6 @@ int generic_ide_ioctl(ide_drive_t *drive return 0; } - case CDROMEJECT: - case CDROMCLOSETRAY: - return scsi_cmd_ioctl(file, bdev->bd_disk->queue, bdev->bd_disk, cmd, p); - case HDIO_GET_BUSSTATE: if (!capable(CAP_SYS_ADMIN)) return -EACCES;