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

2015-09-28 Thread David Laight
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'

2015-09-28 Thread James Bottomley
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'

2015-09-28 Thread David Laight
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'

2015-09-28 Thread James Bottomley
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'

2015-09-28 Thread Arnd Bergmann
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'

2015-09-28 Thread Rafael J. Wysocki
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'

2015-09-28 Thread David Laight
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'

2015-09-27 Thread Viresh Kumar
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'

2015-09-27 Thread Rafael J. Wysocki
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'

2015-09-27 Thread Rafael J. Wysocki
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'

2015-09-26 Thread Viresh Kumar
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'

2015-09-26 Thread James Bottomley
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'

2015-09-25 Thread Johannes Berg
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'

2015-09-25 Thread Viresh Kumar
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'

2015-09-25 Thread Viresh Kumar
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'

2015-09-25 Thread Rafael J. Wysocki
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'

2015-09-25 Thread Rafael J. Wysocki
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'

2015-09-25 Thread Viresh Kumar
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'

2015-09-25 Thread Rafael J. Wysocki
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'

2015-09-25 Thread Viresh Kumar
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'

2015-09-25 Thread Johannes Berg
> 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'

2015-09-25 Thread Rafael J. Wysocki
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'

2015-09-25 Thread Rafael J. Wysocki
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

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

2015-09-25 Thread Rafael J. Wysocki
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