diff mbox

[1/5] qtnfmac: modify full Tx queue error reporting

Message ID 20171015205327.9966-2-sergey.matyukevich.os@quantenna.com (mailing list archive)
State Superseded
Delegated to: Kalle Valo
Headers show

Commit Message

Sergey Matyukevich Oct. 15, 2017, 8:53 p.m. UTC
Under heavy load it is normal that h/w Tx queue is almost full all the time
and reclaim should be done before transmitting next packet. Warning still
should be reported as well as s/w Tx queues should be stopped in the
case when reclaim failed.

Signed-off-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>
---
 drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Kalle Valo Oct. 27, 2017, 8:48 a.m. UTC | #1
Sergey Matyukevich <sergey.matyukevich.os@quantenna.com> wrote:

> Under heavy load it is normal that h/w Tx queue is almost full all the time
> and reclaim should be done before transmitting next packet. Warning still
> should be reported as well as s/w Tx queues should be stopped in the
> case when reclaim failed.
> 
> Signed-off-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>

Failed to apply:

fatal: sha1 information is lacking or useless (drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c).
error: could not build fake ancestor
Applying: qtnfmac: modify full Tx queue recovery
Patch failed at 0001 qtnfmac: modify full Tx queue recovery
The copy of the patch that failed is found in: .git/rebase-apply/patch

5 patches set to Changes Requested.

10007281 [1/5] qtnfmac: modify full Tx queue error reporting
10007279 [2/5] qtnfmac: enable registration of more mgmt frames
10007283 [3/5] qtnfmac: drop nonexistent function declaration
10007285 [4/5] qtnfmac: modify full Tx queue recovery
10007287 [5/5] qtnfmac: advertise support of inactivity timeout
Sergey Matyukevich Oct. 29, 2017, 3:32 p.m. UTC | #2
Hello Kalle,

> Failed to apply:
> 
> fatal: sha1 information is lacking or useless (drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c).
> error: could not build fake ancestor
> Applying: qtnfmac: modify full Tx queue recovery
> Patch failed at 0001 qtnfmac: modify full Tx queue recovery
> The copy of the patch that failed is found in: .git/rebase-apply/patch
> 
> 5 patches set to Changes Requested.
> 
> 10007281 [1/5] qtnfmac: modify full Tx queue error reporting
> 10007279 [2/5] qtnfmac: enable registration of more mgmt frames
> 10007283 [3/5] qtnfmac: drop nonexistent function declaration
> 10007285 [4/5] qtnfmac: modify full Tx queue recovery
> 10007287 [5/5] qtnfmac: advertise support of inactivity timeout

My assumption is that by default all the patches should cleanly apply to wireless-drivers-next.
I could apply the patch in question to wireless-drivers-next without any issues.
Rebase of the whole series on top of wireless-drivers-next looks good as well.
I will resend rebased patches as v2. Meanwhile do you have any idea what could go wrong ?
The error message looks scary...

Regards,
Sergey
Kalle Valo Oct. 30, 2017, 8:02 a.m. UTC | #3
Sergey Matyukevich <sergey.matyukevich.os@quantenna.com> writes:

> Hello Kalle,
>
>> Failed to apply:
>> 
>> fatal: sha1 information is lacking or useless
>> (drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c).
>> error: could not build fake ancestor
>> Applying: qtnfmac: modify full Tx queue recovery
>> Patch failed at 0001 qtnfmac: modify full Tx queue recovery
>> The copy of the patch that failed is found in: .git/rebase-apply/patch
>> 
>> 5 patches set to Changes Requested.
>> 
>> 10007281 [1/5] qtnfmac: modify full Tx queue error reporting
>> 10007279 [2/5] qtnfmac: enable registration of more mgmt frames
>> 10007283 [3/5] qtnfmac: drop nonexistent function declaration
>> 10007285 [4/5] qtnfmac: modify full Tx queue recovery
>> 10007287 [5/5] qtnfmac: advertise support of inactivity timeout
>
> My assumption is that by default all the patches should cleanly apply
> to wireless-drivers-next. I could apply the patch in question to
> wireless-drivers-next without any issues.

Odd. How did you apply it? My script uses 'git am -s -3' individually
for each patch in the series, but to my knowledge that shouldn't cause
any problems.

> Rebase of the whole series on top of wireless-drivers-next looks good
> as well. I will resend rebased patches as v2.

Thanks, sending v2 is the easiest for me. If there are problems again
I'll investigate in detail what's going on.

> Meanwhile do you have any idea what could go wrong ? The error message
> looks scary...

You mean the "sha1 information is lacking", right? It means that git was
not able to find a common ancestor for the file which it could use to
create the 3-way merge. Usually that happens when people have
out-of-tree patches on the branch they are submitting from.

And that's why I recommend to use the w-d-next master branch as the
baseline when submitting patches, and not have any other custom patches
applied on the branch. This should keep sha1 information correct and
make it possible for git to use 3-way merge.
Sergey Matyukevich Oct. 30, 2017, 9:45 a.m. UTC | #4
> > My assumption is that by default all the patches should cleanly apply
> > to wireless-drivers-next. I could apply the patch in question to
> > wireless-drivers-next without any issues.
> 
> Odd. How did you apply it? My script uses 'git am -s -3' individually
> for each patch in the series, but to my knowledge that shouldn't cause
> any problems.

It turns out I mechanically rebased my branch on top of w-d-n before writing
the previous email. Now I could reproduce the same failure when using this
command for the original patches downloaded from patchwork. The problem
was in conflict with qtnfmac fixes pulled to w-d-n from w-d.

Regards,
Sergey
Kalle Valo Oct. 30, 2017, 10:31 a.m. UTC | #5
Sergey Matyukevich <sergey.matyukevich.os@quantenna.com> writes:

>> > My assumption is that by default all the patches should cleanly apply
>> > to wireless-drivers-next. I could apply the patch in question to
>> > wireless-drivers-next without any issues.
>> 
>> Odd. How did you apply it? My script uses 'git am -s -3' individually
>> for each patch in the series, but to my knowledge that shouldn't cause
>> any problems.
>
> It turns out I mechanically rebased my branch on top of w-d-n before writing
> the previous email. Now I could reproduce the same failure when using this
> command for the original patches downloaded from patchwork.

Ok, that makes sense. Most likely when git-rebase does a 3-way merge
automatically and that's why you were able to successfully apply them.
But as git-am was not able to use 3-way merge due to incorrect sha1
information it failed.

> The problem was in conflict with qtnfmac fixes pulled to w-d-n from
> w-d.

Ok, thanks for investigating the root cause.
diff mbox

Patch

diff --git a/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c b/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c
index 502e72b7cdcc..a8f2c46f3a25 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c
@@ -643,11 +643,11 @@  static int qtnf_tx_queue_ready(struct qtnf_pcie_bus_priv *priv)
 {
 	if (!CIRC_SPACE(priv->tx_bd_w_index, priv->tx_bd_r_index,
 			priv->tx_bd_num)) {
-		pr_err_ratelimited("reclaim full Tx queue\n");
 		qtnf_pcie_data_tx_reclaim(priv);
 
 		if (!CIRC_SPACE(priv->tx_bd_w_index, priv->tx_bd_r_index,
 				priv->tx_bd_num)) {
+			pr_warn_ratelimited("reclaim full Tx queue\n");
 			priv->tx_full_count++;
 			return 0;
 		}