* [PATCH net v3 1/6] cxgb4i: fix tx credit calculation
@ 2014-12-09 17:32 Karen Xie
2014-12-09 19:54 ` Sergei Shtylyov
0 siblings, 1 reply; 4+ messages in thread
From: Karen Xie @ 2014-12-09 17:32 UTC (permalink / raw)
To: linux-scsi, netdev
Cc: kxie, hariprasad, anish, hch, James.Bottomley, michaelc, davem
[PATCH net v3 1/6] cxgb4i: fix tx credit calculation
From: Karen Xie <kxie@chelsio.com>
- Only data skbs need the wr header added while control skbs do not. Make sure they are treated differently.
- Any credit related checking should be done before adding the wr header.
- Fixed compiler warning resulted from added cxgbi_skb_test_flag() call in is_ofld_imm().
Signed-off-by: Karen Xie <kxie@chelsio.com>
---
drivers/scsi/cxgbi/cxgb4i/cxgb4i.c | 26 +++++++++++++++++---------
drivers/scsi/cxgbi/libcxgbi.h | 4 ++--
2 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
index 1508125..5c3f15d 100644
--- a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
+++ b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
@@ -173,8 +173,12 @@ static int push_tx_frames(struct cxgbi_sock *, int);
*/
static inline int is_ofld_imm(const struct sk_buff *skb)
{
- return skb->len <= (MAX_IMM_TX_PKT_LEN -
- sizeof(struct fw_ofld_tx_data_wr));
+ int length = skb->len;
+
+ if (likely(cxgbi_skcb_test_flag(skb, SKCBF_TX_NEED_HDR)))
+ length += sizeof(struct fw_ofld_tx_data_wr);
+
+ return length <= MAX_IMM_TX_PKT_LEN;
}
static void send_act_open_req(struct cxgbi_sock *csk, struct sk_buff *skb,
@@ -544,15 +548,17 @@ static inline void make_tx_data_wr(struct cxgbi_sock *csk, struct sk_buff *skb,
unsigned int submode = cxgbi_skcb_ulp_mode(skb) & 3;
unsigned int wr_ulp_mode = 0;
- req = (struct fw_ofld_tx_data_wr *)__skb_push(skb, sizeof(*req));
-
if (is_ofld_imm(skb)) {
+ req = (struct fw_ofld_tx_data_wr *)__skb_push(skb,
+ sizeof(*req));
req->op_to_immdlen = htonl(FW_WR_OP(FW_OFLD_TX_DATA_WR) |
FW_WR_COMPL(1) |
FW_WR_IMMDLEN(dlen));
req->flowid_len16 = htonl(FW_WR_FLOWID(csk->tid) |
FW_WR_LEN16(credits));
} else {
+ req = (struct fw_ofld_tx_data_wr *)__skb_push(skb,
+ sizeof(*req));
req->op_to_immdlen =
cpu_to_be32(FW_WR_OP(FW_OFLD_TX_DATA_WR) |
FW_WR_COMPL(1) |
@@ -597,12 +603,14 @@ static int push_tx_frames(struct cxgbi_sock *csk, int req_completion)
skb_reset_transport_header(skb);
if (is_ofld_imm(skb))
- credits_needed = DIV_ROUND_UP(dlen +
- sizeof(struct fw_ofld_tx_data_wr), 16);
+ credits_needed = DIV_ROUND_UP(dlen, 16);
else
- credits_needed = DIV_ROUND_UP(8*calc_tx_flits_ofld(skb)
- + sizeof(struct fw_ofld_tx_data_wr),
- 16);
+ credits_needed = DIV_ROUND_UP(8*calc_tx_flits_ofld(skb),
+ 16);
+
+ if (likely(cxgbi_skcb_test_flag(skb, SKCBF_TX_NEED_HDR)))
+ credits_needed += DIV_ROUND_UP(
+ sizeof(struct fw_ofld_tx_data_wr), 16);
if (csk->wr_cred < credits_needed) {
log_debug(1 << CXGBI_DBG_PDU_TX,
diff --git a/drivers/scsi/cxgbi/libcxgbi.h b/drivers/scsi/cxgbi/libcxgbi.h
index 2c7cb1c..aba1af7 100644
--- a/drivers/scsi/cxgbi/libcxgbi.h
+++ b/drivers/scsi/cxgbi/libcxgbi.h
@@ -317,8 +317,8 @@ static inline void cxgbi_skcb_clear_flag(struct sk_buff *skb,
__clear_bit(flag, &(cxgbi_skcb_flags(skb)));
}
-static inline int cxgbi_skcb_test_flag(struct sk_buff *skb,
- enum cxgbi_skcb_flags flag)
+static inline int cxgbi_skcb_test_flag(const struct sk_buff *skb,
+ enum cxgbi_skcb_flags flag)
{
return test_bit(flag, &(cxgbi_skcb_flags(skb)));
}
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH net v3 1/6] cxgb4i: fix tx credit calculation
2014-12-09 17:32 [PATCH net v3 1/6] cxgb4i: fix tx credit calculation Karen Xie
@ 2014-12-09 19:54 ` Sergei Shtylyov
2014-12-10 1:38 ` Karen Xie
2014-12-10 1:44 ` Karen Xie
0 siblings, 2 replies; 4+ messages in thread
From: Sergei Shtylyov @ 2014-12-09 19:54 UTC (permalink / raw)
To: Karen Xie, linux-scsi, netdev
Cc: hariprasad, anish, hch, James.Bottomley, michaelc, davem
Hello.
On 12/09/2014 08:32 PM, Karen Xie wrote:
> [PATCH net v3 1/6] cxgb4i: fix tx credit calculation
> From: Karen Xie <kxie@chelsio.com>
> - Only data skbs need the wr header added while control skbs do not. Make sure they are treated differently.
> - Any credit related checking should be done before adding the wr header.
Looks like a spearate issue deserving its own patch?
> - Fixed compiler warning resulted from added cxgbi_skb_test_flag() call in is_ofld_imm().
Isn't it called cxgbi_skcb_test_flag()?
> Signed-off-by: Karen Xie <kxie@chelsio.com>
> ---
> drivers/scsi/cxgbi/cxgb4i/cxgb4i.c | 26 +++++++++++++++++---------
> drivers/scsi/cxgbi/libcxgbi.h | 4 ++--
> 2 files changed, 19 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
> index 1508125..5c3f15d 100644
> --- a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
> +++ b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
[...]
> @@ -544,15 +548,17 @@ static inline void make_tx_data_wr(struct cxgbi_sock *csk, struct sk_buff *skb,
> unsigned int submode = cxgbi_skcb_ulp_mode(skb) & 3;
> unsigned int wr_ulp_mode = 0;
>
> - req = (struct fw_ofld_tx_data_wr *)__skb_push(skb, sizeof(*req));
> -
Perhaps it makes sense to store the result of is_ofld_imm() before
__skb_push() call
instead of duplicating the same call in 2 branches?
> if (is_ofld_imm(skb)) {
> + req = (struct fw_ofld_tx_data_wr *)__skb_push(skb,
> + sizeof(*req));
The continuation line should start right under 'skb' on the previous line.
> req->op_to_immdlen = htonl(FW_WR_OP(FW_OFLD_TX_DATA_WR) |
> FW_WR_COMPL(1) |
> FW_WR_IMMDLEN(dlen));
> req->flowid_len16 = htonl(FW_WR_FLOWID(csk->tid) |
> FW_WR_LEN16(credits));
> } else {
> + req = (struct fw_ofld_tx_data_wr *)__skb_push(skb,
> + sizeof(*req));
Likewise.
> req->op_to_immdlen =
> cpu_to_be32(FW_WR_OP(FW_OFLD_TX_DATA_WR) |
> FW_WR_COMPL(1) |
> @@ -597,12 +603,14 @@ static int push_tx_frames(struct cxgbi_sock *csk, int req_completion)
>
> skb_reset_transport_header(skb);
> if (is_ofld_imm(skb))
> - credits_needed = DIV_ROUND_UP(dlen +
> - sizeof(struct fw_ofld_tx_data_wr), 16);
> + credits_needed = DIV_ROUND_UP(dlen, 16);
> else
> - credits_needed = DIV_ROUND_UP(8*calc_tx_flits_ofld(skb)
> - + sizeof(struct fw_ofld_tx_data_wr),
> - 16);
> + credits_needed = DIV_ROUND_UP(8*calc_tx_flits_ofld(skb),
It would have been good if you added spaces around *, while at it, to keep
it consistent with the general kernel coding style...
[...]
WBR, Sergei
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH net v3 1/6] cxgb4i: fix tx credit calculation
2014-12-09 19:54 ` Sergei Shtylyov
@ 2014-12-10 1:38 ` Karen Xie
2014-12-10 1:44 ` Karen Xie
1 sibling, 0 replies; 4+ messages in thread
From: Karen Xie @ 2014-12-10 1:38 UTC (permalink / raw)
To: Sergei Shtylyov, linux-scsi@vger.kernel.org,
netdev@vger.kernel.org
Cc: Hariprasad S, Anish Bhatt, hch@infradead.org,
James.Bottomley@HansenPartnership.com, michaelc@cs.wisc.edu,
davem@davemloft.net
Thanks for the review comment, have sent the v4 set to incorporate your comments.
-----Original Message-----
From: Sergei Shtylyov [mailto:sergei.shtylyov@cogentembedded.com]
Sent: Tuesday, December 09, 2014 11:55 AM
To: Karen Xie; linux-scsi@vger.kernel.org; netdev@vger.kernel.org
Cc: Hariprasad S; Anish Bhatt; hch@infradead.org; James.Bottomley@HansenPartnership.com; michaelc@cs.wisc.edu; davem@davemloft.net
Subject: Re: [PATCH net v3 1/6] cxgb4i: fix tx credit calculation
Hello.
On 12/09/2014 08:32 PM, Karen Xie wrote:
> [PATCH net v3 1/6] cxgb4i: fix tx credit calculation
> From: Karen Xie <kxie@chelsio.com>
> - Only data skbs need the wr header added while control skbs do not. Make sure they are treated differently.
> - Any credit related checking should be done before adding the wr header.
Looks like a spearate issue deserving its own patch?
> - Fixed compiler warning resulted from added cxgbi_skb_test_flag() call in is_ofld_imm().
Isn't it called cxgbi_skcb_test_flag()?
> Signed-off-by: Karen Xie <kxie@chelsio.com>
> ---
> drivers/scsi/cxgbi/cxgb4i/cxgb4i.c | 26 +++++++++++++++++---------
> drivers/scsi/cxgbi/libcxgbi.h | 4 ++--
> 2 files changed, 19 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
> b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
> index 1508125..5c3f15d 100644
> --- a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
> +++ b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
[...]
> @@ -544,15 +548,17 @@ static inline void make_tx_data_wr(struct cxgbi_sock *csk, struct sk_buff *skb,
> unsigned int submode = cxgbi_skcb_ulp_mode(skb) & 3;
> unsigned int wr_ulp_mode = 0;
>
> - req = (struct fw_ofld_tx_data_wr *)__skb_push(skb, sizeof(*req));
> -
Perhaps it makes sense to store the result of is_ofld_imm() before
__skb_push() call
instead of duplicating the same call in 2 branches?
> if (is_ofld_imm(skb)) {
> + req = (struct fw_ofld_tx_data_wr *)__skb_push(skb,
> + sizeof(*req));
The continuation line should start right under 'skb' on the previous line.
> req->op_to_immdlen = htonl(FW_WR_OP(FW_OFLD_TX_DATA_WR) |
> FW_WR_COMPL(1) |
> FW_WR_IMMDLEN(dlen));
> req->flowid_len16 = htonl(FW_WR_FLOWID(csk->tid) |
> FW_WR_LEN16(credits));
> } else {
> + req = (struct fw_ofld_tx_data_wr *)__skb_push(skb,
> + sizeof(*req));
Likewise.
> req->op_to_immdlen =
> cpu_to_be32(FW_WR_OP(FW_OFLD_TX_DATA_WR) |
> FW_WR_COMPL(1) |
> @@ -597,12 +603,14 @@ static int push_tx_frames(struct cxgbi_sock
> *csk, int req_completion)
>
> skb_reset_transport_header(skb);
> if (is_ofld_imm(skb))
> - credits_needed = DIV_ROUND_UP(dlen +
> - sizeof(struct fw_ofld_tx_data_wr), 16);
> + credits_needed = DIV_ROUND_UP(dlen, 16);
> else
> - credits_needed = DIV_ROUND_UP(8*calc_tx_flits_ofld(skb)
> - + sizeof(struct fw_ofld_tx_data_wr),
> - 16);
> + credits_needed = DIV_ROUND_UP(8*calc_tx_flits_ofld(skb),
It would have been good if you added spaces around *, while at it, to keep it consistent with the general kernel coding style...
[...]
WBR, Sergei
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH net v3 1/6] cxgb4i: fix tx credit calculation
2014-12-09 19:54 ` Sergei Shtylyov
2014-12-10 1:38 ` Karen Xie
@ 2014-12-10 1:44 ` Karen Xie
1 sibling, 0 replies; 4+ messages in thread
From: Karen Xie @ 2014-12-10 1:44 UTC (permalink / raw)
To: Sergei Shtylyov, linux-scsi@vger.kernel.org,
netdev@vger.kernel.org
Cc: Hariprasad S, Anish Bhatt, hch@infradead.org,
James.Bottomley@HansenPartnership.com, michaelc@cs.wisc.edu,
davem@davemloft.net
Please ignore v4, and use v5. Thanks!
-----Original Message-----
From: Karen Xie
Sent: Tuesday, December 09, 2014 5:39 PM
To: 'Sergei Shtylyov'; linux-scsi@vger.kernel.org; netdev@vger.kernel.org
Cc: Hariprasad S; Anish Bhatt; hch@infradead.org; James.Bottomley@HansenPartnership.com; michaelc@cs.wisc.edu; davem@davemloft.net
Subject: RE: [PATCH net v3 1/6] cxgb4i: fix tx credit calculation
Thanks for the review comment, have sent the v4 set to incorporate your comments.
-----Original Message-----
From: Sergei Shtylyov [mailto:sergei.shtylyov@cogentembedded.com]
Sent: Tuesday, December 09, 2014 11:55 AM
To: Karen Xie; linux-scsi@vger.kernel.org; netdev@vger.kernel.org
Cc: Hariprasad S; Anish Bhatt; hch@infradead.org; James.Bottomley@HansenPartnership.com; michaelc@cs.wisc.edu; davem@davemloft.net
Subject: Re: [PATCH net v3 1/6] cxgb4i: fix tx credit calculation
Hello.
On 12/09/2014 08:32 PM, Karen Xie wrote:
> [PATCH net v3 1/6] cxgb4i: fix tx credit calculation
> From: Karen Xie <kxie@chelsio.com>
> - Only data skbs need the wr header added while control skbs do not. Make sure they are treated differently.
> - Any credit related checking should be done before adding the wr header.
Looks like a spearate issue deserving its own patch?
> - Fixed compiler warning resulted from added cxgbi_skb_test_flag() call in is_ofld_imm().
Isn't it called cxgbi_skcb_test_flag()?
> Signed-off-by: Karen Xie <kxie@chelsio.com>
> ---
> drivers/scsi/cxgbi/cxgb4i/cxgb4i.c | 26 +++++++++++++++++---------
> drivers/scsi/cxgbi/libcxgbi.h | 4 ++--
> 2 files changed, 19 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
> b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
> index 1508125..5c3f15d 100644
> --- a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
> +++ b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
[...]
> @@ -544,15 +548,17 @@ static inline void make_tx_data_wr(struct cxgbi_sock *csk, struct sk_buff *skb,
> unsigned int submode = cxgbi_skcb_ulp_mode(skb) & 3;
> unsigned int wr_ulp_mode = 0;
>
> - req = (struct fw_ofld_tx_data_wr *)__skb_push(skb, sizeof(*req));
> -
Perhaps it makes sense to store the result of is_ofld_imm() before
__skb_push() call
instead of duplicating the same call in 2 branches?
> if (is_ofld_imm(skb)) {
> + req = (struct fw_ofld_tx_data_wr *)__skb_push(skb,
> + sizeof(*req));
The continuation line should start right under 'skb' on the previous line.
> req->op_to_immdlen = htonl(FW_WR_OP(FW_OFLD_TX_DATA_WR) |
> FW_WR_COMPL(1) |
> FW_WR_IMMDLEN(dlen));
> req->flowid_len16 = htonl(FW_WR_FLOWID(csk->tid) |
> FW_WR_LEN16(credits));
> } else {
> + req = (struct fw_ofld_tx_data_wr *)__skb_push(skb,
> + sizeof(*req));
Likewise.
> req->op_to_immdlen =
> cpu_to_be32(FW_WR_OP(FW_OFLD_TX_DATA_WR) |
> FW_WR_COMPL(1) |
> @@ -597,12 +603,14 @@ static int push_tx_frames(struct cxgbi_sock
> *csk, int req_completion)
>
> skb_reset_transport_header(skb);
> if (is_ofld_imm(skb))
> - credits_needed = DIV_ROUND_UP(dlen +
> - sizeof(struct fw_ofld_tx_data_wr), 16);
> + credits_needed = DIV_ROUND_UP(dlen, 16);
> else
> - credits_needed = DIV_ROUND_UP(8*calc_tx_flits_ofld(skb)
> - + sizeof(struct fw_ofld_tx_data_wr),
> - 16);
> + credits_needed = DIV_ROUND_UP(8*calc_tx_flits_ofld(skb),
It would have been good if you added spaces around *, while at it, to keep it consistent with the general kernel coding style...
[...]
WBR, Sergei
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-12-10 1:46 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-12-09 17:32 [PATCH net v3 1/6] cxgb4i: fix tx credit calculation Karen Xie
2014-12-09 19:54 ` Sergei Shtylyov
2014-12-10 1:38 ` Karen Xie
2014-12-10 1:44 ` Karen Xie
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).