From: Tomas Henzl <thenzl@redhat.com>
To: Sasikumar PC <sasikumar.pc@broadcom.com>,
jejb@kernel.org, hch@infradead.org
Cc: linux-scsi@vger.kernel.org,
Sathya Prakash Veerichetty <sathya.prakash@broadcom.com>,
linux-kernel@vger.kernel.org,
Christopher Owens <christopher.owens@broadcom.com>,
Kiran Kumar Kasturi <kiran-kumar.kasturi@broadcom.com>
Subject: Re: [PATCH V4 08/11] megaraid_sas: Enable or Disable Fast path based on the PCI Threshold Bandwidth
Date: Thu, 15 Dec 2016 16:10:26 +0100 [thread overview]
Message-ID: <e604955b-86a3-e510-67ab-9632cf16e667@redhat.com> (raw)
In-Reply-To: <459befddaf0e9f9ad1f597f1037f82ba@mail.gmail.com>
On 14.12.2016 22:54, Sasikumar PC wrote:
> Hi Tomas,
>
> Please see my response inline
>
> Thanks
> sasi
>
> -----Original Message-----
> From: Tomas Henzl [mailto:thenzl@redhat.com]
> Sent: Friday, December 09, 2016 8:59 AM
> To: Sasikumar Chandrasekaran; jejb@kernel.org; hch@infradead.org
> Cc: linux-scsi@vger.kernel.org; Sathya.Prakash@broadcom.com;
> linux-kernel@vger.kernel.org; christopher.owens@broadcom.com;
> kiran-kumar.kasturi@broadcom.com
> Subject: Re: [PATCH V4 08/11] megaraid_sas: Enable or Disable Fast path
> based on the PCI Threshold Bandwidth
>
> On 7.12.2016 00:00, Sasikumar Chandrasekaran wrote:
>> Large SEQ IO workload should sent as non fast path commands
>>
>> This patch is depending on patch 7
>>
>> Signed-off-by: Sasikumar Chandrasekaran <sasikumar.pc@broadcom.com>
>> ---
>> drivers/scsi/megaraid/megaraid_sas.h | 8 +++++
>> drivers/scsi/megaraid/megaraid_sas_base.c | 48
> +++++++++++++++++++++++++++++
>> drivers/scsi/megaraid/megaraid_sas_fp.c | 11 +++++--
>> drivers/scsi/megaraid/megaraid_sas_fusion.c | 20 +++++++-----
>> drivers/scsi/megaraid/megaraid_sas_fusion.h | 2 +-
>> 5 files changed, 78 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/scsi/megaraid/megaraid_sas.h
>> b/drivers/scsi/megaraid/megaraid_sas.h
>> index 3e087ab..eb5be2b 100644
>> --- a/drivers/scsi/megaraid/megaraid_sas.h
>> +++ b/drivers/scsi/megaraid/megaraid_sas.h
>> @@ -1429,6 +1429,8 @@ enum FW_BOOT_CONTEXT {
>> #define MFI_1068_FW_HANDSHAKE_OFFSET 0x64
>> #define MFI_1068_FW_READY 0xDDDD0000
>>
>> +#define MEGASAS_RAID1_FAST_PATH_STATUS_CHECK_INTERVAL HZ
>> +
>> #define MR_MAX_REPLY_QUEUES_OFFSET 0X0000001F
>> #define MR_MAX_REPLY_QUEUES_EXT_OFFSET 0X003FC000
>> #define MR_MAX_REPLY_QUEUES_EXT_OFFSET_SHIFT 14
>> @@ -2101,6 +2103,10 @@ struct megasas_instance {
>> atomic_t ldio_outstanding;
>> atomic_t fw_reset_no_pci_access;
>>
>> + atomic64_t bytes_wrote; /* used for raid1 fast path enable or
> disable */
>> + atomic_t r1_write_fp_capable;
> Is a an atomic variable needed for a just boolean variable?
> Sasi - As we need to synchronize timer thread and IO issue threads,
> With atomic, at any point of time the value will be definitive.
> With boolean, it depends, the value could be in transit while
> one thread is reading and other is writing.
This explanation is I think not good enough, as a write of a char value
is atomic on all architectures. If you need synchronisation you probably
need a spinlock.
tomash
>
>> +
>> +
>> struct megasas_instance_template *instancet;
>> struct tasklet_struct isr_tasklet;
>> struct work_struct work_init;
>> @@ -2143,6 +2149,7 @@ struct megasas_instance {
>> long reset_flags;
>> struct mutex reset_mutex;
>> struct timer_list sriov_heartbeat_timer;
>> + struct timer_list r1_fp_hold_timer;
>> char skip_heartbeat_timer_del;
>> u8 requestorId;
>> char PlasmaFW111;
>> @@ -2159,6 +2166,7 @@ struct megasas_instance {
>> bool is_ventura;
>> bool msix_combined;
>> u16 max_raid_mapsize;
>> + u64 pci_threshold_bandwidth; /* used to control the fp writes */
>> };
>> struct MR_LD_VF_MAP {
>> u32 size;
>> diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c
>> b/drivers/scsi/megaraid/megaraid_sas_base.c
>> index 8aafb59..899d25c 100644
>> --- a/drivers/scsi/megaraid/megaraid_sas_base.c
>> +++ b/drivers/scsi/megaraid/megaraid_sas_base.c
>> @@ -1940,6 +1940,9 @@ void megaraid_sas_kill_hba(struct megasas_instance
> *instance)
>> }
>> /* Complete outstanding ioctls when adapter is killed */
>> megasas_complete_outstanding_ioctls(instance);
>> + if (instance->is_ventura)
>> + del_timer_sync(&instance->r1_fp_hold_timer);
>> +
>> }
>>
>> /**
>> @@ -2438,6 +2441,24 @@ void megasas_sriov_heartbeat_handler(unsigned
> long instance_addr)
>> }
>> }
>>
>> +/*Handler for disabling/enabling raid 1 fast paths*/ void
>> +megasas_change_r1_fp_status(unsigned long instance_addr) {
>> + struct megasas_instance *instance =
>> + (struct megasas_instance *)instance_addr;
>> + if (atomic64_read(&instance->bytes_wrote) >=
>> + instance->pci_threshold_bandwidth)
> {
>> +
>> + atomic64_set(&instance->bytes_wrote, 0);
>> + atomic_set(&instance->r1_write_fp_capable, 0);
>> + } else {
>> + atomic64_set(&instance->bytes_wrote, 0);
>> + atomic_set(&instance->r1_write_fp_capable, 1);
>> + }
>> + mod_timer(&instance->r1_fp_hold_timer,
>> + jiffies + MEGASAS_RAID1_FAST_PATH_STATUS_CHECK_INTERVAL);
>> +}
>> +
>> /**
>> * megasas_wait_for_outstanding - Wait for all outstanding cmds
>> * @instance: Adapter soft state
>> @@ -5371,6 +5392,17 @@ static int megasas_init_fw(struct
> megasas_instance *instance)
>> instance->skip_heartbeat_timer_del = 1;
>> }
>>
>> + if (instance->is_ventura) {
>> + atomic64_set(&instance->bytes_wrote, 0);
>> + atomic_set(&instance->r1_write_fp_capable, 1);
>> + megasas_start_timer(instance,
>> + &instance->r1_fp_hold_timer,
>> + megasas_change_r1_fp_status,
>> +
> MEGASAS_RAID1_FAST_PATH_STATUS_CHECK_INTERVAL);
>> + dev_info(&instance->pdev->dev, "starting
> the raid 1 fp timer with interval %d\n",
>> +
> MEGASAS_RAID1_FAST_PATH_STATUS_CHECK_INTERVAL);
>> + }
>> +
>> return 0;
>>
>> fail_get_ld_pd_list:
>> @@ -6161,6 +6193,9 @@ static void megasas_shutdown_controller(struct
> megasas_instance *instance,
>> if (instance->requestorId && !instance->skip_heartbeat_timer_del)
>> del_timer_sync(&instance->sriov_heartbeat_timer);
>>
>> + if (instance->is_ventura)
>> + del_timer_sync(&instance->r1_fp_hold_timer);
>> +
>> megasas_flush_cache(instance);
>> megasas_shutdown_controller(instance, MR_DCMD_HIBERNATE_SHUTDOWN);
>>
>> @@ -6280,6 +6315,16 @@ static void megasas_shutdown_controller(struct
> megasas_instance *instance,
>> megasas_setup_jbod_map(instance);
>> instance->unload = 0;
>>
>> + if (instance->is_ventura) {
>> + atomic64_set(&instance->bytes_wrote, 0);
>> + atomic_set(&instance->r1_write_fp_capable, 1);
>> + megasas_start_timer(instance,
>> + &instance->r1_fp_hold_timer,
>> + megasas_change_r1_fp_status,
>> +
> MEGASAS_RAID1_FAST_PATH_STATUS_CHECK_INTERVAL);
>> + }
>> +
>> +
>> /*
>> * Initiate AEN (Asynchronous Event Notification)
>> */
>> @@ -6368,6 +6413,9 @@ static void megasas_detach_one(struct pci_dev
> *pdev)
>> if (instance->requestorId && !instance->skip_heartbeat_timer_del)
>> del_timer_sync(&instance->sriov_heartbeat_timer);
>>
>> + if (instance->is_ventura)
>> + del_timer_sync(&instance->r1_fp_hold_timer);
>> +
>> if (instance->fw_crash_state != UNAVAILABLE)
>> megasas_free_host_crash_buffer(instance);
>> scsi_remove_host(instance->host);
>> diff --git a/drivers/scsi/megaraid/megaraid_sas_fp.c
>> b/drivers/scsi/megaraid/megaraid_sas_fp.c
>> index a6957a3..7da4685 100644
>> --- a/drivers/scsi/megaraid/megaraid_sas_fp.c
>> +++ b/drivers/scsi/megaraid/megaraid_sas_fp.c
>> @@ -197,14 +197,19 @@ void MR_PopulateDrvRaidMap(struct
>> megasas_instance *instance)
>>
>> if (instance->max_raid_mapsize) {
>> fw_map_dyn = fusion->ld_map[(instance->map_id & 1)];
>> + if (fw_map_dyn->pci_threshold_bandwidth)
>> + instance->pci_threshold_bandwidth =
>> + le64_to_cpu(fw_map_dyn->pci_threshold_bandwidth);
>> #if VD_EXT_DEBUG
>> dev_dbg(&instance->pdev->dev,
>> " raidMapSize 0x%x fw_map_dyn->descTableOffset 0x%x, "
>> - " descTableSize 0x%x descTableNumElements 0x%x\n",
>> + " descTableSize 0x%x descTableNumElements 0x%x, "
>> + " PCIThreasholdBandwidth %llu\n",
>> le32_to_cpu(fw_map_dyn->raid_map_size),
>> le32_to_cpu(fw_map_dyn->desc_table_offset),
>> le32_to_cpu(fw_map_dyn->desc_table_size),
>> - le32_to_cpu(fw_map_dyn->desc_table_num_elements));
>> + le32_to_cpu(fw_map_dyn->desc_table_num_elements),
>> + instance->pci_threshold_bandwidth);
>> dev_dbg(&instance->pdev->dev,
>> "drv map %p ldCount %d\n", drv_map, fw_map_dyn->ld_count);
> #endif
>> @@ -434,6 +439,8 @@ void MR_PopulateDrvRaidMap(struct megasas_instance
> *instance)
>> sizeof(struct MR_DEV_HANDLE_INFO) *
>> MAX_RAIDMAP_PHYSICAL_DEVICES);
>> }
>> + if (instance->is_ventura && !instance->pci_threshold_bandwidth)
>> + instance->pci_threshold_bandwidth = ULLONG_MAX;
>> }
>>
>> /*
>> diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.c
>> b/drivers/scsi/megaraid/megaraid_sas_fusion.c
>> index f968a23..5992153 100644
>> --- a/drivers/scsi/megaraid/megaraid_sas_fusion.c
>> +++ b/drivers/scsi/megaraid/megaraid_sas_fusion.c
>> @@ -95,6 +95,7 @@ void megasas_start_timer(struct megasas_instance
>> *instance, extern unsigned int dual_qdepth_disable; static void
>> megasas_free_rdpq_fusion(struct megasas_instance *instance); static
>> void megasas_free_reply_fusion(struct megasas_instance *instance);
>> +void megasas_change_r1_fp_status(unsigned long instance_addr);
>>
>>
>>
>> @@ -2633,8 +2634,9 @@ void megasas_prepare_secondRaid1_IO(struct
> megasas_instance *instance,
>> * to get new command
>> */
>> if (cmd->is_raid_1_fp_write &&
>> - atomic_inc_return(&instance->fw_outstanding) >
>> - (instance->host->can_queue)) {
>> + (atomic_inc_return(&instance->fw_outstanding) >
>> + (instance->host->can_queue) ||
>> + (!atomic_read(&instance->r1_write_fp_capable)))) {
>> megasas_fpio_to_ldio(instance, cmd, cmd->scmd);
>> atomic_dec(&instance->fw_outstanding);
>> } else if (cmd->is_raid_1_fp_write) { @@ -2643,17 +2645,19 @@ void
>> megasas_prepare_secondRaid1_IO(struct megasas_instance *instance,
>> megasas_prepare_secondRaid1_IO(instance, cmd, r1_cmd);
>> }
>>
>> -
>> /*
>> - * Issue the command to the FW
>> - */
>> + * Issue the command to the FW
>> + */
>> + if (scmd->sc_data_direction == PCI_DMA_TODEVICE &&
> instance->is_ventura)
>> + atomic64_add(scsi_bufflen(scmd), &instance->bytes_wrote);
> You count the bytes written to the ventura card and based on that it is
> asynchronously decided whether the r1_write_fp_capable bit is set in a
> timer function.
> Please explain what should be achieved with this.
>
> Sasi - I am working on this and will be posting the update soon
>
> Thanks,
> Tomas
>
>
>
>
>> megasas_fire_cmd_fusion(instance, req_desc, instance->is_ventura);
>>
>> - if (r1_cmd)
>> + if (r1_cmd) {
>> + atomic64_add(scsi_bufflen(scmd), &instance->bytes_wrote);
>> megasas_fire_cmd_fusion(instance, r1_cmd->request_desc,
>> - instance->is_ventura);
>> -
>> + instance->is_ventura);
>> + }
>>
>> return 0;
>> }
>> diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.h
>> b/drivers/scsi/megaraid/megaraid_sas_fusion.h
>> index c39c4ed..da05790 100644
>> --- a/drivers/scsi/megaraid/megaraid_sas_fusion.h
>> +++ b/drivers/scsi/megaraid/megaraid_sas_fusion.h
>> @@ -977,7 +977,7 @@ struct MR_FW_RAID_MAP_DYNAMIC {
>> u32 desc_table_size; /* Total Size of desc table */
>> /* Total Number of elements in the desc table */
>> u32 desc_table_num_elements;
>> - u64 reserved1;
>> + u64 pci_threshold_bandwidth;
>> u32 reserved2[3]; /*future use */
>> /* timeout value used by driver in FP IOs */
>> u8 fp_pd_io_timeout_sec;
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2016-12-15 15:10 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-12-06 23:00 [PATCH V4 00/11] megaraid_sas: Updates for scsi-next Sasikumar Chandrasekaran
2016-12-06 23:00 ` [PATCH V4 01/11] megaraid_sas: Add new pci device Ids for SAS3.5 Generic Megaraid Controllers Sasikumar Chandrasekaran
2016-12-08 15:17 ` Tomas Henzl
2016-12-06 23:00 ` [PATCH V4 02/11] megaraid_sas: 128 MSIX Support Sasikumar Chandrasekaran
2016-12-08 15:34 ` Tomas Henzl
2016-12-14 21:43 ` Sasikumar PC
2016-12-20 1:50 ` Sasikumar PC
2016-12-20 14:16 ` Tomas Henzl
2016-12-06 23:00 ` [PATCH V4 03/11] megaraid_sas: EEDP Escape Mode Support for SAS3.5 Generic Megaraid Controllers Sasikumar Chandrasekaran
2016-12-08 15:35 ` Tomas Henzl
2016-12-06 23:00 ` [PATCH V4 04/11] megaraid_sas: SAS3.5 Generic Megaraid Controllers Stream Detection and IO Coalescing Sasikumar Chandrasekaran
2016-12-08 16:00 ` Tomas Henzl
2016-12-12 16:38 ` Tomas Henzl
2016-12-14 22:07 ` Sasikumar PC
2016-12-06 23:00 ` [PATCH V4 05/11] megaraid_sas: SAS3.5 Generic Megaraid Controllers Fast Path for RAID 1/10 Writes Sasikumar Chandrasekaran
2016-12-09 10:54 ` Tomas Henzl
2016-12-06 23:00 ` [PATCH V4 06/11] megaraid_sas: Dynamic Raid Map Changes for SAS3.5 Generic Megaraid Controllers Sasikumar Chandrasekaran
2016-12-09 12:55 ` Tomas Henzl
2016-12-14 21:49 ` Sasikumar PC
2016-12-20 1:51 ` Sasikumar PC
2016-12-20 14:20 ` Tomas Henzl
2016-12-20 14:22 ` Sasikumar PC
2016-12-06 23:00 ` [PATCH V4 07/11] megaraid_sas: Add the Support for SAS3.5 Generic Megaraid Controllers Capabilities Sasikumar Chandrasekaran
2016-12-09 13:05 ` Tomas Henzl
2016-12-09 13:13 ` Tomas Henzl
2016-12-14 21:52 ` Sasikumar PC
2016-12-06 23:00 ` [PATCH V4 08/11] megaraid_sas: Enable or Disable Fast path based on the PCI Threshold Bandwidth Sasikumar Chandrasekaran
2016-12-09 13:58 ` Tomas Henzl
2016-12-14 21:54 ` Sasikumar PC
2016-12-15 15:10 ` Tomas Henzl [this message]
2016-12-20 1:51 ` Sasikumar PC
2016-12-20 15:12 ` Tomas Henzl
2016-12-06 23:00 ` [PATCH V4 09/11] megaraid_sas: ldio_outstanding variable is not decremented in completion path Sasikumar Chandrasekaran
2016-12-09 14:01 ` Tomas Henzl
2016-12-06 23:00 ` [PATCH V4 10/11] megaraid_sas: Implement the PD Map support for SAS3.5 Generic Megaraid Controllers Sasikumar Chandrasekaran
2016-12-09 14:07 ` Tomas Henzl
2016-12-06 23:00 ` [PATCH V4 11/11] megaraid_sas: driver version upgrade Sasikumar Chandrasekaran
2016-12-09 14:07 ` Tomas Henzl
2016-12-07 7:31 ` [PATCH V4 00/11] megaraid_sas: Updates for scsi-next Christoph Hellwig
2016-12-11 14:04 ` Sasikumar PC
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e604955b-86a3-e510-67ab-9632cf16e667@redhat.com \
--to=thenzl@redhat.com \
--cc=christopher.owens@broadcom.com \
--cc=hch@infradead.org \
--cc=jejb@kernel.org \
--cc=kiran-kumar.kasturi@broadcom.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=sasikumar.pc@broadcom.com \
--cc=sathya.prakash@broadcom.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®