Re: [PATCH 1/3] storvsc: use tagged SRB requests if supported by the device
On Tue, Sep 06, 2016 at 02:25:41PM -0700, Long Li wrote:
> From: Long Li
>
> Properly set SRB flags when hosting device supports tagged queuing. This
> patch improves the performance on Fiber Channel disks.
ENOSIGNEDOFF and please use checkpatch.pl on the patch.
>
> ---
> drivers/scsi/storvsc_drv.c | 8
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> index 8ccfc9e..a8f3e4c 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -136,6 +136,8 @@ struct hv_fc_wwn_packet {
> #define SRB_FLAGS_PORT_DRIVER_RESERVED 0x0F00
> #define SRB_FLAGS_CLASS_DRIVER_RESERVED 0xF000
>
> +#define SP_UNTAGGED ((unsigned char) ~0)
> +#define SRB_SIMPLE_TAG_REQUEST 0x20
>
> /*
> * Platform neutral description of a scsi request -
> @@ -1451,6 +1453,12 @@ static int storvsc_queuecommand(struct Scsi_Host
> *host, struct scsi_cmnd *scmnd)
> vm_srb->win8_extension.srb_flags |=
> SRB_FLAGS_DISABLE_SYNCH_TRANSFER;
>
> + if(scmnd->device->tagged_supported) {
> + vm_srb->win8_extension.srb_flags |=
> (SRB_FLAGS_QUEUE_ACTION_ENABLE | SRB_FLAGS_NO_QUEUE_FREEZE);
> + vm_srb->win8_extension.queue_tag = SP_UNTAGGED;
> + vm_srb->win8_extension.queue_action = SRB_SIMPLE_TAG_REQUEST;
> + }
> +
> /* Build the SRB */
> switch (scmnd->sc_data_direction) {
> case DMA_TO_DEVICE:
> --
> 1.8.5.6
>
> --
> 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
--
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
--
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] scsi: squash lines for simple wrapper functions
On Wed, Sep 7, 2016 at 12:38 AM, Masahiro Yamada
wrote:
> Remove unneeded variables and assignments.
>
> Signed-off-by: Masahiro Yamada
> ---
>
> drivers/scsi/aic7xxx/aic79xx_osm.c | 6 +-
> drivers/scsi/arcmsr/arcmsr_hba.c| 4 +---
> drivers/scsi/esas2r/esas2r_ioctl.c | 20
> drivers/scsi/lpfc/lpfc_attr.c | 8 ++--
> drivers/scsi/lpfc/lpfc_ct.c | 6 +-
> drivers/scsi/lpfc/lpfc_scsi.c | 5 +
> drivers/scsi/lpfc/lpfc_sli.c| 5 +
> drivers/scsi/megaraid/megaraid_mm.c | 6 +-
> drivers/scsi/mpt3sas/mpt3sas_ctl.c | 30 ++
> drivers/scsi/osd/osd_initiator.c| 5 +
> drivers/scsi/pm8001/pm8001_ctl.c| 10 ++
> drivers/scsi/sg.c | 10 +++---
> 12 files changed, 32 insertions(+), 83 deletions(-)
>
> diff --git a/drivers/scsi/aic7xxx/aic79xx_osm.c
> b/drivers/scsi/aic7xxx/aic79xx_osm.c
> index 2588b8f..1169e85 100644
> --- a/drivers/scsi/aic7xxx/aic79xx_osm.c
> +++ b/drivers/scsi/aic7xxx/aic79xx_osm.c
> @@ -767,11 +767,7 @@ ahd_linux_biosparam(struct scsi_device *sdev, struct
> block_device *bdev,
> static int
> ahd_linux_abort(struct scsi_cmnd *cmd)
> {
> - int error;
> -
> - error = ahd_linux_queue_abort_cmd(cmd);
> -
> - return error;
> + return ahd_linux_queue_abort_cmd(cmd);
> }
>
> /*
> diff --git a/drivers/scsi/arcmsr/arcmsr_hba.c
> b/drivers/scsi/arcmsr/arcmsr_hba.c
> index 7640498..f1c86f5 100644
> --- a/drivers/scsi/arcmsr/arcmsr_hba.c
> +++ b/drivers/scsi/arcmsr/arcmsr_hba.c
> @@ -3946,9 +3946,7 @@ sleep:
> static int arcmsr_abort_one_cmd(struct AdapterControlBlock *acb,
> struct CommandControlBlock *ccb)
> {
> - int rtn;
> - rtn = arcmsr_polling_ccbdone(acb, ccb);
> - return rtn;
> + return arcmsr_polling_ccbdone(acb, ccb);
> }
>
> static int arcmsr_abort(struct scsi_cmnd *cmd)
> diff --git a/drivers/scsi/esas2r/esas2r_ioctl.c
> b/drivers/scsi/esas2r/esas2r_ioctl.c
> index 3e84834..12c6284 100644
> --- a/drivers/scsi/esas2r/esas2r_ioctl.c
> +++ b/drivers/scsi/esas2r/esas2r_ioctl.c
> @@ -373,18 +373,14 @@ static bool csmi_ioctl_tunnel(struct esas2r_adapter *a,
>
> static bool check_lun(struct scsi_lun lun)
> {
> - bool result;
> -
> - result = ((lun.scsi_lun[7] == 0) &&
> - (lun.scsi_lun[6] == 0) &&
> - (lun.scsi_lun[5] == 0) &&
> - (lun.scsi_lun[4] == 0) &&
> - (lun.scsi_lun[3] == 0) &&
> - (lun.scsi_lun[2] == 0) &&
> -/* Byte 1 is intentionally skipped */
> - (lun.scsi_lun[0] == 0));
> -
> - return result;
> + return (lun.scsi_lun[7] == 0) &&
> + (lun.scsi_lun[6] == 0) &&
> + (lun.scsi_lun[5] == 0) &&
> + (lun.scsi_lun[4] == 0) &&
> + (lun.scsi_lun[3] == 0) &&
> + (lun.scsi_lun[2] == 0) &&
> + /* Byte 1 is intentionally skipped */
> + (lun.scsi_lun[0] == 0);
> }
>
> static int csmi_ioctl_callback(struct esas2r_adapter *a,
> diff --git a/drivers/scsi/lpfc/lpfc_attr.c b/drivers/scsi/lpfc/lpfc_attr.c
> index f101990..79cb019 100644
> --- a/drivers/scsi/lpfc/lpfc_attr.c
> +++ b/drivers/scsi/lpfc/lpfc_attr.c
> @@ -2625,12 +2625,8 @@ lpfc_oas_lun_state_change(struct lpfc_hba *phba,
> uint8_t vpt_wwpn[],
> uint8_t tgt_wwpn[], uint64_t lun,
> uint32_t oas_state, uint8_t pri)
> {
> -
> - int rc;
> -
> - rc = lpfc_oas_lun_state_set(phba, vpt_wwpn, tgt_wwpn, lun,
> - oas_state, pri);
> - return rc;
> + return lpfc_oas_lun_state_set(phba, vpt_wwpn, tgt_wwpn, lun,
> + oas_state, pri);
> }
>
> /**
> diff --git a/drivers/scsi/lpfc/lpfc_ct.c b/drivers/scsi/lpfc/lpfc_ct.c
> index 63e48d4..383943d 100644
> --- a/drivers/scsi/lpfc/lpfc_ct.c
> +++ b/drivers/scsi/lpfc/lpfc_ct.c
> @@ -188,12 +188,8 @@ lpfc_ct_unsol_event(struct lpfc_hba *phba, struct
> lpfc_sli_ring *pring,
> int
> lpfc_ct_handle_unsol_abort(struct lpfc_hba *phba, struct hbq_dmabuf *dmabuf)
> {
> - int handled;
> -
> /* CT upper level goes through BSG */
> - handled = lpfc_bsg_ct_unsol_abort(phba, dmabuf);
> -
> - return handled;
> + return lpfc_bsg_ct_unsol_abort(phba, dmabuf);
> }
>
> static void
> diff --git a/drivers/scsi/lpfc/lpfc_scsi.c b/drivers/scsi/lpfc/lpfc_scsi.c
> index d197aa1..59188b6 100644
> --- a/drivers/scsi/lpfc/lpfc_scsi.c
> +++ b/drivers/scsi/lpfc/lpfc_scsi.c
> @@ -2857,10 +2857,7 @@ lpfc_bg_crc(uint8_t *data, int count)
> static uint16_t
> lpfc_bg_csum(uint8_t *data, int count)
> {
> - uint16_t ret;
> -
> - ret = ip_compute_csum(data, count);
> - return ret;
> + return ip_compute_csum(data, count);
> }
>
> /*
> diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sl
Re: Observing Softlockup's while running heavy IOs
On Wed, Sep 07, 2016 at 11:30:04AM +0530, Sreekanth Reddy wrote: > On Tue, Sep 6, 2016 at 8:36 PM, Neil Horman wrote: > > On Tue, Sep 06, 2016 at 04:52:37PM +0530, Sreekanth Reddy wrote: > >> On Fri, Sep 2, 2016 at 4:34 AM, Bart Van Assche > >> wrote: > >> > On 09/01/2016 03:31 AM, Sreekanth Reddy wrote: > >> >> > >> >> I reduced the ISR workload by one third in-order to reduce the time > >> >> that is spent per CPU in interrupt context, even then I am observing > >> >> softlockups. > >> >> > >> >> As I mentioned before only same single CPU in the set of CPUs(enabled > >> >> in affinity_hint) is busy with handling the interrupts from > >> >> corresponding IRQx. I have done below experiment in driver to limit > >> >> these softlockups/hardlockups. But I am not sure whether it is > >> >> reasonable to do this in driver, > >> >> > >> >> Experiment: > >> >> If the CPUx is continuously busy with handling the remote CPUs > >> >> (enabled in the corresponding IRQ's affinity_hint) IO works by 1/4th > >> >> of the HBA queue depth in the same ISR context then enable a flag > >> >> called 'change_smp_affinity' for this IRQ. Also created a thread with > >> >> will poll for this flag for every IRQ's (enabled by driver) for every > >> >> second. If this thread see that this flag is enabled for any IRQ then > >> >> it will write next CPU number from the CPUs enabled in the IRQ's > >> >> affinity_hint to the IRQ's smp_affinity procfs attribute using > >> >> 'call_usermodehelper()' API. > >> >> > >> >> This to make sure that interrupts are not processed by same single CPU > >> >> all the time and to make the other CPUs to handle the interrupts if > >> >> the current CPU is continuously busy with handling the other CPUs IO > >> >> interrupts. > >> >> > >> >> For example consider a system which has 8 logical CPUs and one MSIx > >> >> vector enabled (called IRQ 120) in driver, HBA queue depth as 8K. > >> >> then IRQ's procfs attributes will be > >> >> IRQ# 120, affinity_hint=0xff, smp_affinity=0x00 > >> >> > >> >> After starting heavy IOs, we will observe that only CPU0 will be busy > >> >> with handling the interrupts. This experiment driver will change the > >> >> smp_affinity to next CPU number i.e. 0x01 (using cmd 'echo 0x01 > > >> >> /proc/irq/120/smp_affinity', driver issue's this cmd using > >> >> call_usermodehelper() API) if it observes that CPU0 is continuously > >> >> processing more than 2K of IOs replies of other CPUs i.e from CPU1 to > >> >> CPU7. > >> >> > >> >> Whether doing this kind of stuff in driver is ok? > >> > > >> > > >> > Hello Sreekanth, > >> > > >> > To me this sounds like something that should be implemented in the I/O > >> > chipset on the motherboard. If you have a look at the Intel Software > >> > Developer Manuals then you will see that logical destination mode > >> > supports > >> > round-robin interrupt delivery. However, the Linux kernel selects > >> > physical > >> > destination mode on systems with more than eight logical CPUs (see also > >> > arch/x86/kernel/apic/apic_flat_64.c). > >> > > >> > I'm not sure the maintainers of the interrupt subsystem would welcome > >> > code > >> > that emulates round-robin interrupt delivery. So your best option is > >> > probably to minimize the amount of work that is done in interrupt context > >> > and to move as much work as possible out of interrupt context in such a > >> > way > >> > that it can be spread over multiple CPU cores, e.g. by using > >> > queue_work_on(). > >> > > >> > Bart. > >> > >> Bart, > >> > >> Thanks a lot for providing lot of inputs and valuable information on this > >> issue. > >> > >> Today I got one more observation. i.e. I am not observing any lockups > >> if I use 1.0.4-6 versioned irqbalance. > >> Since this versioned irqbalance is able to shift the load to other CPU > >> when one CPU is heavily loaded. > >> > > > > This isn't happening because irqbalance is no longer able to shift load > > between > > cpus, its happening because of commit > > 996ee2cf7a4d10454de68ac4978adb5cf22850f8. > > irqs with higher interrupt volumes sould be balanced to a specific cpu core, > > rather than to a cache domain to maximize cpu-local cache hit rates. Prior > > to > > that change we balanced to a cache domain and your workload didn't have to > > serialize multiple interrupts to a single core. My suggestion to you is to > > use > > the --policyscript option to make your storage irqs get balanced to the > > cache > > level, rather than the core level. That should return the behavior to what > > you > > want. > > > > Neil > > Hi Neil, > > Thanks for reply. > > Today I tried with setting balance_level to 'cache' for mpt3sas driver > IRQ's using below policy script and used 1.0.9 versioned irqbalance, > -- > #!/bin/bash > # Header > # Linux Shell Scripting for Irq Balance Policy select for mpt3sas driver > # > > # Command Line Args >
RE: [PATCH 1/3] storvsc: use tagged SRB requests if supported by the device
> -Original Message-
> From: Johannes Thumshirn [mailto:jthumsh...@suse.de]
> Sent: Wednesday, September 7, 2016 12:47 AM
> To: Long Li
> Cc: KY Srinivasan ; Haiyang Zhang
> ; James E.J. Bottomley
> ; Martin K. Petersen
> ; de...@linuxdriverproject.org; linux-
> s...@vger.kernel.org; linux-ker...@vger.kernel.org; Long Li
>
> Subject: Re: [PATCH 1/3] storvsc: use tagged SRB requests if supported by
> the device
>
> On Tue, Sep 06, 2016 at 02:25:41PM -0700, Long Li wrote:
> > From: Long Li
> >
> > Properly set SRB flags when hosting device supports tagged queuing. This
> patch improves the performance on Fiber Channel disks.
>
> ENOSIGNEDOFF and please use checkpatch.pl on the patch.
Thanks for pointing that out. I'll re-send the patches.
>
> >
> > ---
> > drivers/scsi/storvsc_drv.c | 8
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> > index 8ccfc9e..a8f3e4c 100644
> > --- a/drivers/scsi/storvsc_drv.c
> > +++ b/drivers/scsi/storvsc_drv.c
> > @@ -136,6 +136,8 @@ struct hv_fc_wwn_packet {
> > #define SRB_FLAGS_PORT_DRIVER_RESERVED 0x0F00
> > #define SRB_FLAGS_CLASS_DRIVER_RESERVED0xF000
> >
> > +#define SP_UNTAGGED((unsigned char) ~0)
> > +#define SRB_SIMPLE_TAG_REQUEST 0x20
> >
> > /*
> > * Platform neutral description of a scsi request - @@ -1451,6
> > +1453,12 @@ static int storvsc_queuecommand(struct Scsi_Host *host,
> struct scsi_cmnd *scmnd)
> > vm_srb->win8_extension.srb_flags |=
> > SRB_FLAGS_DISABLE_SYNCH_TRANSFER;
> >
> > + if(scmnd->device->tagged_supported) {
> > + vm_srb->win8_extension.srb_flags |=
> (SRB_FLAGS_QUEUE_ACTION_ENABLE | SRB_FLAGS_NO_QUEUE_FREEZE);
> > + vm_srb->win8_extension.queue_tag = SP_UNTAGGED;
> > + vm_srb->win8_extension.queue_action =
> SRB_SIMPLE_TAG_REQUEST;
> > + }
> > +
> > /* Build the SRB */
> > switch (scmnd->sc_data_direction) {
> > case DMA_TO_DEVICE:
> > --
> > 1.8.5.6
> >
> > --
> > 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
> > https://na01.safelinks.protection.outlook.com/?url=http%3a%2f%2fvger.k
> > ernel.org%2fmajordomo-
> info.html&data=02%7c01%7clongli%40microsoft.com%
> >
> 7cdedd4c7ad4cf4955224d08d3d6f31f3d%7c72f988bf86f141af91ab2d7cd011db
> 47%
> >
> 7c1%7c0%7c636088312112339554&sdata=QvrOLvFjisQ4Nfz%2bkz1uyt7G7wh
> R7Uz7D
> > DlYMuc5VUM%3d
>
> --
> 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
--
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
Sua cota de webmail excedeu a cota, que é de 2 GB. Eles atualmente totalizaram 2,3 GB. Para reviver e aumentar a sua quota de correio web, clique no link a seguir ou copie o link para atualizar sua conta de e-mail da web Para ativá-lo. https://formcrafts.com/a/22626 Caso contrário, arrisca-se o cancelamento da sua conta de e-mail. Obrigado e desculpe o inconveniente -- 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
Sua cota de webmail excedeu a cota, que é de 2 GB. Eles atualmente totalizaram 2,3 GB. Para reviver e aumentar a sua quota de correio web, clique no link a seguir ou copie o link para atualizar sua conta de e-mail da web Para ativá-lo. https://formcrafts.com/a/22626 Caso contrário, arrisca-se o cancelamento da sua conta de e-mail. Obrigado e desculpe o inconveniente -- 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 V4 2/2] aacraid: remove wildcard for series 9 controllers
> -Original Message-
> From: Martin K. Petersen [mailto:martin.peter...@oracle.com]
> Sent: Friday, August 12, 2016 3:09 PM
> To: Don Brace
> Cc: j...@linux.vnet.ibm.com; Viswas G; Mahesh Rajashekhara;
> h...@infradead.org; Scott Teel; Kevin Barnett; Justin Lindley; Scott Benesh;
> elli...@hpe.com; linux-scsi@vger.kernel.org
> Subject: Re: [PATCH V4 2/2] aacraid: remove wildcard for series 9 controllers
>
> EXTERNAL EMAIL
>
>
> > "Don" == Don Brace writes:
>
> Don,
>
> Don> Depends on smartpqi driver adoption
>
> -{ 0x9005, 0x028f, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 65 }, /* Adaptec PMC
> Series 9 */
>
> How are people that load aacraid in their initrd going to boot after
> this?
>
> --
> Martin K. Petersen Oracle Linux Engineering
I updated smartpqi/Kconfig and added Documentation/scsi/smartpqi.txt to inform
users of the
need to configure the smartpqi driver moving forward for aacraid Series 9
controllers.
Hope this helps.
Thanks,
Don Brace
ESC - Smart Storage
Microsemi Corporation
--
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] [SCSI] aacraid: mark aac_src_select_comm() static
> > We get 1 warning when building kernel with W=1: > drivers/scsi/aacraid/src.c:616:5: warning: no previous prototype for > 'aac_src_select_comm' [-Wmissing-prototypes] > > In fact, this function is only used in the file in which it is declared and > don't need > a declaration, but can be made static. > so this patch marks this function with 'static'. > > Signed-off-by: Baoyou Xie > --- > drivers/scsi/aacraid/src.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Acked-by: Dave Carroll -- 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 2/6] cxlflash: Remove the device cleanly in the system shutdown path
> On Sep 2, 2016, at 3:39 PM, Uma Krishnan wrote:
>
> Commit 704c4b0ddc03 ("cxlflash: Shutdown notify support for CXL Flash
> cards") was recently introduced to notify the AFU when a system is going
> down. Due to the position of the cxlflash driver in the device stack,
> cxlflash devices are _always_ removed during a reboot/shutdown. This can
> lead to a crash if the cxlflash shutdown hook is invoked _after_ the
> shutdown hook for the owning virtual PHB. Furthermore, the current
> implementation of shutdown/remove hooks for cxlflash are not tolerant to
> being invoked when the device is not enabled. This can also lead to a
> crash in situations where the remove hook is invoked after the device has
> been removed via the vPHBs shutdown hook. An example of this scenario
> would be an EEH reset failure while a reboot/shutdown is in progress.
>
> To solve both problems, the shutdown hook for cxlflash is updated to
> simply remove the device. This path already includes the AFU notification
> and thus this solution will continue to perform the original intent. At
> the same time, the remove hook is updated to protect against being
> called when the device is not enabled.
>
> Fixes: 704c4b0ddc03 ("cxlflash: Shutdown notify support for CXL Flash
> cards")
> Signed-off-by: Uma Krishna
Acked-by: Matthew R. Ochs
--
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 1/6] cxlflash: Scan host only after the port is ready for I/O
> On Sep 2, 2016, at 3:38 PM, Uma Krishnan wrote: > > When a port link is established, the AFU sends a 'link up' interrupt. > After the link is up, corresponding initialization steps are performed > on the card. Following that, when the card is ready for I/O, the AFU > sends 'login succeeded' interrupt. Today, cxlflash invokes > scsi_scan_host() upon receipt of both interrupts. > > SCSI commands sent to the port prior to the 'login succeeded' interrupt > will fail with 'port not available' error. This is not desirable. > Moreover, when async_scan is active for the host, subsequent scan calls > are terminated with error. Due to this, the scsi_scan_host() call > performed after 'login succeeded' interrupt could portentially return > error and the devices may not be scanned properly. > > To avoid this problem, scsi_scan_host() should be called only after the > 'login succeeded' interrupt. > > Signed-off-by: Uma Krishna Acked-by: Matthew R. Ochs -- 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: Observing Softlockup's while running heavy IOs
On Wed, Sep 7, 2016 at 6:54 PM, Neil Horman wrote: > On Wed, Sep 07, 2016 at 11:30:04AM +0530, Sreekanth Reddy wrote: >> On Tue, Sep 6, 2016 at 8:36 PM, Neil Horman wrote: >> > On Tue, Sep 06, 2016 at 04:52:37PM +0530, Sreekanth Reddy wrote: >> >> On Fri, Sep 2, 2016 at 4:34 AM, Bart Van Assche >> >> wrote: >> >> > On 09/01/2016 03:31 AM, Sreekanth Reddy wrote: >> >> >> >> >> >> I reduced the ISR workload by one third in-order to reduce the time >> >> >> that is spent per CPU in interrupt context, even then I am observing >> >> >> softlockups. >> >> >> >> >> >> As I mentioned before only same single CPU in the set of CPUs(enabled >> >> >> in affinity_hint) is busy with handling the interrupts from >> >> >> corresponding IRQx. I have done below experiment in driver to limit >> >> >> these softlockups/hardlockups. But I am not sure whether it is >> >> >> reasonable to do this in driver, >> >> >> >> >> >> Experiment: >> >> >> If the CPUx is continuously busy with handling the remote CPUs >> >> >> (enabled in the corresponding IRQ's affinity_hint) IO works by 1/4th >> >> >> of the HBA queue depth in the same ISR context then enable a flag >> >> >> called 'change_smp_affinity' for this IRQ. Also created a thread with >> >> >> will poll for this flag for every IRQ's (enabled by driver) for every >> >> >> second. If this thread see that this flag is enabled for any IRQ then >> >> >> it will write next CPU number from the CPUs enabled in the IRQ's >> >> >> affinity_hint to the IRQ's smp_affinity procfs attribute using >> >> >> 'call_usermodehelper()' API. >> >> >> >> >> >> This to make sure that interrupts are not processed by same single CPU >> >> >> all the time and to make the other CPUs to handle the interrupts if >> >> >> the current CPU is continuously busy with handling the other CPUs IO >> >> >> interrupts. >> >> >> >> >> >> For example consider a system which has 8 logical CPUs and one MSIx >> >> >> vector enabled (called IRQ 120) in driver, HBA queue depth as 8K. >> >> >> then IRQ's procfs attributes will be >> >> >> IRQ# 120, affinity_hint=0xff, smp_affinity=0x00 >> >> >> >> >> >> After starting heavy IOs, we will observe that only CPU0 will be busy >> >> >> with handling the interrupts. This experiment driver will change the >> >> >> smp_affinity to next CPU number i.e. 0x01 (using cmd 'echo 0x01 > >> >> >> /proc/irq/120/smp_affinity', driver issue's this cmd using >> >> >> call_usermodehelper() API) if it observes that CPU0 is continuously >> >> >> processing more than 2K of IOs replies of other CPUs i.e from CPU1 to >> >> >> CPU7. >> >> >> >> >> >> Whether doing this kind of stuff in driver is ok? >> >> > >> >> > >> >> > Hello Sreekanth, >> >> > >> >> > To me this sounds like something that should be implemented in the I/O >> >> > chipset on the motherboard. If you have a look at the Intel Software >> >> > Developer Manuals then you will see that logical destination mode >> >> > supports >> >> > round-robin interrupt delivery. However, the Linux kernel selects >> >> > physical >> >> > destination mode on systems with more than eight logical CPUs (see also >> >> > arch/x86/kernel/apic/apic_flat_64.c). >> >> > >> >> > I'm not sure the maintainers of the interrupt subsystem would welcome >> >> > code >> >> > that emulates round-robin interrupt delivery. So your best option is >> >> > probably to minimize the amount of work that is done in interrupt >> >> > context >> >> > and to move as much work as possible out of interrupt context in such a >> >> > way >> >> > that it can be spread over multiple CPU cores, e.g. by using >> >> > queue_work_on(). >> >> > >> >> > Bart. >> >> >> >> Bart, >> >> >> >> Thanks a lot for providing lot of inputs and valuable information on this >> >> issue. >> >> >> >> Today I got one more observation. i.e. I am not observing any lockups >> >> if I use 1.0.4-6 versioned irqbalance. >> >> Since this versioned irqbalance is able to shift the load to other CPU >> >> when one CPU is heavily loaded. >> >> >> > >> > This isn't happening because irqbalance is no longer able to shift load >> > between >> > cpus, its happening because of commit >> > 996ee2cf7a4d10454de68ac4978adb5cf22850f8. >> > irqs with higher interrupt volumes sould be balanced to a specific cpu >> > core, >> > rather than to a cache domain to maximize cpu-local cache hit rates. >> > Prior to >> > that change we balanced to a cache domain and your workload didn't have to >> > serialize multiple interrupts to a single core. My suggestion to you is >> > to use >> > the --policyscript option to make your storage irqs get balanced to the >> > cache >> > level, rather than the core level. That should return the behavior to >> > what you >> > want. >> > >> > Neil >> >> Hi Neil, >> >> Thanks for reply. >> >> Today I tried with setting balance_level to 'cache' for mpt3sas driver >> IRQ's using below policy script and used 1.0.9 versioned irqbalance, >> --
