Re: [PATCH v4 3/6] mpt3sas: Introdude _scsih_get_shost_and_ioc.

[Suganath Prabu Subramani]

On Thu, Sep 27, 2018 at 2:39 AM Bjorn Helgaas  wrote:
>
> On Wed, Sep 26, 2018 at 09:52:36AM +0530, Suganath Prabu S wrote:
> > The code for getting shost and IOC is redundant so
> > moved that to function "scsih_get_shost_and_ioc".
> > Also checks for NULL are added to IOC and shost.
> >
> > Signed-off-by: Suganath Prabu S 
> > ---
> >  drivers/scsi/mpt3sas/mpt3sas_scsih.c | 98 
> > ++--
> >  1 file changed, 82 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c 
> > b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> > index 566a550..f6e92eb 100644
> > --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> > +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> > @@ -9809,6 +9809,35 @@ _scsih_ir_shutdown(struct MPT3SAS_ADAPTER *ioc)
> >  }
> >
> >  /**
> > + * _scsih_get_shost_and_ioc - get shost and ioc
> > + *   and verify whether they are NULL or not
> > + * @pdev: PCI device struct
> > + * @shost: address of scsi host pointer
> > + * @ioc: address of HBA adapter pointer
> > + *
> > + * Return zero if *shost and *ioc are not NULL otherwise return error 
> > number.
> > + */
> > +static int
> > +_scsih_get_shost_and_ioc(struct pci_dev *pdev,
> > + struct Scsi_Host **shost, struct MPT3SAS_ADAPTER **ioc)
> > +{
> > + *shost = pci_get_drvdata(pdev);
> > + if (*shost == NULL) {
> > + dev_err(>dev, "pdev's driver data is null\n");
> > + return -ENXIO;
> > + }
> > +
> > + *ioc = shost_priv(*shost);
> > + if (*ioc == NULL) {
> > + dev_err(>dev, "shost's private data is null\n");
> > + return -ENXIO;
>
> I think it's better to omit NULL pointer checks like these because
> there should not be a path where we can execute this code when these
> pointers are NULL.
>
> If there *is* such a path, I think that's a serious bug and it's
> better to oops when we try to dereference the NULL pointer.  If we
> just return an error code, it's likely the bug will be ignored and
> never fixed.
>
We have added the ioc and shost checks, because of kernel panic in
below scenario.
Have 3 HBA's in system and perform below operation.
1) Run “poweroff”.
2) Immediate hot unplug HBA.
We have observed, kernel's shutdown process has removed all the 3 HBA
devices smoothly,
and also user have unplugged the HBA device during this time. PCI
subsystem's hot-plug thread is also trying to remove
the hot plugged PCI device which is already removed/cleaned by the
shutdown process. (Which is not expected for the
already removed device) Due to this kernel panic is observed. And we
are not sure whether it
has to fixed from pciehp layer, so we added sanity checks in driver.

[ 1745.605510] BUG: unable to handle kernel NULL pointer dereference
at 0a98
[ 1745.606554] IP: [] scsih_remove+0x20/0x2d0 [mpt3sas]
[ 1745.607609] PGD 0
[ 1745.608621] Oops:  [#1] SMP
[ 1745.622989] CPU: 0 PID: 668 Comm: kworker/0:2 Tainted: G
O4.4.55-1.el7.elrepo.x86_64 #1
[ 1745.624800] Hardware name: PRO-MNU65930231
PRO-NME69559126/BRD-PRO55212588, BIOS 0.51e 05/08/2017
[ 1745.626673] Workqueue: pciehp-3 pciehp_power_thread
[ 1745.628566] task: 881fe50dd880 ti: 881fe88e4000 task.ti:
881fe88e4000
[ 1745.630530] RIP: 0010:[]  []
scsih_remove+0x20/0x2d0 [mpt3sas]
[ 1745.632577] RSP: 0018:881fe88e7c98  EFLAGS: 00010292
[ 1745.634639] RAX: 0001 RBX: 881feef5c000 RCX: 
[ 1745.636718] RDX:  RSI: 0202 RDI: 881feef5c000
[ 1745.638832] RBP: 881fe88e7cc8 R08:  R09: 000180220002
[ 1745.640972] R10: eaf4a401 R11: ea007fabd280 R12: 
[ 1745.643136] R13: a0576020 R14: 881fe9af8240 R15: 
[ 1745.645320] FS:  () GS:881ffde0()
knlGS:
[ 1745.647572] CS:  0010 DS:  ES:  CR0: 80050033
[ 1745.649833] CR2: 0a98 CR3: 001fe76df000 CR4: 003406f0
[ 1745.652147] DR0:  DR1:  DR2: 
[ 1745.654476] DR3:  DR6: fffe0ff0 DR7: 0400
[ 1745.656825] Stack:
[ 1745.659138]  881fe88e7cc8 881feef5c098 881feef5c000
a0576020
[ 1745.661562]  881fe9af8240  881fe88e7cf0
8137f9d9
[ 1745.663990]  881feef5c098 a0576088 881feef5c000
881fe88e7d10
[ 1745.666428] Call Trace:
[ 1745.668830]  [] pci_device_remove+0x39/0xc0
[ 1745.671256]  [] __device_release_driver+0x96/0x130
[ 1745.673664]  [] device_release_driver+0x23/0x30
[ 1745.676071]  [] pci_stop_bus_device+0x8c/0xa0
[ 1745.678485]  [] pci_stop_and_remove_bus_device+0x12/0x20
[ 1745.680909]  [] pciehp_unconfigure_device+0xaa/0x1b0
[ 1745.683331]  [] pciehp_disable_slot+0x52/0xd0
[ 1745.685767]  [] pciehp_power_thread+0x8d/0xb0
[ 1745.688210]  [] process_one_work+0x14f/0x400
[ 1745.690633]  [] worker_thread+0x114/0x470
[ 1745.693080]  

Re: [PATCH v4 2/6] mpt3sas: Separate out mpt3sas_wait_for_ioc_to_operational

[Suganath Prabu Subramani]

Hi Bjorn,

Thanks for reviewing.

On Thu, Sep 27, 2018 at 2:33 AM Bjorn Helgaas  wrote:
>
> On Wed, Sep 26, 2018 at 09:52:35AM +0530, Suganath Prabu S wrote:
> > Introduce mpt3sas_wait_for_ioc_to_operational.
> >
> > This section of code "wait for IOC to be operational"
> > is used in many places across the driver,
> > and hence moved this section of code in to the function
> > "mpt3sas_wait_for_ioc_to_operational".
> >
> > Also added HBA hot unplug checks, and this returns with
> > error code EFAULT, if it detects HBA is hot unplugged
> > or IOC is not in operational state.
>
> This should be two patches:
>
>   1) Factor out the "wait for IOC" code.  This should not cause any
>  functional changes (I didn't verify in your code, but this is the
>  objective).
>
>   2) Add the hot unplug checks.
>
> This makes the patches much easier to review.
> Will move hot unplug check to separate patch.
> > V2 change set:
> > used pci_device_is_present instead of
> > mpt3sas_base_pci_device_is_unplugged
> >
> > v4 Change set:
> > Dont split strings in print statement
>
> I don't know the convention in drivers/scsi, but in driver/pci, I
> remove this sort of v2/v3 commentary from the changelog because it's
> really not of interest after the patch is merged.  It's nice to have
> it in the email, but I think if you put it after a line containing
> only "--" it will be in the email but not the changelog.
>Thanks, Will add change sets after --
> > Signed-off-by: Suganath Prabu S 
> > ---
> >  drivers/scsi/mpt3sas/mpt3sas_base.c  | 92 
> > +++-
> >  drivers/scsi/mpt3sas/mpt3sas_base.h  |  4 ++
> >  drivers/scsi/mpt3sas/mpt3sas_config.c| 28 +++---
> >  drivers/scsi/mpt3sas/mpt3sas_ctl.c   | 26 ++---
> >  drivers/scsi/mpt3sas/mpt3sas_transport.c | 75 +-
> >  5 files changed, 81 insertions(+), 144 deletions(-)
> >
> > diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c 
> > b/drivers/scsi/mpt3sas/mpt3sas_base.c
> > index c880e72..9f1d8fb 100644
> > --- a/drivers/scsi/mpt3sas/mpt3sas_base.c
> > +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
> > @@ -5176,6 +5176,53 @@ _base_send_ioc_reset(struct MPT3SAS_ADAPTER *ioc, u8 
> > reset_type, int timeout)
> >  }
> >
> >  /**
> > + * mpt3sas_wait_for_ioc_to_operational - IOC's operational
> > + *   state and HBA hot unplug status are checked here.
> > + * @ioc: per adapter object
> > + * @wait_count: timeout in seconds
>
> "wait_count" is really a timeout and maybe should be named "timeout".
>Yes, wait_count is timeout, will rename wait_count to timeout
> > + * Return:  Returns EFAULT, if HBA is hot unplugged or IOC is
> > + * not in operational state, within the wait_count.
> > + * And returns 0, If not hot unplugged Or ioc is in
> > + * operational state.
>
> I think you mean something like:
>
>   Waits up to wait_count seconds for the IOC to become operational.
>   Returns 0 if IOC is present and operational; otherwise returns
>   -EFAULT.
> Yes.
> > + */
> > +
> > +int
> > +mpt3sas_wait_for_ioc_to_operational(struct MPT3SAS_ADAPTER *ioc,
> > + int wait_count)

Re: [PATCH v3 1/6] mpt3sas: Introduce mpt3sas_base_pci_device_is_available

[Suganath Prabu Subramani]

On Tue, Sep 25, 2018 at 3:22 PM Andy Shevchenko
 wrote:
>
> On Tue, Sep 25, 2018 at 12:46 PM Suganath Prabu Subramani
>  wrote:
>
> > > > +   pr_err(MPT3SAS_FMT
> > > > +   "%s: pci error recovery reset or"
> > > > +   " pci device unplug occurred\n",
> > >
> > > This should be just one line.
> > Above line crosses over 80 characters, so it is in two lines.
>
> For years there is no such requirement for string literals.
Below is the one i was mentioning,
WARNING: line over 80 characters
#30: FILE: drivers/scsi/mpt3sas/mpt3sas_base.c:6861:
+ pr_err(MPT3SAS_FMT "%s: pci error recovery reset or pci device
unplug occurred\n", ioc->name, __func__);

Warning comes for both characters over 80 chars and string split.
Do you want us to ignore warning and repost like above in single line ?
> Please, don't split string literals in ugly way like this.
>
> --
> With Best Regards,
> Andy Shevchenko

Re: [PATCH v3 1/6] mpt3sas: Introduce mpt3sas_base_pci_device_is_available

[Suganath Prabu Subramani]

Hi Andy,

I forgot to add Lukas in last patch CC list, But i have sent a note
regarding the updated patch to him.
 Also added linux-pci

On Tue, Sep 25, 2018 at 2:10 PM Andy Shevchenko
 wrote:
>
> On Mon, Sep 24, 2018 at 9:36 AM Suganath Prabu S
>  wrote:
> >
> > * Driver uses "pci_device_is_present" to check whether
> > If Hot unplugged:
> > the outstanding IOs with 'DID_NO_CONNECT' before removing the drives
> > attached to the HBA.
> > "DID_NO_CONNECT" status and free the smid, if driver detects that
> > HBA is hot unplugged.
> >
> > * In the hard reset flush out all the outstanding IOs even if diag reset
> > fails and also if driver detects that HBA is hot unplugged.
>
> > +   if (!mpt3sas_base_pci_device_is_available(ioc)) {
>
> > +   pr_err(MPT3SAS_FMT
> > +   "%s: pci error recovery reset or"
> > +   " pci device unplug occurred\n",
>
> This should be just one line.
Above line crosses over 80 characters, so it is in two lines.
>
> > +   ioc->name, __func__);
> > +   return;
> > +   }
>
> --
> With Best Regards,
> Andy Shevchenko

Re: mpt3sas heavy I/O load causes kernel BUG at block/blk-core.c:2695

[Suganath Prabu Subramani]

Hi Douglas,

Can you check if this patch is already part of driver, If not please
try with below patch.
This patch is to fix the completion of abort before the IO completion.
With this, driver will process IO's reply first followed by TM.

authorSuganath prabu Subramani
2016-01-28 12:07:06 +0530
committerMartin K. Petersen 2016-02-23
21:27:02 -0500
commit03d1fb3a65783979f23bd58b5a0387e6992d9e26 (patch)
tree6aca275e2ebe7fbcd5fac1654cedd8f56d0947d0 /drivers/scsi/mpt3sas
parent5c739b6157bd090942e5847ddd12bfb99cd4240d (diff)
downloadlinux-03d1fb3a65783979f23bd58b5a0387e6992d9e26.tar.gz

mpt3sas: Fix for Asynchronous completion of timedout IO and task abort
of timedout IO.
Track msix of each IO and use the same msix for issuing abort to timed
out IO. With this driver will process IO's reply first followed by TM.
Signed-off-by: Suganath prabu Subramani
 Signed-off-by: Chaitra P B
 Reviewed-by: Tomas Henzl
 Signed-off-by: Martin K. Petersen



Thanks,
Suganath Prabu S

On Wed, Jun 6, 2018 at 7:50 PM, Douglas Miller
 wrote:
> Running a heavy I/O load on multipath/dual-ported SSD disks attached to a
> SAS3008 adapter (mpt3sas driver), we are seeing I/Os get aborted and tasks
> stuck in blk_complete_request() and this sometimes results in hitting a
> BUG_ON in blk_start_request(). It would appear that we are seeing two
> completions performed on an I/O, and the second completion is racing with
> re-use of the request for a new I/O.
>
> I saw this upstream commit:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v4.17-rc3=9961c9bbf2b43acaaf030a0fbabc9954d937ad8c
>
> which addresses the case where the normal completion occurs before the abort
> completion. But the situation I am seeing appears to be that the abort
> completion occurs before the normal completion (due to tasks getting delayed
> in blk_complete_request()). I don't find any commit to fix this second case.
>
> Of course, tasks being delayed like this is a concern, and is being worked
> separately. But it seems that the alternate double-completion case is being
> ignored here.
>
> Does everyone concur that this second case needs to be addressed? Is there a
> proposed fix?
>
> Thanks,
>
> Doug
>
> FYI, system is a Power9 running RHEL-ALT 7.5, two SAS3008 adapters connected
> to an IBM EXP24SX SAS Storage Enclosure with 24 HUSMM8040ASS201 drives. FIO
> was being used to drive the I/O load.
>
>

Re: mpt3sas: sleeping function called from invalid context

[Suganath Prabu Subramani]

Hi Bart,

We have root caused the issue and it is same as you mentioned.
"_scsih_get_enclosure_logicalid_chassis_slot()" is called with interrupts
disabled and this function
"_scsih_get_enclosure_logicalid_chassis_slot" again calls
_config_request(), with mutex_lock().

We have patch ready along with few other change and we ll be posting
it by tomorrow after covering BST.

Thanks,
Suganath Prabu S

On Mon, Mar 12, 2018 at 11:53 PM, Bart Van Assche
 wrote:
> Hello,
>
> For the first I/O request after boot that is sent to a disk attached to an
> mpt3sas adapter I see the below complaint appearing in the kernel log. This
> occurs at least with kernels v4.16-rc4 and v4.16-rc5.
>
> What I see in the mpt3sas source code is that
> _scsih_get_enclosure_logicalid_chassis_slot() is called with interrupts
> disabled and also that a function called by that function, namely
> _config_request(), calls mutex_lock().
>
> Can someone who is more familiar than I with the mpt3sas adapter have a look
> at this and propose a fix?
>
> Thanks,
>
> Bart.
>
> BUG: sleeping function called from invalid context at 
> kernel/locking/mutex.c:747
> in_atomic(): 1, irqs_disabled(): 1, pid: 2389, name: kworker/u64:1
> INFO: lockdep is turned off.
> irq event stamp: 278
> hardirqs last  enabled at (277): [<32c577ec>] 
> _raw_spin_unlock_irq+0x24/0x50
> hardirqs last disabled at (278): [<6082e2fa>] __schedule+0x120/0x1010
> softirqs last  enabled at (0): [<8c2eb285>] 
> copy_process.part.45+0x930/0x3470
> softirqs last disabled at (0): [<  (null)>]   (null)
> Preemption disabled at:
> [<>]   (null)
> CPU: 3 PID: 2389 Comm: kworker/u64:1 Tainted: GW
> 4.16.0-rc5-dbg+ #1
> Workqueue: poll_mpt3sas0_statu _base_fault_reset_work [mpt3sas]
> Call Trace:
> dump_stack+0x67/0x90
> ___might_sleep+0x1da/0x2c0
> __mutex_lock+0xb9/0xbb0
> _config_request.constprop.5+0xa3/0xe70 [mpt3sas]
> mpt3sas_config_get_enclosure_pg0+0xb3/0x110 [mpt3sas]
> _scsih_get_enclosure_logicalid_chassis_slot+0xf8/0x160 [mpt3sas]
> mpt3sas_scsih_reset_handler+0x3f6/0xb30 [mpt3sas]
> mpt3sas_base_hard_reset_handler+0x49a/0x7c0 [mpt3sas]
> _base_fault_reset_work+0x1bb/0x260 [mpt3sas]
> process_one_work+0x441/0xa50
> worker_thread+0x76/0x6c0
> kthread+0x1b2/0x1d0
> ret_from_fork+0x24/0x30
>

Re: [PATCH 5/6] mpt3sas: Introduce function to clone mpi request.

[Suganath Prabu Subramani]

Hi tomas,
We have sent V1 version of patches on 7th of Feb. Please review.

Thanks,
Suganath Prabu S

On Mon, Jan 22, 2018 at 8:08 PM, Tomas Henzl  wrote:
> On 01/19/2018 01:37 PM, Suganath Prabu S wrote:
>> 1) Added function _base_clone_mpi_to_sys_mem to clone
>> MPI request into system BAR0 mapped region.
>>
>> 2) Seperate out MPI Endpoint IO submissions to function
>> _base_put_smid_mpi_ep_scsi_io.
>>
>> 3) MPI EP requests are submitted in two 32 bit MMIO writes.
>> from _base_mpi_ep_writeq.
>>
>> Signed-off-by: Suganath Prabu S 
>> ---
>>  drivers/scsi/mpt3sas/mpt3sas_base.c | 131 
>> +---
>>  1 file changed, 123 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c 
>> b/drivers/scsi/mpt3sas/mpt3sas_base.c
>> index 40a1806..0248058 100644
>> --- a/drivers/scsi/mpt3sas/mpt3sas_base.c
>> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
>> @@ -126,6 +126,24 @@ module_param_call(mpt3sas_fwfault_debug, 
>> _scsih_set_fwfault_debug,
>>   param_get_int, _fwfault_debug, 0644);
>>
>>  /**
>> + * _base_clone_mpi_to_sys_mem - Writes/copies MPI frames
>> + *   to system/BAR0 region.
>> + *
>> + * @dst_iomem: Pointer to the destinaltion location in BAR0 space.
>> + * @src: Pointer to the Source data.
>> + * @size: Size of data to be copied.
>> + */
>> +static void
>> +_base_clone_mpi_to_sys_mem(void *dst_iomem, void *src, u32 size)
>> +{
>> + int i;
>> + __le32 *src_virt_mem = (__le32 *)src;
>> +
>> + for (i = 0; i < size/4; i++)
>> + writel(cpu_to_le32(src_virt_mem[i]), dst_iomem + (i * 4));
>> +}
>> +
>> +/**
>>   * _base_clone_to_sys_mem - Writes/copies data to system/BAR0 region
>>   *
>>   * @dst_iomem: Pointer to the destinaltion location in BAR0 space.
>> @@ -3265,6 +3283,29 @@ mpt3sas_base_free_smid(struct MPT3SAS_ADAPTER *ioc, 
>> u16 smid)
>>  }
>>
>>  /**
>> + * _base_mpi_ep_writeq - 32 bit write to MMIO
>> + * @b: data payload
>> + * @addr: address in MMIO space
>> + * @writeq_lock: spin lock
>> + *
>> + * This special handling for MPI EP to take care of 32 bit
>> + * environment where its not quarenteed to send the entire word
>> + * in one transfer.
>
> Hi Suganath,
> so is a single writeq possible ? There already is a _base_writeq function
> which seems to be identical to _base_mpi_ep_writeq.
> Also you may want to add a mmiowb() call.
> tomash
>
>> + */
>> +static inline void
>> +_base_mpi_ep_writeq(__u64 b, volatile void __iomem *addr,
>> + spinlock_t *writeq_lock)
>> +{
>> + unsigned long flags;
>> + __u64 data_out = cpu_to_le64(b);
>> +
>> + spin_lock_irqsave(writeq_lock, flags);
>> + writel((u32)(data_out), addr);
>> + writel((u32)(data_out >> 32), (addr + 4));
>> + spin_unlock_irqrestore(writeq_lock, flags);
>> +}
>> +
>> +/**
>>   * _base_writeq - 64 bit write to MMIO
>>   * @ioc: per adapter object
>>   * @b: data payload
>> @@ -3296,6 +3337,36 @@ _base_writeq(__u64 b, volatile void __iomem *addr, 
>> spinlock_t *writeq_lock)
>>  #endif
>>
>>  /**
>> + * _base_put_smid_mpi_ep_scsi_io - send SCSI_IO request to firmware
>> + * @ioc: per adapter object
>> + * @smid: system request message index
>> + * @handle: device handle
>> + *
>> + * Return nothing.
>> + */
>> +static void
>> +_base_put_smid_mpi_ep_scsi_io(struct MPT3SAS_ADAPTER *ioc, u16 smid, u16 
>> handle)
>> +{
>> + Mpi2RequestDescriptorUnion_t descriptor;
>> + u64 *request = (u64 *)
>> + void *mpi_req_iomem;
>> + __le32 *mfp = (__le32 *)mpt3sas_base_get_msg_frame(ioc, smid);
>> +
>> + _clone_sg_entries(ioc, (void *) mfp, smid);
>> + mpi_req_iomem = (void *)ioc->chip +
>> + MPI_FRAME_START_OFFSET + (smid * ioc->request_sz);
>> + _base_clone_mpi_to_sys_mem(mpi_req_iomem, (void *)mfp,
>> + ioc->request_sz);
>> + descriptor.SCSIIO.RequestFlags = MPI2_REQ_DESCRIPT_FLAGS_SCSI_IO;
>> + descriptor.SCSIIO.MSIxIndex =  _base_get_msix_index(ioc);
>> + descriptor.SCSIIO.SMID = cpu_to_le16(smid);
>> + descriptor.SCSIIO.DevHandle = cpu_to_le16(handle);
>> + descriptor.SCSIIO.LMID = 0;
>> + _base_mpi_ep_writeq(*request, >chip->RequestDescriptorPostLow,
>> + >scsi_lookup_lock);
>> +}
>> +
>> +/**
>>   * _base_put_smid_scsi_io - send SCSI_IO request to firmware
>>   * @ioc: per adapter object
>>   * @smid: system request message index
>> @@ -3356,7 +3427,23 @@ _base_put_smid_hi_priority(struct MPT3SAS_ADAPTER 
>> *ioc, u16 smid,
>>   u16 msix_task)
>>  {
>>   Mpi2RequestDescriptorUnion_t descriptor;
>> - u64 *request = (u64 *)
>> + void *mpi_req_iomem;
>> + u64 *request;
>> +
>> + if (ioc->is_mcpu_endpoint) {
>> + MPI2RequestHeader_t *request_hdr;
>> +
>> + __le32 *mfp = (__le32 *)mpt3sas_base_get_msg_frame(ioc, smid);
>> +
>> + request_hdr = 

Re: [V1 0/6] mpt3sas: Adding MPI Endpoint device support.

[Suganath Prabu Subramani]

Gentle Reminder, Any update on this ?

Thanks,
Suganath Prabu S

On Wed, Feb 7, 2018 at 4:21 PM, Suganath Prabu S
 wrote:
> V1 Change info:
>
> * Few sparse warning fixes over initial patch set.
> * For 32 bit Arch,_base_writeq function is identical
> to _base_mpi_ep_writeq, Removed duplicate code as suggested by Martin.
>
> Andromeda is a PCIe switch, and it has a dedicated management
>  CPU (mCPU), nonvolatile flash memory, RAM etc... and
>  Linux kernel runs on mCPU. MPI Endpoint driver is the
>  management driver for Andromeda.
>
> The Plx Manager driver running on mCPU synthesizes a
>  virtual/Synthetic MPI End point to host.
> Synthetic MPI End point is emulated IT firmware running on
>  Linux operating system, which interfaces with PLX management
>  driver.
>
> PLX Management driver integrates IOCFW in same driver binary.
> At the end of Plx_Mgr driver load, it initializes IOC FW as well.
> Current implementation is single instance
>  of IOC FW (as it supports only one host).
>
>  PLX management driver will provide required resources
> and infrastructure for Synthetic MPI End point.
>
> Existing PLXManagement driver will reserve virtual slot for
>  MPI end point. currently, Virtual slot number 29 is reserved
>  for MPI end point.
>
> Synthetic device in management driver will be marked as
>  new type “PLX_DEV_TYPE_SYNTH_MPI_EP”. PLXmanagement driver
>  will interface with Synthetic MPI Endpoint for any
>  communication happening on PLX_DEV_TYPE_SYNTH_MPI_EP device
>  type from host.
>
> Link between host and PLX C2 is in below diagram.
>
>  ___
>  ___|   |
> |   |   |   |
> | PLX C2|===|HOST   |
> | PCI - |===|   MACHINE |
> |  SWITCH   |   |   |
> |___|   |   |
> ||  |___|
> ||
> ||
>  ___||__
> |   |
> |  MCPU |
> |   |
> |___|
>
>
>
>  After MPI end point implementation -
> (Host will see dedicated Virtual SLOT as MPI End point.)
> In Below single line is logical channel for MPI Endpoint
>  ___
>  ___|   |
> |   |   |   |
> | PLX C2|===|   HOST|
> | PCI - |===|   MACHINE |
> |  SWITCH   |   |   |
> |   |   |  ---  |
> |___|---| | IT DRIVER | |
> ||  |   |  ---  |
> ||  |   |___|
> ||  |
> ||  |
>  ___||__|___
> |   ||  |   |
> |  MCPU |   |
> |___|   |
> |   | PLX MGR|  |
> |   | DRIVER |  |
> |   ||  |
> |   |   |
> |___|_  |
> |   | | |
> |   |IOC FW   | |
> |   |_| |
> |___|
>
> PLXmanagement driver will create MPI end point based on
>  device table definition. PLXManagement driver will also
>  populate Synthetic device tree based on Device Table
>  for each host.
>
> From host it will be seen as IT HBA (Simplified version of SAS2/MPI2)
> (PCI Device, in which emulated IT FW running on mCPU behind Synthetic
>  endpoint of PCISWITCH). For host it is considered as actual
>  Physical Device.
>
> PLX Management driver provide interface to do DMA from mCPU to Host
>  using “MCPU Response Data Buffer“ method. DMA from Host to mCPU using
>  “MCPU Response Data Buffer” is not possible.
>
> Why DMA from host to mCPU is not possible using Responsebuffer ?
>  MCPU Response buffer is not really for reading from host
>  (reading will work, but answer TLP will not come back to the CSR FIFO,
>  but will go to the MCPU root complex - which could be an
>  unexpected read completion!
>
> Existing host driver (mpt2sas) will not work
>  for MPI end point. As the interface to DMA from host to mCPU is
>  not present for Mcpu/MPI Endpoint device, To overcome this
>  Driver should do double copy of those buffer directly to the
>  mCPU memory region via BAR-0 region.
>
> The host BAR0 region is divided into different group to serve Host
>  assisted DMA.
>
>  0- 255 System register(Doorbell, Host Interrupt etc)
>  256  - 4352MPI Frame. (This is based on maxCredit 32)
>  4352 - 4864Reply_free pool (512 byte is reserved considering
> maxCredit 32. Reply needsextra room, for mCPU case
> kept four times of maxCredit)
>  4864 -17152SGE chain element.
> (32 command * 3 chain of 128 byte size = 12288)
>  17152 -x   Host buffer mapped with smid.
> (Each smid 

Re: [PATCH] mpt3sas: fix an out of bound write

[Suganath Prabu Subramani]

Hi,
Please consider this patch as Ack-by: Suganath Prabu S


Thanks.

On Fri, Jan 19, 2018 at 8:52 PM, Tomas Henzl  wrote:
> cpu_msix_table is allocated to store online cpus, but pci_irq_get_affinity
> may return cpu_possible_mask which is then used to access cpu_msix_table.
> That causes bad user experience.
> Fix limits access to only online cpus, I've also added an additonal test
> to protect from an unlikely change in cpu_online_mask.
>
> Fixes: 1d55abc0e98a0bf35f3af80665aac564e3b30572 scsi: mpt3sas: switch to 
> pci_alloc_irq_vectors
>
> Signed-off-by: Tomas Henzl 
> ---
>  drivers/scsi/mpt3sas/mpt3sas_base.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c 
> b/drivers/scsi/mpt3sas/mpt3sas_base.c
> index 13d6e4ec3..59a87ca32 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_base.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
> @@ -2410,8 +2410,11 @@ _base_assign_reply_queues(struct MPT3SAS_ADAPTER *ioc)
> continue;
> }
>
> -   for_each_cpu(cpu, mask)
> +   for_each_cpu_and(cpu, mask, cpu_online_mask) {
> +   if (cpu >= ioc->cpu_msix_table_sz)
> +   break;
> ioc->cpu_msix_table[cpu] = 
> reply_q->msix_index;
> +   }
> }
> return;
> }
> --
> 2.14.3
>

Re: [PATCH 4/6] mpt3sas: Introduce Base function for cloning.

[Suganath Prabu Subramani]

Hi All,

We tried to reproduce below error
"drivers/scsi/mpt3sas/mpt3sas_base.c:315:10: error: implicit
declaration of function 'mpt3sas_scsih_scsi_lookup_get'; did you mean
scmd = mpt3sas_scsih_scsi_lookup_get(ioc, smid);"

with base code and Make file flags (sparse) as mentioned under
reproduce in auto build test log.

We are not seeing this error. We have reviewed the code and it seems
to be fine. Let us know if we miss something here.

base:   https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__

  drivers/scsi/mpt3sas/mpt3sas_base.c: In function '_clone_sg_entries':
   drivers/scsi/mpt3sas/mpt3sas_base.c:315:10: error: implicit
declaration of function 'mpt3sas_scsih_scsi_lookup_get'; did you mean
scmd = mpt3sas_scsih_scsi_lookup_get(ioc, smid);
^
mpt3sas_scsih_issue_locked_tm
   drivers/scsi/mpt3sas/mpt3sas_base.c:315:8: warning: assignment
makes pointer from integer without a cast
scmd = mpt3sas_scsih_scsi_lookup_get(ioc, smid);
^
   At top level:
   drivers/scsi/mpt3sas/mpt3sas_base.c:278:13: warning:
'_clone_sg_entries' defined but not used
static void _clone_sg_entries(struct MPT3SAS_ADAPTER
^
   cc1: some warnings being treated as errors


Thanks,
Suganath Prabu S

On Sat, Jan 20, 2018 at 11:37 PM, kbuild test robot  wrote:
> Hi Suganath,
>
> I love your patch! Perhaps something to improve:
>
> [auto build test WARNING on scsi/for-next]
> [also build test WARNING on v4.15-rc8 next-20180119]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improve the system]
>
> url:
> https://github.com/0day-ci/linux/commits/Suganath-Prabu-S/mpt3sas-Add-PCI-device-ID-for-Andromeda/20180121-002454
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
> reproduce:
> # apt-get install sparse
> make ARCH=x86_64 allmodconfig
> make C=1 CF=-D__CHECK_ENDIAN__
>
>
> sparse warnings: (new ones prefixed by >>)
>
>>> drivers/scsi/mpt3sas/mpt3sas_base.c:142:24: sparse: cast from restricted 
>>> __le32
>drivers/scsi/mpt3sas/mpt3sas_base.c:142:24: sparse: incorrect type in 
> argument 1 (different base types) @@ expected unsigned int val @@ got 
> restrunsigned int val @@
>drivers/scsi/mpt3sas/mpt3sas_base.c:142:24: expected unsigned int val
>drivers/scsi/mpt3sas/mpt3sas_base.c:142:24: got restricted __le32 
>>> drivers/scsi/mpt3sas/mpt3sas_base.c:142:64: sparse: incorrect type in 
>>> argument 2 (different address spaces) @@ expected void volatile @@ got @@
>drivers/scsi/mpt3sas/mpt3sas_base.c:142:64: expected void volatile
>drivers/scsi/mpt3sas/mpt3sas_base.c:142:64: got void COPYING CREDITS 
> Documentation Kbuild Kconfig MAINTAINERS Makefile README arch block certs 
> crypto drivers firmware fs include init ipc kernel lib mm net samples scripts 
> security sound tools usr virt
>drivers/scsi/mpt3sas/mpt3sas_base.c:162:24: sparse: cast removes address 
> space of expression
>drivers/scsi/mpt3sas/mpt3sas_base.c:315:24: sparse: undefined identifier 
> 'mpt3sas_scsih_scsi_lookup_get'
>drivers/scsi/mpt3sas/mpt3sas_base.c:1164:42: sparse: incorrect type in 
> assignment (different base types) @@ expected unsigned short Event @@ got 
> short Event @@
>drivers/scsi/mpt3sas/mpt3sas_base.c:1165:49: sparse: incorrect type in 
> assignment (different base types) @@ expected unsigned int EventContext @@ 
> got ed int EventContext @@
>drivers/scsi/mpt3sas/mpt3sas_base.c:1383:64: sparse: incorrect type in 
> argument 2 (different address spaces) @@ expected void volatile @@ got oid 
> volatile @@
>drivers/scsi/mpt3sas/mpt3sas_base.c:1432:52: sparse: incorrect type in 
> argument 2 (different address spaces) @@ expected void volatile @@ got oid 
> volatile @@
>drivers/scsi/mpt3sas/mpt3sas_base.c:2964:32: sparse: cast removes address 
> space of expression
>drivers/scsi/mpt3sas/mpt3sas_base.c:3256:16: sparse: incorrect type in 
> argument 1 (different base types) @@ expected unsigned long val @@ got 
> restunsigned long val @@
>drivers/scsi/mpt3sas/mpt3sas_base.c:3256:16: sparse: incorrect type in 
> argument 1 (different base types) @@ expected unsigned long val @@ got 
> restunsigned long val @@
>drivers/scsi/mpt3sas/mpt3sas_base.c:3256:16: sparse: incorrect type in 
> argument 1 (different base types) @@ expected unsigned long val @@ got 
> restunsigned long val @@
>drivers/scsi/mpt3sas/mpt3sas_base.c:3256:16: sparse: incorrect type in 
> argument 1 (different base types) @@ expected unsigned long val @@ got 
> restunsigned long val @@
>drivers/scsi/mpt3sas/mpt3sas_base.c:3256:16: sparse: incorrect type in 
> argument 1 (different base types) @@ expected unsigned long val @@ got 
> restunsigned long val @@
>

Re: [PATCH v4 00/14] mpt3sas driver NVMe support:

[Suganath Prabu Subramani]

Hi Martin,

On Fri, Sep 15, 2017 at 6:37 AM, Martin K. Petersen
 wrote:
>
> Suganath,
>
>> Is there any update on the submitted mpt3sas patches.
>
> We are waiting for you to report back your findings on PRP vs. SGL.

We are working on this, since there is h/w dependent, we are in
discussion with H/W & F/W team and doing experiments. If there is no
impact, and if SGL translation has to be removed, this change has to
go through some phase of testing, before we post it to upstream, since
that is not inline with H/W requirement.

The hardware translation of IEEE SGL to NVMe PRPs has limitation.

We have added the below comment in patch 3 as well:

if a command cannot be translated by hardware then it will go
to firmware and the firmware needs to translate it. And this will
have a performance reduction. To avoid that driver proactively
checks whether the translation will be done in hardware or not,
if not then driver try to translate inside the driver

Current code posted to upstream is inline with hardware requirements
and well tested internally.

SGL vs NVMe PRP building in driver is small sanity check for decision making
and it is not going to change in long run.

Also, Making all PRP buffer may or may not need FW changes (assuming
it is possible.),
we may end up into multiple FW version check.

Since this is main IO path and current driver is following H/W limitation,
we should avoid any changes in this area until and unless change is
universal acceptable in FW (for all type of work load).

Hope this clarifies.
>
> --
> Martin K. Petersen  Oracle Linux Engineering

Thanks,
Suganath Prabu S

Re: [PATCH v4 00/14] mpt3sas driver NVMe support:

[Suganath Prabu Subramani]

Hi Martin,

Is there any update on the submitted mpt3sas patches.

Thanks,
Suganath Prabu S

On Fri, Sep 1, 2017 at 2:09 PM, Suganath Prabu Subramani
<suganath-prabu.subram...@broadcom.com> wrote:
> Hi Martin,
>
> On Fri, Sep 1, 2017 at 8:52 AM, Martin K. Petersen
> <martin.peter...@oracle.com> wrote:
>>
>> Hi Suganath,
>>
>>> Let me explain - NVME device fast path is possible in two ways.  IEEE
>>> SGL and PRP SGL. Due to h/w constraint we choose IEEE SGL only for
>>> smaller IO size.  Both above is true h/w Fast Path and no firmware
>>> involvement.
>>
>>> Agree with you. We are planning to see if we can keep only simple Fast
>>> Path using only PRP.
>>
>> That would be great, thank you!
>>
>>> Currently there is no performance issue for UNMAP translation in FW.
>>
>> Good!
>>
>>>> And yet patch 4 circumvents that statement by adding support for
>>>> encapsulated commands to bypass the FW translation...
>>>
>>> This path is not due to performance reason. User wants to interact
>>> with NVME drive in native NVME command for management.
>>
>> Patch 4 states:
>>
>> "This encapsulated NVMe command is used by applications to send direct
>> NVMe commands to NVMe drives or for handling unmap where the translation
>> at controller/firmware level is having performance issues."
>>
>> --
>
> The statement in description of patch 4 is added by mistake, We ll
> correct the description and re sending that.
>
>> Martin K. Petersen  Oracle Linux Engineering
>
> Thanks,
> Suganath Prabu S

Re: [PATCH v4 00/14] mpt3sas driver NVMe support:

[Suganath Prabu Subramani]

Hi Martin,

On Fri, Sep 1, 2017 at 8:52 AM, Martin K. Petersen
 wrote:
>
> Hi Suganath,
>
>> Let me explain - NVME device fast path is possible in two ways.  IEEE
>> SGL and PRP SGL. Due to h/w constraint we choose IEEE SGL only for
>> smaller IO size.  Both above is true h/w Fast Path and no firmware
>> involvement.
>
>> Agree with you. We are planning to see if we can keep only simple Fast
>> Path using only PRP.
>
> That would be great, thank you!
>
>> Currently there is no performance issue for UNMAP translation in FW.
>
> Good!
>
>>> And yet patch 4 circumvents that statement by adding support for
>>> encapsulated commands to bypass the FW translation...
>>
>> This path is not due to performance reason. User wants to interact
>> with NVME drive in native NVME command for management.
>
> Patch 4 states:
>
> "This encapsulated NVMe command is used by applications to send direct
> NVMe commands to NVMe drives or for handling unmap where the translation
> at controller/firmware level is having performance issues."
>
> --

The statement in description of patch 4 is added by mistake, We ll
correct the description and re sending that.

> Martin K. Petersen  Oracle Linux Engineering

Thanks,
Suganath Prabu S

Re: [PATCH v4 00/14] mpt3sas driver NVMe support:

[Suganath Prabu Subramani]

Hi Martin,
Replied inline.

Thanks,
Suganath Prabu S

On Thu, Aug 31, 2017 at 8:35 AM, Martin K. Petersen
 wrote:
>
> Hi Suganath,
>
>> Theoretically we want to use h/w capability (to translate IEEE to PRP)
>> for smaller IO size to leverage h/w capability.
>
> Nobody says we have to use the capability just because the hardware has
> it.
>
> Unlike some other operating systems, Linux will only submit I/Os to the
> driver that conform to the reported underlying constraints of the
> hardware.

 In general, h/w constraints are handled. What we missed is
Fast Path h/w
which is not exposed to OS.

> I fail to understand how letting the HBA firmware translate an
> SGL to a PRP for a subset of I/Os could do anything but add latency.

 Let me explain - NVME device fast path is possible in two ways.
IEEE SGL and PRP SGL. Due to h/w constraint we choose IEEE SGL only for
smaller IO size.
Both above is true h/w Fast Path and no firmware involvement.

> Plus complexity in the hot path of the driver.

 Agree with you. We are planning to see if we can keep only
simple Fast
Path using only PRP. It will take some time to finalize as we have to
engage h/w and f/w team.  BTW - This area is h/w dependent and we do not
see further changes in this area.

>
>> - If the unmap translation in firmware is slow, why don't you translate
>>   WRITE SAME/w UNMAP set to DSM DEALLOCATE without requiring
>>   applications to do encapsulated passthrough?
>
>> => As of now, current FW supports UNMAP command but not WRITE_SAME for
>> NVME drive. We did some experiment to convert UMAP command in driver,
>> but that is not really giving any performance improvement.
>
> It is imperative that the common use case, Linux' discard
> infrastructure, is working correctly and is as performant as any
> application-driven passthrough workaround.
>
> Unlike SCSI-to-SATA translation you have the benefit of a 1:1 mapping
> between UNMAP and DEALLOCATE. I'm not even sure why there would be a
> significant performance penalty in the firmware?

 I agree. Currently there is no performance issue for UNMAP
translation in
FW.


>> We would like to continue with UNMAP (and all other non-read/write
>> commands) to be handled in FW.
>
> And yet patch 4 circumvents that statement by adding support for
> encapsulated commands to bypass the FW translation...

 This path is not due to performance reason. User wants to
interact with
NVME drive in native NVME command for management.

>
> --
> Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH v4 00/14] mpt3sas driver NVMe support:

[Suganath Prabu Subramani]

Hi Martin,

Replied in line.

- I don't understand why you go through all these hoops to decide
  whether to use PRPs or IEEE scatterlists. If the firmware translation
  is slow, why even bother with the SG format in the first place? Set
  the max I/O size to match MDTS and you're done.

=>  We will  set MDTS value as max hw sectors using
blk_queue_max_hw_sectors(). As of now, we see correct MDTS value is
being set to block layer via VPD page 0xb0 (Block limits VPD page )
response from FW for NVME device.
=> I will remove MDTS checks in IO path.


- What's the benefit of using SG for regular I/O commands?

=>  Broadcom's IT Tri-mode HBA hardware has a capability of
translating IEEE SGLs to PRP's only up to ~4 page block size.
If the IO block size is greater than that (along with other condition
described code base_is_prp_possible), driver has to frame the PRP's to
avoid FW intervention. Both the case is a fast path, but for smaller
IO (up to 20K) size will frame IEEE SGL and large IO size will frame
PRP format SGL. Theoretically we want to use h/w capability (to
translate IEEE to PRP) for smaller IO size to leverage h/w capability.
We are investigating if at all we can send all PRP and avoid checks in
driver, but that exercise may take time as we have many different
opinions. We prefer to use existing code as it is stable and in-line
with h/w requirement.


- If the unmap translation in firmware is slow, why don't you translate
  WRITE SAME/w UNMAP set to DSM DEALLOCATE without requiring
  applications to do encapsulated passthrough?

=> As of now, current FW supports UNMAP command but not WRITE_SAME for
NVME drive. We did some experiment to convert UMAP command in driver,
but that is not really giving any performance improvement. We would
like to continue with UNMAP (and all other non-read/write commands) to
be handled in FW.


- Also make sure you attribute your patches correctly (From: root
). And you don't need
that long CC: list. Just send the patch series to
linux-scsi@vger.kernel.org.

=>  I will fix this type of issue going forward

Thanks,
Suganath Prabu S

On Wed, Aug 23, 2017 at 7:48 AM, Martin K. Petersen
 wrote:
>
> Suganath,
>
>>   mpt3sas: SGL to PRP Translation for I/Os to NVMe  devices
>
> I'm still confused about this patch.
>
>  - I don't understand why you go through all these hoops to decide
>whether to use PRPs or IEEE scatterlists. If the firmware translation
>is slow, why even bother with the SG format in the first place? Set
>the max I/O size to match MDTS and you're done.
>
>  - What's the benefit of using SG for regular I/O commands?
>
>  - If the unmap translation in firmware is slow, why don't you translate
>WRITE SAME/w UNMAP set to DSM DEALLOCATE without requiring
>applications to do encapsulated passthrough?
>
> Also make sure you attribute your patches correctly (From: root
> ). And you don't need that
> long CC: list. Just send the patch series to linux-scsi@vger.kernel.org.
>
> Thanks!
>
> --
> Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH v3 01/13] mpt3sas: Update MPI Header

[Suganath Prabu Subramani]

Hi James,
we have fixed the sparse warnings associated with this patch set and
posting V4 of mpt3sas patches soon. We are also working on sparse
error/warnings that existed before this patch set and we ll be posting
it separately.

Thanks,
Suganath Prabu S

On Wed, Aug 9, 2017 at 2:48 AM, J Freyensee
 wrote:
>
>
> Looks like your header has a white space error:
>
> Applying: mpt3sas: Update MPI Header
> .git/rebase-apply/patch:1452: new blank line at EOF.
> +
>
> Also, FYI, this project has a lot of sparse errors that look like existed
> before your patchset.  As your patchset touches a few of the files that
> have sparse warnings (such as mpt3sas_base.c, mpt3sas_scsih.c, etc), maybe
> you'll want to investigate fixing these things?
>
>
> [mainline-linux]$ make C=1
>   CHK include/config/kernel.release
>   CHK include/generated/uapi/linux/version.h
>   CHK include/generated/utsrelease.h
>   CHK include/generated/bounds.h
>   CHK include/generated/timeconst.h
>   CHK include/generated/asm-offsets.h
>   CALLscripts/checksyscalls.sh
>   CHK scripts/mod/devicetable-offsets.h
>   CHK include/generated/compile.h
>   AR  drivers/scsi/mpt3sas/built-in.o
>   CHECK   drivers/scsi/mpt3sas/mpt3sas_base.c
> drivers/scsi/mpt3sas/mpt3sas_base.c:861:42: warning: incorrect type in
> assignment (different base types)
> drivers/scsi/mpt3sas/mpt3sas_base.c:861:42:expected unsigned short
> [unsigned] [usertype] Event
> drivers/scsi/mpt3sas/mpt3sas_base.c:861:42:got restricted __le16
> [usertype] Event
> drivers/scsi/mpt3sas/mpt3sas_base.c:862:49: warning: incorrect type in
> assignment (different base types)
> drivers/scsi/mpt3sas/mpt3sas_base.c:862:49:expected unsigned int
> [unsigned] [usertype] EventContext
> drivers/scsi/mpt3sas/mpt3sas_base.c:862:49:got restricted __le32
> [usertype] EventContext
> drivers/scsi/mpt3sas/mpt3sas_base.c:1080:64: warning: incorrect type in
> argument 2 (different address spaces)
> drivers/scsi/mpt3sas/mpt3sas_base.c:1080:64:expected void volatile
> [noderef] *addr
> drivers/scsi/mpt3sas/mpt3sas_base.c:1080:64:got unsigned long long
> [usertype] *
> drivers/scsi/mpt3sas/mpt3sas_base.c:1129:52: warning: incorrect type in
> argument 2 (different address spaces)
> drivers/scsi/mpt3sas/mpt3sas_base.c:1129:52:expected void volatile
> [noderef] *addr
> drivers/scsi/mpt3sas/mpt3sas_base.c:1129:52:got unsigned long long
> [usertype] *
> drivers/scsi/mpt3sas/mpt3sas_base.c:1519:36: warning: incorrect type in
> assignment (different base types)
> drivers/scsi/mpt3sas/mpt3sas_base.c:1519:36:expected unsigned long long
> [unsigned] [long] [long long] [usertype] 
> drivers/scsi/mpt3sas/mpt3sas_base.c:1519:36:got restricted __le64
> [usertype] 
> drivers/scsi/mpt3sas/mpt3sas_base.c:1532:37: warning: incorrect type in
> assignment (different base types)
> drivers/scsi/mpt3sas/mpt3sas_base.c:1532:37:expected unsigned long long
> [unsigned] [long] [long long] [usertype] 
> drivers/scsi/mpt3sas/mpt3sas_base.c:1532:37:got restricted __le64
> [usertype] 
> drivers/scsi/mpt3sas/mpt3sas_base.c:1552:45: warning: incorrect type in
> assignment (different base types)
> drivers/scsi/mpt3sas/mpt3sas_base.c:1552:45:expected unsigned long long
> [unsigned] [long] [long long] [usertype] 
> drivers/scsi/mpt3sas/mpt3sas_base.c:1552:45:got restricted __le64
> [usertype] 
> drivers/scsi/mpt3sas/mpt3sas_base.c:1565:45: warning: incorrect type in
> assignment (different base types)
> drivers/scsi/mpt3sas/mpt3sas_base.c:1565:45:expected unsigned long long
> [unsigned] [long] [long long] [usertype] 
> drivers/scsi/mpt3sas/mpt3sas_base.c:1565:45:got restricted __le64
> [usertype] 
> drivers/scsi/mpt3sas/mpt3sas_base.c:1575:36: warning: incorrect type in
> assignment (different base types)
> drivers/scsi/mpt3sas/mpt3sas_base.c:1575:36:expected unsigned long long
> [unsigned] [long] [long long] [usertype] 
> drivers/scsi/mpt3sas/mpt3sas_base.c:1575:36:got restricted __le64
> [usertype] 
> drivers/scsi/mpt3sas/mpt3sas_base.c:1594:5: warning: symbol 'base_mod64'
> was not declared. Should it be static?
> drivers/scsi/mpt3sas/mpt3sas_base.c:1717:36: warning: incorrect type in
> assignment (different base types)
> drivers/scsi/mpt3sas/mpt3sas_base.c:1717:36:expected unsigned long long
> [unsigned] [long] [long long] [usertype] 
> drivers/scsi/mpt3sas/mpt3sas_base.c:1717:36:got restricted __le64
> [usertype] 
> drivers/scsi/mpt3sas/mpt3sas_base.c:1722:28: warning: incorrect type in
> assignment (different base types)
> drivers/scsi/mpt3sas/mpt3sas_base.c:1722:28:expected unsigned long long
> [unsigned] [long] [long long] [usertype] 
> drivers/scsi/mpt3sas/mpt3sas_base.c:1722:28:got restricted __le64
> [usertype] 
> drivers/scsi/mpt3sas/mpt3sas_base.c:1620:1: warning: symbol
> 'base_make_prp_nvme' was not declared. Should it be static?
> 

Re: [PATCH v3 03/13] mpt3sas: SGL to PRP Translation for I/Os to NVMe devices

[Suganath Prabu Subramani]

Hi Martin,
This code was added to detect holes, when we started testing with 4.9
kernel. when we disabled "use_blk_mq" and no merges, we are hitting
issues with holes. Anyhow In latest upstream, it got fixed with this
commit 5a8d75a1b8c99bdc926ba69b7b7dbe4fae81a5af

So we are removing the code related to hole detection .

Thanks,
Suganath Prabu S



On Tue, Aug 8, 2017 at 9:42 PM, Martin K. Petersen
 wrote:
>
> Suganath,
>
>> + /*
>> +  ** Below code detects gaps/holes in IO data buffers.
>> +  ** What does holes/gaps mean?
>> +  ** Any SGE except first one in a SGL starts at non NVME page size
>> +  ** aligned address OR Any SGE except last one in a SGL ends at
>> +  ** non NVME page size boundary.
>> +  **
>> +  ** Driver has already informed block layer by setting boundary rules
>> +  ** for bio merging done at NVME page size boundary calling kernel API
>> +  ** blk_queue_virt_boundary inside slave_config.
>> +  ** Still there is possibility of IO coming with holes to driver 
>> because
>> +  ** of IO merging done by IO scheduler.
>
> All this SGL to PRP code needs to go.
>
> If you are seeing anything that's not a valid PRP after setting the
> queue virt boundary then there's a block layer bug that needs to be
> debugged and fixed. Regardless of whether you are using an I/O scheduler
> or not.
>
> --
> Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH v2 00/13] mpt3sas driver NVMe support:

[Suganath Prabu Subramani]

Is there any update on these patches ?

Thanks,
Suganath Prabu S

On Fri, Jul 14, 2017 at 6:52 PM, Suganath Prabu S
 wrote:
> Ventura Series controller are Tri-mode. The controller and
> firmware are capable of supporting NVMe devices and
> PCIe switches to be connected with the controller. This
> patch set adds driver level support for NVMe devices and
> PCIe switches.
>
> Suganath Prabu S (13):
>   mpt3sas: Update MPI Header
>   mpt3sas: Add nvme device support in slave alloc, target alloc and
> probe
>   mpt3sas: SGL to PRP Translation for I/Os to NVMe  devices
>   mpt3sas: Added support for nvme encapsulated request message.
>   mpt3sas: API 's to support NVMe drive addition to SML
>   mpt3sas: API's to remove nvme drive from sml
>   mpt3sas: Handle NVMe PCIe device related events generated
>from firmware.
>   mpt3sas: Set NVMe device queue depth as 128
>   mpt3sas: scan and add nvme device after controller reset
>   mpt3as: Add-Task-management-debug-info-for-NVMe-drives.
>   mpt3sas: NVMe drive support for BTDHMAPPING ioctl command and log
> info
>   mpt3sas: Fix nvme drives checking for tlr.
>   mpt3sas: Update mpt3sas driver version.
>
>  drivers/scsi/mpt3sas/mpi/mpi2.h  |   43 +-
>  drivers/scsi/mpt3sas/mpi/mpi2_cnfg.h |  647 ++-
>  drivers/scsi/mpt3sas/mpi/mpi2_init.h |   11 +-
>  drivers/scsi/mpt3sas/mpi/mpi2_ioc.h  |  331 ++-
>  drivers/scsi/mpt3sas/mpi/mpi2_pci.h  |  142 +++
>  drivers/scsi/mpt3sas/mpi/mpi2_tool.h |   14 +-
>  drivers/scsi/mpt3sas/mpt3sas_base.c  |  710 +++-
>  drivers/scsi/mpt3sas/mpt3sas_base.h  |  171 +++-
>  drivers/scsi/mpt3sas/mpt3sas_config.c|  100 ++
>  drivers/scsi/mpt3sas/mpt3sas_ctl.c   |  158 ++-
>  drivers/scsi/mpt3sas/mpt3sas_scsih.c | 1874 
> --
>  drivers/scsi/mpt3sas/mpt3sas_warpdrive.c |2 +-
>  12 files changed, 4063 insertions(+), 140 deletions(-)
>  create mode 100644 drivers/scsi/mpt3sas/mpi/mpi2_pci.h
>
> Thanks,
> Suganath Prabu S

Re: [PATCH 05/13] mpt3sas: Set NVMe device queue depth as 128

[Suganath Prabu Subramani]

Hi Elliott,
We are maintaining NVMe drives as scsi device in mpt3sas driver.
There are lot of firmware/hardware level dependencies and after lot of
discussions we arrived this value (128).
So, we prefer not to provide module parameter to change this at least for now.

Thanks,
Suganath Prabu S

On Tue, Jul 11, 2017 at 10:53 PM, Elliott, Robert (Persistent Memory)
 wrote:
>> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.h
>> @@ -115,7 +115,7 @@
>>
>>  #define MPT3SAS_RAID_MAX_SECTORS 8192
>>  #define MPT3SAS_HOST_PAGE_SIZE_4K12
>> -
>> +#define MPT3SAS_NVME_QUEUE_DEPTH 128
> ...
>> + /*TODO-right Queue Depth?*/
>> + qdepth = MPT3SAS_NVME_QUEUE_DEPTH;
>> + ds = "NVMe";
>
> The native NVMe driver is getting a modparam to set that value (rather than
> using a #define of 1024) in this patch:
> http://lists.infradead.org/pipermail/linux-nvme/2017-July/011734.html
>
> Perhaps this driver should do the same.
>
> ---
> Robert Elliott, HPE Persistent Memory
>
>

Re: [PATCH 02/13] mpt3sas: SGL to PRP Translation for I/Os to NVMe devices

[Suganath Prabu Subramani]

Hi Keith,
We have made change and submitted V2 of patch set.

Thanks,
Suganath Prabu S

On Wed, Jul 12, 2017 at 5:34 AM, Keith Busch  wrote:
> On Tue, Jul 11, 2017 at 01:55:02AM -0700, Suganath Prabu S wrote:
>> +/**
>> + * _base_check_pcie_native_sgl - This function is called for PCIe end 
>> devices to
>> + * determine if the driver needs to build a native SGL.  If so, that native
>> + * SGL is built in the special contiguous buffers allocated especially for
>> + * PCIe SGL creation.  If the driver will not build a native SGL, return
>> + * TRUE and a normal IEEE SGL will be built.  Currently this routine
>> + * supports NVMe.
>> + * @ioc: per adapter object
>> + * @mpi_request: mf request pointer
>> + * @smid: system request message index
>> + * @scmd: scsi command
>> + * @pcie_device: points to the PCIe device's info
>> + *
>> + * Returns 0 if native SGL was built, 1 if no SGL was built
>> + */
>> +static int
>> +_base_check_pcie_native_sgl(struct MPT3SAS_ADAPTER *ioc,
>> + Mpi25SCSIIORequest_t *mpi_request, u16 smid, struct scsi_cmnd *scmd,
>> + struct _pcie_device *pcie_device)
>> +{
>
> 
>
>> + /* Return 0, indicating we built a native SGL. */
>> + return 1;
>> +}
>
> This function doesn't return 0 ever. Not sure why it's here.
>
> Curious about your device, though, if a nvme native SGL can *not* be
> built, does the HBA firmware then buffer it in its local memory before
> sending/receiving to/from the host?
>
> And if a native SGL can be built, does the NVMe target DMA directly
> to/from host memory, giving a performance boost?

Re: [PATCH 00/13]mpt3sas driver NVMe support:

[Suganath Prabu Subramani]

Is there any update on this ?
Will include  linux-n...@lists.infradead.org in next version of this
patch submission, if there is any change suggestion.

Thanks,
Suganath Prabu S

On Thu, Jun 29, 2017 at 8:01 PM, Johannes Thumshirn  wrote:
> On Thu, Jun 29, 2017 at 07:49:01PM +0530, Suganath Prabu S wrote:
>> Ventura Series controller are Tri-mode. The controller and
>> firmware are capable of supporting NVMe devices and
>> PCIe switches to be connected with the controller. This
>> patch set adds driver level support for NVMe devices and
>> PCIe switches.
>
> Hi Suganath,
> Can you please also Cc linux-n...@lists.infradead.org for NVMe related topics.
>
> Thanks,
> Johannes
> --
> Johannes Thumshirn  Storage
> jthumsh...@suse.de+49 911 74053 689
> SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
> GF: Felix Imendörffer, Jane Smithard, Graham Norton
> HRB 21284 (AG Nürnberg)
> Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH 03/10] mpt3sas: Implement device_remove_in_progress check in IOCTL path

[Suganath Prabu Subramani]

Hi Hannes,

Thanks,
>From facts.MaxDevHandle value from firmware, driver will get to know
the max device handle or devices that it can support. Based on that
value only driver will allocate memory for bit mapping.
And driver will not receive any events from controller, if it exceeds
the max devices it can support. So, there is No possibility that
driver can add more devices than bitmap.

Thanks,
Suganath Prabu S



On Tue, Oct 25, 2016 at 3:21 PM, Hannes Reinecke <h...@suse.de> wrote:
> On 10/25/2016 11:19 AM, Suganath Prabu Subramani wrote:
>> Hi Hannes,
>>
>> Please give us little more info on the third comment. It ll help us to
>> understand better and
>> incorporate required changes.
>>
>> Comment is  "Why don't you need to check for the size of the bitmap here?"
>>
>> i have taken care of other two comments in this patch.
>>
>>>   /* check if device is present */
>>> @@ -5467,6 +5482,7 @@ _scsih_add_device(struct MPT3SAS_ADAPTER *ioc, u16 
>>> handle, u8 phy_num,
>>> sas_device = mpt3sas_get_sdev_by_addr(ioc,
>>> sas_address);
>>> if (sas_device) {
>>> +   clear_bit(handle, ioc->pend_os_device_add);
>>> sas_device_put(sas_device);
>>> return -1;
>>> }
>>
>> Why don't you need to check for the size of the bitmap here?
>>
>>
> Thing is, you are using 'ioc->pend_os_device_add' as a bitmap to track
> which devices to add.
> Which in turn means that the overall number of devices you _can_ add is
> restricted by the size of the bitmap.
> But as you're adding devices you (might) increase the number of devices,
> potentially overflowing the bitmap.
> Hence the question: is it possible that you can add _more_ devices than
> the bitmap can hold?
> Or, to put it the other way round: Why don't you need to check the size
> of the bitmap to avoid accessing an invalid bit beyond the end of the mask?
>
> Cheers,
>
> Hannes
> --
> Dr. Hannes ReineckeTeamlead Storage & Networking
> h...@suse.de   +49 911 74053 688
> SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
> GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
> HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 03/10] mpt3sas: Implement device_remove_in_progress check in IOCTL path

[Suganath Prabu Subramani]

Hi Hannes,

Please give us little more info on the third comment. It ll help us to
understand better and
incorporate required changes.

Comment is  "Why don't you need to check for the size of the bitmap here?"

i have taken care of other two comments in this patch.

>   /* check if device is present */
> @@ -5467,6 +5482,7 @@ _scsih_add_device(struct MPT3SAS_ADAPTER *ioc, u16 
> handle, u8 phy_num,
> sas_device = mpt3sas_get_sdev_by_addr(ioc,
> sas_address);
> if (sas_device) {
> +   clear_bit(handle, ioc->pend_os_device_add);
> sas_device_put(sas_device);
> return -1;
> }

Why don't you need to check for the size of the bitmap here?


Thanks,
Suganath Prabu S

On Mon, Oct 24, 2016 at 8:17 PM, Hannes Reinecke  wrote:
> On 10/20/2016 02:20 PM, Suganath Prabu S wrote:
>>
>> When device missing event arrives, device_remove_in_progress bit will be
>> set and hence driver has to stop sending IOCTL commands.Now the check has
>> been added in IOCTL path to test device_remove_in_progress bit is set, if
>> so then IOCTL will be failed printing failure message.
>>
>> Signed-off-by: Chaitra P B 
>> Signed-off-by: Sathya Prakash 
>> Signed-off-by: Suganath Prabu S 
>> ---
>>  drivers/scsi/mpt3sas/mpt3sas_base.c  | 19 +++
>>  drivers/scsi/mpt3sas/mpt3sas_base.h  |  5 
>>  drivers/scsi/mpt3sas/mpt3sas_ctl.c   | 46
>> ++--
>>  drivers/scsi/mpt3sas/mpt3sas_scsih.c | 24 ++-
>>  4 files changed, 86 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c
>> b/drivers/scsi/mpt3sas/mpt3sas_base.c
>> index 4ea81e1..9ad7f7c 100644
>> --- a/drivers/scsi/mpt3sas/mpt3sas_base.c
>> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
>> @@ -5334,6 +5334,21 @@ mpt3sas_base_attach(struct MPT3SAS_ADAPTER *ioc)
>> goto out_free_resources;
>> }
>>
>> +   /* allocate memory for pending OS device add list */
>> +   ioc->pend_os_device_add_sz = (ioc->facts.MaxDevHandle / 8);
>> +   if (ioc->facts.MaxDevHandle % 8)
>> +   ioc->pend_os_device_add_sz++;
>> +   ioc->pend_os_device_add = kzalloc(ioc->pend_os_device_add_sz,
>> +   GFP_KERNEL);
>> +   if (!ioc->pend_os_device_add)
>> +   goto out_free_resources;
>> +
>> +   ioc->device_remove_in_progress_sz = ioc->pend_os_device_add_sz;
>> +   ioc->device_remove_in_progress =
>> +   kzalloc(ioc->device_remove_in_progress_sz, GFP_KERNEL);
>> +   if (!ioc->device_remove_in_progress)
>> +   goto out_free_resources;
>> +
>> ioc->fwfault_debug = mpt3sas_fwfault_debug;
>>
>> /* base internal command bits */
>> @@ -5416,6 +5431,8 @@ mpt3sas_base_attach(struct MPT3SAS_ADAPTER *ioc)
>> kfree(ioc->reply_post_host_index);
>> kfree(ioc->pd_handles);
>> kfree(ioc->blocking_handles);
>> +   kfree(ioc->device_remove_in_progress);
>> +   kfree(ioc->pend_os_device_add);
>> kfree(ioc->tm_cmds.reply);
>> kfree(ioc->transport_cmds.reply);
>> kfree(ioc->scsih_cmds.reply);
>> @@ -5457,6 +5474,8 @@ mpt3sas_base_detach(struct MPT3SAS_ADAPTER *ioc)
>> kfree(ioc->reply_post_host_index);
>> kfree(ioc->pd_handles);
>> kfree(ioc->blocking_handles);
>> +   kfree(ioc->device_remove_in_progress);
>> +   kfree(ioc->pend_os_device_add);
>> kfree(ioc->pfacts);
>> kfree(ioc->ctl_cmds.reply);
>> kfree(ioc->ctl_cmds.sense);
>> diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.h
>> b/drivers/scsi/mpt3sas/mpt3sas_base.h
>> index 3e71bc1..4221a4d 100644
>> --- a/drivers/scsi/mpt3sas/mpt3sas_base.h
>> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.h
>> @@ -1079,6 +1079,9 @@ struct MPT3SAS_ADAPTER {
>> void*pd_handles;
>> u16 pd_handles_sz;
>>
>> +   void*pend_os_device_add;
>> +   u16 pend_os_device_add_sz;
>> +
>> /* config page */
>> u16 config_page_sz;
>> void*config_page;
>> @@ -1187,6 +1190,8 @@ struct MPT3SAS_ADAPTER {
>> struct SL_WH_EVENT_TRIGGERS_T diag_trigger_event;
>> struct SL_WH_SCSI_TRIGGERS_T diag_trigger_scsi;
>> struct SL_WH_MPI_TRIGGERS_T diag_trigger_mpi;
>> +   void*device_remove_in_progress;
>> +   u16 device_remove_in_progress_sz;
>>  };
>>
>>  typedef u8 (*MPT_CALLBACK)(struct MPT3SAS_ADAPTER *ioc, u16 smid, u8
>> msix_index,
>> diff --git a/drivers/scsi/mpt3sas/mpt3sas_ctl.c
>> b/drivers/scsi/mpt3sas/mpt3sas_ctl.c
>> index 26cdc12..f204ce1 100644
>> --- a/drivers/scsi/mpt3sas/mpt3sas_ctl.c
>> +++ b/drivers/scsi/mpt3sas/mpt3sas_ctl.c
>> @@ -654,6 +654,7 @@ _ctl_do_mpt_command(struct MPT3SAS_ADAPTER 

[PATCH v2 1/2] mpt3sas: Free memory pools before retrying to allocate with different value.

[Suganath Prabu Subramani]

From: Suganath prabu Subramani <suganath-prabu.subram...@avagotech.com>

Deallocate resources before reallocating of the same in retry_allocation
path of _base_allocate_memory_pools()

Signed-off-by: Suganath prabu Subramani <suganath-prabu.subram...@avagotech.com>
Signed-off-by: Chaitra P B <chaitra.basa...@broadcom.com>
---
 drivers/scsi/mpt3sas/mpt3sas_base.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c 
b/drivers/scsi/mpt3sas/mpt3sas_base.c
index afdb13a..5f2 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -3414,6 +3414,7 @@ _base_allocate_memory_pools(struct MPT3SAS_ADAPTER *ioc,  
int sleep_flag)
goto out;
retry_sz = 64;
ioc->hba_queue_depth -= retry_sz;
+   _base_release_memory_pools(ioc);
goto retry_allocation;
}
 
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 2/2] Updating maintainers list for MPT FUSION DRIVERS.

[Suganath Prabu Subramani]

From: Suganath prabu Subramani <suganath-prabu.subram...@avagotech.com>

Updating maintainers list for MPT FUSION DRIVERS,
broadcom support link and email id.

Signed-off-by: Suganath prabu Subramani <suganath-prabu.subram...@avagotech.com>
Signed-off-by: Chaitra P B <chaitra.basa...@broadcom.com>
---
 MAINTAINERS | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 30aca4a..003eef4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6675,13 +6675,12 @@ S:  Maintained
 F: arch/arm/mach-lpc32xx/
 
 LSILOGIC MPT FUSION DRIVERS (FC/SAS/SPI)
-M: Nagalakshmi Nandigama <nagalakshmi.nandig...@avagotech.com>
-M: Praveen Krishnamoorthy <praveen.krishnamoor...@avagotech.com>
-M: Sreekanth Reddy <sreekanth.re...@avagotech.com>
-M: Abhijit Mahajan <abhijit.maha...@avagotech.com>
-L: mpt-fusionlinux@avagotech.com
+M: Sathya Prakash <sathya.prak...@broadcom.com>
+M: Chaitra P B <chaitra.basa...@broadcom.com>
+M: Suganath Prabu Subramani <suganath-prabu.subram...@broadcom.com>
+L: mpt-fusionlinux@broadcom.com
 L: linux-scsi@vger.kernel.org
-W: http://www.lsilogic.com/support
+W: http://www.avagotech.com/support/
 S: Supported
 F: drivers/message/fusion/
 F: drivers/scsi/mpt2sas/
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/1] mpt3sas: Fix - Remove cpumask_clear for zalloc_cpumask_var and dont free free_cpu_mask_var before reply_q

[Suganath prabu Subramani]

From: Suganath prabu Subramani <suganath-prabu.subram...@broadcom.com>

Removed cpumask_clear as it is not required for zalloc_cpumask_var and
free free_cpumask_var before freeing reply_q.

Signed-off-by: Suganath prabu Subramani <suganath-prabu.subram...@broadcom.com>
Signed-off-by: Chaitra P B <chaitra.basa...@broadcom.com>
---
 drivers/scsi/mpt3sas/mpt3sas_base.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c 
b/drivers/scsi/mpt3sas/mpt3sas_base.c
index c0a9d97..afdb13a 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -1855,7 +1855,6 @@ _base_request_irq(struct MPT3SAS_ADAPTER *ioc, u8 index, 
u32 vector)
kfree(reply_q);
return -ENOMEM;
}
-   cpumask_clear(reply_q->affinity_hint);
}
 
atomic_set(_q->busy, 0);
@@ -1870,8 +1869,8 @@ _base_request_irq(struct MPT3SAS_ADAPTER *ioc, u8 index, 
u32 vector)
if (r) {
pr_err(MPT3SAS_FMT "unable to allocate interrupt %d!\n",
reply_q->name, vector);
-   kfree(reply_q);
free_cpumask_var(reply_q->affinity_hint);
+   kfree(reply_q);
return -EBUSY;
}
 
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[mpt3sas driver V1 6/10] mpt3sas: Added smp_affinity_enable module parameter.

[Suganath prabu Subramani]

Module parameter to enable/disable configuring
affinity hint for msix vector.
SMP affinity feature can be enabled/disabled by setting
module parameter "smp_affinity_enable" to 1/0.
By default this feature is enabled. (smp_affinity_enable = 1 enabled).

Signed-off-by: Suganath prabu Subramani <suganath-prabu.subram...@avagotech.com>
Signed-off-by: Chaitra P B <chaitra.basa...@avagotech.com>
---
 drivers/scsi/mpt3sas/mpt3sas_base.c | 37 ++---
 1 file changed, 26 insertions(+), 11 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c 
b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 31838d9a..582ba4b 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -83,6 +83,10 @@ static int msix_disable = -1;
 module_param(msix_disable, int, 0);
 MODULE_PARM_DESC(msix_disable, " disable msix routed interrupts (default=0)");
 
+static int smp_affinity_enable = 1;
+module_param(smp_affinity_enable, int, S_IRUGO);
+MODULE_PARM_DESC(smp_affinity_enable, "SMP affinity feature enable/disbale 
Default: enable(1)");
+
 static int max_msix_vectors = -1;
 module_param(max_msix_vectors, int, 0);
 MODULE_PARM_DESC(max_msix_vectors,
@@ -1812,8 +1816,10 @@ _base_free_irq(struct MPT3SAS_ADAPTER *ioc)
 
list_for_each_entry_safe(reply_q, next, >reply_queue_list, list) {
list_del(_q->list);
-   irq_set_affinity_hint(reply_q->vector, NULL);
-   free_cpumask_var(reply_q->affinity_hint);
+   if (smp_affinity_enable) {
+   irq_set_affinity_hint(reply_q->vector, NULL);
+   free_cpumask_var(reply_q->affinity_hint);
+   }
synchronize_irq(reply_q->vector);
free_irq(reply_q->vector, reply_q);
kfree(reply_q);
@@ -1844,9 +1850,13 @@ _base_request_irq(struct MPT3SAS_ADAPTER *ioc, u8 index, 
u32 vector)
reply_q->msix_index = index;
reply_q->vector = vector;
 
-   if (!alloc_cpumask_var(_q->affinity_hint, GFP_KERNEL))
-   return -ENOMEM;
-   cpumask_clear(reply_q->affinity_hint);
+   if (smp_affinity_enable) {
+   if (!zalloc_cpumask_var(_q->affinity_hint, GFP_KERNEL)) {
+   kfree(reply_q);
+   return -ENOMEM;
+   }
+   cpumask_clear(reply_q->affinity_hint);
+   }
 
atomic_set(_q->busy, 0);
if (ioc->msix_enable)
@@ -1861,6 +1871,7 @@ _base_request_irq(struct MPT3SAS_ADAPTER *ioc, u8 index, 
u32 vector)
pr_err(MPT3SAS_FMT "unable to allocate interrupt %d!\n",
reply_q->name, vector);
kfree(reply_q);
+   free_cpumask_var(reply_q->affinity_hint);
return -EBUSY;
}
 
@@ -1909,16 +1920,17 @@ _base_assign_reply_queues(struct MPT3SAS_ADAPTER *ioc)
 
for (i = 0 ; i < group ; i++) {
ioc->cpu_msix_table[cpu] = index;
-   cpumask_or(reply_q->affinity_hint,
+   if (smp_affinity_enable)
+   cpumask_or(reply_q->affinity_hint,
   reply_q->affinity_hint, get_cpu_mask(cpu));
cpu = cpumask_next(cpu, cpu_online_mask);
}
-
-   if (irq_set_affinity_hint(reply_q->vector,
+   if (smp_affinity_enable)
+   if (irq_set_affinity_hint(reply_q->vector,
   reply_q->affinity_hint))
-   dinitprintk(ioc, pr_info(MPT3SAS_FMT
-   "error setting affinity hint for irq vector %d\n",
-   ioc->name, reply_q->vector));
+   dinitprintk(ioc, pr_info(MPT3SAS_FMT
+"Err setting affinity hint to irq vector %d\n",
+ioc->name, reply_q->vector));
index++;
}
 }
@@ -1976,6 +1988,9 @@ _base_enable_msix(struct MPT3SAS_ADAPTER *ioc)
} else if (max_msix_vectors == 0)
goto try_ioapic;
 
+   if (ioc->msix_vector_count < ioc->cpu_count)
+   smp_affinity_enable = 0;
+
entries = kcalloc(ioc->reply_queue_count, sizeof(struct msix_entry),
GFP_KERNEL);
if (!entries) {
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [mpt3sas driver 06/10] mpt3sas: Added smp_affinity_enable module parameter.

[Suganath Prabu Subramani]

Hi Tomas,
Initially, we posted this patch with "zalloc_cpumassk_var" and Robert
has suggested to use "alloc_cpumask_var". Please check the below link.
"http://www.gossamer-threads.com/lists/linux/kernel/2068280?do=post_view_threaded#2068280;

We incorporated other review comments for freeing reply_q and
cpumask_var in this patch and sending it soon.

Thanks,
Suganath prabu S

On Thu, Feb 4, 2016 at 8:31 PM, Tomas Henzl <the...@redhat.com> wrote:
> On 28.1.2016 07:37, Suganath prabu Subaramani wrote:
>> From: Suganath prabu Subramani <suganath-prabu.subram...@avagotech.com>
>>
>> Module parameter to enable/disable configuring
>> affinity hint for msix vector.
>> SMP affinity feature can be enabled/disabled by setting
>> module parameter "smp_affinity_enable" to 1/0.
>> By default this feature is enabled. (smp_affinity_enable = 1 enabled).
>>
>> Signed-off-by: Suganath prabu Subramani 
>> <suganath-prabu.subram...@avagotech.com>
>> Signed-off-by: Chaitra P B <chaitra.basa...@avagotech.com>
>> ---
>>  drivers/scsi/mpt3sas/mpt3sas_base.c | 34 +++---
>>  1 file changed, 23 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c 
>> b/drivers/scsi/mpt3sas/mpt3sas_base.c
>> index 31838d9a..a1a3b39 100644
>> --- a/drivers/scsi/mpt3sas/mpt3sas_base.c
>> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
>> @@ -83,6 +83,10 @@ static int msix_disable = -1;
>>  module_param(msix_disable, int, 0);
>>  MODULE_PARM_DESC(msix_disable, " disable msix routed interrupts 
>> (default=0)");
>>
>> +static int smp_affinity_enable = 1;
>> +module_param(smp_affinity_enable, int, S_IRUGO);
>> +MODULE_PARM_DESC(smp_affinity_enable, "SMP affinity feature enable/disbale 
>> Default: enable(1)");
>> +
>>  static int max_msix_vectors = -1;
>>  module_param(max_msix_vectors, int, 0);
>>  MODULE_PARM_DESC(max_msix_vectors,
>> @@ -1812,8 +1816,10 @@ _base_free_irq(struct MPT3SAS_ADAPTER *ioc)
>>
>>   list_for_each_entry_safe(reply_q, next, >reply_queue_list, list) {
>>   list_del(_q->list);
>> - irq_set_affinity_hint(reply_q->vector, NULL);
>> - free_cpumask_var(reply_q->affinity_hint);
>> + if (smp_affinity_enable) {
>> + irq_set_affinity_hint(reply_q->vector, NULL);
>> + free_cpumask_var(reply_q->affinity_hint);
>> + }
>>   synchronize_irq(reply_q->vector);
>>   free_irq(reply_q->vector, reply_q);
>>   kfree(reply_q);
>> @@ -1844,9 +1850,11 @@ _base_request_irq(struct MPT3SAS_ADAPTER *ioc, u8 
>> index, u32 vector)
>>   reply_q->msix_index = index;
>>   reply_q->vector = vector;
>>
>> - if (!alloc_cpumask_var(_q->affinity_hint, GFP_KERNEL))
>> - return -ENOMEM;
>> - cpumask_clear(reply_q->affinity_hint);
>> + if (smp_affinity_enable) {
>> + if (!alloc_cpumask_var(_q->affinity_hint, GFP_KERNEL))
>
> a kfree(reply_q); should go here
>
>> + return -ENOMEM;
>> + cpumask_clear(reply_q->affinity_hint);
>
> maybe zalloc_cpumask_var ?
>
>> + }
>
> if (r) {
> pr_err(MPT3SAS_FMT "unable to allocate interrupt %d!\n",
> reply_q->name, vector);
> kfree(reply_q);
> and a  free_cpumask_var (..)  here ?
> return -EBUSY;
>
>
> cheers,
> tomash
>
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html