Revert "scsi: revert "[SCSI] Get rid of scsi_cmnd->done""
authorLinus Torvalds <torvalds@woody.linux-foundation.org>
Sun, 6 Jan 2008 18:17:12 +0000 (10:17 -0800)
committerLinus Torvalds <torvalds@woody.linux-foundation.org>
Sun, 6 Jan 2008 18:17:12 +0000 (10:17 -0800)
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>
drivers/scsi/scsi.c
drivers/scsi/scsi_error.c
drivers/scsi/scsi_lib.c
drivers/scsi/scsi_priv.h
drivers/scsi/sd.c
drivers/scsi/sr.c
include/scsi/scsi_cmnd.h
include/scsi/scsi_driver.h
include/scsi/sd.h

index 7ceb8209e5df416f93dac3916e4f11e1b75616ee..0fb1709ce5e371649c0c90126a3a8558a1fd13a4 100644 (file)
@@ -59,6 +59,7 @@
 #include <scsi/scsi_cmnd.h>
 #include <scsi/scsi_dbg.h>
 #include <scsi/scsi_device.h>
+#include <scsi/scsi_driver.h>
 #include <scsi/scsi_eh.h>
 #include <scsi/scsi_host.h>
 #include <scsi/scsi_tcq.h>
@@ -367,9 +368,8 @@ void scsi_log_send(struct scsi_cmnd *cmd)
                        scsi_print_command(cmd);
                        if (level > 3) {
                                printk(KERN_INFO "buffer = 0x%p, bufflen = %d,"
-                                      " done = 0x%p, queuecommand 0x%p\n",
+                                      " queuecommand 0x%p\n",
                                        scsi_sglist(cmd), scsi_bufflen(cmd),
-                                       cmd->done,
                                        cmd->device->host->hostt->queuecommand);
 
                        }
@@ -654,6 +654,12 @@ void __scsi_done(struct scsi_cmnd *cmd)
        blk_complete_request(rq);
 }
 
+/* Move this to a header if it becomes more generally useful */
+static struct scsi_driver *scsi_cmd_to_driver(struct scsi_cmnd *cmd)
+{
+       return *(struct scsi_driver **)cmd->request->rq_disk->private_data;
+}
+
 /*
  * Function:    scsi_finish_command
  *
@@ -665,6 +671,8 @@ void scsi_finish_command(struct scsi_cmnd *cmd)
 {
        struct scsi_device *sdev = cmd->device;
        struct Scsi_Host *shost = sdev->host;
+       struct scsi_driver *drv;
+       unsigned int good_bytes;
 
        scsi_device_unbusy(sdev);
 
@@ -690,7 +698,13 @@ void scsi_finish_command(struct scsi_cmnd *cmd)
                                "Notifying upper driver of completion "
                                "(result %x)\n", cmd->result));
 
-       cmd->done(cmd);
+       good_bytes = cmd->request_bufflen;
+        if (cmd->request->cmd_type != REQ_TYPE_BLOCK_PC) {
+               drv = scsi_cmd_to_driver(cmd);
+               if (drv->done)
+                       good_bytes = drv->done(cmd);
+       }
+       scsi_io_completion(cmd, good_bytes);
 }
 EXPORT_SYMBOL(scsi_finish_command);
 
index 70700b97c915a482210c8bcab5462d22b0a38889..ebaca4ca4a135787a6ff3fa477a722dbc4b2ec45 100644 (file)
@@ -1699,7 +1699,6 @@ scsi_reset_provider(struct scsi_device *dev, int flag)
        memset(&scmd->cmnd, '\0', sizeof(scmd->cmnd));
     
        scmd->scsi_done         = scsi_reset_provider_done_command;
-       scmd->done                      = NULL;
        scmd->request_buffer            = NULL;
        scmd->request_bufflen           = 0;
 
index 60f77c4b39468d17852e78a503602d42f470f58a..a9ac5b1b1667f339d5db22760e308b18ac9a3a71 100644 (file)
@@ -1092,7 +1092,6 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
        }
        scsi_end_request(cmd, 0, this_count, !result);
 }
-EXPORT_SYMBOL(scsi_io_completion);
 
 /*
  * Function:    scsi_init_io()
@@ -1171,18 +1170,6 @@ static struct scsi_cmnd *scsi_get_cmd_from_req(struct scsi_device *sdev,
        return cmd;
 }
 
-static void scsi_blk_pc_done(struct scsi_cmnd *cmd)
-{
-       BUG_ON(!blk_pc_request(cmd->request));
-       /*
-        * This will complete the whole command with uptodate=1 so
-        * as far as the block layer is concerned the command completed
-        * successfully. Since this is a REQ_BLOCK_PC command the
-        * caller should check the request's errors value
-        */
-       scsi_io_completion(cmd, cmd->request_bufflen);
-}
-
 int scsi_setup_blk_pc_cmnd(struct scsi_device *sdev, struct request *req)
 {
        struct scsi_cmnd *cmd;
@@ -1232,7 +1219,6 @@ int scsi_setup_blk_pc_cmnd(struct scsi_device *sdev, struct request *req)
        cmd->transfersize = req->data_len;
        cmd->allowed = req->retries;
        cmd->timeout_per_command = req->timeout;
-       cmd->done = scsi_blk_pc_done;
        return BLKPREP_OK;
 }
 EXPORT_SYMBOL(scsi_setup_blk_pc_cmnd);
index 1de52b6ded40c2dfdadfd8f23408f07d985856ac..3f34e9376b0aa148c8dafc58ee0c52c65849b1e1 100644 (file)
@@ -68,6 +68,7 @@ extern int scsi_maybe_unblock_host(struct scsi_device *sdev);
 extern void scsi_device_unbusy(struct scsi_device *sdev);
 extern int scsi_queue_insert(struct scsi_cmnd *cmd, int reason);
 extern void scsi_next_command(struct scsi_cmnd *cmd);
+extern void scsi_io_completion(struct scsi_cmnd *, unsigned int);
 extern void scsi_run_host_queues(struct Scsi_Host *shost);
 extern struct request_queue *scsi_alloc_queue(struct scsi_device *sdev);
 extern void scsi_free_queue(struct request_queue *q);
index cb85296d53842ae1bea20b88aa492d021412853d..a69b155f39a2b2a1c4134b440e2cd5404d478e10 100644 (file)
@@ -86,6 +86,19 @@ MODULE_ALIAS_SCSI_DEVICE(TYPE_DISK);
 MODULE_ALIAS_SCSI_DEVICE(TYPE_MOD);
 MODULE_ALIAS_SCSI_DEVICE(TYPE_RBC);
 
+static int  sd_revalidate_disk(struct gendisk *);
+static int  sd_probe(struct device *);
+static int  sd_remove(struct device *);
+static void sd_shutdown(struct device *);
+static int sd_suspend(struct device *, pm_message_t state);
+static int sd_resume(struct device *);
+static void sd_rescan(struct device *);
+static int sd_done(struct scsi_cmnd *);
+static void sd_read_capacity(struct scsi_disk *sdkp, unsigned char *buffer);
+static void scsi_disk_release(struct class_device *cdev);
+static void sd_print_sense_hdr(struct scsi_disk *, struct scsi_sense_hdr *);
+static void sd_print_result(struct scsi_disk *, int);
+
 static DEFINE_IDR(sd_index_idr);
 static DEFINE_SPINLOCK(sd_index_lock);
 
@@ -240,6 +253,7 @@ static struct scsi_driver sd_template = {
                .shutdown       = sd_shutdown,
        },
        .rescan                 = sd_rescan,
+       .done                   = sd_done,
 };
 
 /*
@@ -508,12 +522,6 @@ static int sd_prep_fn(struct request_queue *q, struct request *rq)
        SCpnt->allowed = SD_MAX_RETRIES;
        SCpnt->timeout_per_command = timeout;
 
-       /*
-        * This is the completion routine we use.  This is matched in terms
-        * of capability to this function.
-        */
-       SCpnt->done = sd_rw_intr;
-
        /*
         * This indicates that the command is ready from our end to be
         * queued.
@@ -887,13 +895,13 @@ static struct block_device_operations sd_fops = {
 };
 
 /**
- *     sd_rw_intr - bottom half handler: called when the lower level
+ *     sd_done - bottom half handler: called when the lower level
  *     driver has completed (successfully or otherwise) a scsi command.
  *     @SCpnt: mid-level's per command structure.
  *
  *     Note: potentially run from within an ISR. Must not block.
  **/
-static void sd_rw_intr(struct scsi_cmnd * SCpnt)
+static int sd_done(struct scsi_cmnd *SCpnt)
 {
        int result = SCpnt->result;
        unsigned int xfer_size = SCpnt->request_bufflen;
@@ -914,7 +922,7 @@ static void sd_rw_intr(struct scsi_cmnd * SCpnt)
        SCSI_LOG_HLCOMPLETE(1, scsi_print_result(SCpnt));
        if (sense_valid) {
                SCSI_LOG_HLCOMPLETE(1, scmd_printk(KERN_INFO, SCpnt,
-                                                  "sd_rw_intr: sb[respc,sk,asc,"
+                                                  "sd_done: sb[respc,sk,asc,"
                                                   "ascq]=%x,%x,%x,%x\n",
                                                   sshdr.response_code,
                                                   sshdr.sense_key, sshdr.asc,
@@ -986,7 +994,7 @@ static void sd_rw_intr(struct scsi_cmnd * SCpnt)
                break;
        }
  out:
-       scsi_io_completion(SCpnt, good_bytes);
+       return good_bytes;
 }
 
 static int media_not_present(struct scsi_disk *sdkp,
index a0c4e13d4dabbbe70a962e103e78577ce71c6803..c61999031141ca8912a3ac0cecd441bae53cc2db 100644 (file)
@@ -78,6 +78,7 @@ MODULE_ALIAS_SCSI_DEVICE(TYPE_WORM);
 
 static int sr_probe(struct device *);
 static int sr_remove(struct device *);
+static int sr_done(struct scsi_cmnd *);
 
 static struct scsi_driver sr_template = {
        .owner                  = THIS_MODULE,
@@ -86,6 +87,7 @@ static struct scsi_driver sr_template = {
                .probe          = sr_probe,
                .remove         = sr_remove,
        },
+       .done                   = sr_done,
 };
 
 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)
 }
  
 /*
- * rw_intr is the interrupt routine for the device driver.
+ * sr_done is the interrupt routine for the device driver.
  *
- * It will be notified on the end of a SCSI read / write, and will take on
+ * It will be notified on the end of a SCSI read / write, and will take one
  * of several actions based on success or failure.
  */
-static void rw_intr(struct scsi_cmnd * SCpnt)
+static int sr_done(struct scsi_cmnd *SCpnt)
 {
        int result = SCpnt->result;
        int this_count = SCpnt->request_bufflen;
@@ -286,12 +288,7 @@ static void rw_intr(struct scsi_cmnd * SCpnt)
                }
        }
 
-       /*
-        * This calls the generic completion function, now that we know
-        * how many actual sectors finished, and how many sectors we need
-        * to say have failed.
-        */
-       scsi_io_completion(SCpnt, good_bytes);
+       return good_bytes;
 }
 
 static int sr_prep_fn(struct request_queue *q, struct request *rq)
@@ -427,12 +424,6 @@ static int sr_prep_fn(struct request_queue *q, struct request *rq)
        SCpnt->allowed = MAX_RETRIES;
        SCpnt->timeout_per_command = timeout;
 
-       /*
-        * This is the completion routine we use.  This is matched in terms
-        * of capability to this function.
-        */
-       SCpnt->done = rw_intr;
-
        /*
         * This indicates that the command is ready from our end to be
         * queued.
index 7613c29893703bc52cf532f9ad1896e3444a904d..3f47e522a1ec3bedf6c85b9d30145c7208a626b5 100644 (file)
@@ -34,7 +34,6 @@ struct scsi_cmnd {
        struct list_head list;  /* scsi_cmnd participates in queue lists */
        struct list_head eh_entry; /* entry for the host eh_cmd_q */
        int eh_eflags;          /* Used by error handlr */
-       void (*done) (struct scsi_cmnd *);      /* Mid-level done function */
 
        /*
         * A SCSI Command is assigned a nonzero serial_number before passed
@@ -122,7 +121,6 @@ extern struct scsi_cmnd *__scsi_get_command(struct Scsi_Host *, gfp_t);
 extern void scsi_put_command(struct scsi_cmnd *);
 extern void __scsi_put_command(struct Scsi_Host *, struct scsi_cmnd *,
                               struct device *);
-extern void scsi_io_completion(struct scsi_cmnd *, unsigned int);
 extern void scsi_finish_command(struct scsi_cmnd *cmd);
 extern void scsi_req_abort_cmd(struct scsi_cmnd *cmd);
 
index 56a304709fde0acc626f0ec4e8a6740c1ee2851e..1f5ca7f621165797ff559203e99b5a1deebe1624 100644 (file)
@@ -15,6 +15,7 @@ struct scsi_driver {
        struct device_driver    gendrv;
 
        void (*rescan)(struct device *);
+       int (*done)(struct scsi_cmnd *);
 };
 #define to_scsi_driver(drv) \
        container_of((drv), struct scsi_driver, gendrv)
index aa1e716130108dc40e6b2681e879dfca453ba3e0..f7513313ef0d5d5bdd85194dfb6771ac294cc22d 100644 (file)
@@ -47,19 +47,6 @@ struct scsi_disk {
 };
 #define to_scsi_disk(obj) container_of(obj,struct scsi_disk,cdev)
 
-static int  sd_revalidate_disk(struct gendisk *disk);
-static void sd_rw_intr(struct scsi_cmnd * SCpnt);
-static int  sd_probe(struct device *);
-static int  sd_remove(struct device *);
-static void sd_shutdown(struct device *dev);
-static int sd_suspend(struct device *dev, pm_message_t state);
-static int sd_resume(struct device *dev);
-static void sd_rescan(struct device *);
-static void sd_read_capacity(struct scsi_disk *sdkp, unsigned char *buffer);
-static void scsi_disk_release(struct class_device *cdev);
-static void sd_print_sense_hdr(struct scsi_disk *, struct scsi_sense_hdr *);
-static void sd_print_result(struct scsi_disk *, int);
-
 #define sd_printk(prefix, sdsk, fmt, a...)                             \
         (sdsk)->disk ?                                                 \
        sdev_printk(prefix, (sdsk)->device, "[%s] " fmt,                \