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
