On 8/26/2019 6:40 AM, Dan Carpenter wrote:
Hello James Smart,The patch d79c9e9d4b3d: "scsi: lpfc: Support dynamic unbounded SGL lists on G7 hardware." from Aug 14, 2019, leads to the following static checker warning: drivers/scsi/lpfc/lpfc_init.c:4107 lpfc_new_io_buf() error: not allocating enough data 784 vs 768 drivers/scsi/lpfc/lpfc_init.c 4071 /** 4072 * lpfc_new_io_buf - IO buffer allocator for HBA with SLI4 IF spec 4073 * @vport: The virtual port for which this call being executed. 4074 * @num_to_allocate: The requested number of buffers to allocate. 4075 * 4076 * This routine allocates nvme buffers for device with SLI-4 interface spec, 4077 * the nvme buffer contains all the necessary information needed to initiate 4078 * an I/O. After allocating up to @num_to_allocate IO buffers and put 4079 * them on a list, it post them to the port by using SGL block post. 4080 * 4081 * Return codes: 4082 * int - number of IO buffers that were allocated and posted. 4083 * 0 = failure, less than num_to_alloc is a partial failure. 4084 **/ 4085 int 4086 lpfc_new_io_buf(struct lpfc_hba *phba, int num_to_alloc) 4087 { 4088 struct lpfc_io_buf *lpfc_ncmd; 4089 struct lpfc_iocbq *pwqeq; 4090 uint16_t iotag, lxri = 0; 4091 int bcnt, num_posted; 4092 LIST_HEAD(prep_nblist); 4093 LIST_HEAD(post_nblist); 4094 LIST_HEAD(nvme_nblist); 4095 4096 /* Sanity check to ensure our sizing is right for both SCSI and NVME */ 4097 if (sizeof(struct lpfc_io_buf) > LPFC_COMMON_IO_BUF_SZ) { ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ We made the lpfc_io_buf struct larger so now this check is more likely to trigger. Why don't we make this condition a BUILD_BUG_ON()? 4098 lpfc_printf_log(phba, KERN_ERR, LOG_FCP, 4099 "6426 Common buffer size %zd exceeds %d\n", 4100 sizeof(struct lpfc_io_buf), 4101 LPFC_COMMON_IO_BUF_SZ); 4102 return 0; Zero means we're returning failure on this path. 4103 } 4104 4105 phba->sli4_hba.io_xri_cnt = 0; 4106 for (bcnt = 0; bcnt < num_to_alloc; bcnt++) { 4107 lpfc_ncmd = kzalloc(LPFC_COMMON_IO_BUF_SZ, GFP_KERNEL); ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Smatch generates a warning here. It's obviously not a problem, because of the earlier check. I guess I don't really understand why LPFC_COMMON_IO_BUF_SZ is useful when it's so close to sizeof(*lpfc_ncmd).
Completely agree - I'll clean this up. -- james
