From: Andrew Jones <andrew.jones@linux.dev>
To: kvm-riscv@lists.infradead.org
Subject: [kvm-unit-tests PATCH v2 1/4] lib/report: Add helper methods to clear multiple prefixes
Date: Tue, 27 Aug 2024 18:49:56 +0200 [thread overview]
Message-ID: <20240827-bd7768e88d0b307a8923cfb6@orel> (raw)
In-Reply-To: <20240825170824.107467-2-jamestiotio@gmail.com>
On Mon, Aug 26, 2024 at 01:08:21AM GMT, James Raphael Tiovalen wrote:
> Add a method to pop a specified number of prefixes and another method to
> clear all prefixes.
>
> Suggested-by: Andrew Jones <andrew.jones@linux.dev>
> Signed-off-by: James Raphael Tiovalen <jamestiotio@gmail.com>
> ---
> lib/libcflat.h | 2 ++
> lib/report.c | 13 +++++++++++++
> 2 files changed, 15 insertions(+)
>
> diff --git a/lib/libcflat.h b/lib/libcflat.h
> index 16a83880..0286ddec 100644
> --- a/lib/libcflat.h
> +++ b/lib/libcflat.h
> @@ -96,6 +96,8 @@ void report_prefix_pushf(const char *prefix_fmt, ...)
> __attribute__((format(printf, 1, 2)));
> extern void report_prefix_push(const char *prefix);
> extern void report_prefix_pop(void);
> +extern void report_prefix_popn(int n);
> +extern void report_prefix_clear(void);
> extern void report(bool pass, const char *msg_fmt, ...)
> __attribute__((format(printf, 2, 3), nonnull(2)));
> extern void report_xfail(bool xfail, bool pass, const char *msg_fmt, ...)
> diff --git a/lib/report.c b/lib/report.c
> index 7f3c4f05..d45afedc 100644
> --- a/lib/report.c
> +++ b/lib/report.c
> @@ -80,6 +80,19 @@ void report_prefix_pop(void)
> spin_unlock(&lock);
> }
>
> +void report_prefix_popn(int n)
> +{
> + while (n--)
> + report_prefix_pop();
I think I suggested this implementation, but thinking about it some more
this won't work well with other cpus pushing/popping simultaneously. We
need something like
static void __report_prefix_pop(void)
{
char *p, *q;
if (!*prefixes)
return;
for (p = prefixes, q = strstr(p, PREFIX_DELIMITER) + 2;
*q;
p = q, q = strstr(p, PREFIX_DELIMITER) + 2)
;
*p = '\0';
}
void report_prefix_pop(void)
{
spin_lock(&lock);
__report_prefix_pop();
spin_unlock(&lock);
}
void report_prefix_popn(int n)
{
spin_lock(&lock);
while (n--)
__report_prefix_pop();
spin_unlock(&lock);
}
> +}
> +
> +void report_prefix_clear(void)
> +{
> + spin_lock(&lock);
> + prefixes[0] = '\0';
> + spin_unlock(&lock);
> +}
I'm also second guessing the utility of this one. We'd probably almost
never want to do this since most tests are designed with a
main()
{
report_prefix_push("mytest");
subtest1();
subtest2();
...
report_prefix_pop();
...
}
type pattern and we wouldn't want to lose that "mytest" prefix when some
subtest calls clear. Let's just drop report_prefix_clear() for now.
> +
> static void va_report(const char *msg_fmt,
> bool pass, bool xfail, bool kfail, bool skip, va_list va)
> {
> --
> 2.43.0
>
Thanks,
drew
WARNING: multiple messages have this Message-ID (diff)
From: Andrew Jones <andrew.jones@linux.dev>
To: James Raphael Tiovalen <jamestiotio@gmail.com>
Cc: kvm@vger.kernel.org, kvm-riscv@lists.infradead.org,
atishp@rivosinc.com, cade.richard@berkeley.edu
Subject: Re: [kvm-unit-tests PATCH v2 1/4] lib/report: Add helper methods to clear multiple prefixes
Date: Tue, 27 Aug 2024 18:49:56 +0200 [thread overview]
Message-ID: <20240827-bd7768e88d0b307a8923cfb6@orel> (raw)
In-Reply-To: <20240825170824.107467-2-jamestiotio@gmail.com>
On Mon, Aug 26, 2024 at 01:08:21AM GMT, James Raphael Tiovalen wrote:
> Add a method to pop a specified number of prefixes and another method to
> clear all prefixes.
>
> Suggested-by: Andrew Jones <andrew.jones@linux.dev>
> Signed-off-by: James Raphael Tiovalen <jamestiotio@gmail.com>
> ---
> lib/libcflat.h | 2 ++
> lib/report.c | 13 +++++++++++++
> 2 files changed, 15 insertions(+)
>
> diff --git a/lib/libcflat.h b/lib/libcflat.h
> index 16a83880..0286ddec 100644
> --- a/lib/libcflat.h
> +++ b/lib/libcflat.h
> @@ -96,6 +96,8 @@ void report_prefix_pushf(const char *prefix_fmt, ...)
> __attribute__((format(printf, 1, 2)));
> extern void report_prefix_push(const char *prefix);
> extern void report_prefix_pop(void);
> +extern void report_prefix_popn(int n);
> +extern void report_prefix_clear(void);
> extern void report(bool pass, const char *msg_fmt, ...)
> __attribute__((format(printf, 2, 3), nonnull(2)));
> extern void report_xfail(bool xfail, bool pass, const char *msg_fmt, ...)
> diff --git a/lib/report.c b/lib/report.c
> index 7f3c4f05..d45afedc 100644
> --- a/lib/report.c
> +++ b/lib/report.c
> @@ -80,6 +80,19 @@ void report_prefix_pop(void)
> spin_unlock(&lock);
> }
>
> +void report_prefix_popn(int n)
> +{
> + while (n--)
> + report_prefix_pop();
I think I suggested this implementation, but thinking about it some more
this won't work well with other cpus pushing/popping simultaneously. We
need something like
static void __report_prefix_pop(void)
{
char *p, *q;
if (!*prefixes)
return;
for (p = prefixes, q = strstr(p, PREFIX_DELIMITER) + 2;
*q;
p = q, q = strstr(p, PREFIX_DELIMITER) + 2)
;
*p = '\0';
}
void report_prefix_pop(void)
{
spin_lock(&lock);
__report_prefix_pop();
spin_unlock(&lock);
}
void report_prefix_popn(int n)
{
spin_lock(&lock);
while (n--)
__report_prefix_pop();
spin_unlock(&lock);
}
> +}
> +
> +void report_prefix_clear(void)
> +{
> + spin_lock(&lock);
> + prefixes[0] = '\0';
> + spin_unlock(&lock);
> +}
I'm also second guessing the utility of this one. We'd probably almost
never want to do this since most tests are designed with a
main()
{
report_prefix_push("mytest");
subtest1();
subtest2();
...
report_prefix_pop();
...
}
type pattern and we wouldn't want to lose that "mytest" prefix when some
subtest calls clear. Let's just drop report_prefix_clear() for now.
> +
> static void va_report(const char *msg_fmt,
> bool pass, bool xfail, bool kfail, bool skip, va_list va)
> {
> --
> 2.43.0
>
Thanks,
drew
next prev parent reply other threads:[~2024-08-27 16:49 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-08-25 17:08 [kvm-unit-tests PATCH v2 0/4] riscv: sbi: Add support to test HSM extension James Raphael Tiovalen
2024-08-25 17:08 ` James Raphael Tiovalen
2024-08-25 17:08 ` [kvm-unit-tests PATCH v2 1/4] lib/report: Add helper methods to clear multiple prefixes James Raphael Tiovalen
2024-08-25 17:08 ` James Raphael Tiovalen
2024-08-27 16:49 ` Andrew Jones [this message]
2024-08-27 16:49 ` Andrew Jones
2024-08-27 16:55 ` Andrew Jones
2024-08-27 16:55 ` Andrew Jones
2024-08-25 17:08 ` [kvm-unit-tests PATCH v2 2/4] riscv: sbi: Add IPI extension support James Raphael Tiovalen
2024-08-25 17:08 ` James Raphael Tiovalen
2024-08-27 16:53 ` Andrew Jones
2024-08-27 16:53 ` Andrew Jones
2024-08-25 17:08 ` [kvm-unit-tests PATCH v2 3/4] riscv: sbi: Add HSM extension functions James Raphael Tiovalen
2024-08-25 17:08 ` James Raphael Tiovalen
2024-08-29 11:13 ` Andrew Jones
2024-08-29 11:13 ` Andrew Jones
2024-08-25 17:08 ` [kvm-unit-tests PATCH v2 4/4] riscv: sbi: Add tests for HSM extension James Raphael Tiovalen
2024-08-25 17:08 ` James Raphael Tiovalen
2024-08-29 13:30 ` Andrew Jones
2024-08-29 13:30 ` Andrew Jones
2024-08-29 16:00 ` Andrew Jones
2024-08-29 16:00 ` Andrew Jones
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240827-bd7768e88d0b307a8923cfb6@orel \
--to=andrew.jones@linux.dev \
--cc=kvm-riscv@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.