Re: [PATCH 1/1] target:separate tx/rx cmd_puds
[replying to an old thread] On Sun, 8 Apr 2018 11:57:18 +0800, Zhang Zhuoyu wrote: > > On Thu, 5 Apr 2018 13:12:12 +0200, David Disseldorp wrote: > > > > > > -CONFIGFS_ATTR_RO(target_stat_tgt_port_, in_cmds); > > > > +CONFIGFS_ATTR_RO(target_stat_tgt_port_, tx_cmds); > > > > +CONFIGFS_ATTR_RO(target_stat_tgt_port_, rx_cmds); > > > > > > I don't think the in_cmds metric should be deleted here. It could be > > > calculated on the fly via tx_cmds + rx_cmds + nodata_cmds. > > > > @Zhang Zhuoyu: How about something like the following? > > https://git.samba.org/?p=ddiss/linux.git;a=commitdiff;h=73723ccf433424721 > > 830797d70cfb88d4596e0fc > > > > Mmm... This patch is much better. > Looks good to me. Looks like I never followed up with a proper submission for this patch. I'll do so later today. Cheers, David
RE: [PATCH 1/1] target:separate tx/rx cmd_puds
> -Original Message- > From: David Disseldorp [mailto:dd...@suse.de] > Sent: Friday, April 6, 2018 8:10 PM > To: Zhang Zhuoyu > Cc: n...@linux-iscsi.org; linux-scsi@vger.kernel.org; target- > de...@vger.kernel.org; linux-ker...@vger.kernel.org > Subject: Re: [PATCH 1/1] target:separate tx/rx cmd_puds > > On Thu, 5 Apr 2018 13:12:12 +0200, David Disseldorp wrote: > > > > -CONFIGFS_ATTR_RO(target_stat_tgt_port_, in_cmds); > > > +CONFIGFS_ATTR_RO(target_stat_tgt_port_, tx_cmds); > > > +CONFIGFS_ATTR_RO(target_stat_tgt_port_, rx_cmds); > > > > I don't think the in_cmds metric should be deleted here. It could be > > calculated on the fly via tx_cmds + rx_cmds + nodata_cmds. > > @Zhang Zhuoyu: How about something like the following? > https://git.samba.org/?p=ddiss/linux.git;a=commitdiff;h=73723ccf433424721 > 830797d70cfb88d4596e0fc > Mmm... This patch is much better. Looks good to me. Zhuoyu > ...this keeps the in_cmds metric, and renames tx/rx_cmds read/write_cmds > respectively. read/write_cmds is still a bit ambiguous, as it refers to the > command data direction rather than SCSI READ/WRITE CDBs, but IMO it's > clearer, and more consistent with the read/write_mbytes metrics. > > Cheers, David
Re: [PATCH 1/1] target:separate tx/rx cmd_puds
On Thu, 5 Apr 2018 13:12:12 +0200, David Disseldorp wrote: > > -CONFIGFS_ATTR_RO(target_stat_tgt_port_, in_cmds); > > +CONFIGFS_ATTR_RO(target_stat_tgt_port_, tx_cmds); > > +CONFIGFS_ATTR_RO(target_stat_tgt_port_, rx_cmds); > > I don't think the in_cmds metric should be deleted here. It could be > calculated on the fly via tx_cmds + rx_cmds + nodata_cmds. @Zhang Zhuoyu: How about something like the following? https://git.samba.org/?p=ddiss/linux.git;a=commitdiff;h=73723ccf433424721830797d70cfb88d4596e0fc ...this keeps the in_cmds metric, and renames tx/rx_cmds read/write_cmds respectively. read/write_cmds is still a bit ambiguous, as it refers to the command data direction rather than SCSI READ/WRITE CDBs, but IMO it's clearer, and more consistent with the read/write_mbytes metrics. Cheers, David
Re: [PATCH 1/1] target:separate tx/rx cmd_puds
Hi, The commit summary has a typo (cmd_puds). That said, this change isn't iSCSI specific, so using "pdu" here doesn't make much sense IMO. On Wed, 21 Mar 2018 17:52:43 +0800, Zhang Zhuoyu wrote: > Separate tx/rx cmd_pdus in order to distinguish LUN read/write IOPS. > > Signed-off-by: Zhang Zhuoyu > --- > drivers/target/target_core_stat.c | 26 ++ > drivers/target/target_core_transport.c | 11 ++- > include/target/target_core_base.h | 3 ++- > 3 files changed, 34 insertions(+), 6 deletions(-) > > diff --git a/drivers/target/target_core_stat.c > b/drivers/target/target_core_stat.c > index f0db91e..9099494 100644 > --- a/drivers/target/target_core_stat.c > +++ b/drivers/target/target_core_stat.c > @@ -706,7 +722,8 @@ static ssize_t > target_stat_tgt_port_hs_in_cmds_show(struct config_item *item, > CONFIGFS_ATTR_RO(target_stat_tgt_port_, indx); > CONFIGFS_ATTR_RO(target_stat_tgt_port_, name); > CONFIGFS_ATTR_RO(target_stat_tgt_port_, port_index); > -CONFIGFS_ATTR_RO(target_stat_tgt_port_, in_cmds); > +CONFIGFS_ATTR_RO(target_stat_tgt_port_, tx_cmds); > +CONFIGFS_ATTR_RO(target_stat_tgt_port_, rx_cmds); I don't think the in_cmds metric should be deleted here. It could be calculated on the fly via tx_cmds + rx_cmds + nodata_cmds. Cheers, David