[Devel] [PATCH 1/3] target: Move scsi_port_stats from se_port to se_lun

Vasily Averin vvs at virtuozzo.com
Wed Dec 13 17:58:12 MSK 2017


On 2017-12-13 17:37, Andrey Grafin wrote:
> It was sent an hour ago. Have you received it?
I don't.
Kostja?

> ________________________________________
> From: Vasily Averin <vvs at virtuozzo.com>
> Sent: Wednesday, December 13, 2017 4:20 PM
> To: Andrey Grafin; Konstantin Khorenko
> Cc: Kirill Korotaev; devel at openvz.org; Andrei Vagin; azaitsev at virtuozzo.com; Ludmila Ivanichkina
> Subject: Re: [Devel] [PATCH 1/3] target: Move scsi_port_stats from se_port to se_lun
> 
> We can see this letter,
> I have added auto-approval for your emails in devel at openvz.org mailing list,
> hope it will be enough,
> however anyway, please resend the 2nd patch once again.
> 
> Thank you,
>         Vasily Averin
> 
> On 2017-12-13 16:13, Andrey Grafin wrote:
>> I don't know what's happening, all cc from @acronis got all three patches
>> ________________________________________
>> From: Konstantin Khorenko <khorenko at virtuozzo.com>
>> Sent: Wednesday, December 13, 2017 4:01 PM
>> To: Andrey Grafin
>> Cc: Andrei Vagin; azaitsev at virtuozzo.com; Kirill Korotaev; devel at openvz.org; Ludmila Ivanichkina
>> Subject: Re: [Devel] [PATCH 1/3] target: Move scsi_port_stats from se_port to se_lun
>>
>> Please resend with 2nd path included.
>>
>> --
>> Best regards,
>>
>> Konstantin Khorenko,
>> Virtuozzo Linux Kernel Team
>>
>> On 12/11/2017 10:36 PM, Andrei Vagin wrote:
>>> I haven't got the second patch and it isn't listed in the mailing list
>>> archive:
>>> https://lists.openvz.org/pipermail/devel/2017-December/thread.html
>>>
>>> On Mon, Dec 11, 2017 at 08:53:03PM +0300, Andrey Grafin wrote:
>>>> This patch moves scsi_port_stats from se_port to se_lun
>>>> and changes stats counters type to atomic_long_t.
>>>>
>>>> This changes remove the next superfluous actions in collecting stats:
>>>> - the check for the existence se_port;
>>>> - spin_lock usage.
>>>>
>>>> This patch is based on the mainstream patches adf653f92f38e and
>>>> 4cc987eaff914 that can't be backported directly because there are
>>>> too many changes before them. But the idea of that patches
>>>> simplifies stats collecting.
>>>>
>>>> Signed-off-by: Andrey Grafin <Andrey.Grafin at acronis.com>
>>>> ---
>>>>  drivers/target/target_core_stat.c      | 41 ++++++++++------------------------
>>>>  drivers/target/target_core_tpg.c       |  3 +++
>>>>  drivers/target/target_core_transport.c | 30 +++++++------------------
>>>>  include/target/target_core_base.h      | 15 +++++++------
>>>>  4 files changed, 31 insertions(+), 58 deletions(-)
>>>>
>>>> diff --git a/drivers/target/target_core_stat.c b/drivers/target/target_core_stat.c
>>>> index 59830a27f50..8dacf57620f 100644
>>>> --- a/drivers/target/target_core_stat.c
>>>> +++ b/drivers/target/target_core_stat.c
>>>> @@ -794,17 +794,12 @@ static ssize_t target_stat_scsi_tgt_port_show_attr_in_cmds(
>>>>      struct se_port_stat_grps *pgrps, char *page)
>>>>  {
>>>>      struct se_lun *lun = container_of(pgrps, struct se_lun, port_stat_grps);
>>>> -    struct se_port *sep;
>>>> -    ssize_t ret;
>>>> +    ssize_t ret = -ENODEV;
>>>>
>>>>      spin_lock(&lun->lun_sep_lock);
>>>> -    sep = lun->lun_sep;
>>>> -    if (!sep) {
>>>> -            spin_unlock(&lun->lun_sep_lock);
>>>> -            return -ENODEV;
>>>> -    }
>>>> -
>>>> -    ret = snprintf(page, PAGE_SIZE, "%llu\n", sep->sep_stats.cmd_pdus);
>>>> +    if (lun->lun_sep)
>>>> +            ret = snprintf(page, PAGE_SIZE, "%lu\n",
>>>> +                    atomic_long_read(&lun->lun_stats.cmd_pdus));
>>>>      spin_unlock(&lun->lun_sep_lock);
>>>>      return ret;
>>>>  }
>>>> @@ -814,18 +809,12 @@ static ssize_t target_stat_scsi_tgt_port_show_attr_write_mbytes(
>>>>      struct se_port_stat_grps *pgrps, char *page)
>>>>  {
>>>>      struct se_lun *lun = container_of(pgrps, struct se_lun, port_stat_grps);
>>>> -    struct se_port *sep;
>>>> -    ssize_t ret;
>>>> +    ssize_t ret = -ENODEV;
>>>>
>>>>      spin_lock(&lun->lun_sep_lock);
>>>> -    sep = lun->lun_sep;
>>>> -    if (!sep) {
>>>> -            spin_unlock(&lun->lun_sep_lock);
>>>> -            return -ENODEV;
>>>> -    }
>>>> -
>>>> -    ret = snprintf(page, PAGE_SIZE, "%u\n",
>>>> -                    (u32)(sep->sep_stats.rx_data_octets >> 20));
>>>> +    if (lun->lun_sep)
>>>> +            ret = snprintf(page, PAGE_SIZE, "%lu\n",
>>>> +                    atomic_long_read(&lun->lun_stats.rx_data_octets) >> 20);
>>>>      spin_unlock(&lun->lun_sep_lock);
>>>>      return ret;
>>>>  }
>>>> @@ -835,18 +824,12 @@ static ssize_t target_stat_scsi_tgt_port_show_attr_read_mbytes(
>>>>      struct se_port_stat_grps *pgrps, char *page)
>>>>  {
>>>>      struct se_lun *lun = container_of(pgrps, struct se_lun, port_stat_grps);
>>>> -    struct se_port *sep;
>>>> -    ssize_t ret;
>>>> +    ssize_t ret = -ENODEV;
>>>>
>>>>      spin_lock(&lun->lun_sep_lock);
>>>> -    sep = lun->lun_sep;
>>>> -    if (!sep) {
>>>> -            spin_unlock(&lun->lun_sep_lock);
>>>> -            return -ENODEV;
>>>> -    }
>>>> -
>>>> -    ret = snprintf(page, PAGE_SIZE, "%u\n",
>>>> -                    (u32)(sep->sep_stats.tx_data_octets >> 20));
>>>> +    if (lun->lun_sep)
>>>> +            ret = snprintf(page, PAGE_SIZE, "%lu\n",
>>>> +                    atomic_long_read(&lun->lun_stats.tx_data_octets) >> 20);
>>>>      spin_unlock(&lun->lun_sep_lock);
>>>>      return ret;
>>>>  }
>>>> diff --git a/drivers/target/target_core_tpg.c b/drivers/target/target_core_tpg.c
>>>> index 0696de9553d..7ee2a94463b 100644
>>>> --- a/drivers/target/target_core_tpg.c
>>>> +++ b/drivers/target/target_core_tpg.c
>>>> @@ -832,6 +832,9 @@ int core_tpg_add_lun(
>>>>      }
>>>>
>>>>      spin_lock(&tpg->tpg_lun_lock);
>>>> +    atomic_long_set(&lun->lun_stats.cmd_pdus, 0);
>>>> +    atomic_long_set(&lun->lun_stats.rx_data_octets, 0);
>>>> +    atomic_long_set(&lun->lun_stats.tx_data_octets, 0);
>>>>      lun->lun_access = lun_access;
>>>>      lun->lun_status = TRANSPORT_LUN_STATUS_ACTIVE;
>>>>      spin_unlock(&tpg->tpg_lun_lock);
>>>> diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
>>>> index 25b5581ad78..4675bcc70cb 100644
>>>> --- a/drivers/target/target_core_transport.c
>>>> +++ b/drivers/target/target_core_transport.c
>>>> @@ -1243,10 +1243,7 @@ target_setup_cmd_from_cdb(struct se_cmd *cmd, unsigned char *cdb)
>>>>
>>>>      cmd->se_cmd_flags |= SCF_SUPPORTED_SAM_OPCODE;
>>>>
>>>> -    spin_lock(&cmd->se_lun->lun_sep_lock);
>>>> -    if (cmd->se_lun->lun_sep)
>>>> -            cmd->se_lun->lun_sep->sep_stats.cmd_pdus++;
>>>> -    spin_unlock(&cmd->se_lun->lun_sep_lock);
>>>> +    atomic_long_inc(&cmd->se_lun->lun_stats.cmd_pdus);
>>>>      return 0;
>>>>  }
>>>>  EXPORT_SYMBOL(target_setup_cmd_from_cdb);
>>>> @@ -2028,12 +2025,9 @@ static void target_complete_ok_work(struct work_struct *work)
>>>>  queue_rsp:
>>>>      switch (cmd->data_direction) {
>>>>      case DMA_FROM_DEVICE:
>>>> -            spin_lock(&cmd->se_lun->lun_sep_lock);
>>>> -            if (cmd->se_lun->lun_sep) {
>>>> -                    cmd->se_lun->lun_sep->sep_stats.tx_data_octets +=
>>>> -                                    cmd->data_length;
>>>> -            }
>>>> -            spin_unlock(&cmd->se_lun->lun_sep_lock);
>>>> +            atomic_long_add(cmd->data_length,
>>>> +                    &cmd->se_lun->lun_stats.tx_data_octets);
>>>> +
>>>>              /*
>>>>               * Perform READ_STRIP of PI using software emulation when
>>>>               * backend had PI enabled, if the transport will not be
>>>> @@ -2057,22 +2051,14 @@ queue_rsp:
>>>>                      goto queue_full;
>>>>              break;
>>>>      case DMA_TO_DEVICE:
>>>> -            spin_lock(&cmd->se_lun->lun_sep_lock);
>>>> -            if (cmd->se_lun->lun_sep) {
>>>> -                    cmd->se_lun->lun_sep->sep_stats.rx_data_octets +=
>>>> -                            cmd->data_length;
>>>> -            }
>>>> -            spin_unlock(&cmd->se_lun->lun_sep_lock);
>>>> +            atomic_long_add(cmd->data_length,
>>>> +                    &cmd->se_lun->lun_stats.rx_data_octets);
>>>>              /*
>>>>               * Check if we need to send READ payload for BIDI-COMMAND
>>>>               */
>>>>              if (cmd->se_cmd_flags & SCF_BIDI) {
>>>> -                    spin_lock(&cmd->se_lun->lun_sep_lock);
>>>> -                    if (cmd->se_lun->lun_sep) {
>>>> -                            cmd->se_lun->lun_sep->sep_stats.tx_data_octets +=
>>>> -                                    cmd->data_length;
>>>> -                    }
>>>> -                    spin_unlock(&cmd->se_lun->lun_sep_lock);
>>>> +                    atomic_long_add(cmd->data_length,
>>>> +                            &cmd->se_lun->lun_stats.tx_data_octets);
>>>>                      ret = cmd->se_tfo->queue_data_in(cmd);
>>>>                      if (ret)
>>>>                              goto queue_full;
>>>> diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
>>>> index 189b73b514c..3010f63e085 100644
>>>> --- a/include/target/target_core_base.h
>>>> +++ b/include/target/target_core_base.h
>>>> @@ -710,6 +710,12 @@ struct se_port_stat_grps {
>>>>      struct config_group scsi_transport_group;
>>>>  };
>>>>
>>>> +struct scsi_port_stats {
>>>> +    atomic_long_t   cmd_pdus;
>>>> +    atomic_long_t   tx_data_octets;
>>>> +    atomic_long_t   rx_data_octets;
>>>> +};
>>>> +
>>>>  struct se_lun {
>>>>  #define SE_LUN_LINK_MAGIC                   0xffff7771
>>>>      u32                     lun_link_magic;
>>>> @@ -729,6 +735,8 @@ struct se_lun {
>>>>      struct se_port_stat_grps port_stat_grps;
>>>>      struct completion       lun_ref_comp;
>>>>      struct percpu_ref       lun_ref;
>>>> +
>>>> +    struct scsi_port_stats  lun_stats;
>>>>  };
>>>>
>>>>  struct se_dev_stat_grps {
>>>> @@ -835,19 +843,12 @@ struct se_hba {
>>>>      struct se_subsystem_api *transport;
>>>>  };
>>>>
>>>> -struct scsi_port_stats {
>>>> -       u64     cmd_pdus;
>>>> -       u64     tx_data_octets;
>>>> -       u64     rx_data_octets;
>>>> -};
>>>> -
>>>>  struct se_port {
>>>>      /* RELATIVE TARGET PORT IDENTIFER */
>>>>      u16             sep_rtpi;
>>>>      int             sep_tg_pt_secondary_stat;
>>>>      int             sep_tg_pt_secondary_write_md;
>>>>      u32             sep_index;
>>>> -    struct scsi_port_stats sep_stats;
>>>>      /* Used for ALUA Target Port Groups membership */
>>>>      atomic_t        sep_tg_pt_secondary_offline;
>>>>      /* Used for PR ALL_TG_PT=1 */
>>>> --
>>>> 2.11.0
>>>>
>>> _______________________________________________
>>> Devel mailing list
>>> Devel at openvz.org
>>> https://lists.openvz.org/mailman/listinfo/devel
>>> .
>>>
>>
>> _______________________________________________
>> Devel mailing list
>> Devel at openvz.org
>> https://lists.openvz.org/mailman/listinfo/devel
>>
> 


More information about the Devel mailing list