[patch 8/8] ide-scsi: add basic refcounting

From: Bartlomiej Zolnierkiewicz (bzolnier_at_elka.pw.edu.pl)
Date: 01/22/05

  • Next message: William Lee Irwin III: "Re: Kernel Panic with LTP on 2.6.11-rc1 (was Re: LTP Results for 2.6.x and 2.4.x)"
    Date:	Sat, 22 Jan 2005 00:38:42 +0100 (CET)
    To: linux-ide@vger.kernel.org
    
    

    * pointers to a SCSI host and a drive are added to idescsi_scsi_t
    * pointer to the SCSI host is stored in disk->private_data
    * ide_scsi_{get,put}() is used to {get,put} reference to the SCSI host

    Unfortunately this is not complete fix for ->open() vs ->cleanup()
    race, there are two TODO items left (as stated by FIXMEs):

    * don't use drive->driver_data
    * add driver's private struct gendisk [already DONE]

    Last minute note: it seems we can't get rid of drive->driver_data
    because of interactions between ide-scsi and IDE core (no surprise),
    drive->driver_data should be set to NULL when SCSI host object is
    being destroyed - callback from scsi_host_dev_release() is needed.

    This will help in fixing another problem - for IDE we need to wait
    until driver is really no longer using the device because we have more
    than 1 driver for the same class of devices (ide-{cd,floppy,tape} vs
    ide-scsi, also there is ide-default hack but it will die soon).

    diff -Nru a/drivers/scsi/ide-scsi.c b/drivers/scsi/ide-scsi.c
    --- a/drivers/scsi/ide-scsi.c 2005-01-21 23:41:42 +01:00
    +++ b/drivers/scsi/ide-scsi.c 2005-01-21 23:41:42 +01:00
    @@ -96,14 +96,39 @@
      */
     #define IDESCSI_LOG_CMD 0 /* Log SCSI commands */

    -typedef struct {
    - ide_drive_t *drive;
    +typedef struct ide_scsi_obj {
    + ide_drive_t *drive;
    + struct Scsi_Host *host;
    +
             idescsi_pc_t *pc; /* Current packet command */
             unsigned long flags; /* Status/Action flags */
             unsigned long transform; /* SCSI cmd translation layer */
             unsigned long log; /* log flags */
     } idescsi_scsi_t;

    +static DECLARE_MUTEX(idescsi_ref_sem);
    +
    +#define ide_scsi_g(disk) ((disk)->private_data)
    +
    +static struct ide_scsi_obj *ide_scsi_get(struct gendisk *disk)
    +{
    + struct ide_scsi_obj *scsi = NULL;
    +
    + down(&idescsi_ref_sem);
    + scsi = ide_scsi_g(disk);
    + if (scsi)
    + scsi_host_get(scsi->host);
    + up(&idescsi_ref_sem);
    + return scsi;
    +}
    +
    +static void ide_scsi_put(struct ide_scsi_obj *scsi)
    +{
    + down(&idescsi_ref_sem);
    + scsi_host_put(scsi->host);
    + up(&idescsi_ref_sem);
    +}
    +
     static inline idescsi_scsi_t *scsihost_to_idescsi(struct Scsi_Host *host)
     {
             return (idescsi_scsi_t*) (&host[1]);
    @@ -693,16 +718,20 @@
     static int idescsi_cleanup (ide_drive_t *drive)
     {
             struct Scsi_Host *scsihost = drive->driver_data;
    + struct gendisk *g = drive->disk;

             if (ide_unregister_subdriver(drive))
                     return 1;
    -
    - /* FIXME?: Are these two statements necessary? */
    +
    + /* FIXME: drive->driver_data shouldn't be used */
             drive->driver_data = NULL;
    - drive->disk->fops = ide_fops;
    + /* FIXME: add driver's private struct gendisk */
    + g->private_data = NULL;
    + g->fops = ide_fops;

             scsi_remove_host(scsihost);
    - scsi_host_put(scsihost);
    + ide_scsi_put(scsihost_to_idescsi(scsihost));
    +
             return 0;
     }

    @@ -739,15 +768,30 @@

     static int idescsi_ide_open(struct inode *inode, struct file *filp)
     {
    - ide_drive_t *drive = inode->i_bdev->bd_disk->private_data;
    + struct gendisk *disk = inode->i_bdev->bd_disk;
    + struct ide_scsi_obj *scsi;
    + ide_drive_t *drive;
    +
    + if (!(scsi = ide_scsi_get(disk)))
    + return -ENXIO;
    +
    + drive = scsi->drive;
    +
             drive->usage++;
    +
             return 0;
     }

     static int idescsi_ide_release(struct inode *inode, struct file *filp)
     {
    - ide_drive_t *drive = inode->i_bdev->bd_disk->private_data;
    + struct gendisk *disk = inode->i_bdev->bd_disk;
    + struct ide_scsi_obj *scsi = ide_scsi_g(disk);
    + ide_drive_t *drive = scsi->drive;
    +
             drive->usage--;
    +
    + ide_scsi_put(scsi);
    +
             return 0;
     }

    @@ -755,8 +799,8 @@
                             unsigned int cmd, unsigned long arg)
     {
             struct block_device *bdev = inode->i_bdev;
    - ide_drive_t *drive = bdev->bd_disk->private_data;
    - return generic_ide_ioctl(drive, file, bdev, cmd, arg);
    + struct ide_scsi_obj *scsi = ide_scsi_g(bdev->bd_disk);
    + return generic_ide_ioctl(scsi->drive, file, bdev, cmd, arg);
     }

     static struct block_device_operations idescsi_ops = {
    @@ -1043,6 +1087,7 @@
     {
             idescsi_scsi_t *idescsi;
             struct Scsi_Host *host;
    + struct gendisk *g = drive->disk;
             static int warned;
             int err;

    @@ -1071,10 +1116,12 @@
             drive->driver_data = host;
             idescsi = scsihost_to_idescsi(host);
             idescsi->drive = drive;
    + idescsi->host = host;
             err = ide_register_subdriver(drive, &idescsi_driver);
             if (!err) {
                     idescsi_setup (drive, idescsi);
    - drive->disk->fops = &idescsi_ops;
    + g->fops = &idescsi_ops;
    + g->private_data = idescsi;
                     err = scsi_add_host(host, &drive->gendev);
                     if (!err) {
                             scsi_scan_host(host);
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/


  • Next message: William Lee Irwin III: "Re: Kernel Panic with LTP on 2.6.11-rc1 (was Re: LTP Results for 2.6.x and 2.4.x)"

    Relevant Pages

    • [PATCH 2/3] kernel-doc: print arrays in declarations correctly.
      ... Do not convert arrays into pointers while generating documentation for them. ... struct sk_buff { ... send the line "unsubscribe linux-kernel" in ...
      (Linux-Kernel)
    • Re: kblockd/1: page allocation failure in 2.6.9
      ... On Thu, Dec 23 2004, Frank Steiner wrote: ... This looks fishy - this is GFP_ATOMIC | GFP_DMA, ... way I can see this fail is if the scsi host free_list is not filled for ... send the line "unsubscribe linux-kernel" in ...
      (Linux-Kernel)
    • Re: [BK PATCH] USB update for 2.6.3
      ... > pointers attached to the struct device inherited from the bus ... platform-specific pointer to whatever data structure that platform needs ... pointers wouldn't have any function pointers there, ... send the line "unsubscribe linux-kernel" in ...
      (Linux-Kernel)
    • Re: RFC [patch 13/34] PID Virtualization Define new task_pid api
      ... linked list of struct task_weakref in the struct task_struct, and when the task struct is about to go away, run around all of the weakrefs and change their pointers to NULL. ... it because life wouldn't have any meaning for them if they didn't. ... send the line "unsubscribe linux-kernel" in ...
      (Linux-Kernel)
    • Re: [ERROR] in ini9100.c scsi driver
      ... and here it's the debug output that the kernel show's me! ... This is not a safe way to run your SCSI host ... The error handling must be added to this driver ... send the line "unsubscribe linux-kernel" in ...
      (Linux-Kernel)