[1/4] target: Move scsi_port_stats from se_port to se_lun

Submitted by Andrey Grafin on Dec. 20, 2017, 10:10 a.m.

Details

Message ID 20171220101014.751-1-Andrey.Grafin@acronis.com
State New
Series "Series without cover letter"
Headers show

Commit Message

Andrey Grafin Dec. 20, 2017, 10:10 a.m.
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@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(-)

Patch hide | download patch | download mbox

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 */

Comments

Andrei Vagin Dec. 28, 2017, 2:30 a.m.
For all patches:

Acked-by: Andrei Vagin <avagin@virtuozzo.com>

On Wed, Dec 20, 2017 at 01:10:11PM +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@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
>