[RFC] breakage in bsg

[RFC] breakage in bsg

[Al Viro]

	* with "bsg: bind bsg to request_queue instead of gendisk" we
get all requests with NULL ->rq_disk.  SCSI can cope with that, anything
else is not promised to be able to surive that.  Indeed, quite a few
drivers do not.

	* WTF are these per-bd flags doing, seeing that you set them
on every ->read() and ->write()?  Just what will happen if you have
two openers?

	* cmd_filter thing is broken as well (we have no access to
gendisk in question and there's a lot of obvious issues when you have
several openers).

#2 and part of #3 are fixable, but I really don't see what to do with #1.
If nothing else, it's absolutely incompatible with cmd_filter, even if you
leave aside the issue with non-IDE/non-SCSI drivers.

Comments?

Re: [RFC] breakage in bsg

[Al Viro]

On Sun, Jul 20, 2008 at 05:34:06PM +0100, Al Viro wrote:

> 	* cmd_filter thing is broken as well (we have no access to
> gendisk in question and there's a lot of obvious issues when you have
> several openers).
> 
> #2 and part of #3 are fixable, but I really don't see what to do with #1.
> If nothing else, it's absolutely incompatible with cmd_filter, even if you
> leave aside the issue with non-IDE/non-SCSI drivers.

Actually, cmd_filter changeset breaks all users of blk_verify_command().
It expects that struct file passed to will be that of a block device:
        inode = file->f_dentry->d_inode;
        if (!inode)
                return -EINVAL;

        disk = inode->i_bdev->bd_disk;

        return blk_cmd_filter_verify_command(&disk->cmd_filter,
                                                 cmd, &file->f_mode);

Unfortunately, all of those can get struct file of *character* device, with
obvious results.

The minimal fix is to pass disk / mode explicitly.  I'll post the patch,
but that still leaves bsg broken.

Re: [RFC] breakage in bsg

[James Bottomley]

[Sorry for breaking threading ... this is a cut and paste from mark
since you didn't post to any list I'm subscribed to]

> 	* with "bsg: bind bsg to request_queue instead of gendisk" we
> get all requests with NULL ->rq_disk.  SCSI can cope with that, anything
> else is not promised to be able to surive that.  Indeed, quite a few
> drivers do not.

That's pretty much by design since we need to allow bsg to be used on
request queues that don't have a gendisk attached (classic example are
the transient queues belonging to SAS expanders).  Since drivers have to
actively allow bsg, we can do the local conversions as necessary.

> 	* WTF are these per-bd flags doing, seeing that you set them
> on every ->read() and ->write()?  Just what will happen if you have
> two openers?

This isn't really my area, but there's only one flag: BSG_F_BLOCK which
is used to signal a temporary blockage of the device ... by definition
that applies to all openers.

> 	* cmd_filter thing is broken as well (we have no access to
> gendisk in question and there's a lot of obvious issues when you have
> several openers).

Well, this is philosophical.  I've always maintained the command filter
to be broken by design.  BSG reinforces that because now we have actual
uses that aren't SCSI (the main use I have is to transmit Management
frames to SAS).  You can't filter something if you don't understand the
protocols and protocol independence was a requirement for the next
generation of SG.

> #2 and part of #3 are fixable, but I really don't see what to do with #1.
> If nothing else, it's absolutely incompatible with cmd_filter, even if you
> leave aside the issue with non-IDE/non-SCSI drivers.
> 
> Comments?

James