RE: [PATCH V4 1/2] ACPI / EC: Fix broken 64bit big-endian users of 'global_lock'

From: James Bottomley [mailto:james.bottom...@hansenpartnership.com] > Sent: 28 September 2015 15:27 > On Mon, 2015-09-28 at 08:58 +, David Laight wrote: > > From: Rafael J. Wysocki > > > Sent: 27 September 2015 15:09 > > ... > > > > > Say you have three adjacent fields in a structure, x, y,

Re: [PATCH V4 1/2] ACPI / EC: Fix broken 64bit big-endian users of 'global_lock'

On Mon, 2015-09-28 at 14:50 +, David Laight wrote: > From: James Bottomley [mailto:james.bottom...@hansenpartnership.com] > > Sent: 28 September 2015 15:27 > > On Mon, 2015-09-28 at 08:58 +, David Laight wrote: > > > From: Rafael J. Wysocki > > > > Sent: 27 September 2015 15:09 > > > ... >

RE: [PATCH V4 1/2] ACPI / EC: Fix broken 64bit big-endian users of 'global_lock'

From: James Bottomley > Sent: 28 September 2015 16:12 > > > > The x86 cpus will also do 32bit wide rmw cycles for the 'bit' > > > > operations. > > > > > > That's different: it's an atomic RMW operation. The problem with the > > > alpha was that the operation wasn't atomic (meaning that it

Re: [PATCH V4 1/2] ACPI / EC: Fix broken 64bit big-endian users of 'global_lock'

On Mon, 2015-09-28 at 08:58 +, David Laight wrote: > From: Rafael J. Wysocki > > Sent: 27 September 2015 15:09 > ... > > > > Say you have three adjacent fields in a structure, x, y, z, each one > > > > byte long. > > > > Initially, all of them are equal to 0. > > > > > > > > CPU A writes 1 to

Re: [PATCH V4 1/2] ACPI / EC: Fix broken 64bit big-endian users of 'global_lock'

On Sunday 27 September 2015 16:10:48 Rafael J. Wysocki wrote: > On Saturday, September 26, 2015 09:33:56 PM Arnd Bergmann wrote: > > On Saturday 26 September 2015 11:40:00 Viresh Kumar wrote: > > > On 25 September 2015 at 15:19, Rafael J. Wysocki > > > wrote: > > > > So if you

Re: [PATCH V4 1/2] ACPI / EC: Fix broken 64bit big-endian users of 'global_lock'

On Monday, September 28, 2015 10:24:58 AM Arnd Bergmann wrote: > On Sunday 27 September 2015 16:10:48 Rafael J. Wysocki wrote: > > On Saturday, September 26, 2015 09:33:56 PM Arnd Bergmann wrote: > > > On Saturday 26 September 2015 11:40:00 Viresh Kumar wrote: > > > > On 25 September 2015 at

RE: [PATCH V4 1/2] ACPI / EC: Fix broken 64bit big-endian users of 'global_lock'

From: Rafael J. Wysocki > Sent: 27 September 2015 15:09 ... > > > Say you have three adjacent fields in a structure, x, y, z, each one byte > > > long. > > > Initially, all of them are equal to 0. > > > > > > CPU A writes 1 to x and CPU B writes 2 to y at the same time. > > > > > > What's the

Re: [PATCH V4 1/2] ACPI / EC: Fix broken 64bit big-endian users of 'global_lock'

On 26 September 2015 at 22:31, Jiri Slaby wrote: > But this has to crash whenever the file is read as val's storage is gone at > that moment already, right? Yeah, its fixed now in the new version. This was a *really* bad idea :( -- To unsubscribe from this list: send the

Re: [PATCH V4 1/2] ACPI / EC: Fix broken 64bit big-endian users of 'global_lock'

On Saturday, September 26, 2015 12:52:08 PM James Bottomley wrote: > On Fri, 2015-09-25 at 22:58 +0200, Rafael J. Wysocki wrote: > > On Friday, September 25, 2015 01:25:49 PM Viresh Kumar wrote: > > > On 25 September 2015 at 13:33, Rafael J. Wysocki > > > wrote: > > > >

Re: [PATCH V4 1/2] ACPI / EC: Fix broken 64bit big-endian users of 'global_lock'

On Saturday, September 26, 2015 09:33:56 PM Arnd Bergmann wrote: > On Saturday 26 September 2015 11:40:00 Viresh Kumar wrote: > > On 25 September 2015 at 15:19, Rafael J. Wysocki wrote: > > > So if you allow something like debugfs to update your structure, how > > > do you make

Re: [PATCH V4 1/2] ACPI / EC: Fix broken 64bit big-endian users of 'global_lock'

On 25 September 2015 at 15:19, Rafael J. Wysocki wrote: > So if you allow something like debugfs to update your structure, how > do you make sure there is the proper locking? Not really sure at all.. Isn't there some debugfs locking that will jump in, to avoid updation of

Re: [PATCH V4 1/2] ACPI / EC: Fix broken 64bit big-endian users of 'global_lock'

On Fri, 2015-09-25 at 22:58 +0200, Rafael J. Wysocki wrote: > On Friday, September 25, 2015 01:25:49 PM Viresh Kumar wrote: > > On 25 September 2015 at 13:33, Rafael J. Wysocki wrote: > > > You're going to change that into bool in the next patch, right? > > > > Yeah. > > > >

Re: [PATCH V4 1/2] ACPI / EC: Fix broken 64bit big-endian users of 'global_lock'

On Fri, 2015-09-25 at 09:41 -0700, Viresh Kumar wrote: > Signed-off-by: Viresh Kumar > --- > V3->V4: > - Create a local variable instead of changing type of global_lock > (Rafael) Err, surely that wasn't what Rafael meant, since it's clearly impossible to use a

Re: [PATCH V4 1/2] ACPI / EC: Fix broken 64bit big-endian users of 'global_lock'

On 25-09-15, 22:58, Rafael J. Wysocki wrote: > Say you have three adjacent fields in a structure, x, y, z, each one byte > long. > Initially, all of them are equal to 0. > > CPU A writes 1 to x and CPU B writes 2 to y at the same time. > > What's the result? But then two CPUs can update the

Re: [PATCH V4 1/2] ACPI / EC: Fix broken 64bit big-endian users of 'global_lock'

On 25 September 2015 at 13:33, Rafael J. Wysocki wrote: > You're going to change that into bool in the next patch, right? Yeah. > So what if bool is a byte and the field is not word-aligned Its between two 'unsigned long' variables today, and the struct isn't packed. So, it

Re: [PATCH V4 1/2] ACPI / EC: Fix broken 64bit big-endian users of 'global_lock'

On Friday, September 25, 2015 01:25:49 PM Viresh Kumar wrote: > On 25 September 2015 at 13:33, Rafael J. Wysocki wrote: > > You're going to change that into bool in the next patch, right? > > Yeah. > > > So what if bool is a byte and the field is not word-aligned > > Its

Re: [PATCH V4 1/2] ACPI / EC: Fix broken 64bit big-endian users of 'global_lock'

On Friday, September 25, 2015 09:41:37 AM Viresh Kumar wrote: > global_lock is defined as an unsigned long and accessing only its lower > 32 bits from sysfs is incorrect, as we need to consider other 32 bits > for big endian 64 bit systems. There are no such platforms yet, but the > code needs to

Re: [PATCH V4 1/2] ACPI / EC: Fix broken 64bit big-endian users of 'global_lock'

On 25-09-15, 20:49, Johannes Berg wrote: > Ok, then, but that means Rafael is completely wrong ... > debugfs_create_bool() takes a *pointer* and it needs to be long-lived, > it can't be on the stack. You also don't get a call when it changes. Ahh, ofcourse. My bad as well... I think we can

Re: [PATCH V4 1/2] ACPI / EC: Fix broken 64bit big-endian users of 'global_lock'

On Friday, September 25, 2015 10:18:13 PM Rafael J. Wysocki wrote: > On Friday, September 25, 2015 09:41:37 AM Viresh Kumar wrote: > > global_lock is defined as an unsigned long and accessing only its lower > > 32 bits from sysfs is incorrect, as we need to consider other 32 bits > > for big

Re: [PATCH V4 1/2] ACPI / EC: Fix broken 64bit big-endian users of 'global_lock'

On 25-09-15, 19:42, Johannes Berg wrote: > On Fri, 2015-09-25 at 09:41 -0700, Viresh Kumar wrote: > > > Signed-off-by: Viresh Kumar > > --- > > V3->V4: > > - Create a local variable instead of changing type of global_lock > > (Rafael) > > Err, surely that wasn't what

Re: [PATCH V4 1/2] ACPI / EC: Fix broken 64bit big-endian users of 'global_lock'

> Rafael wrote: > > Actually, what about adding a local u32 variable, say val, here and > > doing > > > > > if (!debugfs_create_x32("gpe", 0444, dev_dir, (u32 > > > *)_ec->gpe)) > > > goto error; > > > if (!debugfs_create_bool("use_global_lock", 0444, > > > dev_dir, > > > -

Re: [PATCH V4 1/2] ACPI / EC: Fix broken 64bit big-endian users of 'global_lock'

On Friday, September 25, 2015 11:52:56 AM Viresh Kumar wrote: > On 25-09-15, 20:49, Johannes Berg wrote: > > Ok, then, but that means Rafael is completely wrong ... > > debugfs_create_bool() takes a *pointer* and it needs to be long-lived, > > it can't be on the stack. You also don't get a call

Re: [PATCH V4 1/2] ACPI / EC: Fix broken 64bit big-endian users of 'global_lock'

On Friday, September 25, 2015 10:26:22 PM Rafael J. Wysocki wrote: > On Friday, September 25, 2015 11:52:56 AM Viresh Kumar wrote: > > On 25-09-15, 20:49, Johannes Berg wrote: > > > Ok, then, but that means Rafael is completely wrong ... > > > debugfs_create_bool() takes a *pointer* and it needs

[PATCH V4 1/2] ACPI / EC: Fix broken 64bit big-endian users of 'global_lock'

global_lock is defined as an unsigned long and accessing only its lower 32 bits from sysfs is incorrect, as we need to consider other 32 bits for big endian 64 bit systems. There are no such platforms yet, but the code needs to be robust for such a case. Fix that by passing a local variable to

Re: [PATCH V4 1/2] ACPI / EC: Fix broken 64bit big-endian users of 'global_lock'

On Fri, Sep 25, 2015 at 11:44 PM, Viresh Kumar wrote: > On 25-09-15, 22:58, Rafael J. Wysocki wrote: >> Say you have three adjacent fields in a structure, x, y, z, each one byte >> long. >> Initially, all of them are equal to 0. >> >> CPU A writes 1 to x and CPU B writes