Re: [PATCH ] scsi-misc-2.6: File System going into read-only mode

[James Bottomley <James.Bottomley@suse.de>]

On Tue, 2009-09-29 at 14:17 +0530, Penchala Narasimha Reddy Chilakala,
TLS-Chennai wrote:

> @@ -593,10 +593,10 @@ static int aac_send_raw_srb(struct aac_d
>                                 u64 addr;
>                                 void* p;
>                                 if (upsg->sg[i].count >
> -                                   (dev->adapter_info.options &
> +                                   ((dev->adapter_info.options &
>                                      AAC_OPT_NEW_COMM) ?
>                                       (dev->scsi_host_ptr->max_sectors
> << 9) :
> -                                     65536) {
> +                                     65536)) {
>                                         rcode = -EINVAL;
>                                         goto cleanup;
>                                 }
> @@ -645,10 +645,10 @@ static int aac_send_raw_srb(struct aac_d
>                                 u64 addr;
>                                 void* p;
>                                 if (usg->sg[i].count >
> -                                   (dev->adapter_info.options &
> +                                   ((dev->adapter_info.options &
>                                      AAC_OPT_NEW_COMM) ?
>                                       (dev->scsi_host_ptr->max_sectors
> << 9) :
> -                                     65536) {
> +                                     65536)) {
>                                         rcode = -EINVAL;
>                                         goto cleanup;
>                                 }
> @@ -695,10 +695,10 @@ static int aac_send_raw_srb(struct aac_d
>                                 uintptr_t addr;
>                                 void* p;
>                                 if (usg->sg[i].count >
> -                                   (dev->adapter_info.options &
> +                                   ((dev->adapter_info.options &
>                                      AAC_OPT_NEW_COMM) ?
>                                       (dev->scsi_host_ptr->max_sectors
> << 9) :
> -                                     65536) {
> +                                     65536)) {
>                                         rcode = -EINVAL;
>                                         goto cleanup;
>                                 }
> @@ -734,10 +734,10 @@ static int aac_send_raw_srb(struct aac_d
>                                 dma_addr_t addr;
>                                 void* p;
>                                 if (upsg->sg[i].count >
> -                                   (dev->adapter_info.options &
> +                                   ((dev->adapter_info.options &
>                                      AAC_OPT_NEW_COMM) ?
>                                       (dev->scsi_host_ptr->max_sectors
> << 9) :
> -                                     65536) {
> +                                     65536)) {
>                                         rcode = -EINVAL;
>                                         goto cleanup;
>                                 }

This is all spurious bracket addition; it doesn't really have any place
in a code fix.


> @@ -842,13 +842,22 @@ static int aac_get_pci_info(struct aac_d
>  int aac_do_ioctl(struct aac_dev * dev, int cmd, void __user *arg)
>  {
>         int status;
> +       unsigned long mflags;
>  
>         /*
>          *      HBA gets first crack
>          */
>  
> +       spin_lock_irqsave(&dev->manage_lock, mflags);
> +       if (dev->management_fib_count > AAC_NUM_MGT_FIB) {
> +               printk(KERN_INFO "No management Fibs Available:%d\n",
> +                                               dev->management_fib_count);
> +               spin_unlock_irqrestore(&dev->manage_lock, mflags);
> +               return -EBUSY;
> +       }
> +       spin_unlock_irqrestore(&dev->manage_lock, mflags);
>         status = aac_dev_ioctl(dev, cmd, arg);

I can see what you're trying to do: limit the number of ioctl created
fibs to prevent starvation, but this mechanism is completely racy.  The
check is too far away from the increment.  It looks fixable just by
doing the increment within the lock here and decrementing if something
fails.

James