diff mbox series

[2/2] selftests/user_events: Fix abi_test for BE archs

Message ID 20230925230829.341-3-beaub@linux.microsoft.com
State New
Headers
Series tracing/user_events: Fix alignment issues for 32 on 64-bit and BE |

Commit Message

Beau Belgrave Sept. 25, 2023, 11:08 p.m. UTC 
  The abi_test currently uses a long sized test value for enablement
checks. On LE this works fine, however, on BE this results in inaccurate
assert checks due to a bit being used and assuming it's value is the
same on both LE and BE.

Use int type for 32-bit values and long type for 64-bit values to ensure
appropriate behavior on both LE and BE.

Fixes: 60b1af8de8c1 ("tracing/user_events: Add ABI self-test")
Signed-off-by: Beau Belgrave <beaub@linux.microsoft.com>
---
 tools/testing/selftests/user_events/abi_test.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

Comments

Steven Rostedt Oct. 4, 2023, 12:59 a.m. UTC | #1 
Note, this doesn't seem to apply to my tree so I only added the first
patch. I think this needs to go through Shuah's tree.

-- Steve


On Mon, 25 Sep 2023 23:08:29 +0000
Beau Belgrave <beaub@linux.microsoft.com> wrote:

> The abi_test currently uses a long sized test value for enablement
> checks. On LE this works fine, however, on BE this results in inaccurate
> assert checks due to a bit being used and assuming it's value is the
> same on both LE and BE.
> 
> Use int type for 32-bit values and long type for 64-bit values to ensure
> appropriate behavior on both LE and BE.
> 
> Fixes: 60b1af8de8c1 ("tracing/user_events: Add ABI self-test")
> Signed-off-by: Beau Belgrave <beaub@linux.microsoft.com>
> ---
>  tools/testing/selftests/user_events/abi_test.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/testing/selftests/user_events/abi_test.c b/tools/testing/selftests/user_events/abi_test.c
> index 5125c42efe65..67af4c491c0c 100644
> --- a/tools/testing/selftests/user_events/abi_test.c
> +++ b/tools/testing/selftests/user_events/abi_test.c
> @@ -46,7 +46,7 @@ static int change_event(bool enable)
>  	return ret;
>  }
>  
> -static int reg_enable(long *enable, int size, int bit)
> +static int reg_enable(void *enable, int size, int bit)
>  {
>  	struct user_reg reg = {0};
>  	int fd = open(data_file, O_RDWR);
> @@ -68,7 +68,7 @@ static int reg_enable(long *enable, int size, int bit)
>  	return ret;
>  }
>  
> -static int reg_disable(long *enable, int bit)
> +static int reg_disable(void *enable, int bit)
>  {
>  	struct user_unreg reg = {0};
>  	int fd = open(data_file, O_RDWR);
> @@ -89,12 +89,14 @@ static int reg_disable(long *enable, int bit)
>  }
>  
>  FIXTURE(user) {
> -	long check;
> +	int check;
> +	long check_long;
>  };
>  
>  FIXTURE_SETUP(user) {
>  	change_event(false);
>  	self->check = 0;
> +	self->check_long = 0;
>  }
>  
>  FIXTURE_TEARDOWN(user) {
> @@ -131,9 +133,9 @@ TEST_F(user, bit_sizes) {
>  
>  #if BITS_PER_LONG == 8
>  	/* Allow 0-64 bits for 64-bit */
> -	ASSERT_EQ(0, reg_enable(&self->check, sizeof(long), 63));
> -	ASSERT_NE(0, reg_enable(&self->check, sizeof(long), 64));
> -	ASSERT_EQ(0, reg_disable(&self->check, 63));
> +	ASSERT_EQ(0, reg_enable(&self->check_long, sizeof(long), 63));
> +	ASSERT_NE(0, reg_enable(&self->check_long, sizeof(long), 64));
> +	ASSERT_EQ(0, reg_disable(&self->check_long, 63));
>  #endif
>  
>  	/* Disallowed sizes (everything beside 4 and 8) */
> @@ -195,7 +197,7 @@ static int clone_check(void *check)
>  	for (i = 0; i < 10; ++i) {
>  		usleep(100000);
>  
> -		if (*(long *)check)
> +		if (*(int *)check)
>  			return 0;
>  	}
>
Shuah Khan Oct. 4, 2023, 3:10 p.m. UTC | #2 
On 10/3/23 18:59, Steven Rostedt wrote:
> 
> Note, this doesn't seem to apply to my tree so I only added the first
> patch. I think this needs to go through Shuah's tree.
> 
> -- Steve
> 
> 

Yes. I sent a fix up for rc4 - I can pull these two patches into
linux-kselftest next

Steve! Does that work for you?

thanks,
-- Shuah
Steven Rostedt Oct. 4, 2023, 3:14 p.m. UTC | #3 
On Wed, 4 Oct 2023 09:10:52 -0600
Shuah Khan <skhan@linuxfoundation.org> wrote:

> On 10/3/23 18:59, Steven Rostedt wrote:
> > 
> > Note, this doesn't seem to apply to my tree so I only added the first
> > patch. I think this needs to go through Shuah's tree.
> > 
> > -- Steve
> > 
> >   
> 
> Yes. I sent a fix up for rc4 - I can pull these two patches into
> linux-kselftest next
> 
> Steve! Does that work for you?
> 

I applied the first patch to my tree, I think the second patch is fine to go
separately through your tree.

-- Steve
Shuah Khan Oct. 4, 2023, 4:38 p.m. UTC | #4 
On 10/4/23 09:14, Steven Rostedt wrote:
> On Wed, 4 Oct 2023 09:10:52 -0600
> Shuah Khan <skhan@linuxfoundation.org> wrote:
> 
>> On 10/3/23 18:59, Steven Rostedt wrote:
>>>
>>> Note, this doesn't seem to apply to my tree so I only added the first
>>> patch. I think this needs to go through Shuah's tree.
>>>
>>> -- Steve
>>>
>>>    
>>
>> Yes. I sent a fix up for rc4 - I can pull these two patches into
>> linux-kselftest next
>>
>> Steve! Does that work for you?
>>
> 
> I applied the first patch to my tree, I think the second patch is fine to go
> separately through your tree.
> 


Yes I will apply this to linux-kselftest fixes branch once my PR
clears.

thanks,
-- Shuah
Shuah Khan Oct. 5, 2023, 2:48 p.m. UTC | #5 
On 10/4/23 10:38, Shuah Khan wrote:
> On 10/4/23 09:14, Steven Rostedt wrote:
>> On Wed, 4 Oct 2023 09:10:52 -0600
>> Shuah Khan <skhan@linuxfoundation.org> wrote:
>>
>>> On 10/3/23 18:59, Steven Rostedt wrote:
>>>>
>>>> Note, this doesn't seem to apply to my tree so I only added the first
>>>> patch. I think this needs to go through Shuah's tree.
>>>>
>>>> -- Steve
>>>>
>>>
>>> Yes. I sent a fix up for rc4 - I can pull these two patches into
>>> linux-kselftest next
>>>
>>> Steve! Does that work for you?
>>>
>>
>> I applied the first patch to my tree, I think the second patch is fine to go
>> separately through your tree.
>>
> 
> 
> Yes I will apply this to linux-kselftest fixes branch once my PR
> clears.
> 

Hmm. Which tree is this patch based on? This doesn't apply to
linux-kselftest fixes - I thought this was based on top of fixes
since I sent in a fix for Linux 6.6-rc4 for user_events

Beau, Please rebase to the correct tree/branch and send v2 for
this patch.

thanks,
-- Shuah
Steven Rostedt Oct. 5, 2023, 3:08 p.m. UTC | #6 
On Thu, 5 Oct 2023 08:48:14 -0600
Shuah Khan <skhan@linuxfoundation.org> wrote:

> Hmm. Which tree is this patch based on? This doesn't apply to
> linux-kselftest fixes - I thought this was based on top of fixes
> since I sent in a fix for Linux 6.6-rc4 for user_events
> 
> Beau, Please rebase to the correct tree/branch and send v2 for
> this patch.

Hmm, so this didn't apply to my tree nor yours.

Beau, can you verify which tree this goes to?

-- Steve
Beau Belgrave Oct. 5, 2023, 4:52 p.m. UTC | #7 
On Thu, Oct 05, 2023 at 11:08:15AM -0400, Steven Rostedt wrote:
> On Thu, 5 Oct 2023 08:48:14 -0600
> Shuah Khan <skhan@linuxfoundation.org> wrote:
> 
> > Hmm. Which tree is this patch based on? This doesn't apply to
> > linux-kselftest fixes - I thought this was based on top of fixes
> > since I sent in a fix for Linux 6.6-rc4 for user_events
> > 
> > Beau, Please rebase to the correct tree/branch and send v2 for
> > this patch.
> 
> Hmm, so this didn't apply to my tree nor yours.
> 
> Beau, can you verify which tree this goes to?
> 
> -- Steve

It was based on tracing/for-next.

I'll get a v2 out rebased upon linux-kselftest, does that work?

Thanks,
-Beau
Steven Rostedt Oct. 5, 2023, 5:46 p.m. UTC | #8 
On Thu, 5 Oct 2023 09:52:30 -0700
Beau Belgrave <beaub@linux.microsoft.com> wrote:


> It was based on tracing/for-next.
> 
> I'll get a v2 out rebased upon linux-kselftest, does that work?

Hmm, then it should have applied to my tree. I didn't look too deep.

Can you see if it applies to:

  git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace.git

    trace/for-next ?

Thanks,

-- Steve
diff mbox series

Patch

diff --git a/tools/testing/selftests/user_events/abi_test.c b/tools/testing/selftests/user_events/abi_test.c
index 5125c42efe65..67af4c491c0c 100644
--- a/tools/testing/selftests/user_events/abi_test.c
+++ b/tools/testing/selftests/user_events/abi_test.c
@@ -46,7 +46,7 @@  static int change_event(bool enable)
 	return ret;
 }
 
-static int reg_enable(long *enable, int size, int bit)
+static int reg_enable(void *enable, int size, int bit)
 {
 	struct user_reg reg = {0};
 	int fd = open(data_file, O_RDWR);
@@ -68,7 +68,7 @@  static int reg_enable(long *enable, int size, int bit)
 	return ret;
 }
 
-static int reg_disable(long *enable, int bit)
+static int reg_disable(void *enable, int bit)
 {
 	struct user_unreg reg = {0};
 	int fd = open(data_file, O_RDWR);
@@ -89,12 +89,14 @@  static int reg_disable(long *enable, int bit)
 }
 
 FIXTURE(user) {
-	long check;
+	int check;
+	long check_long;
 };
 
 FIXTURE_SETUP(user) {
 	change_event(false);
 	self->check = 0;
+	self->check_long = 0;
 }
 
 FIXTURE_TEARDOWN(user) {
@@ -131,9 +133,9 @@  TEST_F(user, bit_sizes) {
 
 #if BITS_PER_LONG == 8
 	/* Allow 0-64 bits for 64-bit */
-	ASSERT_EQ(0, reg_enable(&self->check, sizeof(long), 63));
-	ASSERT_NE(0, reg_enable(&self->check, sizeof(long), 64));
-	ASSERT_EQ(0, reg_disable(&self->check, 63));
+	ASSERT_EQ(0, reg_enable(&self->check_long, sizeof(long), 63));
+	ASSERT_NE(0, reg_enable(&self->check_long, sizeof(long), 64));
+	ASSERT_EQ(0, reg_disable(&self->check_long, 63));
 #endif
 
 	/* Disallowed sizes (everything beside 4 and 8) */
@@ -195,7 +197,7 @@  static int clone_check(void *check)
 	for (i = 0; i < 10; ++i) {
 		usleep(100000);
 
-		if (*(long *)check)
+		if (*(int *)check)
 			return 0;
 	}