All of lore.kernel.org
 help / color / mirror / Atom feed
From: Douglas Gilbert <dougg@torque.net>
To: Peter Osterlund <petero2@telia.com>, linux-kernel@vger.kernel.org
Cc: linux-scsi@vger.kernel.org, Jens Axboe <axboe@suse.de>
Subject: Re: CDROM ioctl bug (fwd)
Date: Thu, 29 Nov 2001 09:21:14 -0500	[thread overview]
Message-ID: <3C06445A.2551CFA9@torque.net> (raw)

Peter,
That patch is flawed as Jens and I found out the hard way
in the sg driver. The scsi_do_req() can lead to the pointer
chain on the following assignment into q being invalid (in 
the worst case).

The easy fix is to move the assignment into q _before_
the call to scsi_do_req().

Doug Gilbert


vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
Peter Osterlund <petero2@telia.com> wrote:

Ron Lawrence <rlawrence@netraverse.com> writes:

> busy. Here are the symptoms of my problem : doing reads from a CDROM
> device intermingled with CDROM_MEDIA_CHANGED ioctls causes long pauses
> during the ioctl. This behavior started in 2.4.10. The ioctl can take a
> very long time to return, especially if reading large chunks.

This patch fixes the problem for my USB CDROM device. Maybe a similar
patch is needed for the IDE case, I haven't looked yet.

In general, who is responsible for unplugging the request queue after
queuing an ioctl command?

--- linux/drivers/scsi/scsi.c.old       Thu Nov 29 00:42:16 2001
+++ linux/drivers/scsi/scsi.c   Thu Nov 29 00:32:28 2001
@@ -767,14 +767,17 @@
 void scsi_wait_req (Scsi_Request * SRpnt, const void *cmnd ,
                  void *buffer, unsigned bufflen, 
                  int timeout, int retries)
 {
+       request_queue_t *q;
        DECLARE_COMPLETION(wait);
        
        SRpnt->sr_request.waiting = &wait;
        SRpnt->sr_request.rq_status = RQ_SCSI_BUSY;
        scsi_do_req (SRpnt, (void *) cmnd,
                buffer, bufflen, scsi_wait_done, timeout, retries);
+       q = &SRpnt->sr_device->request_queue;
+       generic_unplug_device(q);
        wait_for_completion(&wait);
        SRpnt->sr_request.waiting = NULL;
        if( SRpnt->sr_command != NULL )
        {

             reply	other threads:[~2001-11-29 14:20 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2001-11-29 14:21 Douglas Gilbert [this message]
  -- strict thread matches above, loose matches on Subject: below --
2001-11-28 18:44 CDROM ioctl bug (fwd) Ron Lawrence
2001-11-28 23:51 ` Peter Osterlund
2001-11-29 17:17   ` Ron Lawrence
2001-11-29 17:27   ` Jens Axboe
2001-11-29 18:38     ` Ron Lawrence
2001-11-29 18:48       ` Jens Axboe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3C06445A.2551CFA9@torque.net \
    --to=dougg@torque.net \
    --cc=axboe@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=petero2@telia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.