Message ID | 20230725161331.27481-7-james@equiv.tech |
---|---|
State | Superseded |
Headers | show |
Series | scsi: mpt3sas: Use flexible arrays and do a few cleanups | expand |
On Tue, Jul 25, 2023 at 09:13:31AM -0700, James Seo wrote: > This dynamic allocation can be replaced with a local variable. > > Signed-off-by: James Seo <james@equiv.tech> > --- > drivers/scsi/mpt3sas/mpt3sas_base.c | 19 +++++-------------- > 1 file changed, 5 insertions(+), 14 deletions(-) > > diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c > index cd6f36094159..a32a6fa728a7 100644 > --- a/drivers/scsi/mpt3sas/mpt3sas_base.c > +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c > @@ -5361,10 +5361,9 @@ _base_update_diag_trigger_pages(struct MPT3SAS_ADAPTER *ioc) > static int _base_assign_fw_reported_qd(struct MPT3SAS_ADAPTER *ioc) > { > Mpi2ConfigReply_t mpi_reply; > - Mpi2SasIOUnitPage1_t *sas_iounit_pg1 = NULL; > + Mpi2SasIOUnitPage1_t sas_iounit_pg1; > Mpi26PCIeIOUnitPage1_t pcie_iounit_pg1; > u16 depth; > - int sz; > int rc = 0; > > ioc->max_wideport_qd = MPT3SAS_SAS_QUEUE_DEPTH; > @@ -5374,28 +5373,21 @@ static int _base_assign_fw_reported_qd(struct MPT3SAS_ADAPTER *ioc) > if (!ioc->is_gen35_ioc) > goto out; > /* sas iounit page 1 */ > - sz = offsetof(Mpi2SasIOUnitPage1_t, PhyData); > - sas_iounit_pg1 = kzalloc(sizeof(Mpi2SasIOUnitPage1_t), GFP_KERNEL); Hunh. So Mpi2SasIOUnitPage1_t is used without the flexarray at all? -Kees > - if (!sas_iounit_pg1) { > - pr_err("%s: failure at %s:%d/%s()!\n", > - ioc->name, __FILE__, __LINE__, __func__); > - return rc; > - } > rc = mpt3sas_config_get_sas_iounit_pg1(ioc, &mpi_reply, > - sas_iounit_pg1, sz); > + &sas_iounit_pg1, sizeof(Mpi2SasIOUnitPage1_t)); > if (rc) { > pr_err("%s: failure at %s:%d/%s()!\n", > ioc->name, __FILE__, __LINE__, __func__); > goto out; > } > > - depth = le16_to_cpu(sas_iounit_pg1->SASWideMaxQueueDepth); > + depth = le16_to_cpu(sas_iounit_pg1.SASWideMaxQueueDepth); > ioc->max_wideport_qd = (depth ? depth : MPT3SAS_SAS_QUEUE_DEPTH); > > - depth = le16_to_cpu(sas_iounit_pg1->SASNarrowMaxQueueDepth); > + depth = le16_to_cpu(sas_iounit_pg1.SASNarrowMaxQueueDepth); > ioc->max_narrowport_qd = (depth ? depth : MPT3SAS_SAS_QUEUE_DEPTH); > > - depth = sas_iounit_pg1->SATAMaxQDepth; > + depth = sas_iounit_pg1.SATAMaxQDepth; > ioc->max_sata_qd = (depth ? depth : MPT3SAS_SATA_QUEUE_DEPTH); > > /* pcie iounit page 1 */ > @@ -5414,7 +5406,6 @@ static int _base_assign_fw_reported_qd(struct MPT3SAS_ADAPTER *ioc) > "MaxWidePortQD: 0x%x MaxNarrowPortQD: 0x%x MaxSataQD: 0x%x MaxNvmeQD: 0x%x\n", > ioc->max_wideport_qd, ioc->max_narrowport_qd, > ioc->max_sata_qd, ioc->max_nvme_qd)); > - kfree(sas_iounit_pg1); > return rc; > } > > -- > 2.39.2 >
On Fri, Jul 28, 2023 at 03:29:17PM -0700, Kees Cook wrote: > On Tue, Jul 25, 2023 at 09:13:31AM -0700, James Seo wrote: >> This dynamic allocation can be replaced with a local variable. >> >> Signed-off-by: James Seo <james@equiv.tech> >> --- >> drivers/scsi/mpt3sas/mpt3sas_base.c | 19 +++++-------------- >> 1 file changed, 5 insertions(+), 14 deletions(-) >> >> diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c >> index cd6f36094159..a32a6fa728a7 100644 >> --- a/drivers/scsi/mpt3sas/mpt3sas_base.c >> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c >> @@ -5361,10 +5361,9 @@ _base_update_diag_trigger_pages(struct MPT3SAS_ADAPTER *ioc) >> static int _base_assign_fw_reported_qd(struct MPT3SAS_ADAPTER *ioc) >> { >> Mpi2ConfigReply_t mpi_reply; >> - Mpi2SasIOUnitPage1_t *sas_iounit_pg1 = NULL; >> + Mpi2SasIOUnitPage1_t sas_iounit_pg1; >> Mpi26PCIeIOUnitPage1_t pcie_iounit_pg1; >> u16 depth; >> - int sz; >> int rc = 0; >> >> ioc->max_wideport_qd = MPT3SAS_SAS_QUEUE_DEPTH; >> @@ -5374,28 +5373,21 @@ static int _base_assign_fw_reported_qd(struct MPT3SAS_ADAPTER *ioc) >> if (!ioc->is_gen35_ioc) >> goto out; >> /* sas iounit page 1 */ >> - sz = offsetof(Mpi2SasIOUnitPage1_t, PhyData); >> - sas_iounit_pg1 = kzalloc(sizeof(Mpi2SasIOUnitPage1_t), GFP_KERNEL); > > Hunh. So Mpi2SasIOUnitPage1_t is used without the flexarray at all? > > -Kees You call it "dead code" and "unused struct members". mpt3sas evidently calls it "documentation" ;) Anyway, does this commit get your "Reviewed-by:"? James
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c index cd6f36094159..a32a6fa728a7 100644 --- a/drivers/scsi/mpt3sas/mpt3sas_base.c +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c @@ -5361,10 +5361,9 @@ _base_update_diag_trigger_pages(struct MPT3SAS_ADAPTER *ioc) static int _base_assign_fw_reported_qd(struct MPT3SAS_ADAPTER *ioc) { Mpi2ConfigReply_t mpi_reply; - Mpi2SasIOUnitPage1_t *sas_iounit_pg1 = NULL; + Mpi2SasIOUnitPage1_t sas_iounit_pg1; Mpi26PCIeIOUnitPage1_t pcie_iounit_pg1; u16 depth; - int sz; int rc = 0; ioc->max_wideport_qd = MPT3SAS_SAS_QUEUE_DEPTH; @@ -5374,28 +5373,21 @@ static int _base_assign_fw_reported_qd(struct MPT3SAS_ADAPTER *ioc) if (!ioc->is_gen35_ioc) goto out; /* sas iounit page 1 */ - sz = offsetof(Mpi2SasIOUnitPage1_t, PhyData); - sas_iounit_pg1 = kzalloc(sizeof(Mpi2SasIOUnitPage1_t), GFP_KERNEL); - if (!sas_iounit_pg1) { - pr_err("%s: failure at %s:%d/%s()!\n", - ioc->name, __FILE__, __LINE__, __func__); - return rc; - } rc = mpt3sas_config_get_sas_iounit_pg1(ioc, &mpi_reply, - sas_iounit_pg1, sz); + &sas_iounit_pg1, sizeof(Mpi2SasIOUnitPage1_t)); if (rc) { pr_err("%s: failure at %s:%d/%s()!\n", ioc->name, __FILE__, __LINE__, __func__); goto out; } - depth = le16_to_cpu(sas_iounit_pg1->SASWideMaxQueueDepth); + depth = le16_to_cpu(sas_iounit_pg1.SASWideMaxQueueDepth); ioc->max_wideport_qd = (depth ? depth : MPT3SAS_SAS_QUEUE_DEPTH); - depth = le16_to_cpu(sas_iounit_pg1->SASNarrowMaxQueueDepth); + depth = le16_to_cpu(sas_iounit_pg1.SASNarrowMaxQueueDepth); ioc->max_narrowport_qd = (depth ? depth : MPT3SAS_SAS_QUEUE_DEPTH); - depth = sas_iounit_pg1->SATAMaxQDepth; + depth = sas_iounit_pg1.SATAMaxQDepth; ioc->max_sata_qd = (depth ? depth : MPT3SAS_SATA_QUEUE_DEPTH); /* pcie iounit page 1 */ @@ -5414,7 +5406,6 @@ static int _base_assign_fw_reported_qd(struct MPT3SAS_ADAPTER *ioc) "MaxWidePortQD: 0x%x MaxNarrowPortQD: 0x%x MaxSataQD: 0x%x MaxNvmeQD: 0x%x\n", ioc->max_wideport_qd, ioc->max_narrowport_qd, ioc->max_sata_qd, ioc->max_nvme_qd)); - kfree(sas_iounit_pg1); return rc; }
This dynamic allocation can be replaced with a local variable. Signed-off-by: James Seo <james@equiv.tech> --- drivers/scsi/mpt3sas/mpt3sas_base.c | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-)