Revert "scsi: revert "[SCSI] Get rid of scsi_cmnd->done""

2008-01-06 Thread Linux Kernel Mailing List
Gitweb: 
http://git.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=7b3d9545f9ac8b31528dd2d6d8ec8d19922917b8
Commit: 7b3d9545f9ac8b31528dd2d6d8ec8d19922917b8
Parent: 911833440b498e3e4fe2f12c1ae2bd44400c7004
Author: Linus Torvalds <[EMAIL PROTECTED]>
AuthorDate: Sun Jan 6 10:17:12 2008 -0800
Committer:  Linus Torvalds <[EMAIL PROTECTED]>
CommitDate: Sun Jan 6 10:17:12 2008 -0800

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 <[EMAIL PROTECTED]>
Cc: James Bottomley <[EMAIL PROTECTED]>
Cc: Matthew Wilcox <[EMAIL PROTECTED]>
Cc: Ingo Molnar <[EMAIL PROTECTED]>
Cc: Andrew Morton <[EMAIL PROTECTED]>
Signed-off-by: Linus Torvalds <[EMAIL PROTECTED]>
---
 drivers/scsi/scsi.c|   20 +---
 drivers/scsi/scsi_error.c  |1 -
 drivers/scsi/scsi_lib.c|   14 --
 drivers/scsi/scsi_priv.h   |1 +
 drivers/scsi/sd.c  |   28 ++--
 drivers/scsi/sr.c  |   21 ++---
 include/scsi/scsi_cmnd.h   |2 --
 include/scsi/scsi_driver.h |1 +
 include/scsi/sd.h  |   13 -
 9 files changed, 43 insertions(+), 58 deletions(-)

diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 7ceb820..0fb1709 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -59,6 +59,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -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-

scsi: revert "[SCSI] Get rid of scsi_cmnd->done"

2008-01-02 Thread Linux Kernel Mailing List
Gitweb: 
http://git.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=ac40532ef0b8649e6f7f83859ea0de1c4ed08a19
Commit: ac40532ef0b8649e6f7f83859ea0de1c4ed08a19
Parent: 158a962422e4a54dc256b6a9b9562f3d30d34d9c
Author: Ingo Molnar <[EMAIL PROTECTED]>
AuthorDate: Wed Jan 2 17:25:34 2008 +0100
Committer:  Linus Torvalds <[EMAIL PROTECTED]>
CommitDate: Wed Jan 2 13:11:06 2008 -0800

scsi: revert "[SCSI] Get rid of scsi_cmnd->done"

This reverts commit 6f5391c283d7fdcf24bf40786ea79061919d1e1d ("[SCSI]
Get rid of scsi_cmnd->done") that was supposed to be a cleanup commit,
but apparently it causes regressions:

  Bug 9370 - v2.6.24-rc2-409-g9418d5d: attempt to access beyond end of 
device
  http://bugzilla.kernel.org/show_bug.cgi?id=9370

this patch should be reintroduced in a more split-up form to make
testing of it easier.

Signed-off-by: Ingo Molnar <[EMAIL PROTECTED]>
Acked-by: Matthew Wilcox <[EMAIL PROTECTED]>
Cc: James Bottomley <[EMAIL PROTECTED]>
Signed-off-by: Linus Torvalds <[EMAIL PROTECTED]>
---
 drivers/scsi/scsi.c|   20 +++-
 drivers/scsi/scsi_error.c  |1 +
 drivers/scsi/scsi_lib.c|   14 ++
 drivers/scsi/scsi_priv.h   |1 -
 drivers/scsi/sd.c  |   28 ++--
 drivers/scsi/sr.c  |   21 +++--
 include/scsi/scsi_cmnd.h   |2 ++
 include/scsi/scsi_driver.h |1 -
 include/scsi/sd.h  |   13 +
 9 files changed, 58 insertions(+), 43 deletions(-)

diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 0fb1709..7ceb820 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -59,7 +59,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -368,8 +367,9 @@ void scsi_log_send(struct scsi_cmnd *cmd)
scsi_print_command(cmd);
if (level > 3) {
printk(KERN_INFO "buffer = 0x%p, bufflen = %d,"
-  " queuecommand 0x%p\n",
+  " done = 0x%p, queuecommand 0x%p\n",
scsi_sglist(cmd), scsi_bufflen(cmd),
+   cmd->done,
cmd->device->host->hostt->queuecommand);
 
}
@@ -654,12 +654,6 @@ 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
  *
@@ -671,8 +665,6 @@ 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);
 
@@ -698,13 +690,7 @@ void scsi_finish_command(struct scsi_cmnd *cmd)
"Notifying upper driver of completion "
"(result %x)\n", cmd->result));
 
-   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);
+   cmd->done(cmd);
 }
 EXPORT_SYMBOL(scsi_finish_command);
 
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index ebaca4c..70700b9 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -1699,6 +1699,7 @@ 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;
 
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 0e81e4c..8df8267 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1092,6 +1092,7 @@ 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()
@@ -1170,6 +1171,18 @@ 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 complet