Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

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>

+43 -58
+17 -3
drivers/scsi/scsi.c
··· 59 59 #include <scsi/scsi_cmnd.h> 60 60 #include <scsi/scsi_dbg.h> 61 61 #include <scsi/scsi_device.h> 62 + #include <scsi/scsi_driver.h> 62 63 #include <scsi/scsi_eh.h> 63 64 #include <scsi/scsi_host.h> 64 65 #include <scsi/scsi_tcq.h> ··· 368 367 scsi_print_command(cmd); 369 368 if (level > 3) { 370 369 printk(KERN_INFO "buffer = 0x%p, bufflen = %d," 371 - " done = 0x%p, queuecommand 0x%p\n", 370 + " queuecommand 0x%p\n", 372 371 scsi_sglist(cmd), scsi_bufflen(cmd), 373 - cmd->done, 374 372 cmd->device->host->hostt->queuecommand); 375 373 376 374 } ··· 654 654 blk_complete_request(rq); 655 655 } 656 656 657 + /* Move this to a header if it becomes more generally useful */ 658 + static struct scsi_driver *scsi_cmd_to_driver(struct scsi_cmnd *cmd) 659 + { 660 + return *(struct scsi_driver **)cmd->request->rq_disk->private_data; 661 + } 662 + 657 663 /* 658 664 * Function: scsi_finish_command 659 665 * ··· 671 665 { 672 666 struct scsi_device *sdev = cmd->device; 673 667 struct Scsi_Host *shost = sdev->host; 668 + struct scsi_driver *drv; 669 + unsigned int good_bytes; 674 670 675 671 scsi_device_unbusy(sdev); 676 672 ··· 698 690 "Notifying upper driver of completion " 699 691 "(result %x)\n", cmd->result)); 700 692 701 - cmd->done(cmd); 693 + good_bytes = cmd->request_bufflen; 694 + if (cmd->request->cmd_type != REQ_TYPE_BLOCK_PC) { 695 + drv = scsi_cmd_to_driver(cmd); 696 + if (drv->done) 697 + good_bytes = drv->done(cmd); 698 + } 699 + scsi_io_completion(cmd, good_bytes); 702 700 } 703 701 EXPORT_SYMBOL(scsi_finish_command); 704 702
-1
drivers/scsi/scsi_error.c
··· 1699 1699 memset(&scmd->cmnd, '\0', sizeof(scmd->cmnd)); 1700 1700 1701 1701 scmd->scsi_done = scsi_reset_provider_done_command; 1702 - scmd->done = NULL; 1703 1702 scmd->request_buffer = NULL; 1704 1703 scmd->request_bufflen = 0; 1705 1704
-14
drivers/scsi/scsi_lib.c
··· 1092 1092 } 1093 1093 scsi_end_request(cmd, 0, this_count, !result); 1094 1094 } 1095 - EXPORT_SYMBOL(scsi_io_completion); 1096 1095 1097 1096 /* 1098 1097 * Function: scsi_init_io() ··· 1170 1171 return cmd; 1171 1172 } 1172 1173 1173 - static void scsi_blk_pc_done(struct scsi_cmnd *cmd) 1174 - { 1175 - BUG_ON(!blk_pc_request(cmd->request)); 1176 - /* 1177 - * This will complete the whole command with uptodate=1 so 1178 - * as far as the block layer is concerned the command completed 1179 - * successfully. Since this is a REQ_BLOCK_PC command the 1180 - * caller should check the request's errors value 1181 - */ 1182 - scsi_io_completion(cmd, cmd->request_bufflen); 1183 - } 1184 - 1185 1174 int scsi_setup_blk_pc_cmnd(struct scsi_device *sdev, struct request *req) 1186 1175 { 1187 1176 struct scsi_cmnd *cmd; ··· 1219 1232 cmd->transfersize = req->data_len; 1220 1233 cmd->allowed = req->retries; 1221 1234 cmd->timeout_per_command = req->timeout; 1222 - cmd->done = scsi_blk_pc_done; 1223 1235 return BLKPREP_OK; 1224 1236 } 1225 1237 EXPORT_SYMBOL(scsi_setup_blk_pc_cmnd);
+1
drivers/scsi/scsi_priv.h
··· 68 68 extern void scsi_device_unbusy(struct scsi_device *sdev); 69 69 extern int scsi_queue_insert(struct scsi_cmnd *cmd, int reason); 70 70 extern void scsi_next_command(struct scsi_cmnd *cmd); 71 + extern void scsi_io_completion(struct scsi_cmnd *, unsigned int); 71 72 extern void scsi_run_host_queues(struct Scsi_Host *shost); 72 73 extern struct request_queue *scsi_alloc_queue(struct scsi_device *sdev); 73 74 extern void scsi_free_queue(struct request_queue *q);
+18 -10
drivers/scsi/sd.c
··· 86 86 MODULE_ALIAS_SCSI_DEVICE(TYPE_MOD); 87 87 MODULE_ALIAS_SCSI_DEVICE(TYPE_RBC); 88 88 89 + static int sd_revalidate_disk(struct gendisk *); 90 + static int sd_probe(struct device *); 91 + static int sd_remove(struct device *); 92 + static void sd_shutdown(struct device *); 93 + static int sd_suspend(struct device *, pm_message_t state); 94 + static int sd_resume(struct device *); 95 + static void sd_rescan(struct device *); 96 + static int sd_done(struct scsi_cmnd *); 97 + static void sd_read_capacity(struct scsi_disk *sdkp, unsigned char *buffer); 98 + static void scsi_disk_release(struct class_device *cdev); 99 + static void sd_print_sense_hdr(struct scsi_disk *, struct scsi_sense_hdr *); 100 + static void sd_print_result(struct scsi_disk *, int); 101 + 89 102 static DEFINE_IDR(sd_index_idr); 90 103 static DEFINE_SPINLOCK(sd_index_lock); 91 104 ··· 253 240 .shutdown = sd_shutdown, 254 241 }, 255 242 .rescan = sd_rescan, 243 + .done = sd_done, 256 244 }; 257 245 258 246 /* ··· 521 507 SCpnt->underflow = this_count << 9; 522 508 SCpnt->allowed = SD_MAX_RETRIES; 523 509 SCpnt->timeout_per_command = timeout; 524 - 525 - /* 526 - * This is the completion routine we use. This is matched in terms 527 - * of capability to this function. 528 - */ 529 - SCpnt->done = sd_rw_intr; 530 510 531 511 /* 532 512 * This indicates that the command is ready from our end to be ··· 895 887 }; 896 888 897 889 /** 898 - * sd_rw_intr - bottom half handler: called when the lower level 890 + * sd_done - bottom half handler: called when the lower level 899 891 * driver has completed (successfully or otherwise) a scsi command. 900 892 * @SCpnt: mid-level's per command structure. 901 893 * 902 894 * Note: potentially run from within an ISR. Must not block. 903 895 **/ 904 - static void sd_rw_intr(struct scsi_cmnd * SCpnt) 896 + static int sd_done(struct scsi_cmnd *SCpnt) 905 897 { 906 898 int result = SCpnt->result; 907 899 unsigned int xfer_size = SCpnt->request_bufflen; ··· 922 914 SCSI_LOG_HLCOMPLETE(1, scsi_print_result(SCpnt)); 923 915 if (sense_valid) { 924 916 SCSI_LOG_HLCOMPLETE(1, scmd_printk(KERN_INFO, SCpnt, 925 - "sd_rw_intr: sb[respc,sk,asc," 917 + "sd_done: sb[respc,sk,asc," 926 918 "ascq]=%x,%x,%x,%x\n", 927 919 sshdr.response_code, 928 920 sshdr.sense_key, sshdr.asc, ··· 994 986 break; 995 987 } 996 988 out: 997 - scsi_io_completion(SCpnt, good_bytes); 989 + return good_bytes; 998 990 } 999 991 1000 992 static int media_not_present(struct scsi_disk *sdkp,
+6 -15
drivers/scsi/sr.c
··· 78 78 79 79 static int sr_probe(struct device *); 80 80 static int sr_remove(struct device *); 81 + static int sr_done(struct scsi_cmnd *); 81 82 82 83 static struct scsi_driver sr_template = { 83 84 .owner = THIS_MODULE, ··· 87 86 .probe = sr_probe, 88 87 .remove = sr_remove, 89 88 }, 89 + .done = sr_done, 90 90 }; 91 91 92 92 static unsigned long sr_index_bits[SR_DISKS / BITS_PER_LONG]; ··· 210 208 } 211 209 212 210 /* 213 - * rw_intr is the interrupt routine for the device driver. 211 + * sr_done is the interrupt routine for the device driver. 214 212 * 215 - * It will be notified on the end of a SCSI read / write, and will take on 213 + * It will be notified on the end of a SCSI read / write, and will take one 216 214 * of several actions based on success or failure. 217 215 */ 218 - static void rw_intr(struct scsi_cmnd * SCpnt) 216 + static int sr_done(struct scsi_cmnd *SCpnt) 219 217 { 220 218 int result = SCpnt->result; 221 219 int this_count = SCpnt->request_bufflen; ··· 288 286 } 289 287 } 290 288 291 - /* 292 - * This calls the generic completion function, now that we know 293 - * how many actual sectors finished, and how many sectors we need 294 - * to say have failed. 295 - */ 296 - scsi_io_completion(SCpnt, good_bytes); 289 + return good_bytes; 297 290 } 298 291 299 292 static int sr_prep_fn(struct request_queue *q, struct request *rq) ··· 423 426 SCpnt->underflow = this_count << 9; 424 427 SCpnt->allowed = MAX_RETRIES; 425 428 SCpnt->timeout_per_command = timeout; 426 - 427 - /* 428 - * This is the completion routine we use. This is matched in terms 429 - * of capability to this function. 430 - */ 431 - SCpnt->done = rw_intr; 432 429 433 430 /* 434 431 * This indicates that the command is ready from our end to be
-2
include/scsi/scsi_cmnd.h
··· 34 34 struct list_head list; /* scsi_cmnd participates in queue lists */ 35 35 struct list_head eh_entry; /* entry for the host eh_cmd_q */ 36 36 int eh_eflags; /* Used by error handlr */ 37 - void (*done) (struct scsi_cmnd *); /* Mid-level done function */ 38 37 39 38 /* 40 39 * A SCSI Command is assigned a nonzero serial_number before passed ··· 121 122 extern void scsi_put_command(struct scsi_cmnd *); 122 123 extern void __scsi_put_command(struct Scsi_Host *, struct scsi_cmnd *, 123 124 struct device *); 124 - extern void scsi_io_completion(struct scsi_cmnd *, unsigned int); 125 125 extern void scsi_finish_command(struct scsi_cmnd *cmd); 126 126 extern void scsi_req_abort_cmd(struct scsi_cmnd *cmd); 127 127
+1
include/scsi/scsi_driver.h
··· 15 15 struct device_driver gendrv; 16 16 17 17 void (*rescan)(struct device *); 18 + int (*done)(struct scsi_cmnd *); 18 19 }; 19 20 #define to_scsi_driver(drv) \ 20 21 container_of((drv), struct scsi_driver, gendrv)
-13
include/scsi/sd.h
··· 47 47 }; 48 48 #define to_scsi_disk(obj) container_of(obj,struct scsi_disk,cdev) 49 49 50 - static int sd_revalidate_disk(struct gendisk *disk); 51 - static void sd_rw_intr(struct scsi_cmnd * SCpnt); 52 - static int sd_probe(struct device *); 53 - static int sd_remove(struct device *); 54 - static void sd_shutdown(struct device *dev); 55 - static int sd_suspend(struct device *dev, pm_message_t state); 56 - static int sd_resume(struct device *dev); 57 - static void sd_rescan(struct device *); 58 - static void sd_read_capacity(struct scsi_disk *sdkp, unsigned char *buffer); 59 - static void scsi_disk_release(struct class_device *cdev); 60 - static void sd_print_sense_hdr(struct scsi_disk *, struct scsi_sense_hdr *); 61 - static void sd_print_result(struct scsi_disk *, int); 62 - 63 50 #define sd_printk(prefix, sdsk, fmt, a...) \ 64 51 (sdsk)->disk ? \ 65 52 sdev_printk(prefix, (sdsk)->device, "[%s] " fmt, \