block: Make CDROMEJECT more robust
PATCH 1/1 block: Make CDROMEJECT more robust
Reference: https://bugzilla.ubuntu.com/5049
The eject command was failing for a large group of users for removable devices. The "eject -r" command, which uses the CDROMEJECT ioctl would not work, however "eject -s", which uses SG_IO did work, but required root access.
Since SG_IO was using the same mechanism as CDROMEJECT, there should be no difference. The main reason for getting the CDROMEJECT ioctl working was because it didn't need root privileges like the SG_IO commands did.
One bug was noticed, and that is CDROMEJECT was setting the blk request to a WRITE operation, when in fact it wasn't. The block layer did not like getting WRITE requests when data_len==0 and data==NULL.
The other issue was that "eject -s" was sending two extra commands that seemed to work for just about any removable device. CDROMEJECT works as such:
START_STOP : 0x02
While "eject -s" sent(via SG_IO):
ALLOW_MEDIUM_REMOVALSTART_STOP : 0x01START_STOP : 0x02
This patch fixes the WRITE vs READ issue, and also sends the extra two commands. Anyone with an iPod connected via USB (not sure about firewire) should be able to reproduce this issue, and verify the patch.
block/scsi_ioctl.c +36 -14
@@ -442,11 +442,38 @@ error:return err; } +/* This is only useful for the following macros */ +static int __blk_send_generic (request_queue_t *q,struct gendisk *bd_disk,int cmd,int data) +{ +struct request *rq; +int err; + + rq =blk_get_request (q,READ , __GFP_WAIT); + rq->flags |=REQ_BLOCK_PC ; + rq->data =NULL ; + rq->data_len =0 ; + rq->timeout =BLK_DEFAULT_TIMEOUT ; +memset (rq->cmd,0 ,sizeof (rq->cmd)); + rq->cmd[0 ] = cmd; + rq->cmd[4 ] = data; + rq->cmd_len =6 ; + err =blk_execute_rq (q, bd_disk, rq,0 ); +blk_put_request (rq); + +return err; +} + +#define blk_send_start_stop (q, bd_disk, data) \ + __blk_send_generic(q, bd_disk, GPCMD_START_STOP_UNIT, data) + +#define blk_send_allow_medium_removal (q, bd_disk) \ + __blk_send_generic(q, bd_disk, GPCMD_PREVENT_ALLOW_MEDIUM_REMOVAL, 0) + +int scsi_cmd_ioctl (struct file *file,struct gendisk *bd_disk,unsigned int cmd,void __user *arg) {request_queue_t *q; -struct request *rq; -int close =0 , err; +int err; q = bd_disk->queue;if (!q)
@@ -564,19 +591,14 @@ int scsi_cmd_ioctl(struct file *file, st err =sg_scsi_ioctl (file, q, bd_disk, arg);break ;case CDROMCLOSETRAY : - close =1 ; + err =blk_send_start_stop (q, bd_disk,0x03 ); +break ;case CDROMEJECT : - rq =blk_get_request (q,WRITE , __GFP_WAIT); - rq->flags |=REQ_BLOCK_PC ; - rq->data =NULL ; - rq->data_len =0 ; - rq->timeout =BLK_DEFAULT_TIMEOUT ; -memset (rq->cmd,0 ,sizeof (rq->cmd)); - rq->cmd[0 ] =GPCMD_START_STOP_UNIT ; - rq->cmd[4 ] =0x02 + (close !=0 ); - rq->cmd_len =6 ; - err =blk_execute_rq (q, bd_disk, rq,0 ); -blk_put_request (rq); + err =0 ; + + err |=blk_send_allow_medium_removal (q, bd_disk); + err |=blk_send_start_stop (q, bd_disk,0x01 ); + err |=blk_send_start_stop (q, bd_disk,0x02 );break ;default : err = -ENOTTY ;
Quoted 6 lines · Ben Collins
Reference: https://bugzilla.ubuntu.com/5049
The eject command was failing for a large group of users for removable devices. The "eject -r" command, which uses the CDROMEJECT ioctl would not work, however "eject -s", which uses
SG_IOdid work, but required root access.
Quoted 7 lines
Since
SG_IOwas using the same mechanism as CDROMEJECT, there should be no difference. The main reason for getting the CDROMEJECT ioctl working was because it didn't need root privileges like theSG_IOcommands did.One bug was noticed, and that is CDROMEJECT was setting the blk request to a WRITE operation, when in fact it wasn't. The block layer did not like getting WRITE requests when data_len==0 and data==NULL.
False, it can't be a write request if there's no data attached. Write is simply used there because read requests are usually more precious.
Quoted 3 lines
This patch fixes the WRITE vs READ issue, and also sends the extra two commands. Anyone with an iPod connected via USB (not sure about firewire) should be able to reproduce this issue, and verify the patch.
The bug was in the SCSI layer, and James already has the fix integrated for that. It really should make 2.6.15, James are you sending it upwards for that?
Quoted 7 lines
block/scsi_ioctl.c +0
-6case CDROMEJECT : - rq =blk_get_request (q,WRITE , __GFP_WAIT); - rq->flags |=REQ_BLOCK_PC ; - rq->data =NULL ; - rq->data_len =0 ; - rq->timeout =BLK_DEFAULT_TIMEOUT ; -memset (rq->cmd,0 ,sizeof (rq->cmd));
block/scsi_ioctl.c int scsi_cmd_ioctl(struct file *file, st · line 601
Quoted 10 lines
block/scsi_ioctl.c +5
-5- rq->cmd[0 ] =GPCMD_START_STOP_UNIT ; - rq->cmd[4 ] =0x02 + (close !=0 ); - rq->cmd_len =6 ; - err =blk_execute_rq (q, bd_disk, rq,0 ); -blk_put_request (rq); + err =0 ; + + err |=blk_send_allow_medium_removal (q, bd_disk); + err |=blk_send_start_stop (q, bd_disk,0x01 ); + err |=blk_send_start_stop (q, bd_disk,0x02 );
Do this in the eject tool, if it's required for some devices.
Signature · 1 line
Jens Axboe
Quoted 7 lines · Jens Axboe
Quoted 3 lines
This patch fixes the WRITE vs READ issue, and also sends the extra two commands. Anyone with an iPod connected via USB (not sure about firewire) should be able to reproduce this issue, and verify the patch.
The bug was in the SCSI layer, and James already has the fix integrated for that. It really should make 2.6.15, James are you sending it upwards for that?
I missed the fact that it is already in -rc6, so no problems there.
Signature · 1 line
Jens Axboe
Quoted 11 lines · Jens Axboe
Quoted 10 lines · Ben Collins
Reference: https://bugzilla.ubuntu.com/5049
The eject command was failing for a large group of users for removable devices. The "eject -r" command, which uses the CDROMEJECT ioctl would not work, however "eject -s", which uses
SG_IOdid work, but required root access.Since
SG_IOwas using the same mechanism as CDROMEJECT, there should be no difference. The main reason for getting the CDROMEJECT ioctl working was because it didn't need root privileges like theSG_IOcommands did.
Quoted 6 lines
Quoted 3 lines
One bug was noticed, and that is CDROMEJECT was setting the blk request to a WRITE operation, when in fact it wasn't. The block layer did not like getting WRITE requests when data_len==0 and data==NULL.
False, it can't be a write request if there's no data attached. Write is simply used there because read requests are usually more precious.
Did you mean "can be a write request"? If not, then you just repeated what I said.
Quoted 7 lines
Quoted 3 lines
This patch fixes the WRITE vs READ issue, and also sends the extra two commands. Anyone with an iPod connected via USB (not sure about firewire) should be able to reproduce this issue, and verify the patch.
The bug was in the SCSI layer, and James already has the fix integrated for that. It really should make 2.6.15, James are you sending it upwards for that?
Can you point me to this fix? Also, does the "fix" fix the case for IDE CDROM's too?
Quoted 11 lines
Quoted 11 lines
block/scsi_ioctl.c +0
-10case CDROMEJECT : - rq =blk_get_request (q,WRITE , __GFP_WAIT); - rq->flags |=REQ_BLOCK_PC ; - rq->data =NULL ; - rq->data_len =0 ; - rq->timeout =BLK_DEFAULT_TIMEOUT ; -memset (rq->cmd,0 ,sizeof (rq->cmd)); - rq->cmd[0 ] =GPCMD_START_STOP_UNIT ; - rq->cmd[4 ] =0x02 + (close !=0 ); - rq->cmd_len =6 ; - err =blk_execute_rq (q, bd_disk, rq,0 );
Quoted 8 lines
block/scsi_ioctl.c int scsi_cmd_ioctl(struct file *file, st · line 601
Quoted 6 lines
block/scsi_ioctl.c +5
-1-blk_put_request (rq); + err =0 ; + + err |=blk_send_allow_medium_removal (q, bd_disk); + err |=blk_send_start_stop (q, bd_disk,0x01 ); + err |=blk_send_start_stop (q, bd_disk,0x02 );Do this in the eject tool, if it's required for some devices.
It already is in eject tool, but as described, that requires root access. Not something I want to force a user to do in order to eject their CDROM/iPod/USBStick in gnome. What exactly is wrong with the commands? If they are harmless for devices that don't need it, and fix a huge number of problems (did you see the Cc list on the bug report?) for users with affected devices, then what's the harm?
Signature · 3 lines
Ben Collins <[email protected]> Developer Ubuntu Linux
Quoted 16 lines · Ben Collins
Quoted 15 lines · Jens Axboe
Quoted 13 lines · Ben Collins
Reference: https://bugzilla.ubuntu.com/5049
The eject command was failing for a large group of users for removable devices. The "eject -r" command, which uses the CDROMEJECT ioctl would not work, however "eject -s", which uses
SG_IOdid work, but required root access.Since
SG_IOwas using the same mechanism as CDROMEJECT, there should be no difference. The main reason for getting the CDROMEJECT ioctl working was because it didn't need root privileges like theSG_IOcommands did.One bug was noticed, and that is CDROMEJECT was setting the blk request to a WRITE operation, when in fact it wasn't. The block layer did not like getting WRITE requests when data_len==0 and data==NULL.
Quoted 5 lines
Quoted 2 lines
False, it can't be a write request if there's no data attached. Write is simply used there because read requests are usually more precious.
Did you mean "can be a write request"? If not, then you just repeated what I said.
No, you are misreading me. Definition:
- Read request: direction bit not set, non-zero data length
- Write request: direction bit set, non-zero data length
How can you read or write anything, when there's nothing to read or write?
Quoted 10 lines
Quoted 7 lines
Quoted 3 lines
This patch fixes the WRITE vs READ issue, and also sends the extra two commands. Anyone with an iPod connected via USB (not sure about firewire) should be able to reproduce this issue, and verify the patch.
The bug was in the SCSI layer, and James already has the fix integrated for that. It really should make 2.6.15, James are you sending it upwards for that?
Can you point me to this fix? Also, does the "fix" fix the case for IDE CDROM's too?
What is the problem case for for ide-cd?
The SCSI fix is here:
and followed up by a fix for James to cater to all paths:
Quoted 20 lines
Quoted 11 lines
Quoted 11 lines
block/scsi_ioctl.c +0
-10case CDROMEJECT : - rq =blk_get_request (q,WRITE , __GFP_WAIT); - rq->flags |=REQ_BLOCK_PC ; - rq->data =NULL ; - rq->data_len =0 ; - rq->timeout =BLK_DEFAULT_TIMEOUT ; -memset (rq->cmd,0 ,sizeof (rq->cmd)); - rq->cmd[0 ] =GPCMD_START_STOP_UNIT ; - rq->cmd[4 ] =0x02 + (close !=0 ); - rq->cmd_len =6 ; - err =blk_execute_rq (q, bd_disk, rq,0 );Quoted 8 lines
block/scsi_ioctl.c int scsi_cmd_ioctl(struct file *file, st · line 601
Quoted 6 lines
block/scsi_ioctl.c +5
-1-blk_put_request (rq); + err =0 ; + + err |=blk_send_allow_medium_removal (q, bd_disk); + err |=blk_send_start_stop (q, bd_disk,0x01 ); + err |=blk_send_start_stop (q, bd_disk,0x02 );Do this in the eject tool, if it's required for some devices.
Quoted 6 lines
It already is in eject tool, but as described, that requires root access. Not something I want to force a user to do in order to eject their CDROM/iPod/USBStick in gnome. What exactly is wrong with the commands? If they are harmless for devices that don't need it, and fix a huge number of problems (did you see the Cc list on the bug report?) for users with affected devices, then what's the harm?
So the medium removal command does require write permission on the deviec, but it doesn't require root. If they need to rw to the device fs, surely they need write permissions on the device in the first place?
Signature · 1 line
Jens Axboe
Quoted 23 lines · Jens Axboe
Quoted 16 lines · Ben Collins
Quoted 15 lines · Jens Axboe
Quoted 13 lines · Ben Collins
Reference: https://bugzilla.ubuntu.com/5049
The eject command was failing for a large group of users for removable devices. The "eject -r" command, which uses the CDROMEJECT ioctl would not work, however "eject -s", which uses
SG_IOdid work, but required root access.Since
SG_IOwas using the same mechanism as CDROMEJECT, there should be no difference. The main reason for getting the CDROMEJECT ioctl working was because it didn't need root privileges like theSG_IOcommands did.One bug was noticed, and that is CDROMEJECT was setting the blk request to a WRITE operation, when in fact it wasn't. The block layer did not like getting WRITE requests when data_len==0 and data==NULL.
Quoted 5 lines
Quoted 2 lines
False, it can't be a write request if there's no data attached. Write is simply used there because read requests are usually more precious.
Did you mean "can be a write request"? If not, then you just repeated what I said.
Quoted 7 lines
No, you are misreading me. Definition:
- Read request: direction bit not set, non-zero data length
- Write request: direction bit set, non-zero data lengthHow can you read or write anything, when there's nothing to read or write?
Exactly, but the block layer (and I'd have to trace back through the code to find the exact failure point) has a check somewhere in there that checks the direction, and if it's a WRITE, checks data_len, and fails if it is 0.
Note, my patch does not fix the case for the bug if I leave it as WRITE. It only works when it is READ.
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 3 lines
This patch fixes the WRITE vs READ issue, and also sends the extra two commands. Anyone with an iPod connected via USB (not sure about firewire) should be able to reproduce this issue, and verify the patch.
Quoted 8 lines
Quoted 6 lines
Quoted 3 lines
The bug was in the SCSI layer, and James already has the fix integrated for that. It really should make 2.6.15, James are you sending it upwards for that?
Can you point me to this fix? Also, does the "fix" fix the case for IDE CDROM's too?
What is the problem case for for ide-cd?
Yes, I know these patches. I was in the Cc when they were all being discussed. It's has nothing to do with my patch. It's the commands that are required. It's not about the bug in the SCSI layer.
Quoted 35 lines
The SCSI fix is here:
and followed up by a fix for James to cater to all paths:
Quoted 20 lines
Quoted 11 lines
Quoted 11 lines
block/scsi_ioctl.c +0
-10case CDROMEJECT : - rq =blk_get_request (q,WRITE , __GFP_WAIT); - rq->flags |=REQ_BLOCK_PC ; - rq->data =NULL ; - rq->data_len =0 ; - rq->timeout =BLK_DEFAULT_TIMEOUT ; -memset (rq->cmd,0 ,sizeof (rq->cmd)); - rq->cmd[0 ] =GPCMD_START_STOP_UNIT ; - rq->cmd[4 ] =0x02 + (close !=0 ); - rq->cmd_len =6 ; - err =blk_execute_rq (q, bd_disk, rq,0 );Quoted 8 lines
block/scsi_ioctl.c int scsi_cmd_ioctl(struct file *file, st · line 601
Quoted 6 lines
block/scsi_ioctl.c +5
-1-blk_put_request (rq); + err =0 ; + + err |=blk_send_allow_medium_removal (q, bd_disk); + err |=blk_send_start_stop (q, bd_disk,0x01 ); + err |=blk_send_start_stop (q, bd_disk,0x02 );Do this in the eject tool, if it's required for some devices.
Quoted 6 lines
It already is in eject tool, but as described, that requires root access. Not something I want to force a user to do in order to eject their CDROM/iPod/USBStick in gnome. What exactly is wrong with the commands? If they are harmless for devices that don't need it, and fix a huge number of problems (did you see the Cc list on the bug report?) for users with affected devices, then what's the harm?
Quoted 3 lines
So the medium removal command does require write permission on the deviec, but it doesn't require root. If they need to rw to the device fs, surely they need write permissions on the device in the first place?
bcollins@colorless:~$ id -a uid=1000(bcollins) gid=1000(bcollins) groups=4(adm),20(dialout),24(cdrom),25(floppy),29(audio),30(dip),44(video),46(plugdev),104(lpadmin),105(scanner),106(admin),1000(bcollins) bcollins@colorless:~$ ls -l /dev/hdc brw-rw---- 1 root plugdev 22, 0 Dec 19 2005 /dev/hdc bcollins@colorless:~$ eject -s /dev/hdc eject: unable to eject, last error: Operation not permitted bcollins@colorless:~$ eject -r /dev/hdc bcollins@colorless:~$
Write permissions is not enough.
Signature · 3 lines
Ben Collins <[email protected]> Developer Ubuntu Linux
Quoted 10 lines · Ben Collins
Quoted 5 lines
- Read request: direction bit not set, non-zero data length
- Write request: direction bit set, non-zero data lengthHow can you read or write anything, when there's nothing to read or write?
Exactly, but the block layer (and I'd have to trace back through the code to find the exact failure point) has a check somewhere in there that checks the direction, and if it's a WRITE, checks
data_len, and fails if it is 0.
I'd like to see that, if it's true it wants fixing of course.
Quoted 2 lines
Note, my patch does not fix the case for the bug if I leave it as WRITE. It only works when it is READ.
We need to get that fixed as well, it should not make a difference.
Quoted 4 lines
Quoted 3 lines
So the medium removal command does require write permission on the deviec, but it doesn't require root. If they need to rw to the device fs, surely they need write permissions on the device in the first place?
Quoted 8 lines
bcollins@colorless:~$ id -a uid=1000(bcollins) gid=1000(bcollins) groups=4(adm),20(dialout),24(cdrom),25(floppy),29(audio),30(dip),44(video),46(plugdev),104(lpadmin),105(scanner),106(admin),1000(bcollins) bcollins@colorless:~$ ls -l /dev/hdc brw-rw---- 1 root plugdev 22, 0 Dec 19 2005 /dev/hdc bcollins@colorless:~$ eject -s /dev/hdc eject: unable to eject, last error: Operation not permitted bcollins@colorless:~$ eject -r /dev/hdc
Does eject open the device O_RDWR?
Signature · 1 line
Jens Axboe
Quoted 14 lines · Jens Axboe
Quoted 4 lines · Ben Collins
Quoted 3 lines
So the medium removal command does require write permission on the deviec, but it doesn't require root. If they need to rw to the device fs, surely they need write permissions on the device in the first place?
Quoted 8 lines
bcollins@colorless:~$ id -a uid=1000(bcollins) gid=1000(bcollins) groups=4(adm),20(dialout),24(cdrom),25(floppy),29(audio),30(dip),44(video),46(plugdev),104(lpadmin),105(scanner),106(admin),1000(bcollins) bcollins@colorless:~$ ls -l /dev/hdc brw-rw---- 1 root plugdev 22, 0 Dec 19 2005 /dev/hdc bcollins@colorless:~$ eject -s /dev/hdc eject: unable to eject, last error: Operation not permitted bcollins@colorless:~$ eject -r /dev/hdc
Quoted 1 line
Does eject open the device
O_RDWR?
No, it was opening it RDONLY. I changed it to RDWR, and it allowed the SG_IO commands to work without root. I'll send a patch to eject for that.
Signature · 3 lines
Ben Collins <[email protected]> Developer Ubuntu Linux
Quoted 21 lines · Ben Collins
Quoted 12 lines
Quoted 12 lines
block/scsi_ioctl.c +0
-10case CDROMEJECT : - rq =blk_get_request (q,WRITE , __GFP_WAIT); - rq->flags |=REQ_BLOCK_PC ; - rq->data =NULL ; - rq->data_len =0 ; - rq->timeout =BLK_DEFAULT_TIMEOUT ; -memset (rq->cmd,0 ,sizeof (rq->cmd)); - rq->cmd[0 ] =GPCMD_START_STOP_UNIT ; - rq->cmd[4 ] =0x02 + (close !=0 ); - rq->cmd_len =6 ; - err =blk_execute_rq (q, bd_disk, rq,0 );Quoted 8 lines
block/scsi_ioctl.c int scsi_cmd_ioctl(struct file *file, st · line 601
Quoted 6 lines
block/scsi_ioctl.c +5
-1-blk_put_request (rq); + err =0 ; + + err |=blk_send_allow_medium_removal (q, bd_disk); + err |=blk_send_start_stop (q, bd_disk,0x01 ); + err |=blk_send_start_stop (q, bd_disk,0x02 );Do this in the eject tool, if it's required for some devices.
Quoted 6 lines
It already is in eject tool, but as described, that requires root access. Not something I want to force a user to do in order to eject their CDROM/iPod/USBStick in gnome. What exactly is wrong with the commands? If they are harmless for devices that don't need it, and fix a huge number of problems (did you see the Cc list on the bug report?) for users with affected devices, then what's the harm?
I do agree that the suggested patch seems to be a real cleanup, regardless of whether the original code bug has now been fixed or not.
Are there devices that really want the old sequence?
Also, do we really need to send fist a start_stop 1 and then a 2?
Wouldn't the _logical_ thing be to replace the old code with just a cleaned-up-version of what the old code did, ie just do
err = blk_send_start_stop(q, bd_disk, 0x02);
for the eject case? That way we could do the patch as a pure cleanup, and then a separate patch might change the singe "start_stop 2" with the more complex sequence.
(IOW, I'd prefer to separate out the cleanup from the "make the eject sequence more complete" part).
Linus
Quoted 31 lines · Linus Torvalds
Quoted 21 lines · Ben Collins
Quoted 12 lines
Quoted 12 lines
block/scsi_ioctl.c +0
-10case CDROMEJECT : - rq =blk_get_request (q,WRITE , __GFP_WAIT); - rq->flags |=REQ_BLOCK_PC ; - rq->data =NULL ; - rq->data_len =0 ; - rq->timeout =BLK_DEFAULT_TIMEOUT ; -memset (rq->cmd,0 ,sizeof (rq->cmd)); - rq->cmd[0 ] =GPCMD_START_STOP_UNIT ; - rq->cmd[4 ] =0x02 + (close !=0 ); - rq->cmd_len =6 ; - err =blk_execute_rq (q, bd_disk, rq,0 );Quoted 8 lines
block/scsi_ioctl.c int scsi_cmd_ioctl(struct file *file, st · line 601
Quoted 6 lines
block/scsi_ioctl.c +5
-1-blk_put_request (rq); + err =0 ; + + err |=blk_send_allow_medium_removal (q, bd_disk); + err |=blk_send_start_stop (q, bd_disk,0x01 ); + err |=blk_send_start_stop (q, bd_disk,0x02 );Do this in the eject tool, if it's required for some devices.
Quoted 6 lines
It already is in eject tool, but as described, that requires root access. Not something I want to force a user to do in order to eject their CDROM/iPod/USBStick in gnome. What exactly is wrong with the commands? If they are harmless for devices that don't need it, and fix a huge number of problems (did you see the Cc list on the bug report?) for users with affected devices, then what's the harm?
Quoted 2 lines
I do agree that the suggested patch seems to be a real cleanup, regardless of whether the original code bug has now been fixed or not.
Apparently two seperate issues.
Quoted 4 lines
Are there devices that really want the old sequence?
Also, do we really need to send fist a
start_stop1 and then a 2?
The 0x01 looks really suspicious to me, it should just cause extra wait and activity on most devices.
Quoted 8 lines
Wouldn't the _logical_ thing be to replace the old code with just a cleaned-up-version of what the old code did, ie just do
err = blk_send_start_stop(q, bd_disk, 0x02);for the eject case? That way we could do the patch as a pure cleanup, and then a separate patch might change the singe "
start_stop2" with the more complex sequence.
That would work.
Signature · 1 line
Jens Axboe
Quoted 6 lines · Linus Torvalds
for the eject case? That way we could do the patch as a pure cleanup, and then a separate patch might change the singe "
start_stop2" with the more complex sequence.(IOW, I'd prefer to separate out the cleanup from the "make the eject sequence more complete" part).
Sure thing.
Signature · 3 lines
Ben Collins <[email protected]> Developer Ubuntu Linux
Quoted 8 lines · Jens Axboe
Quoted 3 lines · Ben Collins
This patch fixes the WRITE vs READ issue, and also sends the extra two commands. Anyone with an iPod connected via USB (not sure about firewire) should be able to reproduce this issue, and verify the patch.
The bug was in the SCSI layer, and James already has the fix integrated for that. It really should make 2.6.15, James are you sending it upwards for that?
You mean this patch?
James Bottomley:
[SCSI] Consolidate REQ_BLOCK_PC handling path (fix ipod panic)
This fixes an oops with data direction because sbp2 was not checking enough itself.
I seriously doubt this will fix the issue being reported. Changing the blk request to a READ did not fix the problem. The problem was only fixed by sending the extra two commands. The direction was just a side issue.
Is there a problem with sending the commands? If they don't bother unaffected devices, but it does fix a large number of other devices, what's the problem?
Signature · 3 lines
Ben Collins <[email protected]> Developer Ubuntu Linux
Quoted 15 lines · Ben Collins
Quoted 8 lines · Jens Axboe
Quoted 3 lines · Ben Collins
This patch fixes the WRITE vs READ issue, and also sends the extra two commands. Anyone with an iPod connected via USB (not sure about firewire) should be able to reproduce this issue, and verify the patch.
The bug was in the SCSI layer, and James already has the fix integrated for that. It really should make 2.6.15, James are you sending it upwards for that?
You mean this patch?
James Bottomley:
[SCSI] ConsolidateREQ_BLOCK_PChandling path (fix ipod panic)
Quoted 7 lines
This fixes an oops with data direction because sbp2 was not checking enough itself.
I seriously doubt this will fix the issue being reported. Changing the blk request to a READ did not fix the problem. The problem was only fixed by sending the extra two commands. The direction was just a side issue.
Your report surely made it seem like an issue, hence I pointed you at the fix(es). What part of the block layer didn't like these commands?
Quoted 3 lines
Is there a problem with sending the commands? If they don't bother unaffected devices, but it does fix a large number of other devices, what's the problem?
It's not necessarily a problem, especially if other paths end up doing it in some way or another. Hard to say whether it bothers other device, but I agree it should not. Please resend after 2.6.15 is released (and do make those macros functions, thanks).
Signature · 1 line
Jens Axboe