diff options
author | Linus Torvalds <torvalds@woody.linux-foundation.org> | 2008-01-06 13:17:12 -0500 |
---|---|---|
committer | Linus Torvalds <torvalds@woody.linux-foundation.org> | 2008-01-06 13:17:12 -0500 |
commit | 7b3d9545f9ac8b31528dd2d6d8ec8d19922917b8 (patch) | |
tree | e8af5ec41abf8ec3a678b5643de5580db417d16f /drivers/scsi/sr.c | |
parent | 911833440b498e3e4fe2f12c1ae2bd44400c7004 (diff) |
Revert "scsi: revert "[SCSI] Get rid of scsi_cmnd->done""
This reverts commit ac40532ef0b8649e6f7f83859ea0de1c4ed08a19, which gets
us back the original cleanup of 6f5391c283d7fdcf24bf40786ea79061919d1e1d.
It turns out that the bug that was triggered by that commit was
apparently not actually triggered by that commit at all, and just the
testing conditions had changed enough to make it appear to be due to it.
The real problem seems to have been found by Peter Osterlund:
"pktcdvd sets it [block device size] when opening the /dev/pktcdvd
device, but when the drive is later opened as /dev/scd0, there is
nothing that sets it back. (Btw, 40944 is possible if the disk is a
CDRW that was formatted with "cdrwtool -m 10236".)
The problem is that pktcdvd opens the cd device in non-blocking mode
when pktsetup is run, and doesn't close it again until pktsetup -d is
run. The effect is that if you meanwhile open the cd device,
blkdev.c:do_open() doesn't call bd_set_size() because
bdev->bd_openers is non-zero."
In particular, to repeat the bug (regardless of whether commit
6f5391c283d7fdcf24bf40786ea79061919d1e1d is applied or not):
" 1. Start with an empty drive.
2. pktsetup 0 /dev/scd0
3. Insert a CD containing an isofs filesystem.
4. mount /dev/pktcdvd/0 /mnt/tmp
5. umount /mnt/tmp
6. Press the eject button.
7. Insert a DVD containing a non-writable filesystem.
8. mount /dev/scd0 /mnt/tmp
9. find /mnt/tmp -type f -print0 | xargs -0 sha1sum >/dev/null
10. If the DVD contains data beyond the physical size of a CD, you
get I/O errors in the terminal, and dmesg reports lots of
"attempt to access beyond end of device" errors."
which in turn is because the nested open after the media change won't
cause the size to be set properly (because the original open still holds
the block device, and we only do the bd_set_size() when we don't have
other people holding the device open).
The proper fix for that is probably to just do something like
bdev->bd_inode->i_size = (loff_t)get_capacity(disk)<<9;
in fs/block_dev.c:do_open() even for the cases where we're not the
original opener (but *not* call bd_set_size(), since that will also
change the block size of the device).
Cc: Peter Osterlund <petero2@telia.com>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
Cc: Matthew Wilcox <matthew@wil.cx>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Diffstat (limited to 'drivers/scsi/sr.c')
-rw-r--r-- | drivers/scsi/sr.c | 21 |
1 files changed, 6 insertions, 15 deletions
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c index a0c4e13d4dab..c61999031141 100644 --- a/drivers/scsi/sr.c +++ b/drivers/scsi/sr.c | |||
@@ -78,6 +78,7 @@ MODULE_ALIAS_SCSI_DEVICE(TYPE_WORM); | |||
78 | 78 | ||
79 | static int sr_probe(struct device *); | 79 | static int sr_probe(struct device *); |
80 | static int sr_remove(struct device *); | 80 | static int sr_remove(struct device *); |
81 | static int sr_done(struct scsi_cmnd *); | ||
81 | 82 | ||
82 | static struct scsi_driver sr_template = { | 83 | static struct scsi_driver sr_template = { |
83 | .owner = THIS_MODULE, | 84 | .owner = THIS_MODULE, |
@@ -86,6 +87,7 @@ static struct scsi_driver sr_template = { | |||
86 | .probe = sr_probe, | 87 | .probe = sr_probe, |
87 | .remove = sr_remove, | 88 | .remove = sr_remove, |
88 | }, | 89 | }, |
90 | .done = sr_done, | ||
89 | }; | 91 | }; |
90 | 92 | ||
91 | static unsigned long sr_index_bits[SR_DISKS / BITS_PER_LONG]; | 93 | static unsigned long sr_index_bits[SR_DISKS / BITS_PER_LONG]; |
@@ -208,12 +210,12 @@ static int sr_media_change(struct cdrom_device_info *cdi, int slot) | |||
208 | } | 210 | } |
209 | 211 | ||
210 | /* | 212 | /* |
211 | * rw_intr is the interrupt routine for the device driver. | 213 | * sr_done is the interrupt routine for the device driver. |
212 | * | 214 | * |
213 | * It will be notified on the end of a SCSI read / write, and will take on | 215 | * It will be notified on the end of a SCSI read / write, and will take one |
214 | * of several actions based on success or failure. | 216 | * of several actions based on success or failure. |
215 | */ | 217 | */ |
216 | static void rw_intr(struct scsi_cmnd * SCpnt) | 218 | static int sr_done(struct scsi_cmnd *SCpnt) |
217 | { | 219 | { |
218 | int result = SCpnt->result; | 220 | int result = SCpnt->result; |
219 | int this_count = SCpnt->request_bufflen; | 221 | int this_count = SCpnt->request_bufflen; |
@@ -286,12 +288,7 @@ static void rw_intr(struct scsi_cmnd * SCpnt) | |||
286 | } | 288 | } |
287 | } | 289 | } |
288 | 290 | ||
289 | /* | 291 | return good_bytes; |
290 | * This calls the generic completion function, now that we know | ||
291 | * how many actual sectors finished, and how many sectors we need | ||
292 | * to say have failed. | ||
293 | */ | ||
294 | scsi_io_completion(SCpnt, good_bytes); | ||
295 | } | 292 | } |
296 | 293 | ||
297 | static int sr_prep_fn(struct request_queue *q, struct request *rq) | 294 | static int sr_prep_fn(struct request_queue *q, struct request *rq) |
@@ -428,12 +425,6 @@ static int sr_prep_fn(struct request_queue *q, struct request *rq) | |||
428 | SCpnt->timeout_per_command = timeout; | 425 | SCpnt->timeout_per_command = timeout; |
429 | 426 | ||
430 | /* | 427 | /* |
431 | * This is the completion routine we use. This is matched in terms | ||
432 | * of capability to this function. | ||
433 | */ | ||
434 | SCpnt->done = rw_intr; | ||
435 | |||
436 | /* | ||
437 | * This indicates that the command is ready from our end to be | 428 | * This indicates that the command is ready from our end to be |
438 | * queued. | 429 | * queued. |
439 | */ | 430 | */ |