Re: [PATCH]: scsi_dh_rdac: fix BUG_ON in get_rdac_data from within send_mode_select

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----Original Message-----
> From: Menny_Hamburger@xxxxxxxx [mailto:Menny_Hamburger@xxxxxxxx]
> Sent: Tuesday, February 01, 2011 2:03 AM
> To: Moger, Babu
> Cc: dm-devel@xxxxxxxxxx
> Subject: RE:  [PATCH]: scsi_dh_rdac: fix BUG_ON in get_rdac_data
> from within send_mode_select
> 
> Hi Babu,
> 
> I ran the tests with my earlier patch intact.
> 
> I reviewed your patches and I can see how they solve the same issue.
> I also acknowledge that we have two different approaches here:
> 1) The patches you suggest solves the issue in a wider range (for all SCSI
> device handlers).
>    The patches are quite complex (several pieces of code) so the potential
> for bugs is higher, and they will require a wider range of testing.
> 2) The patch I submitted solves this issue only in the RDAC hardware
> driver.
>    Since we do have time to market issues involved, if the patch has no
> logical flaws and solves the problem locally I think I will apply it
>    on our side.
> 
> I would like to suggest these action items:
> 1) After you modify the patches to fit the current code, I will test the
> patches here with our RDAC environment that seems quite adequate for
> detecting
>    these problems.
> 2) Please review my patch and see if it has any logical flaws. I tested
> the patch in our test scenario a few hundreds of times and it
>    I seems to be doing what it's supposed to.
> 
> Best Regards,
> Menny
> This panic comes from get_rdac_data, which is called from within the
> send_mode_select code.
> My analysis picked up the following problem:
> The RDAC detach code nullifies the scsi_dh_data before flushing the mode
> select workqueue, and as result a pending send_mode_select will BUG_ON in
> get_rdac_data.
> The following patch (tested over RHEL54) flushes the workqueue before
> setting scsi_dh_data to NULL:


Your code looks bit different than what I have. I don't have RHEL5.4. I looked at RHEL 5.3. I don't see get_rdac_data in send_mode_select. However, I got the logic behind your code. Your logic helps in cases where get_rdac_data is used in the same context as mode select submission. But, if you use get_rdac_data in other places then this code does not help.  I see get_rdac_data in rdac_activate which is used before send_mode_select.

 
> diff -r -U 2 a/drivers/scsi/device_handler/scsi_dh_rdac.c
> b/drivers/scsi/device_handler/scsi_dh_rdac.c
> --- a/drivers/scsi/device_handler/scsi_dh_rdac.c        2011-01-30
> 11:02:31.271426000 +0200
> +++ b/drivers/scsi/device_handler/scsi_dh_rdac.c        2011-01-30
> 11:02:31.327069000 +0200
> @@ -853,8 +853,22 @@
>         spin_lock_irqsave(sdev->request_queue->queue_lock, flags);
>         scsi_dh_data = retrieve_scsi_dh_data(sdev);
> -       store_scsi_dh_data(sdev, NULL);
>         spin_unlock_irqrestore(sdev->request_queue->queue_lock, flags);
> 
>         h = (struct rdac_dh_data *) scsi_dh_data->buf;
> +       if (h->ctlr) {
> +               int flush;
> +
> +               spin_lock(&h->ctlr->ms_lock);
> +               flush = (h->ctlr->ms_sdev == sdev);
> +               spin_unlock(&h->ctlr->ms_lock);
> +
> +               if (flush)
> +                       flush_workqueue(kmpath_rdacd);
> +       }
> +
> +       spin_lock_irqsave(sdev->request_queue->queue_lock, flags);
> +       store_scsi_dh_data(sdev, NULL);
> +       spin_unlock_irqrestore(sdev->request_queue->queue_lock, flags);
> +
>         if (h->ctlr)
>                 kref_put(&h->ctlr->kref, release_controller);
> 
> 
> 
> The patch ensures that we run flush only when detaching the device the
> "owns" the send_mode_select (ms_sdev).
> 
> Menny
> 
> 


--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel


[Index of Archives]     [DM Crypt]     [Fedora Desktop]     [ATA RAID]     [Fedora Marketing]     [Fedora Packaging]     [Fedora SELinux]     [Yosemite Discussion]     [KDE Users]     [Fedora Docs]

  Powered by Linux