Re: [TCWG CI] Regression caused by linux: net: ipa: rework gsi_channel_tx_update()

On 6/26/22 1:55 AM, ci_notify@linaro.org wrote:

[TCWG CI] Regression caused by linux: net: ipa: rework gsi_channel_tx_update(): commit c5bddecbb97bcf0400354dc954cdbd89276e0ddb Author: Alex Elder elder@linaro.org

 net: ipa: rework gsi_channel_tx_update()

I'm really at a loss for what to do with this.

It isn't clear from this message what the problem is.

And I just tried to reproduce it using the steps in this message and hit an error (I'll note it below).

Is there anyone who can help explain this?

Results regressed to # reset_artifacts: -10 # build_abe binutils: -9 # build_abe stage1: -5 # build_abe qemu: -2 # linux_n_obj: 31579 # linux build successful: all # First few build errors in logs:

from # reset_artifacts: -10 # build_abe binutils: -9 # build_abe stage1: -5 # build_abe qemu: -2 # linux_n_obj: 31579 # linux build successful: all # linux boot successful: boot

THIS IS THE END OF INTERESTING STUFF. BELOW ARE LINKS TO BUILDS, REPRODUCTION INSTRUCTIONS, AND THE RAW COMMIT.

This commit has regressed these CI configurations:

• tcwg_kernel/gnu-release-aarch64-next-allmodconfig

First_bad build: https://ci.linaro.org/job/tcwg_kernel-gnu-bisect-gnu-release-aarch64-next-al... Last_good build: https://ci.linaro.org/job/tcwg_kernel-gnu-bisect-gnu-release-aarch64-next-al... Baseline build: https://ci.linaro.org/job/tcwg_kernel-gnu-bisect-gnu-release-aarch64-next-al... Even more details: https://ci.linaro.org/job/tcwg_kernel-gnu-bisect-gnu-release-aarch64-next-al...

Reproduce builds:

Here is where I started running the commands supplied.

<cut> mkdir investigate-linux-c5bddecbb97bcf0400354dc954cdbd89276e0ddb cd investigate-linux-c5bddecbb97bcf0400354dc954cdbd89276e0ddb

All good.

# Fetch scripts git clone https://git.linaro.org/toolchain/jenkins-scripts

# Fetch manifests and test.sh script mkdir -p artifacts/manifests curl -o artifacts/manifests/build-baseline.sh https://ci.linaro.org/job/tcwg_kernel-gnu-bisect-gnu-release-aarch64-next-al... --fail curl -o artifacts/manifests/build-parameters.sh https://ci.linaro.org/job/tcwg_kernel-gnu-bisect-gnu-release-aarch64-next-al... --fail curl -o artifacts/test.sh https://ci.linaro.org/job/tcwg_kernel-gnu-bisect-gnu-release-aarch64-next-al... --fail chmod +x artifacts/test.sh

All good to here.

# Reproduce the baseline build (build all pre-requisites) ./jenkins-scripts/tcwg_kernel-build.sh @@ artifacts/manifests/build-baseline.sh

Here's what I get here:

{2014} elder@presto-> ./jenkins-scripts/tcwg_kernel-build.sh @@ artifacts/manifests/build-baseline.sh

run_step_init: starting at step "" finishing at step "update_baseline"

RUNNING reset_artifacts; see tail -f /tmp/investigate-linux-c5bddecbb97bcf0400354dc954cdbd89276e0ddb/artifacts/build-baseline/01-reset_artifacts/console.log

STOPPING at reset_artifacts due to internal failure

{2015} elder@presto->

# Save baseline build state (which is then restored in artifacts/test.sh) mkdir -p ./bisect rsync -a --del --delete-excluded --exclude /bisect/ --exclude /artifacts/ --exclude /linux/ ./ ./bisect/baseline/

cd linux

I tried to proceed anyway, but got a failure here too (the first problem is what I'm mainly concerned with).

{2016} elder@presto-> mkdir -p ./bisect

{2017} elder@presto-> rsync -a --del --delete-excluded --exclude /bisect/ --exclude /artifacts/ --exclude /linux/ ./ ./bisect/baseline/

{2018} elder@presto-> cd linux

-bash: cd: linux: No such file or directory

{2019} elder@presto-> ls

Thanks.

-Alex

# Reproduce first_bad build git checkout --detach c5bddecbb97bcf0400354dc954cdbd89276e0ddb ../artifacts/test.sh

# Reproduce last_good build git checkout --detach dbad2fa71914a6a1a41bcba8bdc3e9213a982f82 ../artifacts/test.sh

cd ..

</cut>

Full commit (up to 1000 lines):

<cut> commit c5bddecbb97bcf0400354dc954cdbd89276e0ddb Author: Alex Elder <elder@linaro.org> Date: Mon Jun 13 12:17:59 2022 -0500

 net: ipa: rework gsi_channel_tx_update()
 
 Rename gsi_channel_tx_update() to be gsi_trans_tx_completed(), and
 pass it just the transaction pointer, deriving the channel from the
 transaction.  Update the comments above the function to provide a
 more concise description of how statistics for TX endpoints are
 maintained and used.
 
 Signed-off-by: Alex Elder <elder@linaro.org>
 Signed-off-by: David S. Miller <davem@davemloft.net>

---

drivers/net/ipa/gsi.c | 50 +++++++++++++++++++++++--------------------------- 1 file changed, 23 insertions(+), 27 deletions(-)

diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c index c2cafd9247a7..df8af1f00fc8 100644 --- a/drivers/net/ipa/gsi.c +++ b/drivers/net/ipa/gsi.c @@ -1021,40 +1021,36 @@ void gsi_trans_tx_queued(struct gsi_trans *trans) } /**

• • gsi_channel_tx_update() - Report completed TX transfers
• • @channel: Channel that has completed transmitting packets
• • @trans: Last transation known to be complete

• • gsi_trans_tx_completed() - Report completed TX transactions
• • @trans: TX channel transaction that has completed

• • Compute the number of transactions and bytes that have been transferred
• • over a TX channel since the given transaction was committed. Report this
• • information to the network stack.

• • Report that a transaction on a TX channel has completed. At the time a
• • transaction is committed, we record *in the transaction* its channel's
• • committed transaction and byte counts. Transactions are completed in
• • order, and the difference between the channel's byte/transaction count
• • when the transaction was committed and when it completes tells us
• • exactly how much data has been transferred while the transaction was
• • pending.

• • At the time a transaction is committed, we record its channel's
• • committed transaction and byte counts *in the transaction*.
• • Completions are signaled by the hardware with an interrupt, and
• • we can determine the latest completed transaction at that time.
• • The difference between the byte/transaction count recorded in
• • the transaction and the count last time we recorded a completion
• • tells us exactly how much data has been transferred between
• • completions.
• • Calling this each time we learn of a newly-completed transaction
• • allows us to provide accurate information to the network stack
• • about how much work has been completed by the hardware at a given
• • point in time.

• • We report this information to the network stack, which uses it to manage
• • the rate at which data is sent to hardware.
  */

-static void -gsi_channel_tx_update(struct gsi_channel *channel, struct gsi_trans *trans) +static void gsi_trans_tx_completed(struct gsi_trans *trans) {

• u64 trans_count = trans->trans_count - channel->compl_trans_count;
• u64 byte_count = trans->byte_count - channel->compl_byte_count;

• u32 channel_id = trans->channel_id;
• struct gsi *gsi = trans->gsi;
• struct gsi_channel *channel;
• u32 trans_count;
• u32 byte_count;
• channel = &gsi->channel[channel_id];
• trans_count = trans->trans_count - channel->compl_trans_count;
• byte_count = trans->byte_count - channel->compl_byte_count;

channel->compl_trans_count += trans_count; channel->compl_byte_count += byte_count;

• ipa_gsi_channel_tx_completed(channel->gsi, gsi_channel_id(channel),

• 		     trans_count, byte_count);
  

• ipa_gsi_channel_tx_completed(gsi, channel_id, trans_count, byte_count); }

/* Channel control interrupt handler */ @@ -1504,7 +1500,7 @@ static struct gsi_trans *gsi_channel_update(struct gsi_channel *channel) * up the network stack. */ if (channel->toward_ipa)

• gsi_channel_tx_update(channel, trans);
  

• gsi_trans_tx_completed(trans);
  

  else gsi_evt_ring_rx_update(evt_ring, index);

</cut>