> -----Original Message----- > From: Tomas Henzl [mailto:thenzl@xxxxxxxxxx] > Sent: Tuesday, September 09, 2014 7:01 PM > To: Sumit.Saxena@xxxxxxxxxxxxx; linux-scsi@xxxxxxxxxxxxxxx > Cc: martin.petersen@xxxxxxxxxx; hch@xxxxxxxxxxxxx; > jbottomley@xxxxxxxxxxxxx; kashyap.desai@xxxxxxxxxxxxx; > aradford@xxxxxxxxx > Subject: Re: [PATCH 02/11] megaraid_sas : Use writeq for 64bit pci write to > avoid spinlock overhead > > On 09/06/2014 03:25 PM, Sumit.Saxena@xxxxxxxxxxxxx wrote: > > Use writeq() for 64bit PCI write instead of writel() to avoid additional lock > overhead. > > > > Signed-off-by: Sumit Saxena <sumit.saxena@xxxxxxxxxxxxx> > > Signed-off-by: Kashyap Desai <kashyap.desai@xxxxxxxxxxxxx> > > --- > > drivers/scsi/megaraid/megaraid_sas_fusion.c | 8 ++++++++ > > 1 file changed, 8 insertions(+) > > > > diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.c > > b/drivers/scsi/megaraid/megaraid_sas_fusion.c > > index 57b47fe..c69c1ac 100644 > > --- a/drivers/scsi/megaraid/megaraid_sas_fusion.c > > +++ b/drivers/scsi/megaraid/megaraid_sas_fusion.c > > @@ -1065,6 +1065,13 @@ megasas_fire_cmd_fusion(struct > megasas_instance *instance, > > u32 req_desc_hi, > > struct megasas_register_set __iomem *regs) > > Hi Sumit, > the fn params are a bit confusing req_desc_lo is of type dma_addr_t and > req_desc_hi is u32, is it possible to unite it in the future? Agree. We should make changes here. We will do it in separate patch. Originally fire_cmd() was written for MFI controller and carry forward for all generation. In MFI it use second argument as 32 bit address and third argument as frame count, but later in Fusion adapter it started using differently. > > > { > > +#if defined(writeq) && defined(CONFIG_64BIT) > > On a similar place mpt2sas(_base_writeq) uses only "#ifndef writeq" > if it's incorrect fix it there too or remove the CONFIG_64 here We would like to change at mpt2sas as we have all the code with below check for writeq() Original discuss was started when we submitted this change in mpt2sas, but we have delta from day-1. LSI/Avago internal source has "#if defined(writeq) && defined(CONFIG_64BIT)" check in mpt2sas. I think now writeq() is implemented in all arch, so we can safely remove check for #if writeq(). But we can keep this check as it is to continue for older Distribution to take direct advantage without maintaining any separate patch. > > > + u64 req_data = 0; > > + > > + req_data = req_desc_hi; > > + req_data = ((req_data << 32) | (u32)req_desc_lo); > > This seems to be critical path (you are removing an spinlock to avoid > overhead), so why do you have three consecutive assignments to the same > variable? > (~(u64 req_data = r_hi << 32 | r_lo)) Agree. We will be doing this change and re-submit the patch to address this. > > Cheers, > Tomas > > > + writeq(le64_to_cpu(req_data), &(regs)- > >inbound_low_queue_port); > > +#else > > unsigned long flags; > > > > spin_lock_irqsave(&instance->hba_lock, flags); @@ -1072,6 +1079,7 > @@ > > megasas_fire_cmd_fusion(struct megasas_instance *instance, > > writel(le32_to_cpu(req_desc_lo), &(regs)- > >inbound_low_queue_port); > > writel(le32_to_cpu(req_desc_hi), &(regs)- > >inbound_high_queue_port); > > spin_unlock_irqrestore(&instance->hba_lock, flags); > > +#endif > > } > > > > /** -- 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
