[PATCH v2 0/3] trace: Fix flyrecord alignment issue
Hi,
sandbox is getting bigger and bigger and I have reached the case that adding some more functions ends up in CI loop failure. After some investigation I found that flyrecord header have incorrect information about data offset which is caused by incorrect alignment calculation. That's why this series is fixing alignment calculation. I have done it via 3 patches where the first two are just preparing code for actual fixup.
Thanks, Michal
Changes in v2: - s/start_addr/start_ofs/g'
Michal Simek (3): trace: Use 64bit variable for start and len trace: Move trace_clocks description above record offset calculation trace: Fix alignment logic in flyrecord header
tools/proftool.c | 39 ++++++++++++++++++++++++++++++++++----- 1 file changed, 34 insertions(+), 5 deletions(-)
tputq() requires variables to have 64bit width that's why make them 64bit to clean alignment requirement.
Signed-off-by: Michal Simek michal.simek@amd.com Reviewed-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/proftool.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/proftool.c b/tools/proftool.c index 101bcb63334e..869a2a32c510 100644 --- a/tools/proftool.c +++ b/tools/proftool.c @@ -1493,7 +1493,8 @@ static int write_pages(struct twriter *tw, enum out_format_t out_format, static int write_flyrecord(struct twriter *tw, enum out_format_t out_format, int *missing_countp, int *skip_countp) { - int start, ret, len; + unsigned long long start, len; + int ret; FILE *fout = tw->fout; char str[200];
tputq() requires variables to have 64bit width that's why make them 64bit to clean alignment requirement.
Signed-off-by: Michal Simek michal.simek@amd.com Reviewed-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/proftool.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
Applied to u-boot-dm, thanks!
Flyrecord tracing data are page aligned that's why it is necessary to calculate alignment properly. Because trace_clocks description is the part of record length it is necessary to have information about length earlier.
Signed-off-by: Michal Simek michal.simek@amd.com Reviewed-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/proftool.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/tools/proftool.c b/tools/proftool.c index 869a2a32c510..7c95a94482fc 100644 --- a/tools/proftool.c +++ b/tools/proftool.c @@ -1500,6 +1500,10 @@ static int write_flyrecord(struct twriter *tw, enum out_format_t out_format,
tw->ptr += fprintf(fout, "flyrecord%c", 0);
+ snprintf(str, sizeof(str), + "[local] global counter uptime perf mono mono_raw boot x86-tsc\n"); + len = strlen(str); + /* trace data */ start = ALIGN(tw->ptr + 16, TRACE_PAGE_SIZE); tw->ptr += tputq(fout, start); @@ -1510,9 +1514,6 @@ static int write_flyrecord(struct twriter *tw, enum out_format_t out_format, return -1; tw->ptr += ret;
- snprintf(str, sizeof(str), - "[local] global counter uptime perf mono mono_raw boot x86-tsc\n"); - len = strlen(str); tw->ptr += tputq(fout, len); tw->ptr += tputs(fout, str);
Flyrecord tracing data are page aligned that's why it is necessary to calculate alignment properly. Because trace_clocks description is the part of record length it is necessary to have information about length earlier.
Signed-off-by: Michal Simek michal.simek@amd.com Reviewed-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/proftool.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
Applied to u-boot-dm, thanks!
Current alignment which is using 16 bytes is not correct in connection to trace_clocks description and it's length. That's why use start_addr variable and record proper size based on used entries.
Fixes: be16fc81b2ed ("trace: Update proftool to use new binary format"). Signed-off-by: Michal Simek michal.simek@amd.com Reviewed-by: Simon Glass sjg@chromium.org ---
Changes in v2: - s/start_addr/start_ofs/g'
tools/proftool.c | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-)
diff --git a/tools/proftool.c b/tools/proftool.c index 7c95a94482fc..fca45e4a5af6 100644 --- a/tools/proftool.c +++ b/tools/proftool.c @@ -1493,19 +1493,43 @@ static int write_pages(struct twriter *tw, enum out_format_t out_format, static int write_flyrecord(struct twriter *tw, enum out_format_t out_format, int *missing_countp, int *skip_countp) { - unsigned long long start, len; + unsigned long long start, start_ofs, len; int ret; FILE *fout = tw->fout; char str[200];
+ /* Record start pointer */ + start_ofs = tw->ptr; + debug("Start of flyrecord header at: 0x%llx\n", start_ofs); + tw->ptr += fprintf(fout, "flyrecord%c", 0);
+ /* flyrecord\0 - allocated 10 bytes */ + start_ofs += 10; + + /* + * 8 bytes that are a 64-bit word containing the offset into the file + * that holds the data for the CPU. + * + * 8 bytes that are a 64-bit word containing the size of the CPU + * data at that offset. + */ + start_ofs += 16; + snprintf(str, sizeof(str), "[local] global counter uptime perf mono mono_raw boot x86-tsc\n"); len = strlen(str);
+ /* trace clock length - 8 bytes */ + start_ofs += 8; + /* trace clock data */ + start_ofs += len; + + debug("Calculated flyrecord header end at: 0x%llx, trace clock len: 0x%llx\n", + start_ofs, len); + /* trace data */ - start = ALIGN(tw->ptr + 16, TRACE_PAGE_SIZE); + start = ALIGN(start_ofs, TRACE_PAGE_SIZE); tw->ptr += tputq(fout, start);
/* use a placeholder for the size */ @@ -1517,6 +1541,9 @@ static int write_flyrecord(struct twriter *tw, enum out_format_t out_format, tw->ptr += tputq(fout, len); tw->ptr += tputs(fout, str);
+ debug("End of flyrecord header at: 0x%x, offset: 0x%llx\n", + tw->ptr, start); + debug("trace text base %lx, map file %lx\n", text_base, text_offset);
ret = write_pages(tw, out_format, missing_countp, skip_countp);
Current alignment which is using 16 bytes is not correct in connection to trace_clocks description and it's length. That's why use start_addr variable and record proper size based on used entries.
Fixes: be16fc81b2ed ("trace: Update proftool to use new binary format"). Signed-off-by: Michal Simek michal.simek@amd.com Reviewed-by: Simon Glass sjg@chromium.org ---
Changes in v2: - s/start_addr/start_ofs/g'
tools/proftool.c | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-)
Applied to u-boot-dm, thanks!
Hi Simon,
On 9/23/23 20:13, Simon Glass wrote:
Current alignment which is using 16 bytes is not correct in connection to trace_clocks description and it's length. That's why use start_addr variable and record proper size based on used entries.
Fixes: be16fc81b2ed ("trace: Update proftool to use new binary format"). Signed-off-by: Michal Simek michal.simek@amd.com Reviewed-by: Simon Glass sjg@chromium.org
Changes in v2:
s/start_addr/start_ofs/g'
tools/proftool.c | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-)
Applied to u-boot-dm, thanks!
FYI: I have merged it to my tree and already sent pull request to Tom. Without it I couldn't pass CI loop to get all reviewed features in.
https://lore.kernel.org/all/ab72c480-e9f8-416e-adf5-726f7d40c4f5@amd.com/
Thanks, Michal
Hi Michal,
On Mon, 25 Sept 2023 at 00:06, Michal Simek michal.simek@amd.com wrote:
Hi Simon,
On 9/23/23 20:13, Simon Glass wrote:
Current alignment which is using 16 bytes is not correct in connection to trace_clocks description and it's length. That's why use start_addr variable and record proper size based on used entries.
Fixes: be16fc81b2ed ("trace: Update proftool to use new binary format"). Signed-off-by: Michal Simek michal.simek@amd.com Reviewed-by: Simon Glass sjg@chromium.org
Changes in v2:
s/start_addr/start_ofs/g'
tools/proftool.c | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-)
Applied to u-boot-dm, thanks!
FYI: I have merged it to my tree and already sent pull request to Tom. Without it I couldn't pass CI loop to get all reviewed features in.
https://lore.kernel.org/all/ab72c480-e9f8-416e-adf5-726f7d40c4f5@amd.com/
Ah OK, well that's fine. It was in my patchwork queue still, which suggests that the patches were not set to 'applied'?
Regards, Simon
On 9/25/23 15:10, Simon Glass wrote:
Hi Michal,
On Mon, 25 Sept 2023 at 00:06, Michal Simek michal.simek@amd.com wrote:
Hi Simon,
On 9/23/23 20:13, Simon Glass wrote:
Current alignment which is using 16 bytes is not correct in connection to trace_clocks description and it's length. That's why use start_addr variable and record proper size based on used entries.
Fixes: be16fc81b2ed ("trace: Update proftool to use new binary format"). Signed-off-by: Michal Simek michal.simek@amd.com Reviewed-by: Simon Glass sjg@chromium.org
Changes in v2:
s/start_addr/start_ofs/g'
tools/proftool.c | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-)
Applied to u-boot-dm, thanks!
FYI: I have merged it to my tree and already sent pull request to Tom. Without it I couldn't pass CI loop to get all reviewed features in.
https://lore.kernel.org/all/ab72c480-e9f8-416e-adf5-726f7d40c4f5@amd.com/
Ah OK, well that's fine. It was in my patchwork queue still, which suggests that the patches were not set to 'applied'?
I am not using patchwork. But I expect my reply to cover letter was recorded there.
Thanks, Michal
Hi Michal,
On Mon, 25 Sept 2023 at 07:38, Michal Simek michal.simek@amd.com wrote:
On 9/25/23 15:10, Simon Glass wrote:
Hi Michal,
On Mon, 25 Sept 2023 at 00:06, Michal Simek michal.simek@amd.com wrote:
Hi Simon,
On 9/23/23 20:13, Simon Glass wrote:
Current alignment which is using 16 bytes is not correct in connection to trace_clocks description and it's length. That's why use start_addr variable and record proper size based on used entries.
Fixes: be16fc81b2ed ("trace: Update proftool to use new binary format"). Signed-off-by: Michal Simek michal.simek@amd.com Reviewed-by: Simon Glass sjg@chromium.org
Changes in v2:
s/start_addr/start_ofs/g'
tools/proftool.c | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-)
Applied to u-boot-dm, thanks!
FYI: I have merged it to my tree and already sent pull request to Tom. Without it I couldn't pass CI loop to get all reviewed features in.
https://lore.kernel.org/all/ab72c480-e9f8-416e-adf5-726f7d40c4f5@amd.com/
Ah OK, well that's fine. It was in my patchwork queue still, which suggests that the patches were not set to 'applied'?
I am not using patchwork. But I expect my reply to cover letter was recorded there.
Probably. If you reply to each patch, it shows up in the patch, but the cover letter is hidden somewhere else.
If you are not using patchwork, how come you are a custodian? Is someone else dealing with patchwork for you?
Regards, Simon
Hi Simon,
On 9/25/23 16:01, Simon Glass wrote:
Hi Michal,
On Mon, 25 Sept 2023 at 07:38, Michal Simek michal.simek@amd.com wrote:
On 9/25/23 15:10, Simon Glass wrote:
Hi Michal,
On Mon, 25 Sept 2023 at 00:06, Michal Simek michal.simek@amd.com wrote:
Hi Simon,
On 9/23/23 20:13, Simon Glass wrote:
Current alignment which is using 16 bytes is not correct in connection to trace_clocks description and it's length. That's why use start_addr variable and record proper size based on used entries.
Fixes: be16fc81b2ed ("trace: Update proftool to use new binary format"). Signed-off-by: Michal Simek michal.simek@amd.com Reviewed-by: Simon Glass sjg@chromium.org
Changes in v2:
s/start_addr/start_ofs/g'
tools/proftool.c | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-)
Applied to u-boot-dm, thanks!
FYI: I have merged it to my tree and already sent pull request to Tom. Without it I couldn't pass CI loop to get all reviewed features in.
https://lore.kernel.org/all/ab72c480-e9f8-416e-adf5-726f7d40c4f5@amd.com/
Ah OK, well that's fine. It was in my patchwork queue still, which suggests that the patches were not set to 'applied'?
I am not using patchwork. But I expect my reply to cover letter was recorded there.
Probably. If you reply to each patch, it shows up in the patch, but the cover letter is hidden somewhere else.
I have never started to like patchwork. I installed that client long time ago, I also have account for quite a long time.
If you are not using patchwork, how come you are a custodian? Is someone else dealing with patchwork for you?
Not really. I am just keep track on it via emails.
DT folks did wire CI loop on every patch which they get. I am not aware about any feature like this which would bring me something. That's why I am considering patchwork as unneeded layer. And I also don't think that I have read anywhere that all custodians should be using patchwork.
Thanks, Michal
On Mon, Sep 25, 2023 at 04:10:38PM +0200, Michal Simek wrote:
Hi Simon,
On 9/25/23 16:01, Simon Glass wrote:
Hi Michal,
On Mon, 25 Sept 2023 at 07:38, Michal Simek michal.simek@amd.com wrote:
On 9/25/23 15:10, Simon Glass wrote:
Hi Michal,
On Mon, 25 Sept 2023 at 00:06, Michal Simek michal.simek@amd.com wrote:
Hi Simon,
On 9/23/23 20:13, Simon Glass wrote:
Current alignment which is using 16 bytes is not correct in connection to trace_clocks description and it's length. That's why use start_addr variable and record proper size based on used entries.
Fixes: be16fc81b2ed ("trace: Update proftool to use new binary format"). Signed-off-by: Michal Simek michal.simek@amd.com Reviewed-by: Simon Glass sjg@chromium.org
Changes in v2:
s/start_addr/start_ofs/g'
tools/proftool.c | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-)
Applied to u-boot-dm, thanks!
FYI: I have merged it to my tree and already sent pull request to Tom. Without it I couldn't pass CI loop to get all reviewed features in.
https://lore.kernel.org/all/ab72c480-e9f8-416e-adf5-726f7d40c4f5@amd.com/
Ah OK, well that's fine. It was in my patchwork queue still, which suggests that the patches were not set to 'applied'?
I am not using patchwork. But I expect my reply to cover letter was recorded there.
Probably. If you reply to each patch, it shows up in the patch, but the cover letter is hidden somewhere else.
I have never started to like patchwork. I installed that client long time ago, I also have account for quite a long time.
If you are not using patchwork, how come you are a custodian? Is someone else dealing with patchwork for you?
Not really. I am just keep track on it via emails.
DT folks did wire CI loop on every patch which they get. I am not aware about any feature like this which would bring me something. That's why I am considering patchwork as unneeded layer. And I also don't think that I have read anywhere that all custodians should be using patchwork.
Right, patchwork isn't required, but can be helpful. Part of how patchwork is maintained for everyone (in U-Boot) is that I have a script that will update the status of patches to accepted and add the githash, based on the "patchwork hash" of a given commit. There's a number of automated tooling things that other projects use which could be helpful here, but due to lack of time/resources, we haven't tried them here.
On 9/25/23 16:19, Tom Rini wrote:
On Mon, Sep 25, 2023 at 04:10:38PM +0200, Michal Simek wrote:
Hi Simon,
On 9/25/23 16:01, Simon Glass wrote:
Hi Michal,
On Mon, 25 Sept 2023 at 07:38, Michal Simek michal.simek@amd.com wrote:
On 9/25/23 15:10, Simon Glass wrote:
Hi Michal,
On Mon, 25 Sept 2023 at 00:06, Michal Simek michal.simek@amd.com wrote:
Hi Simon,
On 9/23/23 20:13, Simon Glass wrote: > Current alignment which is using 16 bytes is not correct in connection to > trace_clocks description and it's length. > That's why use start_addr variable and record proper size based on used > entries. > > Fixes: be16fc81b2ed ("trace: Update proftool to use new binary format"). > Signed-off-by: Michal Simek michal.simek@amd.com > Reviewed-by: Simon Glass sjg@chromium.org > --- > > Changes in v2: > - s/start_addr/start_ofs/g' > > tools/proftool.c | 31 +++++++++++++++++++++++++++++-- > 1 file changed, 29 insertions(+), 2 deletions(-) > > Applied to u-boot-dm, thanks!
FYI: I have merged it to my tree and already sent pull request to Tom. Without it I couldn't pass CI loop to get all reviewed features in.
https://lore.kernel.org/all/ab72c480-e9f8-416e-adf5-726f7d40c4f5@amd.com/
Ah OK, well that's fine. It was in my patchwork queue still, which suggests that the patches were not set to 'applied'?
I am not using patchwork. But I expect my reply to cover letter was recorded there.
Probably. If you reply to each patch, it shows up in the patch, but the cover letter is hidden somewhere else.
I have never started to like patchwork. I installed that client long time ago, I also have account for quite a long time.
If you are not using patchwork, how come you are a custodian? Is someone else dealing with patchwork for you?
Not really. I am just keep track on it via emails.
DT folks did wire CI loop on every patch which they get. I am not aware about any feature like this which would bring me something. That's why I am considering patchwork as unneeded layer. And I also don't think that I have read anywhere that all custodians should be using patchwork.
Right, patchwork isn't required, but can be helpful. Part of how patchwork is maintained for everyone (in U-Boot) is that I have a script that will update the status of patches to accepted and add the githash, based on the "patchwork hash" of a given commit. There's a number of automated tooling things that other projects use which could be helpful here, but due to lack of time/resources, we haven't tried them here.
Can you share that script? I am happy to run it and pretty much close my list. I am using b4 for applying patches that's why all message-ids are listed in the history which will uniquely identify that patches.
Thanks, Michal
On Mon, Sep 25, 2023 at 04:21:17PM +0200, Michal Simek wrote:
On 9/25/23 16:19, Tom Rini wrote:
On Mon, Sep 25, 2023 at 04:10:38PM +0200, Michal Simek wrote:
Hi Simon,
On 9/25/23 16:01, Simon Glass wrote:
Hi Michal,
On Mon, 25 Sept 2023 at 07:38, Michal Simek michal.simek@amd.com wrote:
On 9/25/23 15:10, Simon Glass wrote:
Hi Michal,
On Mon, 25 Sept 2023 at 00:06, Michal Simek michal.simek@amd.com wrote: > > Hi Simon, > > > On 9/23/23 20:13, Simon Glass wrote: > > Current alignment which is using 16 bytes is not correct in connection to > > trace_clocks description and it's length. > > That's why use start_addr variable and record proper size based on used > > entries. > > > > Fixes: be16fc81b2ed ("trace: Update proftool to use new binary format"). > > Signed-off-by: Michal Simek michal.simek@amd.com > > Reviewed-by: Simon Glass sjg@chromium.org > > --- > > > > Changes in v2: > > - s/start_addr/start_ofs/g' > > > > tools/proftool.c | 31 +++++++++++++++++++++++++++++-- > > 1 file changed, 29 insertions(+), 2 deletions(-) > > > > Applied to u-boot-dm, thanks! > > FYI: I have merged it to my tree and already sent pull request to Tom. > Without it I couldn't pass CI loop to get all reviewed features in. > > https://lore.kernel.org/all/ab72c480-e9f8-416e-adf5-726f7d40c4f5@amd.com/
Ah OK, well that's fine. It was in my patchwork queue still, which suggests that the patches were not set to 'applied'?
I am not using patchwork. But I expect my reply to cover letter was recorded there.
Probably. If you reply to each patch, it shows up in the patch, but the cover letter is hidden somewhere else.
I have never started to like patchwork. I installed that client long time ago, I also have account for quite a long time.
If you are not using patchwork, how come you are a custodian? Is someone else dealing with patchwork for you?
Not really. I am just keep track on it via emails.
DT folks did wire CI loop on every patch which they get. I am not aware about any feature like this which would bring me something. That's why I am considering patchwork as unneeded layer. And I also don't think that I have read anywhere that all custodians should be using patchwork.
Right, patchwork isn't required, but can be helpful. Part of how patchwork is maintained for everyone (in U-Boot) is that I have a script that will update the status of patches to accepted and add the githash, based on the "patchwork hash" of a given commit. There's a number of automated tooling things that other projects use which could be helpful here, but due to lack of time/resources, we haven't tried them here.
Can you share that script? I am happy to run it and pretty much close my list. I am using b4 for applying patches that's why all message-ids are listed in the history which will uniquely identify that patches.
If you like, yes, you can run the following. Note that when I run it myself between tags, it will still re-update things. This requires having patchwork cloned from git as well and is a slight modification of both tools/patchwork-update-commits and tools/post-receive.hook:
#!/bin/bash
# Patchwork - automated patch tracking system # Copyright (C) 2010 martin f. krafft madduck@madduck.net # # SPDX-License-Identifier: GPL-2.0-or-later
# Git post-receive hook to update Patchwork patches after Git pushes set -u
PW_DIR=/home/trini/work/u-boot/patchwork/patchwork
#TODO: the state map should really live in the repo's git-config STATE_MAP=".git/refs/heads/master:Accepted"
# ignore all commits already present in these refs # e.g., # EXCLUDE="refs/heads/upstream refs/heads/other-project" EXCLUDE=""
do_exit=0 trap "do_exit=1" INT
get_patchwork_hash() { local hash hash=$(git diff "$1~..$1" | python3 $PW_DIR/hasher.py) echo "$hash" test -n "$hash" }
get_patchwork_hash_harder() { local hash hash=$(git diff "$1~..$1" | sed -e 's/^ $//g' | python3 $PW_DIR/hasher.py) echo "$hash" test -n "$hash" }
get_patch_id() { local id id=$(curl -s "http://patchwork.ozlabs.org/api/patches/?project=uboot&hash=$1" | \ jq '.[-1] | .id') echo "$id" }
set_patch_state() { pwclient update -s "$2" -c "$3" "$1" 2>&1 }
update_patches() { local cnt; cnt=0 for rev in $(git rev-parse --not ${EXCLUDE} | git rev-list --stdin --no-merges --reverse "${1}".."${2}"); do if [ "$do_exit" = 1 ]; then echo "I: exiting..." >&2 break fi hash=$(get_patchwork_hash "$rev") if [ -z "$hash" ]; then echo "E: failed to hash rev $rev." >&2 continue fi id=$(get_patch_id "$hash") if [ "$id" = "null" ]; then hash=$(get_patchwork_hash_harder "$rev") id=$(get_patch_id "$hash") fi if [ "$id" = "null" ]; then echo "E: failed to find patch for rev $rev." >&2 continue fi reason="$(set_patch_state "$id" "$3" "$rev")" if [ -n "$reason" ]; then echo "E: failed to update patch #$id${reason:+: $reason}." >&2 continue fi echo "I: patch #$id updated using rev $rev." >&2 cnt=$((cnt + 1)) done
echo "I: $cnt patch(es) updated to state $3." >&2 }
oldrev=$1 newrev=$2 refname=".git/refs/heads/master" found=0 for i in $STATE_MAP; do key="${i%:*}" if [ "$key" = "$refname" ]; then update_patches "$oldrev" "$newrev" ${i#*:} found=1 break fi done if [ $found -eq 0 ]; then echo "E: STATE_MAP has no mapping for branch $refname" >&2 fi
Hi Tom,
On 9/25/23 16:33, Tom Rini wrote:
On Mon, Sep 25, 2023 at 04:21:17PM +0200, Michal Simek wrote:
On 9/25/23 16:19, Tom Rini wrote:
On Mon, Sep 25, 2023 at 04:10:38PM +0200, Michal Simek wrote:
Hi Simon,
On 9/25/23 16:01, Simon Glass wrote:
Hi Michal,
On Mon, 25 Sept 2023 at 07:38, Michal Simek michal.simek@amd.com wrote:
On 9/25/23 15:10, Simon Glass wrote: > Hi Michal, > > On Mon, 25 Sept 2023 at 00:06, Michal Simek michal.simek@amd.com wrote: >> >> Hi Simon, >> >> >> On 9/23/23 20:13, Simon Glass wrote: >>> Current alignment which is using 16 bytes is not correct in connection to >>> trace_clocks description and it's length. >>> That's why use start_addr variable and record proper size based on used >>> entries. >>> >>> Fixes: be16fc81b2ed ("trace: Update proftool to use new binary format"). >>> Signed-off-by: Michal Simek michal.simek@amd.com >>> Reviewed-by: Simon Glass sjg@chromium.org >>> --- >>> >>> Changes in v2: >>> - s/start_addr/start_ofs/g' >>> >>> tools/proftool.c | 31 +++++++++++++++++++++++++++++-- >>> 1 file changed, 29 insertions(+), 2 deletions(-) >>> >>> Applied to u-boot-dm, thanks! >> >> FYI: I have merged it to my tree and already sent pull request to Tom. >> Without it I couldn't pass CI loop to get all reviewed features in. >> >> https://lore.kernel.org/all/ab72c480-e9f8-416e-adf5-726f7d40c4f5@amd.com/ > > Ah OK, well that's fine. It was in my patchwork queue still, which > suggests that the patches were not set to 'applied'?
I am not using patchwork. But I expect my reply to cover letter was recorded there.
Probably. If you reply to each patch, it shows up in the patch, but the cover letter is hidden somewhere else.
I have never started to like patchwork. I installed that client long time ago, I also have account for quite a long time.
If you are not using patchwork, how come you are a custodian? Is someone else dealing with patchwork for you?
Not really. I am just keep track on it via emails.
DT folks did wire CI loop on every patch which they get. I am not aware about any feature like this which would bring me something. That's why I am considering patchwork as unneeded layer. And I also don't think that I have read anywhere that all custodians should be using patchwork.
Right, patchwork isn't required, but can be helpful. Part of how patchwork is maintained for everyone (in U-Boot) is that I have a script that will update the status of patches to accepted and add the githash, based on the "patchwork hash" of a given commit. There's a number of automated tooling things that other projects use which could be helpful here, but due to lack of time/resources, we haven't tried them here.
Can you share that script? I am happy to run it and pretty much close my list. I am using b4 for applying patches that's why all message-ids are listed in the history which will uniquely identify that patches.
If you like, yes, you can run the following. Note that when I run it myself between tags, it will still re-update things. This requires having patchwork cloned from git as well and is a slight modification of both tools/patchwork-update-commits and tools/post-receive.hook:
#!/bin/bash
# Patchwork - automated patch tracking system # Copyright (C) 2010 martin f. krafft madduck@madduck.net # # SPDX-License-Identifier: GPL-2.0-or-later
# Git post-receive hook to update Patchwork patches after Git pushes set -u
PW_DIR=/home/trini/work/u-boot/patchwork/patchwork
#TODO: the state map should really live in the repo's git-config STATE_MAP=".git/refs/heads/master:Accepted"
# ignore all commits already present in these refs # e.g., # EXCLUDE="refs/heads/upstream refs/heads/other-project" EXCLUDE=""
do_exit=0 trap "do_exit=1" INT
get_patchwork_hash() { local hash hash=$(git diff "$1~..$1" | python3 $PW_DIR/hasher.py) echo "$hash" test -n "$hash" }
get_patchwork_hash_harder() { local hash hash=$(git diff "$1~..$1" | sed -e 's/^ $//g' | python3 $PW_DIR/hasher.py) echo "$hash" test -n "$hash" }
get_patch_id() { local id id=$(curl -s "http://patchwork.ozlabs.org/api/patches/?project=uboot&hash=$1" | \ jq '.[-1] | .id') echo "$id" }
set_patch_state() { pwclient update -s "$2" -c "$3" "$1" 2>&1 }
update_patches() { local cnt; cnt=0 for rev in $(git rev-parse --not ${EXCLUDE} | git rev-list --stdin --no-merges --reverse "${1}".."${2}"); do if [ "$do_exit" = 1 ]; then echo "I: exiting..." >&2 break fi hash=$(get_patchwork_hash "$rev") if [ -z "$hash" ]; then echo "E: failed to hash rev $rev." >&2 continue fi id=$(get_patch_id "$hash") if [ "$id" = "null" ]; then hash=$(get_patchwork_hash_harder "$rev") id=$(get_patch_id "$hash") fi if [ "$id" = "null" ]; then echo "E: failed to find patch for rev $rev." >&2 continue fi reason="$(set_patch_state "$id" "$3" "$rev")" if [ -n "$reason" ]; then echo "E: failed to update patch #$id${reason:+: $reason}." >&2 continue fi echo "I: patch #$id updated using rev $rev." >&2 cnt=$((cnt + 1)) done
echo "I: $cnt patch(es) updated to state $3." >&2}
oldrev=$1 newrev=$2 refname=".git/refs/heads/master" found=0 for i in $STATE_MAP; do key="${i%:*}" if [ "$key" = "$refname" ]; then update_patches "$oldrev" "$newrev" ${i#*:} found=1 break fi done if [ $found -eq 0 ]; then echo "E: STATE_MAP has no mapping for branch $refname" >&2 fi
Sorry for delay on this. I played with your script and also look at git-pw client and cleaned my queue. Pretty much incorrect series, rfcs, etc should be only patches listed.
If you are running it between tags there is no need for me to run it.
Thanks, Michal
On Tue, Oct 24, 2023 at 02:33:26PM +0200, Michal Simek wrote:
Hi Tom,
On 9/25/23 16:33, Tom Rini wrote:
On Mon, Sep 25, 2023 at 04:21:17PM +0200, Michal Simek wrote:
On 9/25/23 16:19, Tom Rini wrote:
On Mon, Sep 25, 2023 at 04:10:38PM +0200, Michal Simek wrote:
Hi Simon,
On 9/25/23 16:01, Simon Glass wrote:
Hi Michal,
On Mon, 25 Sept 2023 at 07:38, Michal Simek michal.simek@amd.com wrote: > > > > On 9/25/23 15:10, Simon Glass wrote: > > Hi Michal, > > > > On Mon, 25 Sept 2023 at 00:06, Michal Simek michal.simek@amd.com wrote: > > > > > > Hi Simon, > > > > > > > > > On 9/23/23 20:13, Simon Glass wrote: > > > > Current alignment which is using 16 bytes is not correct in connection to > > > > trace_clocks description and it's length. > > > > That's why use start_addr variable and record proper size based on used > > > > entries. > > > > > > > > Fixes: be16fc81b2ed ("trace: Update proftool to use new binary format"). > > > > Signed-off-by: Michal Simek michal.simek@amd.com > > > > Reviewed-by: Simon Glass sjg@chromium.org > > > > --- > > > > > > > > Changes in v2: > > > > - s/start_addr/start_ofs/g' > > > > > > > > tools/proftool.c | 31 +++++++++++++++++++++++++++++-- > > > > 1 file changed, 29 insertions(+), 2 deletions(-) > > > > > > > > Applied to u-boot-dm, thanks! > > > > > > FYI: I have merged it to my tree and already sent pull request to Tom. > > > Without it I couldn't pass CI loop to get all reviewed features in. > > > > > > https://lore.kernel.org/all/ab72c480-e9f8-416e-adf5-726f7d40c4f5@amd.com/ > > > > Ah OK, well that's fine. It was in my patchwork queue still, which > > suggests that the patches were not set to 'applied'? > > I am not using patchwork. But I expect my reply to cover letter was recorded there.
Probably. If you reply to each patch, it shows up in the patch, but the cover letter is hidden somewhere else.
I have never started to like patchwork. I installed that client long time ago, I also have account for quite a long time.
If you are not using patchwork, how come you are a custodian? Is someone else dealing with patchwork for you?
Not really. I am just keep track on it via emails.
DT folks did wire CI loop on every patch which they get. I am not aware about any feature like this which would bring me something. That's why I am considering patchwork as unneeded layer. And I also don't think that I have read anywhere that all custodians should be using patchwork.
Right, patchwork isn't required, but can be helpful. Part of how patchwork is maintained for everyone (in U-Boot) is that I have a script that will update the status of patches to accepted and add the githash, based on the "patchwork hash" of a given commit. There's a number of automated tooling things that other projects use which could be helpful here, but due to lack of time/resources, we haven't tried them here.
Can you share that script? I am happy to run it and pretty much close my list. I am using b4 for applying patches that's why all message-ids are listed in the history which will uniquely identify that patches.
If you like, yes, you can run the following. Note that when I run it myself between tags, it will still re-update things. This requires having patchwork cloned from git as well and is a slight modification of both tools/patchwork-update-commits and tools/post-receive.hook:
#!/bin/bash
# Patchwork - automated patch tracking system # Copyright (C) 2010 martin f. krafft madduck@madduck.net # # SPDX-License-Identifier: GPL-2.0-or-later
# Git post-receive hook to update Patchwork patches after Git pushes set -u
PW_DIR=/home/trini/work/u-boot/patchwork/patchwork
#TODO: the state map should really live in the repo's git-config STATE_MAP=".git/refs/heads/master:Accepted"
# ignore all commits already present in these refs # e.g., # EXCLUDE="refs/heads/upstream refs/heads/other-project" EXCLUDE=""
do_exit=0 trap "do_exit=1" INT
get_patchwork_hash() { local hash hash=$(git diff "$1~..$1" | python3 $PW_DIR/hasher.py) echo "$hash" test -n "$hash" }
get_patchwork_hash_harder() { local hash hash=$(git diff "$1~..$1" | sed -e 's/^ $//g' | python3 $PW_DIR/hasher.py) echo "$hash" test -n "$hash" }
get_patch_id() { local id id=$(curl -s "http://patchwork.ozlabs.org/api/patches/?project=uboot&hash=$1" | \ jq '.[-1] | .id') echo "$id" }
set_patch_state() { pwclient update -s "$2" -c "$3" "$1" 2>&1 }
update_patches() { local cnt; cnt=0 for rev in $(git rev-parse --not ${EXCLUDE} | git rev-list --stdin --no-merges --reverse "${1}".."${2}"); do if [ "$do_exit" = 1 ]; then echo "I: exiting..." >&2 break fi hash=$(get_patchwork_hash "$rev") if [ -z "$hash" ]; then echo "E: failed to hash rev $rev." >&2 continue fi id=$(get_patch_id "$hash") if [ "$id" = "null" ]; then hash=$(get_patchwork_hash_harder "$rev") id=$(get_patch_id "$hash") fi if [ "$id" = "null" ]; then echo "E: failed to find patch for rev $rev." >&2 continue fi reason="$(set_patch_state "$id" "$3" "$rev")" if [ -n "$reason" ]; then echo "E: failed to update patch #$id${reason:+: $reason}." >&2 continue fi echo "I: patch #$id updated using rev $rev." >&2 cnt=$((cnt + 1)) done
echo "I: $cnt patch(es) updated to state $3." >&2}
oldrev=$1 newrev=$2 refname=".git/refs/heads/master" found=0 for i in $STATE_MAP; do key="${i%:*}" if [ "$key" = "$refname" ]; then update_patches "$oldrev" "$newrev" ${i#*:} found=1 break fi done if [ $found -eq 0 ]; then echo "E: STATE_MAP has no mapping for branch $refname" >&2 fi
Sorry for delay on this. I played with your script and also look at git-pw client and cleaned my queue. Pretty much incorrect series, rfcs, etc should be only patches listed.
If you are running it between tags there is no need for me to run it.
I do this after each tag so yes, you don't have to run it as well if it's not helpful to you (it may be helpful to custodians that do use patchwork, perhaps with the upstream commit they're basing their PR on and then their own tag for me, to then clean up their queue?).
On 10/24/23 20:03, Tom Rini wrote:
On Tue, Oct 24, 2023 at 02:33:26PM +0200, Michal Simek wrote:
Hi Tom,
On 9/25/23 16:33, Tom Rini wrote:
On Mon, Sep 25, 2023 at 04:21:17PM +0200, Michal Simek wrote:
On 9/25/23 16:19, Tom Rini wrote:
On Mon, Sep 25, 2023 at 04:10:38PM +0200, Michal Simek wrote:
Hi Simon,
On 9/25/23 16:01, Simon Glass wrote: > Hi Michal, > > On Mon, 25 Sept 2023 at 07:38, Michal Simek michal.simek@amd.com wrote: >> >> >> >> On 9/25/23 15:10, Simon Glass wrote: >>> Hi Michal, >>> >>> On Mon, 25 Sept 2023 at 00:06, Michal Simek michal.simek@amd.com wrote: >>>> >>>> Hi Simon, >>>> >>>> >>>> On 9/23/23 20:13, Simon Glass wrote: >>>>> Current alignment which is using 16 bytes is not correct in connection to >>>>> trace_clocks description and it's length. >>>>> That's why use start_addr variable and record proper size based on used >>>>> entries. >>>>> >>>>> Fixes: be16fc81b2ed ("trace: Update proftool to use new binary format"). >>>>> Signed-off-by: Michal Simek michal.simek@amd.com >>>>> Reviewed-by: Simon Glass sjg@chromium.org >>>>> --- >>>>> >>>>> Changes in v2: >>>>> - s/start_addr/start_ofs/g' >>>>> >>>>> tools/proftool.c | 31 +++++++++++++++++++++++++++++-- >>>>> 1 file changed, 29 insertions(+), 2 deletions(-) >>>>> >>>>> Applied to u-boot-dm, thanks! >>>> >>>> FYI: I have merged it to my tree and already sent pull request to Tom. >>>> Without it I couldn't pass CI loop to get all reviewed features in. >>>> >>>> https://lore.kernel.org/all/ab72c480-e9f8-416e-adf5-726f7d40c4f5@amd.com/ >>> >>> Ah OK, well that's fine. It was in my patchwork queue still, which >>> suggests that the patches were not set to 'applied'? >> >> I am not using patchwork. But I expect my reply to cover letter was recorded there. > > Probably. If you reply to each patch, it shows up in the patch, but > the cover letter is hidden somewhere else.
I have never started to like patchwork. I installed that client long time ago, I also have account for quite a long time.
> If you are not using patchwork, how come you are a custodian? Is > someone else dealing with patchwork for you?
Not really. I am just keep track on it via emails.
DT folks did wire CI loop on every patch which they get. I am not aware about any feature like this which would bring me something. That's why I am considering patchwork as unneeded layer. And I also don't think that I have read anywhere that all custodians should be using patchwork.
Right, patchwork isn't required, but can be helpful. Part of how patchwork is maintained for everyone (in U-Boot) is that I have a script that will update the status of patches to accepted and add the githash, based on the "patchwork hash" of a given commit. There's a number of automated tooling things that other projects use which could be helpful here, but due to lack of time/resources, we haven't tried them here.
Can you share that script? I am happy to run it and pretty much close my list. I am using b4 for applying patches that's why all message-ids are listed in the history which will uniquely identify that patches.
If you like, yes, you can run the following. Note that when I run it myself between tags, it will still re-update things. This requires having patchwork cloned from git as well and is a slight modification of both tools/patchwork-update-commits and tools/post-receive.hook:
#!/bin/bash
# Patchwork - automated patch tracking system # Copyright (C) 2010 martin f. krafft madduck@madduck.net # # SPDX-License-Identifier: GPL-2.0-or-later
# Git post-receive hook to update Patchwork patches after Git pushes set -u
PW_DIR=/home/trini/work/u-boot/patchwork/patchwork
#TODO: the state map should really live in the repo's git-config STATE_MAP=".git/refs/heads/master:Accepted"
# ignore all commits already present in these refs # e.g., # EXCLUDE="refs/heads/upstream refs/heads/other-project" EXCLUDE=""
do_exit=0 trap "do_exit=1" INT
get_patchwork_hash() { local hash hash=$(git diff "$1~..$1" | python3 $PW_DIR/hasher.py) echo "$hash" test -n "$hash" }
get_patchwork_hash_harder() { local hash hash=$(git diff "$1~..$1" | sed -e 's/^ $//g' | python3 $PW_DIR/hasher.py) echo "$hash" test -n "$hash" }
get_patch_id() { local id id=$(curl -s "http://patchwork.ozlabs.org/api/patches/?project=uboot&hash=$1" | \ jq '.[-1] | .id') echo "$id" }
set_patch_state() { pwclient update -s "$2" -c "$3" "$1" 2>&1 }
update_patches() { local cnt; cnt=0 for rev in $(git rev-parse --not ${EXCLUDE} | git rev-list --stdin --no-merges --reverse "${1}".."${2}"); do if [ "$do_exit" = 1 ]; then echo "I: exiting..." >&2 break fi hash=$(get_patchwork_hash "$rev") if [ -z "$hash" ]; then echo "E: failed to hash rev $rev." >&2 continue fi id=$(get_patch_id "$hash") if [ "$id" = "null" ]; then hash=$(get_patchwork_hash_harder "$rev") id=$(get_patch_id "$hash") fi if [ "$id" = "null" ]; then echo "E: failed to find patch for rev $rev." >&2 continue fi reason="$(set_patch_state "$id" "$3" "$rev")" if [ -n "$reason" ]; then echo "E: failed to update patch #$id${reason:+: $reason}." >&2 continue fi echo "I: patch #$id updated using rev $rev." >&2 cnt=$((cnt + 1)) done
echo "I: $cnt patch(es) updated to state $3." >&2}
oldrev=$1 newrev=$2 refname=".git/refs/heads/master" found=0 for i in $STATE_MAP; do key="${i%:*}" if [ "$key" = "$refname" ]; then update_patches "$oldrev" "$newrev" ${i#*:} found=1 break fi done if [ $found -eq 0 ]; then echo "E: STATE_MAP has no mapping for branch $refname" >&2 fi
Sorry for delay on this. I played with your script and also look at git-pw client and cleaned my queue. Pretty much incorrect series, rfcs, etc should be only patches listed.
If you are running it between tags there is no need for me to run it.
I do this after each tag so yes, you don't have to run it as well if it's not helpful to you (it may be helpful to custodians that do use patchwork, perhaps with the upstream commit they're basing their PR on and then their own tag for me, to then clean up their queue?).
Actually I have updated that git filter above just to list sha1 from git for me as committer and that could serve purpose for custodians for cleaning up the queue. Would be maybe worth to commit this script to u-boot repository that other custodians can start to use it.
Thanks, Michal
On Wed, Oct 25, 2023 at 09:33:03AM +0200, Michal Simek wrote:
On 10/24/23 20:03, Tom Rini wrote:
On Tue, Oct 24, 2023 at 02:33:26PM +0200, Michal Simek wrote:
Hi Tom,
On 9/25/23 16:33, Tom Rini wrote:
On Mon, Sep 25, 2023 at 04:21:17PM +0200, Michal Simek wrote:
On 9/25/23 16:19, Tom Rini wrote:
On Mon, Sep 25, 2023 at 04:10:38PM +0200, Michal Simek wrote: > Hi Simon, > > On 9/25/23 16:01, Simon Glass wrote: > > Hi Michal, > > > > On Mon, 25 Sept 2023 at 07:38, Michal Simek michal.simek@amd.com wrote: > > > > > > > > > > > > On 9/25/23 15:10, Simon Glass wrote: > > > > Hi Michal, > > > > > > > > On Mon, 25 Sept 2023 at 00:06, Michal Simek michal.simek@amd.com wrote: > > > > > > > > > > Hi Simon, > > > > > > > > > > > > > > > On 9/23/23 20:13, Simon Glass wrote: > > > > > > Current alignment which is using 16 bytes is not correct in connection to > > > > > > trace_clocks description and it's length. > > > > > > That's why use start_addr variable and record proper size based on used > > > > > > entries. > > > > > > > > > > > > Fixes: be16fc81b2ed ("trace: Update proftool to use new binary format"). > > > > > > Signed-off-by: Michal Simek michal.simek@amd.com > > > > > > Reviewed-by: Simon Glass sjg@chromium.org > > > > > > --- > > > > > > > > > > > > Changes in v2: > > > > > > - s/start_addr/start_ofs/g' > > > > > > > > > > > > tools/proftool.c | 31 +++++++++++++++++++++++++++++-- > > > > > > 1 file changed, 29 insertions(+), 2 deletions(-) > > > > > > > > > > > > Applied to u-boot-dm, thanks! > > > > > > > > > > FYI: I have merged it to my tree and already sent pull request to Tom. > > > > > Without it I couldn't pass CI loop to get all reviewed features in. > > > > > > > > > > https://lore.kernel.org/all/ab72c480-e9f8-416e-adf5-726f7d40c4f5@amd.com/ > > > > > > > > Ah OK, well that's fine. It was in my patchwork queue still, which > > > > suggests that the patches were not set to 'applied'? > > > > > > I am not using patchwork. But I expect my reply to cover letter was recorded there. > > > > Probably. If you reply to each patch, it shows up in the patch, but > > the cover letter is hidden somewhere else. > > I have never started to like patchwork. I installed that client long time > ago, I also have account for quite a long time. > > > If you are not using patchwork, how come you are a custodian? Is > > someone else dealing with patchwork for you? > > Not really. I am just keep track on it via emails. > > DT folks did wire CI loop on every patch which they get. I am not aware > about any feature like this which would bring me something. That's why I am > considering patchwork as unneeded layer. And I also don't think that I have > read anywhere that all custodians should be using patchwork.
Right, patchwork isn't required, but can be helpful. Part of how patchwork is maintained for everyone (in U-Boot) is that I have a script that will update the status of patches to accepted and add the githash, based on the "patchwork hash" of a given commit. There's a number of automated tooling things that other projects use which could be helpful here, but due to lack of time/resources, we haven't tried them here.
Can you share that script? I am happy to run it and pretty much close my list. I am using b4 for applying patches that's why all message-ids are listed in the history which will uniquely identify that patches.
If you like, yes, you can run the following. Note that when I run it myself between tags, it will still re-update things. This requires having patchwork cloned from git as well and is a slight modification of both tools/patchwork-update-commits and tools/post-receive.hook:
#!/bin/bash
# Patchwork - automated patch tracking system # Copyright (C) 2010 martin f. krafft madduck@madduck.net # # SPDX-License-Identifier: GPL-2.0-or-later
# Git post-receive hook to update Patchwork patches after Git pushes set -u
PW_DIR=/home/trini/work/u-boot/patchwork/patchwork
#TODO: the state map should really live in the repo's git-config STATE_MAP=".git/refs/heads/master:Accepted"
# ignore all commits already present in these refs # e.g., # EXCLUDE="refs/heads/upstream refs/heads/other-project" EXCLUDE=""
do_exit=0 trap "do_exit=1" INT
get_patchwork_hash() { local hash hash=$(git diff "$1~..$1" | python3 $PW_DIR/hasher.py) echo "$hash" test -n "$hash" }
get_patchwork_hash_harder() { local hash hash=$(git diff "$1~..$1" | sed -e 's/^ $//g' | python3 $PW_DIR/hasher.py) echo "$hash" test -n "$hash" }
get_patch_id() { local id id=$(curl -s "http://patchwork.ozlabs.org/api/patches/?project=uboot&hash=$1" | \ jq '.[-1] | .id') echo "$id" }
set_patch_state() { pwclient update -s "$2" -c "$3" "$1" 2>&1 }
update_patches() { local cnt; cnt=0 for rev in $(git rev-parse --not ${EXCLUDE} | git rev-list --stdin --no-merges --reverse "${1}".."${2}"); do if [ "$do_exit" = 1 ]; then echo "I: exiting..." >&2 break fi hash=$(get_patchwork_hash "$rev") if [ -z "$hash" ]; then echo "E: failed to hash rev $rev." >&2 continue fi id=$(get_patch_id "$hash") if [ "$id" = "null" ]; then hash=$(get_patchwork_hash_harder "$rev") id=$(get_patch_id "$hash") fi if [ "$id" = "null" ]; then echo "E: failed to find patch for rev $rev." >&2 continue fi reason="$(set_patch_state "$id" "$3" "$rev")" if [ -n "$reason" ]; then echo "E: failed to update patch #$id${reason:+: $reason}." >&2 continue fi echo "I: patch #$id updated using rev $rev." >&2 cnt=$((cnt + 1)) done
echo "I: $cnt patch(es) updated to state $3." >&2}
oldrev=$1 newrev=$2 refname=".git/refs/heads/master" found=0 for i in $STATE_MAP; do key="${i%:*}" if [ "$key" = "$refname" ]; then update_patches "$oldrev" "$newrev" ${i#*:} found=1 break fi done if [ $found -eq 0 ]; then echo "E: STATE_MAP has no mapping for branch $refname" >&2 fi
Sorry for delay on this. I played with your script and also look at git-pw client and cleaned my queue. Pretty much incorrect series, rfcs, etc should be only patches listed.
If you are running it between tags there is no need for me to run it.
I do this after each tag so yes, you don't have to run it as well if it's not helpful to you (it may be helpful to custodians that do use patchwork, perhaps with the upstream commit they're basing their PR on and then their own tag for me, to then clean up their queue?).
Actually I have updated that git filter above just to list sha1 from git for me as committer and that could serve purpose for custodians for cleaning up the queue. Would be maybe worth to commit this script to u-boot repository that other custodians can start to use it.
It's just some small tweaks to the upstream git hook. I should write doc/develop/patchwork.rst and mention that hook, and some work flows, as the most generic answer here. And I'm also not super proud of that set of hacks to an upstream script :)
On Mon, Sep 25, 2023 at 08:01:41AM -0600, Simon Glass wrote:
Hi Michal,
On Mon, 25 Sept 2023 at 07:38, Michal Simek michal.simek@amd.com wrote:
On 9/25/23 15:10, Simon Glass wrote:
Hi Michal,
On Mon, 25 Sept 2023 at 00:06, Michal Simek michal.simek@amd.com wrote:
Hi Simon,
On 9/23/23 20:13, Simon Glass wrote:
Current alignment which is using 16 bytes is not correct in connection to trace_clocks description and it's length. That's why use start_addr variable and record proper size based on used entries.
Fixes: be16fc81b2ed ("trace: Update proftool to use new binary format"). Signed-off-by: Michal Simek michal.simek@amd.com Reviewed-by: Simon Glass sjg@chromium.org
Changes in v2:
s/start_addr/start_ofs/g'
tools/proftool.c | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-)
Applied to u-boot-dm, thanks!
FYI: I have merged it to my tree and already sent pull request to Tom. Without it I couldn't pass CI loop to get all reviewed features in.
https://lore.kernel.org/all/ab72c480-e9f8-416e-adf5-726f7d40c4f5@amd.com/
Ah OK, well that's fine. It was in my patchwork queue still, which suggests that the patches were not set to 'applied'?
I am not using patchwork. But I expect my reply to cover letter was recorded there.
Probably. If you reply to each patch, it shows up in the patch, but the cover letter is hidden somewhere else.
Patchwork doesn't, but b4 does, handle tags sent to the cover letter rather than individually. Patchwork does have a link to the cover letter, on individual patch links under the "expand" link on the "Series" line.
On 9/25/23 16:28, Tom Rini wrote:
On Mon, Sep 25, 2023 at 08:01:41AM -0600, Simon Glass wrote:
Hi Michal,
On Mon, 25 Sept 2023 at 07:38, Michal Simek michal.simek@amd.com wrote:
On 9/25/23 15:10, Simon Glass wrote:
Hi Michal,
On Mon, 25 Sept 2023 at 00:06, Michal Simek michal.simek@amd.com wrote:
Hi Simon,
On 9/23/23 20:13, Simon Glass wrote:
Current alignment which is using 16 bytes is not correct in connection to trace_clocks description and it's length. That's why use start_addr variable and record proper size based on used entries.
Fixes: be16fc81b2ed ("trace: Update proftool to use new binary format"). Signed-off-by: Michal Simek michal.simek@amd.com Reviewed-by: Simon Glass sjg@chromium.org
Changes in v2:
s/start_addr/start_ofs/g'
tools/proftool.c | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-)
Applied to u-boot-dm, thanks!
FYI: I have merged it to my tree and already sent pull request to Tom. Without it I couldn't pass CI loop to get all reviewed features in.
https://lore.kernel.org/all/ab72c480-e9f8-416e-adf5-726f7d40c4f5@amd.com/
Ah OK, well that's fine. It was in my patchwork queue still, which suggests that the patches were not set to 'applied'?
I am not using patchwork. But I expect my reply to cover letter was recorded there.
Probably. If you reply to each patch, it shows up in the patch, but the cover letter is hidden somewhere else.
Patchwork doesn't, but b4 does, handle tags sent to the cover letter rather than individually.
For tags yes it works nicely with b4. Pretty much just reply to cover letter and b4 copy it to all patches. But this was more about that you can't see my Applied reply there. I don't think that b4 will be able to catch it but I would love to be wrong.
M
On 9/15/23 14:12, Michal Simek wrote:
Hi,
sandbox is getting bigger and bigger and I have reached the case that adding some more functions ends up in CI loop failure. After some investigation I found that flyrecord header have incorrect information about data offset which is caused by incorrect alignment calculation. That's why this series is fixing alignment calculation. I have done it via 3 patches where the first two are just preparing code for actual fixup.
Thanks, Michal
Changes in v2:
- s/start_addr/start_ofs/g'
Michal Simek (3): trace: Use 64bit variable for start and len trace: Move trace_clocks description above record offset calculation trace: Fix alignment logic in flyrecord header
tools/proftool.c | 39 ++++++++++++++++++++++++++++++++++----- 1 file changed, 34 insertions(+), 5 deletions(-)
Applied. M
participants (3)
-
Michal Simek -
Simon Glass -
Tom Rini