[bug report] scsi: megaraid_sas: Add the Support for SAS3.5 Generic Megaraid Controllers Capabilities

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

 



Hello Sasikumar Chandrasekaran,

This is a semi-automatic email about new static checker warnings.

The patch 9581ebebbe35: "scsi: megaraid_sas: Add the Support for 
SAS3.5 Generic Megaraid Controllers Capabilities" from Jan 10, 2017, 
leads to the following Smatch complaint:

drivers/scsi/megaraid/megaraid_sas_base.c:5245 megasas_init_fw()
	 error: we previously assumed 'fusion' could be null (see line 5069)

drivers/scsi/megaraid/megaraid_sas_base.c
  5068	
  5069		if (fusion)
                    ^^^^^^
Patch introduces a new NULL check.

  5070			instance->instancet = &megasas_instance_template_fusion;
  5071		else {
  5072			switch (instance->pdev->device) {
  5073			case PCI_DEVICE_ID_LSI_SAS1078R:
  5074			case PCI_DEVICE_ID_LSI_SAS1078DE:
  5075				instance->instancet = &megasas_instance_template_ppc;
  5076				break;
  5077			case PCI_DEVICE_ID_LSI_SAS1078GEN2:
  5078			case PCI_DEVICE_ID_LSI_SAS0079GEN2:
  5079				instance->instancet = &megasas_instance_template_gen2;
  5080				break;
  5081			case PCI_DEVICE_ID_LSI_SAS0073SKINNY:
  5082			case PCI_DEVICE_ID_LSI_SAS0071SKINNY:
  5083				instance->instancet = &megasas_instance_template_skinny;
  5084				break;
  5085			case PCI_DEVICE_ID_LSI_SAS1064R:
  5086			case PCI_DEVICE_ID_DELL_PERC5:
  5087			default:
  5088				instance->instancet = &megasas_instance_template_xscale;
  5089				instance->pd_list_not_supported = 1;
  5090				break;
  5091			}
  5092		}
  5093	
  5094		if (megasas_transition_to_ready(instance, 0)) {
  5095			atomic_set(&instance->fw_reset_no_pci_access, 1);
  5096			instance->instancet->adp_reset
  5097				(instance, instance->reg_set);
  5098			atomic_set(&instance->fw_reset_no_pci_access, 0);
  5099			dev_info(&instance->pdev->dev,
  5100				"FW restarted successfully from %s!\n",
  5101				__func__);
  5102	
  5103			/*waitting for about 30 second before retry*/
  5104			ssleep(30);
  5105	
  5106			if (megasas_transition_to_ready(instance, 0))
  5107				goto fail_ready_state;
  5108		}
  5109	
  5110		if (instance->is_ventura) {
  5111			scratch_pad_3 =
  5112				readl(&instance->reg_set->outbound_scratch_pad_3);
  5113	#if VD_EXT_DEBUG
  5114			dev_info(&instance->pdev->dev, "scratch_pad3 0x%x\n",
  5115				scratch_pad_3);
  5116	#endif
  5117			instance->max_raid_mapsize = ((scratch_pad_3 >>
  5118				MR_MAX_RAID_MAP_SIZE_OFFSET_SHIFT) &
  5119				MR_MAX_RAID_MAP_SIZE_MASK);
  5120		}
  5121	
  5122		/* Check if MSI-X is supported while in ready state */
  5123		msix_enable = (instance->instancet->read_fw_status_reg(reg_set) &
  5124			       0x4000000) >> 0x1a;
  5125		if (msix_enable && !msix_disable) {
  5126			int irq_flags = PCI_IRQ_MSIX;
  5127	
  5128			scratch_pad_2 = readl
  5129				(&instance->reg_set->outbound_scratch_pad_2);
  5130			/* Check max MSI-X vectors */
  5131			if (fusion) {
  5132				if (fusion->adapter_type == THUNDERBOLT_SERIES) { /* Thunderbolt Series*/
  5133					instance->msix_vectors = (scratch_pad_2
  5134						& MR_MAX_REPLY_QUEUES_OFFSET) + 1;
  5135					fw_msix_count = instance->msix_vectors;
  5136				} else { /* Invader series supports more than 8 MSI-x vectors*/
  5137					instance->msix_vectors = ((scratch_pad_2
  5138						& MR_MAX_REPLY_QUEUES_EXT_OFFSET)
  5139						>> MR_MAX_REPLY_QUEUES_EXT_OFFSET_SHIFT) + 1;
  5140					if (instance->msix_vectors > 16)
  5141						instance->msix_combined = true;
  5142	
  5143					if (rdpq_enable)
  5144						instance->is_rdpq = (scratch_pad_2 & MR_RDPQ_MODE_OFFSET) ?
  5145									1 : 0;
  5146					fw_msix_count = instance->msix_vectors;
  5147					/* Save 1-15 reply post index address to local memory
  5148					 * Index 0 is already saved from reg offset
  5149					 * MPI2_REPLY_POST_HOST_INDEX_OFFSET
  5150					 */
  5151					for (loop = 1; loop < MR_MAX_MSIX_REG_ARRAY; loop++) {
  5152						instance->reply_post_host_index_addr[loop] =
  5153							(u32 __iomem *)
  5154							((u8 __iomem *)instance->reg_set +
  5155							MPI2_SUP_REPLY_POST_HOST_INDEX_OFFSET
  5156							+ (loop * 0x10));
  5157					}
  5158				}
  5159				if (msix_vectors)
  5160					instance->msix_vectors = min(msix_vectors,
  5161						instance->msix_vectors);
  5162			} else /* MFI adapters */
  5163				instance->msix_vectors = 1;
  5164			/* Don't bother allocating more MSI-X vectors than cpus */
  5165			instance->msix_vectors = min(instance->msix_vectors,
  5166						     (unsigned int)num_online_cpus());
  5167			if (smp_affinity_enable)
  5168				irq_flags |= PCI_IRQ_AFFINITY;
  5169			i = pci_alloc_irq_vectors(instance->pdev, 1,
  5170						  instance->msix_vectors, irq_flags);
  5171			if (i > 0)
  5172				instance->msix_vectors = i;
  5173			else
  5174				instance->msix_vectors = 0;
  5175		}
  5176		/*
  5177		 * MSI-X host index 0 is common for all adapter.
  5178		 * It is used for all MPT based Adapters.
  5179		 */
  5180		if (instance->msix_combined) {
  5181			instance->reply_post_host_index_addr[0] =
  5182					(u32 *)((u8 *)instance->reg_set +
  5183					MPI2_SUP_REPLY_POST_HOST_INDEX_OFFSET);
  5184		} else {
  5185			instance->reply_post_host_index_addr[0] =
  5186				(u32 *)((u8 *)instance->reg_set +
  5187				MPI2_REPLY_POST_HOST_INDEX_OFFSET);
  5188		}
  5189	
  5190		i = pci_alloc_irq_vectors(instance->pdev, 1, 1, PCI_IRQ_LEGACY);
  5191		if (i < 0)
  5192			goto fail_setup_irqs;
  5193	
  5194		dev_info(&instance->pdev->dev,
  5195			"firmware supports msix\t: (%d)", fw_msix_count);
  5196		dev_info(&instance->pdev->dev,
  5197			"current msix/online cpus\t: (%d/%d)\n",
  5198			instance->msix_vectors, (unsigned int)num_online_cpus());
  5199		dev_info(&instance->pdev->dev,
  5200			"RDPQ mode\t: (%s)\n", instance->is_rdpq ? "enabled" : "disabled");
  5201	
  5202		tasklet_init(&instance->isr_tasklet, instance->instancet->tasklet,
  5203			(unsigned long)instance);
  5204	
  5205		instance->ctrl_info = kzalloc(sizeof(struct megasas_ctrl_info),
  5206					GFP_KERNEL);
  5207		if (instance->ctrl_info == NULL)
  5208			goto fail_init_adapter;
  5209	
  5210		/*
  5211		 * Below are default value for legacy Firmware.
  5212		 * non-fusion based controllers
  5213		 */
  5214		instance->fw_supported_vd_count = MAX_LOGICAL_DRIVES;
  5215		instance->fw_supported_pd_count = MAX_PHYSICAL_DEVICES;
  5216		/* Get operational params, sge flags, send init cmd to controller */
  5217		if (instance->instancet->init_adapter(instance))
  5218			goto fail_init_adapter;
  5219	
  5220		if (instance->msix_vectors ?
  5221			megasas_setup_irqs_msix(instance, 1) :
  5222			megasas_setup_irqs_ioapic(instance))
  5223			goto fail_init_adapter;
  5224	
  5225		instance->instancet->enable_intr(instance);
  5226	
  5227		dev_info(&instance->pdev->dev, "INIT adapter done\n");
  5228	
  5229		megasas_setup_jbod_map(instance);
  5230	
  5231		/** for passthrough
  5232		 * the following function will get the PD LIST.
  5233		 */
  5234		memset(instance->pd_list, 0,
  5235			(MEGASAS_MAX_PD * sizeof(struct megasas_pd_list)));
  5236		if (megasas_get_pd_list(instance) < 0) {
  5237			dev_err(&instance->pdev->dev, "failed to get PD list\n");
  5238			goto fail_get_pd_list;
  5239		}
  5240	
  5241		memset(instance->ld_ids, 0xff, MEGASAS_MAX_LD_IDS);
  5242	
  5243		/* stream detection initialization */
  5244		if (instance->is_ventura) {
  5245			fusion->stream_detect_by_ld =
  5246			kzalloc(sizeof(struct LD_STREAM_DETECT *)
  5247			* MAX_LOGICAL_DRIVES_EXT,
  5248                  GFP_KERNEL);

Ugh...  What's with the indenting here?  Normally, I spent some time
reviewing these static checker warnings before I email the reports but
it's hard to even look at this code.  It makes me discouraged to look at
code this ugly.

Everyone hates newbies coming in and sending hundreds of style cleanups
but to me it means that basic style is pretty easy for everyone.  Just
making the code look nice shows me that you at least care a little bit
instead of just pooping out code and merging it into upstream.

Someone emailed me this morning appologizing for introducing an ERR_PTR
dereference.  I'm thinking, that's just a mistake.  We all make mistakes
it's part of being human.  But when I look at this code, I feel sad
because at least put some effort into it...

Anyway, I suspect that this is a false positive but it's too painful to
look at this code so I haven't checked.

  5249                  if (!fusion->stream_detect_by_ld) {
  5250                          dev_err(&instance->pdev->dev,
  5251                                          "unable to allocate stream detection for pool of LDs\n");
  5252                          goto fail_get_ld_pd_list;
  5253                  }
  5254                  for (i = 0; i < MAX_LOGICAL_DRIVES_EXT; ++i) {
  5255                          fusion->stream_detect_by_ld[i] =
  5256                                  kmalloc(sizeof(struct LD_STREAM_DETECT),
  5257                                  GFP_KERNEL);
  5258                          if (!fusion->stream_detect_by_ld[i]) {
  5259                                  dev_err(&instance->pdev->dev,
  5260                                          "unable to allocate stream detect by LD\n ");
  5261                                  for (j = 0; j < i; ++j)
  5262                                          kfree(fusion->stream_detect_by_ld[j]);
  5263                                  kfree(fusion->stream_detect_by_ld);
  5264                                  fusion->stream_detect_by_ld = NULL;
  5265                                  goto fail_get_ld_pd_list;
  5266                          }
  5267                          fusion->stream_detect_by_ld[i]->mru_bit_map
  5268                                  = MR_STREAM_BITMAP;
  5269                  }
  5270          }

regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux