Message ID | 20220428000326.3622230-1-ipylypiv@google.com |
---|---|
State | New |
Headers | show |
Series | scsi: pm80xx: Remove pm8001_tag_init() | expand |
On 28/04/2022 01:03, Igor Pylypiv wrote: > In commit 5a141315ed7c ("scsi: pm80xx: Increase the number of outstanding > I/O supported to 1024") the pm8001_ha->tags allocation was moved into > pm8001_init_ccb_tag(). This changed the execution order of allocation. > pm8001_tag_init() used to be called after the pm8001_ha->tags allocation > and now it is called before the allocation. > > Before: > > pm8001_pci_probe() > `--> pm8001_pci_alloc() > `--> pm8001_alloc() > `--> pm8001_ha->tags = kzalloc(...) > `--> pm8001_tag_init(pm8001_ha); // OK: tags are allocated > > After: > > pm8001_pci_probe() > `--> pm8001_pci_alloc() > | `--> pm8001_alloc() > | `--> pm8001_tag_init(pm8001_ha); // NOK: tags are not allocated > | > `--> pm8001_init_ccb_tag() > `--> pm8001_ha->tags = kzalloc(...) // today it is bitmap_zalloc() > > Since pm8001_ha->tags_num is zero when pm8001_tag_init() is called it does > nothing. Tags memory is allocated with bitmap_zalloc() so there is no need > to manually clear each bit with pm8001_tag_free(). Your change looks ok. But please note the following discussed in the following link, there was/is a bug in the lateness in which the tags are allocated: https://lore.kernel.org/linux-scsi/PH0PR11MB5112D8C17D9EA268C197893FEC579@PH0PR11MB5112.namprd11.prod.outlook.com/ I don't think that it has been fixed yet... Thanks, John > > Fixes: 5a141315ed7c ("scsi: pm80xx: Increase the number of outstanding > I/O supported to 1024") > Reviewed-by: Changyuan Lyu <changyuanl@google.com> > Signed-off-by: Igor Pylypiv <ipylypiv@google.com> > --- > drivers/scsi/pm8001/pm8001_init.c | 2 -- > drivers/scsi/pm8001/pm8001_sas.c | 7 ------- > drivers/scsi/pm8001/pm8001_sas.h | 1 - > 3 files changed, 10 deletions(-) > > diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c > index 9b04f1a6a67d..7040cecd861b 100644 > --- a/drivers/scsi/pm8001/pm8001_init.c > +++ b/drivers/scsi/pm8001/pm8001_init.c > @@ -420,8 +420,6 @@ static int pm8001_alloc(struct pm8001_hba_info *pm8001_ha, > atomic_set(&pm8001_ha->devices[i].running_req, 0); > } > pm8001_ha->flags = PM8001F_INIT_TIME; > - /* Initialize tags */ > - pm8001_tag_init(pm8001_ha); > return 0; > > err_out_nodev: > diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c > index 3a863d776724..dc689055341b 100644 > --- a/drivers/scsi/pm8001/pm8001_sas.c > +++ b/drivers/scsi/pm8001/pm8001_sas.c > @@ -92,13 +92,6 @@ int pm8001_tag_alloc(struct pm8001_hba_info *pm8001_ha, u32 *tag_out) > return 0; > } > > -void pm8001_tag_init(struct pm8001_hba_info *pm8001_ha) > -{ > - int i; > - for (i = 0; i < pm8001_ha->tags_num; ++i) > - pm8001_tag_free(pm8001_ha, i); > -} > - > /** > * pm8001_mem_alloc - allocate memory for pm8001. > * @pdev: pci device. > diff --git a/drivers/scsi/pm8001/pm8001_sas.h b/drivers/scsi/pm8001/pm8001_sas.h > index 060ab680a7ed..ba959f986c1e 100644 > --- a/drivers/scsi/pm8001/pm8001_sas.h > +++ b/drivers/scsi/pm8001/pm8001_sas.h > @@ -633,7 +633,6 @@ extern struct workqueue_struct *pm8001_wq; > > /******************** function prototype *********************/ > int pm8001_tag_alloc(struct pm8001_hba_info *pm8001_ha, u32 *tag_out); > -void pm8001_tag_init(struct pm8001_hba_info *pm8001_ha); > u32 pm8001_get_ncq_tag(struct sas_task *task, u32 *tag); > void pm8001_ccb_task_free(struct pm8001_hba_info *pm8001_ha, > struct pm8001_ccb_info *ccb);
diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c index 9b04f1a6a67d..7040cecd861b 100644 --- a/drivers/scsi/pm8001/pm8001_init.c +++ b/drivers/scsi/pm8001/pm8001_init.c @@ -420,8 +420,6 @@ static int pm8001_alloc(struct pm8001_hba_info *pm8001_ha, atomic_set(&pm8001_ha->devices[i].running_req, 0); } pm8001_ha->flags = PM8001F_INIT_TIME; - /* Initialize tags */ - pm8001_tag_init(pm8001_ha); return 0; err_out_nodev: diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c index 3a863d776724..dc689055341b 100644 --- a/drivers/scsi/pm8001/pm8001_sas.c +++ b/drivers/scsi/pm8001/pm8001_sas.c @@ -92,13 +92,6 @@ int pm8001_tag_alloc(struct pm8001_hba_info *pm8001_ha, u32 *tag_out) return 0; } -void pm8001_tag_init(struct pm8001_hba_info *pm8001_ha) -{ - int i; - for (i = 0; i < pm8001_ha->tags_num; ++i) - pm8001_tag_free(pm8001_ha, i); -} - /** * pm8001_mem_alloc - allocate memory for pm8001. * @pdev: pci device. diff --git a/drivers/scsi/pm8001/pm8001_sas.h b/drivers/scsi/pm8001/pm8001_sas.h index 060ab680a7ed..ba959f986c1e 100644 --- a/drivers/scsi/pm8001/pm8001_sas.h +++ b/drivers/scsi/pm8001/pm8001_sas.h @@ -633,7 +633,6 @@ extern struct workqueue_struct *pm8001_wq; /******************** function prototype *********************/ int pm8001_tag_alloc(struct pm8001_hba_info *pm8001_ha, u32 *tag_out); -void pm8001_tag_init(struct pm8001_hba_info *pm8001_ha); u32 pm8001_get_ncq_tag(struct sas_task *task, u32 *tag); void pm8001_ccb_task_free(struct pm8001_hba_info *pm8001_ha, struct pm8001_ccb_info *ccb);