Re: [PATCH] ACPI / EC: Clear stale EC events on Samsung systems

2014-02-26 Thread Li Guang

Kieran Clancy wrote:

On Thu, Feb 27, 2014 at 12:29 PM, Li Guang  wrote:
   

+#define ACPI_EC_CLEAR_MAX  20  /* Maximum number of events to
query
+* when trying to clear the EC */

   


20 is enough?
the query index is length of a byte.
 

On my machine, 8 seems to be enough, so 20 seems to be a conservative
maximum. Just reading your other email, maybe we should set this to
32? or 40? 100?

If it's not enough, hopefully anyone seeing bugs will notice the
warning "maximum of X stale EC events cleared".

Here's what happens if I plug/replug the AC lots of times (more than
8) during suspend:

[ 8807.019800] ACPI : EC: --->  status = 0x29
[ 8807.019804] ACPI : EC: --->  data = 0x66
[ 8807.020790] ACPI : EC: --->  status = 0x29
[ 8807.020793] ACPI : EC: --->  data = 0x66
[ 8807.021793] ACPI : EC: --->  status = 0x29
[ 8807.021798] ACPI : EC: --->  data = 0x66
[ 8807.022831] ACPI : EC: --->  status = 0x29
[ 8807.022834] ACPI : EC: --->  data = 0x66
[ 8807.023788] ACPI : EC: --->  status = 0x29
[ 8807.023792] ACPI : EC: --->  data = 0x66
[ 8807.024787] ACPI : EC: --->  status = 0x29
[ 8807.024791] ACPI : EC: --->  data = 0x66
[ 8807.025787] ACPI : EC: --->  status = 0x29
[ 8807.025790] ACPI : EC: --->  data = 0x66
[ 8807.026787] ACPI : EC: --->  status = 0x29
[ 8807.026790] ACPI : EC: --->  data = 0x66
[ 8807.027786] ACPI : EC: --->  status = 0x09
[ 8807.027790] ACPI : EC: --->  data = 0x00
[ 8807.027792] ACPI : EC: 8 stale EC events cleared

Note that most of these have SCI_EVT set, but the OS is not notified
according to ACPI specs (seemingly because these events happened
during sleep).

The _Q66 method in my DSDT, is:

 P8XH (Zero, 0x66)
 If (LEqual (B1EX, One))
 {
 Notify (BAT1, 0x80)
 }

So, basically, this is supposed to notify that the battery (BAT1 =
PNP0C0A) has changed state, but they are stale events so we don't run
the handlers.

   

+static int EC_FLAGS_CLEAR_ON_RESUME; /* EC should be polled on
boot/resume */
   

seems name is implicit, what about EC_FLAGS_QEVENT_CLR_ON_RESUME?
seems too long :-)
 

In my mind this is referring to the function name (acpi_ec_)clear.
Perhaps we could just make the connection more explicit in the
comment:

/* needs acpi_ec_clear() on boot/resume */

Not sure if this is better?

   

+   /* Some hardware may need the EC to be cleared before use */
   

description is implicit, should specify what we clear is Q event, not EC.
 

Are Q events the only thing we can get from the EC data port? I've
read the relevant parts of the ACPI spec and I can't say I am 100%
sure.

   

I guess you want to clear Q events here,
EC usually has ACPI space to be read by cmd 80.

Thanks!



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] ACPI / EC: Clear stale EC events on Samsung systems

2014-02-26 Thread Li Guang

Juan Manuel Cabo wrote:

On 02/27/2014 12:14 AM, Li Guang wrote:
   

oh, sorry, I'm referring internal EC firmware code
for Q event queuing, not ACPI SPEC, ;-)
for machine you tested, 8 is the queue size,
but for some unknown also nasty EC firmwares(let's suppose it exists),
it may queue more Q events.
and I saw several firmwares queued 32 events by default,
then, let's say, they be used for some samsung products,
and also they also forgot to deal with sleep/resume state,
then, we'll also leave stale Q event there.

Thanks!

 

We tested each on our different samsung models (intel, amd), and it
was 8 across. But you're right, there might be more in the future.

  I even saw a bug report in ubuntu's launchpad of an HP with a similar
sounding problem, ( 
https://bugs.launchpad.net/ubuntu/+source/linux-source-2.6.20/+bug/89860 )
which I have no idea if it was caused by the same issue, but if in the future,
the flag ec_clear_on_resume is used to match other DMI's, it might
be a good idea to make the max iteration count bigger.

   The only reason that there is a max iteration count, was to prevent
an unexpected case in which an unknown EC never returns 0 after
queue emptied. So far it hasn't been the case. Can we count on it?.
The loop currently does finish early when there are no more events.

I guess changing it 255 or 1000 would be enough, right?

   


can't imagine 1K bytes be dissipated on Q event,
EC's ram is usually expensive,
I think 255 is really enough. :-)

Thanks!



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] ACPI / EC: Clear stale EC events on Samsung systems

2014-02-26 Thread Li Guang

Juan Manuel Cabo wrote:


   

that's really nasty EC firmware!
 

Yes!
And this bug has been unsolved for about two years.

   

20 is enough?
the query index is length of a byte.

 

According to our humble tests, 8 is the maximum number of
accumulated events. For instance, if the one plugs or unplugs the PSU
16 times, (or battery % changes 16 times) during S3 sleep, then the
EC returns no more than 8 events when polled by this patch or by
my userspace util.
And having reached 8 events, it won't produce its GPE until queried.
   


that surely indicts their EC firmware only queued 8 events,

   

the query index is length of a byte.
 

There is no query index, unless you refer to something else (I'm sorry if its
something that escapes me).
   


oh, sorry, I'm referring internal EC firmware code
for Q event queuing, not ACPI SPEC, ;-)
for machine you tested, 8 is the queue size,
but for some unknown also nasty EC firmwares(let's suppose it exists),
it may queue more Q events.
and I saw several firmwares queued 32 events by default,
then, let's say, they be used for some samsung products,
and also they also forgot to deal with sleep/resume state,
then, we'll also leave stale Q event there.

Thanks!


 For us, a query is just: send 0x84 through EC CMD port, and read status
from CMD port and event type from EC DATA port. This is done with
the usual ec.c functions that would handle a query after a GPE interrupt,
but using them instead to poll (not GPE initiated) at awake. The EC would
then return status without 0x20 mask and 'event type'==0 when no more left.

--
Juan Manuel Cabo



   

   enum {
   EC_FLAGS_QUERY_PENDING,/* Query is pending */
@@ -116,6 +118,7 @@ EXPORT_SYMBOL(first_ec);
   static int EC_FLAGS_MSI; /* Out-of-spec MSI controller */
   static int EC_FLAGS_VALIDATE_ECDT; /* ASUStec ECDTs need to be validated */
   static int EC_FLAGS_SKIP_DSDT_SCAN; /* Not all BIOS survive early DSDT scan 
*/
+static int EC_FLAGS_CLEAR_ON_RESUME; /* EC should be polled on boot/resume */

   

seems name is implicit, what about EC_FLAGS_QEVENT_CLR_ON_RESUME?
seems too long :-)

 

   /* --
Transaction Management
@@ -440,6 +443,26 @@ acpi_handle ec_get_handle(void)

   EXPORT_SYMBOL(ec_get_handle);

+static int acpi_ec_query_unlocked(struct acpi_ec *ec, u8 *data);
+
+/* run with locked ec mutex */
+static void acpi_ec_clear(struct acpi_ec *ec)
+{
+int i, status;
+u8 value = 0;
+
+for (i = 0; i<   ACPI_EC_CLEAR_MAX; i++) {
+status = acpi_ec_query_unlocked(ec,);
+if (status || !value)
+break;
+}
+
+if (i == ACPI_EC_CLEAR_MAX)
+pr_warn("Warning: Maximum of %d stale EC events cleared\n", i);
+else
+pr_info("%d stale EC events cleared\n", i);
+}
+
   void acpi_ec_block_transactions(void)
   {
   struct acpi_ec *ec = first_ec;
@@ -463,6 +486,10 @@ void acpi_ec_unblock_transactions(void)
   mutex_lock(>mutex);
   /* Allow transactions to be carried out again */
   clear_bit(EC_FLAGS_BLOCKED,>flags);
+
+if (EC_FLAGS_CLEAR_ON_RESUME)
+acpi_ec_clear(ec);
+
   mutex_unlock(>mutex);
   }

@@ -821,6 +848,13 @@ static int acpi_ec_add(struct acpi_device *device)

   /* EC is fully operational, allow queries */
   clear_bit(EC_FLAGS_QUERY_PENDING,>flags);
+
+/* Some hardware may need the EC to be cleared before use */

   

description is implicit, should specify what we clear is Q event, not EC.

Thanks!
Li Guang

 

+if (EC_FLAGS_CLEAR_ON_RESUME) {
+mutex_lock(>mutex);
+acpi_ec_clear(ec);
+mutex_unlock(>mutex);
+}
   return ret;
   }

@@ -922,6 +956,30 @@ static int ec_enlarge_storm_threshold(const struct 
dmi_system_id *id)
   return 0;
   }

+/*
+ * On some hardware it is necessary to clear events accumulated by the EC 
during
+ * sleep. These ECs stop reporting GPEs until they are manually polled, if too
+ * many events are accumulated. (e.g. Samsung Series 5/9 notebooks)
+ *
+ * https://bugzilla.kernel.org/show_bug.cgi?id=44161
+ *
+ * Ideally, the EC should also be instructed not to accumulate events during
+ * sleep (which Windows seems to do somehow), but the interface to control this
+ * behaviour is not known at this time.
+ *
+ * Models known to be affected are Samsung 530Uxx/535Uxx/540Uxx/550Pxx/900Xxx,
+ * however it is very likely that other Samsung models are affected.
+ *
+ * On systems which don't accumulate EC events during sleep, this extra check
+ * should be harmless.
+ */
+static int ec_clear_on_resume(const struct dmi_system_id *id)
+{
+pr_debug("Detected system needing EC poll on resume.\n");
+EC_FLAGS_CLEAR_ON_RESUME = 1;
+return 0;
+}
+
   static struct dmi_system_id ec_dmi_table[] __initdata = {
   {
   ec_skip_dsdt_scan, &

Re: [PATCH] ACPI / EC: Clear stale EC events on Samsung systems

2014-02-26 Thread Li Guang

Kieran Clancy wrote:

A number of Samsung notebooks (530Uxx/535Uxx/540Uxx/550Pxx/900Xxx/etc)
continue to log events during sleep (lid open/close, AC plug/unplug,
battery level change), which accumulate in the EC until a buffer fills.
After the buffer is full (tests suggest it holds 8 events), GPEs stop
being triggered for new events. This state persists on wake or even on
power cycle, and prevents new events from being registered until the EC
is manually polled.

   


that's really nasty EC firmware!


This is the root cause of a number of bugs, including AC not being
detected properly, lid close not triggering suspend, and low ambient
light not triggering the keyboard backlight. The bug also seemed to be
responsible for performance issues on at least one user's machine.

Juan Manuel Cabo found the cause of bug and the workaround of polling
the EC manually on wake.

This patch:
- Adds a function acpi_ec_clear() which polls the EC, at most
   ACPI_EC_CLEAR_MAX (currently 20) times. A warning is logged if this
   limit is reached.
- Adds a flag EC_FLAGS_CLEAR_ON_RESUME which is set to 1 if the DMI
   system vendor is Samsung. This check could be replaced by several more
   specific DMI vendor/product pairs, but it's likely that the bug
   affects more Samsung products than just the five series mentioned
   above. Further, it should not be harmful to run acpi_ec_clear() on
   systems without the bug; it will return immediately after finding no
   data waiting.
- Runs acpi_ec_clear() on initialisation (boot), from acpi_ec_add()
- Runs acpi_ec_clear() on wake, from acpi_ec_unblock_transactions()

References: https://bugzilla.kernel.org/show_bug.cgi?id=44161
References: https://bugzilla.kernel.org/show_bug.cgi?id=45461
References: https://bugzilla.kernel.org/show_bug.cgi?id=57271
Signed-off-by: Kieran Clancy
Signed-off-by: Lan Tianyu
Signed-off-by: Juan Manuel Cabo
Signed-off-by: Dennis Jansen
Tested-by: Maurizio D'Addona
Tested-by: San Zamoyski
---
  drivers/acpi/ec.c | 61 +++
  1 file changed, 61 insertions(+)

diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index 959d41a..f437d9a 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -67,6 +67,8 @@ enum ec_command {
  #define ACPI_EC_DELAY 500 /* Wait 500ms max. during EC ops */
  #define ACPI_EC_UDELAY_GLK1000/* Wait 1ms max. to get global lock */
  #define ACPI_EC_MSI_UDELAY550 /* Wait 550us for MSI EC */
+#define ACPI_EC_CLEAR_MAX  20  /* Maximum number of events to query
+* when trying to clear the EC */
   


20 is enough?
the query index is length of a byte.



  enum {
EC_FLAGS_QUERY_PENDING, /* Query is pending */
@@ -116,6 +118,7 @@ EXPORT_SYMBOL(first_ec);
  static int EC_FLAGS_MSI; /* Out-of-spec MSI controller */
  static int EC_FLAGS_VALIDATE_ECDT; /* ASUStec ECDTs need to be validated */
  static int EC_FLAGS_SKIP_DSDT_SCAN; /* Not all BIOS survive early DSDT scan */
+static int EC_FLAGS_CLEAR_ON_RESUME; /* EC should be polled on boot/resume */
   


seems name is implicit, what about EC_FLAGS_QEVENT_CLR_ON_RESUME?
seems too long :-)



  /* --
   Transaction Management
@@ -440,6 +443,26 @@ acpi_handle ec_get_handle(void)

  EXPORT_SYMBOL(ec_get_handle);

+static int acpi_ec_query_unlocked(struct acpi_ec *ec, u8 *data);
+
+/* run with locked ec mutex */
+static void acpi_ec_clear(struct acpi_ec *ec)
+{
+   int i, status;
+   u8 value = 0;
+
+   for (i = 0; i<  ACPI_EC_CLEAR_MAX; i++) {
+   status = acpi_ec_query_unlocked(ec,);
+   if (status || !value)
+   break;
+   }
+
+   if (i == ACPI_EC_CLEAR_MAX)
+   pr_warn("Warning: Maximum of %d stale EC events cleared\n", i);
+   else
+   pr_info("%d stale EC events cleared\n", i);
+}
+
  void acpi_ec_block_transactions(void)
  {
struct acpi_ec *ec = first_ec;
@@ -463,6 +486,10 @@ void acpi_ec_unblock_transactions(void)
mutex_lock(>mutex);
/* Allow transactions to be carried out again */
clear_bit(EC_FLAGS_BLOCKED,>flags);
+
+   if (EC_FLAGS_CLEAR_ON_RESUME)
+   acpi_ec_clear(ec);
+
mutex_unlock(>mutex);
  }

@@ -821,6 +848,13 @@ static int acpi_ec_add(struct acpi_device *device)

/* EC is fully operational, allow queries */
clear_bit(EC_FLAGS_QUERY_PENDING,>flags);
+
+   /* Some hardware may need the EC to be cleared before use */
   


description is implicit, should specify what we clear is Q event, not EC.

Thanks!
Li Guang


+   if (EC_FLAGS_CLEAR_ON_RESUME) {
+   mutex_lock(>mutex);
+   acpi_ec_clear(ec);
+   mutex_unlock(>mutex);
+   }
return ret;
  }

@@ -922,6 +956,30 @@ static in

Re: [PATCH] ACPI / EC: Clear stale EC events on Samsung systems

2014-02-26 Thread Li Guang

Kieran Clancy wrote:

A number of Samsung notebooks (530Uxx/535Uxx/540Uxx/550Pxx/900Xxx/etc)
continue to log events during sleep (lid open/close, AC plug/unplug,
battery level change), which accumulate in the EC until a buffer fills.
After the buffer is full (tests suggest it holds 8 events), GPEs stop
being triggered for new events. This state persists on wake or even on
power cycle, and prevents new events from being registered until the EC
is manually polled.

   


that's really nasty EC firmware!


This is the root cause of a number of bugs, including AC not being
detected properly, lid close not triggering suspend, and low ambient
light not triggering the keyboard backlight. The bug also seemed to be
responsible for performance issues on at least one user's machine.

Juan Manuel Cabo found the cause of bug and the workaround of polling
the EC manually on wake.

This patch:
- Adds a function acpi_ec_clear() which polls the EC, at most
   ACPI_EC_CLEAR_MAX (currently 20) times. A warning is logged if this
   limit is reached.
- Adds a flag EC_FLAGS_CLEAR_ON_RESUME which is set to 1 if the DMI
   system vendor is Samsung. This check could be replaced by several more
   specific DMI vendor/product pairs, but it's likely that the bug
   affects more Samsung products than just the five series mentioned
   above. Further, it should not be harmful to run acpi_ec_clear() on
   systems without the bug; it will return immediately after finding no
   data waiting.
- Runs acpi_ec_clear() on initialisation (boot), from acpi_ec_add()
- Runs acpi_ec_clear() on wake, from acpi_ec_unblock_transactions()

References: https://bugzilla.kernel.org/show_bug.cgi?id=44161
References: https://bugzilla.kernel.org/show_bug.cgi?id=45461
References: https://bugzilla.kernel.org/show_bug.cgi?id=57271
Signed-off-by: Kieran Clancyclancy.kie...@gmail.com
Signed-off-by: Lan Tianyutianyu@intel.com
Signed-off-by: Juan Manuel Cabojuanmanuel.c...@gmail.com
Signed-off-by: Dennis Jansendennis.jan...@web.de
Tested-by: Maurizio D'Addonamauritiusd...@gmail.com
Tested-by: San Zamoyskis...@plusnet.pl
---
  drivers/acpi/ec.c | 61 +++
  1 file changed, 61 insertions(+)

diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index 959d41a..f437d9a 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -67,6 +67,8 @@ enum ec_command {
  #define ACPI_EC_DELAY 500 /* Wait 500ms max. during EC ops */
  #define ACPI_EC_UDELAY_GLK1000/* Wait 1ms max. to get global lock */
  #define ACPI_EC_MSI_UDELAY550 /* Wait 550us for MSI EC */
+#define ACPI_EC_CLEAR_MAX  20  /* Maximum number of events to query
+* when trying to clear the EC */
   


20 is enough?
the query index is length of a byte.



  enum {
EC_FLAGS_QUERY_PENDING, /* Query is pending */
@@ -116,6 +118,7 @@ EXPORT_SYMBOL(first_ec);
  static int EC_FLAGS_MSI; /* Out-of-spec MSI controller */
  static int EC_FLAGS_VALIDATE_ECDT; /* ASUStec ECDTs need to be validated */
  static int EC_FLAGS_SKIP_DSDT_SCAN; /* Not all BIOS survive early DSDT scan */
+static int EC_FLAGS_CLEAR_ON_RESUME; /* EC should be polled on boot/resume */
   


seems name is implicit, what about EC_FLAGS_QEVENT_CLR_ON_RESUME?
seems too long :-)



  /* --
   Transaction Management
@@ -440,6 +443,26 @@ acpi_handle ec_get_handle(void)

  EXPORT_SYMBOL(ec_get_handle);

+static int acpi_ec_query_unlocked(struct acpi_ec *ec, u8 *data);
+
+/* run with locked ec mutex */
+static void acpi_ec_clear(struct acpi_ec *ec)
+{
+   int i, status;
+   u8 value = 0;
+
+   for (i = 0; i  ACPI_EC_CLEAR_MAX; i++) {
+   status = acpi_ec_query_unlocked(ec,value);
+   if (status || !value)
+   break;
+   }
+
+   if (i == ACPI_EC_CLEAR_MAX)
+   pr_warn(Warning: Maximum of %d stale EC events cleared\n, i);
+   else
+   pr_info(%d stale EC events cleared\n, i);
+}
+
  void acpi_ec_block_transactions(void)
  {
struct acpi_ec *ec = first_ec;
@@ -463,6 +486,10 @@ void acpi_ec_unblock_transactions(void)
mutex_lock(ec-mutex);
/* Allow transactions to be carried out again */
clear_bit(EC_FLAGS_BLOCKED,ec-flags);
+
+   if (EC_FLAGS_CLEAR_ON_RESUME)
+   acpi_ec_clear(ec);
+
mutex_unlock(ec-mutex);
  }

@@ -821,6 +848,13 @@ static int acpi_ec_add(struct acpi_device *device)

/* EC is fully operational, allow queries */
clear_bit(EC_FLAGS_QUERY_PENDING,ec-flags);
+
+   /* Some hardware may need the EC to be cleared before use */
   


description is implicit, should specify what we clear is Q event, not EC.

Thanks!
Li Guang


+   if (EC_FLAGS_CLEAR_ON_RESUME) {
+   mutex_lock(ec-mutex);
+   acpi_ec_clear(ec

Re: [PATCH] ACPI / EC: Clear stale EC events on Samsung systems

2014-02-26 Thread Li Guang

Juan Manuel Cabo wrote:


   

that's really nasty EC firmware!
 

Yes!
And this bug has been unsolved for about two years.

   

20 is enough?
the query index is length of a byte.

 

According to our humble tests, 8 is the maximum number of
accumulated events. For instance, if the one plugs or unplugs the PSU
16 times, (or battery % changes 16 times) during S3 sleep, then the
EC returns no more than 8 events when polled by this patch or by
my userspace util.
And having reached 8 events, it won't produce its GPE until queried.
   


that surely indicts their EC firmware only queued 8 events,

   

the query index is length of a byte.
 

There is no query index, unless you refer to something else (I'm sorry if its
something that escapes me).
   


oh, sorry, I'm referring internal EC firmware code
for Q event queuing, not ACPI SPEC, ;-)
for machine you tested, 8 is the queue size,
but for some unknown also nasty EC firmwares(let's suppose it exists),
it may queue more Q events.
and I saw several firmwares queued 32 events by default,
then, let's say, they be used for some samsung products,
and also they also forgot to deal with sleep/resume state,
then, we'll also leave stale Q event there.

Thanks!


 For us, a query is just: send 0x84 through EC CMD port, and read status
from CMD port and event type from EC DATA port. This is done with
the usual ec.c functions that would handle a query after a GPE interrupt,
but using them instead to poll (not GPE initiated) at awake. The EC would
then return status without 0x20 mask and 'event type'==0 when no more left.

--
Juan Manuel Cabojuanmanuel.c...@gmail.com



   

   enum {
   EC_FLAGS_QUERY_PENDING,/* Query is pending */
@@ -116,6 +118,7 @@ EXPORT_SYMBOL(first_ec);
   static int EC_FLAGS_MSI; /* Out-of-spec MSI controller */
   static int EC_FLAGS_VALIDATE_ECDT; /* ASUStec ECDTs need to be validated */
   static int EC_FLAGS_SKIP_DSDT_SCAN; /* Not all BIOS survive early DSDT scan 
*/
+static int EC_FLAGS_CLEAR_ON_RESUME; /* EC should be polled on boot/resume */

   

seems name is implicit, what about EC_FLAGS_QEVENT_CLR_ON_RESUME?
seems too long :-)

 

   /* --
Transaction Management
@@ -440,6 +443,26 @@ acpi_handle ec_get_handle(void)

   EXPORT_SYMBOL(ec_get_handle);

+static int acpi_ec_query_unlocked(struct acpi_ec *ec, u8 *data);
+
+/* run with locked ec mutex */
+static void acpi_ec_clear(struct acpi_ec *ec)
+{
+int i, status;
+u8 value = 0;
+
+for (i = 0; i   ACPI_EC_CLEAR_MAX; i++) {
+status = acpi_ec_query_unlocked(ec,value);
+if (status || !value)
+break;
+}
+
+if (i == ACPI_EC_CLEAR_MAX)
+pr_warn(Warning: Maximum of %d stale EC events cleared\n, i);
+else
+pr_info(%d stale EC events cleared\n, i);
+}
+
   void acpi_ec_block_transactions(void)
   {
   struct acpi_ec *ec = first_ec;
@@ -463,6 +486,10 @@ void acpi_ec_unblock_transactions(void)
   mutex_lock(ec-mutex);
   /* Allow transactions to be carried out again */
   clear_bit(EC_FLAGS_BLOCKED,ec-flags);
+
+if (EC_FLAGS_CLEAR_ON_RESUME)
+acpi_ec_clear(ec);
+
   mutex_unlock(ec-mutex);
   }

@@ -821,6 +848,13 @@ static int acpi_ec_add(struct acpi_device *device)

   /* EC is fully operational, allow queries */
   clear_bit(EC_FLAGS_QUERY_PENDING,ec-flags);
+
+/* Some hardware may need the EC to be cleared before use */

   

description is implicit, should specify what we clear is Q event, not EC.

Thanks!
Li Guang

 

+if (EC_FLAGS_CLEAR_ON_RESUME) {
+mutex_lock(ec-mutex);
+acpi_ec_clear(ec);
+mutex_unlock(ec-mutex);
+}
   return ret;
   }

@@ -922,6 +956,30 @@ static int ec_enlarge_storm_threshold(const struct 
dmi_system_id *id)
   return 0;
   }

+/*
+ * On some hardware it is necessary to clear events accumulated by the EC 
during
+ * sleep. These ECs stop reporting GPEs until they are manually polled, if too
+ * many events are accumulated. (e.g. Samsung Series 5/9 notebooks)
+ *
+ * https://bugzilla.kernel.org/show_bug.cgi?id=44161
+ *
+ * Ideally, the EC should also be instructed not to accumulate events during
+ * sleep (which Windows seems to do somehow), but the interface to control this
+ * behaviour is not known at this time.
+ *
+ * Models known to be affected are Samsung 530Uxx/535Uxx/540Uxx/550Pxx/900Xxx,
+ * however it is very likely that other Samsung models are affected.
+ *
+ * On systems which don't accumulate EC events during sleep, this extra check
+ * should be harmless.
+ */
+static int ec_clear_on_resume(const struct dmi_system_id *id)
+{
+pr_debug(Detected system needing EC poll on resume.\n);
+EC_FLAGS_CLEAR_ON_RESUME = 1;
+return 0;
+}
+
   static struct dmi_system_id ec_dmi_table[] __initdata = {
   {
   ec_skip_dsdt_scan, Compal JFL92, {
@@ -965,6

Re: [PATCH] ACPI / EC: Clear stale EC events on Samsung systems

2014-02-26 Thread Li Guang

Juan Manuel Cabo wrote:

On 02/27/2014 12:14 AM, Li Guang wrote:
   

oh, sorry, I'm referring internal EC firmware code
for Q event queuing, not ACPI SPEC, ;-)
for machine you tested, 8 is the queue size,
but for some unknown also nasty EC firmwares(let's suppose it exists),
it may queue more Q events.
and I saw several firmwares queued 32 events by default,
then, let's say, they be used for some samsung products,
and also they also forgot to deal with sleep/resume state,
then, we'll also leave stale Q event there.

Thanks!

 

We tested each on our different samsung models (intel, amd), and it
was 8 across. But you're right, there might be more in the future.

  I even saw a bug report in ubuntu's launchpad of an HP with a similar
sounding problem, ( 
https://bugs.launchpad.net/ubuntu/+source/linux-source-2.6.20/+bug/89860 )
which I have no idea if it was caused by the same issue, but if in the future,
the flag ec_clear_on_resume is used to match other DMI's, it might
be a good idea to make the max iteration count bigger.

   The only reason that there is a max iteration count, was to prevent
an unexpected case in which an unknown EC never returns 0 after
queue emptied. So far it hasn't been the case. Can we count on it?.
The loop currently does finish early when there are no more events.

I guess changing it 255 or 1000 would be enough, right?

   


can't imagine 1K bytes be dissipated on Q event,
EC's ram is usually expensive,
I think 255 is really enough. :-)

Thanks!



--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] ACPI / EC: Clear stale EC events on Samsung systems

2014-02-26 Thread Li Guang

Kieran Clancy wrote:

On Thu, Feb 27, 2014 at 12:29 PM, Li Guanglig.f...@cn.fujitsu.com  wrote:
   

+#define ACPI_EC_CLEAR_MAX  20  /* Maximum number of events to
query
+* when trying to clear the EC */

   


20 is enough?
the query index is length of a byte.
 

On my machine, 8 seems to be enough, so 20 seems to be a conservative
maximum. Just reading your other email, maybe we should set this to
32? or 40? 100?

If it's not enough, hopefully anyone seeing bugs will notice the
warning maximum of X stale EC events cleared.

Here's what happens if I plug/replug the AC lots of times (more than
8) during suspend:

[ 8807.019800] ACPI : EC: ---  status = 0x29
[ 8807.019804] ACPI : EC: ---  data = 0x66
[ 8807.020790] ACPI : EC: ---  status = 0x29
[ 8807.020793] ACPI : EC: ---  data = 0x66
[ 8807.021793] ACPI : EC: ---  status = 0x29
[ 8807.021798] ACPI : EC: ---  data = 0x66
[ 8807.022831] ACPI : EC: ---  status = 0x29
[ 8807.022834] ACPI : EC: ---  data = 0x66
[ 8807.023788] ACPI : EC: ---  status = 0x29
[ 8807.023792] ACPI : EC: ---  data = 0x66
[ 8807.024787] ACPI : EC: ---  status = 0x29
[ 8807.024791] ACPI : EC: ---  data = 0x66
[ 8807.025787] ACPI : EC: ---  status = 0x29
[ 8807.025790] ACPI : EC: ---  data = 0x66
[ 8807.026787] ACPI : EC: ---  status = 0x29
[ 8807.026790] ACPI : EC: ---  data = 0x66
[ 8807.027786] ACPI : EC: ---  status = 0x09
[ 8807.027790] ACPI : EC: ---  data = 0x00
[ 8807.027792] ACPI : EC: 8 stale EC events cleared

Note that most of these have SCI_EVT set, but the OS is not notified
according to ACPI specs (seemingly because these events happened
during sleep).

The _Q66 method in my DSDT, is:

 P8XH (Zero, 0x66)
 If (LEqual (B1EX, One))
 {
 Notify (BAT1, 0x80)
 }

So, basically, this is supposed to notify that the battery (BAT1 =
PNP0C0A) has changed state, but they are stale events so we don't run
the handlers.

   

+static int EC_FLAGS_CLEAR_ON_RESUME; /* EC should be polled on
boot/resume */
   

seems name is implicit, what about EC_FLAGS_QEVENT_CLR_ON_RESUME?
seems too long :-)
 

In my mind this is referring to the function name (acpi_ec_)clear.
Perhaps we could just make the connection more explicit in the
comment:

/* needs acpi_ec_clear() on boot/resume */

Not sure if this is better?

   

+   /* Some hardware may need the EC to be cleared before use */
   

description is implicit, should specify what we clear is Q event, not EC.
 

Are Q events the only thing we can get from the EC data port? I've
read the relevant parts of the ACPI spec and I can't say I am 100%
sure.

   

I guess you want to clear Q events here,
EC usually has ACPI space to be read by cmd 80.

Thanks!



--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC v2 0/3] add cpu physically hotplug driver

2013-06-09 Thread li guang
在 2013-06-10一的 12:51 +0900,Yasuaki Ishimatsu写道:
> 2013/06/10 9:36, li guang wrote:
> > Hi, Rafael
> >
> > 在 2013-06-06四的 13:00 +0200,Rafael J. Wysocki写道:
> >> On Thursday, June 06, 2013 09:40:32 AM liguang wrote:
> >>> This patch-set try to support physically hot-plug/unplug
> >>> a cpu automatically, that is:
> >>> if you offline a cpu, it will automatically actually remove
> >>> a cpu, and if you hot-plug a cpu, then it will online this
> >>> cpu automatically.
> >>
> >> No and no.
> >
> > Hmm... are you saying cpu online/offline designed to distinguish
> > with real cpu plug/unplug?
> > but, what the actual usage of online/offline?
> > forgive my foolish.
> >
> >>
> >> Why do you need this?
> >>
> >
> > e.g. for QEMU case, if hot-plug a cpu,
> 
> > we have to online a cpu manually if there's
> > no user space support like udev to do it automatically.
> 
> I could not understand why you do not use udev.
> Please explain in detail.

I did not say I can't use udev now,
just for in case someone can't.

> 
> > and also, I think maybe online/offline should be naturally
> > integrated with real plug/unplug process of CPU.
> 
> I could not understand this explanation too.
> Why do we need it?
> 

actually, I am asking if CPU online/offline have real purpose,
you offline a CPU, then threads can't run on it,
you online a CPU, then threads can run on it,
so, the purpose here is just bouncing threads?
obviously not, any profound historical reasons?

I boldly think maybe online/offline should be integrated into
real plug/unplug process of CPU.

Thanks!

> >
> >>
> >>
> >>> so, offline is just like eject, but eject attribute seems not
> >>> available since recent kernel(can't figure out when), with
> >>> this driver, if allowed, it will trigger a eject cpu process.
> >>> and for automatically online, it was said there are objections,
> >>> don't know the reason, so, send this patch-set boldly.
> >>>
> >>> of course, this approach is for QEMU 's hotplug cpu emulation
> >>> only, but not limited, if someone like to explore ec space to
> >>> implment cpu hot-plug/unplug for real platform please
> >>> feel free to continue.
> >>>
> >>> Li Guang (3)
> >>>drivers/platform/x86: add cpu physically hotplug driver
> >>>ec: add ec space notifier
> >>>cpu_physic_hotplug: register handler for ec space notifier
> >>>
> >>> drivers/acpi/ec.c | 32 
> >>> 
> >>> drivers/platform/x86/Kconfig  |  8 
> >>> drivers/platform/x86/Makefile |  1 +
> >>> drivers/platform/x86/cpu_physic_hotplug.c | 90 
> >>> +
> >>> include/linux/acpi.h  |  2 ++
> >>> 5 files changed, 130 insertions(+), 3 deletions(-)
> >>>   create mode 100644 drivers/platform/x86/cpu_physic_hotplug.c
> >
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majord...@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/
> >
> 
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC v2 2/3] ec: add ec space notifier

2013-06-09 Thread li guang
在 2013-06-06四的 12:59 +0200,Rafael J. Wysocki写道:
> On Thursday, June 06, 2013 09:40:34 AM liguang wrote:
> > add a notifier for anyone who are instresting in
> > ec space changing.
> > 
> > Signed-off-by: liguang 
> 
> I'm not going to apply this anyway, but can you please explain what's the
> problem you're trying to solve here?

OK, currently it is for QEMU to do cpu online automatically after
hot-plug a cpu,
and maybe potentially for real platform's cpu
plug/unplug implementation.
with this notifier, we don't need any GPIOes,IO ports, and other
hardware cost if we do have an EC chip in board to trigger kernel's 
cpu process for this.

Yep, you said you'll reject this anyway,
but I have to ask do this notifier offend
any rules of your development?
or some other reasons?

Thanks!

> 
> > ---
> >  drivers/acpi/ec.c|   32 
> >  include/linux/acpi.h |2 ++
> >  2 files changed, 34 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
> > index edc0081..dee3417 100644
> > --- a/drivers/acpi/ec.c
> > +++ b/drivers/acpi/ec.c
> > @@ -124,6 +124,35 @@ static int EC_FLAGS_MSI; /* Out-of-spec MSI controller 
> > */
> >  static int EC_FLAGS_VALIDATE_ECDT; /* ASUStec ECDTs need to be validated */
> >  static int EC_FLAGS_SKIP_DSDT_SCAN; /* Not all BIOS survive early DSDT 
> > scan */
> >  
> > +/* notifier chain for who are intresting in ec space changing */
> > +static RAW_NOTIFIER_HEAD(ec_space_chain);
> > +
> > +int __ref register_ec_space_notifier(struct notifier_block *nb)
> > +{
> > +int ret;
> > +
> > +ret = raw_notifier_chain_register(_space_chain, nb);
> > +
> > +return ret;
> > +}
> > +EXPORT_SYMBOL(register_ec_space_notifier);
> > +
> > +void __ref unregister_ec_space_notifier(struct notifier_block *nb)
> > +{
> > +
> > +raw_notifier_chain_unregister(_space_chain, nb);
> > +}
> > +EXPORT_SYMBOL(unregister_ec_space_notifier);
> > +
> > +static int ec_space_notify(void *data)
> > +{
> > +int ret;
> > +
> > +ret = __raw_notifier_call_chain(_space_chain, 0, data, -1, NULL);
> > +
> > + return notifier_to_errno(ret);
> > +}
> > +
> >  /* 
> > --
> >   Transaction Management
> > 
> > -- 
> > */
> > @@ -638,6 +667,9 @@ static u32 acpi_ec_gpe_handler(acpi_handle gpe_device,
> > wake_up(>wait);
> > ec_check_sci(ec, acpi_ec_read_status(ec));
> > }
> > +
> > +   ec_space_notify(data);
> > +
> > return ACPI_INTERRUPT_HANDLED | ACPI_REENABLE_GPE;
> >  }
> >  
> > diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> > index 17b5b59..4fe2247 100644
> > --- a/include/linux/acpi.h
> > +++ b/include/linux/acpi.h
> > @@ -158,6 +158,8 @@ extern int ec_transaction(u8 command,
> >const u8 *wdata, unsigned wdata_len,
> >u8 *rdata, unsigned rdata_len);
> >  extern acpi_handle ec_get_handle(void);
> > +extern int register_ec_space_notifier(struct notifier_block *nb);
> > +extern void unregister_ec_space_notifier(struct notifier_block *nb);
> >  
> >  #if defined(CONFIG_ACPI_WMI) || defined(CONFIG_ACPI_WMI_MODULE)
> >  
> > 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC v2 0/3] add cpu physically hotplug driver

2013-06-09 Thread li guang
Hi, Rafael

在 2013-06-06四的 13:00 +0200,Rafael J. Wysocki写道:
> On Thursday, June 06, 2013 09:40:32 AM liguang wrote:
> > This patch-set try to support physically hot-plug/unplug
> > a cpu automatically, that is:
> > if you offline a cpu, it will automatically actually remove
> > a cpu, and if you hot-plug a cpu, then it will online this
> > cpu automatically.
> 
> No and no.

Hmm... are you saying cpu online/offline designed to distinguish 
with real cpu plug/unplug?
but, what the actual usage of online/offline?
forgive my foolish.

> 
> Why do you need this?
> 

e.g. for QEMU case, if hot-plug a cpu,
we have to online a cpu manually if there's 
no user space support like udev to do it automatically.
and also, I think maybe online/offline should be naturally
integrated with real plug/unplug process of CPU.

> 
> 
> > so, offline is just like eject, but eject attribute seems not
> > available since recent kernel(can't figure out when), with
> > this driver, if allowed, it will trigger a eject cpu process.
> > and for automatically online, it was said there are objections,
> > don't know the reason, so, send this patch-set boldly.
> > 
> > of course, this approach is for QEMU 's hotplug cpu emulation 
> > only, but not limited, if someone like to explore ec space to
> > implment cpu hot-plug/unplug for real platform please
> > feel free to continue.
> > 
> > Li Guang (3)
> >  drivers/platform/x86: add cpu physically hotplug driver
> >  ec: add ec space notifier
> >  cpu_physic_hotplug: register handler for ec space notifier
> > 
> > drivers/acpi/ec.c | 32 
> > 
> > drivers/platform/x86/Kconfig  |  8 
> > drivers/platform/x86/Makefile |  1 +
> > drivers/platform/x86/cpu_physic_hotplug.c | 90 +
> > include/linux/acpi.h  |  2 ++
> > 5 files changed, 130 insertions(+), 3 deletions(-)
> >  create mode 100644 drivers/platform/x86/cpu_physic_hotplug.c


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC v2 0/3] add cpu physically hotplug driver

2013-06-09 Thread li guang
Hi, Rafael

在 2013-06-06四的 13:00 +0200,Rafael J. Wysocki写道:
 On Thursday, June 06, 2013 09:40:32 AM liguang wrote:
  This patch-set try to support physically hot-plug/unplug
  a cpu automatically, that is:
  if you offline a cpu, it will automatically actually remove
  a cpu, and if you hot-plug a cpu, then it will online this
  cpu automatically.
 
 No and no.

Hmm... are you saying cpu online/offline designed to distinguish 
with real cpu plug/unplug?
but, what the actual usage of online/offline?
forgive my foolish.

 
 Why do you need this?
 

e.g. for QEMU case, if hot-plug a cpu,
we have to online a cpu manually if there's 
no user space support like udev to do it automatically.
and also, I think maybe online/offline should be naturally
integrated with real plug/unplug process of CPU.

 
 
  so, offline is just like eject, but eject attribute seems not
  available since recent kernel(can't figure out when), with
  this driver, if allowed, it will trigger a eject cpu process.
  and for automatically online, it was said there are objections,
  don't know the reason, so, send this patch-set boldly.
  
  of course, this approach is for QEMU 's hotplug cpu emulation 
  only, but not limited, if someone like to explore ec space to
  implment cpu hot-plug/unplug for real platform please
  feel free to continue.
  
  Li Guang (3)
   drivers/platform/x86: add cpu physically hotplug driver
   ec: add ec space notifier
   cpu_physic_hotplug: register handler for ec space notifier
  
  drivers/acpi/ec.c | 32 
  
  drivers/platform/x86/Kconfig  |  8 
  drivers/platform/x86/Makefile |  1 +
  drivers/platform/x86/cpu_physic_hotplug.c | 90 +
  include/linux/acpi.h  |  2 ++
  5 files changed, 130 insertions(+), 3 deletions(-)
   create mode 100644 drivers/platform/x86/cpu_physic_hotplug.c


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC v2 2/3] ec: add ec space notifier

2013-06-09 Thread li guang
在 2013-06-06四的 12:59 +0200,Rafael J. Wysocki写道:
 On Thursday, June 06, 2013 09:40:34 AM liguang wrote:
  add a notifier for anyone who are instresting in
  ec space changing.
  
  Signed-off-by: liguang lig.f...@cn.fujitsu.com
 
 I'm not going to apply this anyway, but can you please explain what's the
 problem you're trying to solve here?

OK, currently it is for QEMU to do cpu online automatically after
hot-plug a cpu,
and maybe potentially for real platform's cpu
plug/unplug implementation.
with this notifier, we don't need any GPIOes,IO ports, and other
hardware cost if we do have an EC chip in board to trigger kernel's 
cpu process for this.

Yep, you said you'll reject this anyway,
but I have to ask do this notifier offend
any rules of your development?
or some other reasons?

Thanks!

 
  ---
   drivers/acpi/ec.c|   32 
   include/linux/acpi.h |2 ++
   2 files changed, 34 insertions(+), 0 deletions(-)
  
  diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
  index edc0081..dee3417 100644
  --- a/drivers/acpi/ec.c
  +++ b/drivers/acpi/ec.c
  @@ -124,6 +124,35 @@ static int EC_FLAGS_MSI; /* Out-of-spec MSI controller 
  */
   static int EC_FLAGS_VALIDATE_ECDT; /* ASUStec ECDTs need to be validated */
   static int EC_FLAGS_SKIP_DSDT_SCAN; /* Not all BIOS survive early DSDT 
  scan */
   
  +/* notifier chain for who are intresting in ec space changing */
  +static RAW_NOTIFIER_HEAD(ec_space_chain);
  +
  +int __ref register_ec_space_notifier(struct notifier_block *nb)
  +{
  +int ret;
  +
  +ret = raw_notifier_chain_register(ec_space_chain, nb);
  +
  +return ret;
  +}
  +EXPORT_SYMBOL(register_ec_space_notifier);
  +
  +void __ref unregister_ec_space_notifier(struct notifier_block *nb)
  +{
  +
  +raw_notifier_chain_unregister(ec_space_chain, nb);
  +}
  +EXPORT_SYMBOL(unregister_ec_space_notifier);
  +
  +static int ec_space_notify(void *data)
  +{
  +int ret;
  +
  +ret = __raw_notifier_call_chain(ec_space_chain, 0, data, -1, NULL);
  +
  + return notifier_to_errno(ret);
  +}
  +
   /* 
  --
Transaction Management
  
  -- 
  */
  @@ -638,6 +667,9 @@ static u32 acpi_ec_gpe_handler(acpi_handle gpe_device,
  wake_up(ec-wait);
  ec_check_sci(ec, acpi_ec_read_status(ec));
  }
  +
  +   ec_space_notify(data);
  +
  return ACPI_INTERRUPT_HANDLED | ACPI_REENABLE_GPE;
   }
   
  diff --git a/include/linux/acpi.h b/include/linux/acpi.h
  index 17b5b59..4fe2247 100644
  --- a/include/linux/acpi.h
  +++ b/include/linux/acpi.h
  @@ -158,6 +158,8 @@ extern int ec_transaction(u8 command,
 const u8 *wdata, unsigned wdata_len,
 u8 *rdata, unsigned rdata_len);
   extern acpi_handle ec_get_handle(void);
  +extern int register_ec_space_notifier(struct notifier_block *nb);
  +extern void unregister_ec_space_notifier(struct notifier_block *nb);
   
   #if defined(CONFIG_ACPI_WMI) || defined(CONFIG_ACPI_WMI_MODULE)
   
  


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC v2 0/3] add cpu physically hotplug driver

2013-06-09 Thread li guang
在 2013-06-10一的 12:51 +0900,Yasuaki Ishimatsu写道:
 2013/06/10 9:36, li guang wrote:
  Hi, Rafael
 
  在 2013-06-06四的 13:00 +0200,Rafael J. Wysocki写道:
  On Thursday, June 06, 2013 09:40:32 AM liguang wrote:
  This patch-set try to support physically hot-plug/unplug
  a cpu automatically, that is:
  if you offline a cpu, it will automatically actually remove
  a cpu, and if you hot-plug a cpu, then it will online this
  cpu automatically.
 
  No and no.
 
  Hmm... are you saying cpu online/offline designed to distinguish
  with real cpu plug/unplug?
  but, what the actual usage of online/offline?
  forgive my foolish.
 
 
  Why do you need this?
 
 
  e.g. for QEMU case, if hot-plug a cpu,
 
  we have to online a cpu manually if there's
  no user space support like udev to do it automatically.
 
 I could not understand why you do not use udev.
 Please explain in detail.

I did not say I can't use udev now,
just for in case someone can't.

 
  and also, I think maybe online/offline should be naturally
  integrated with real plug/unplug process of CPU.
 
 I could not understand this explanation too.
 Why do we need it?
 

actually, I am asking if CPU online/offline have real purpose,
you offline a CPU, then threads can't run on it,
you online a CPU, then threads can run on it,
so, the purpose here is just bouncing threads?
obviously not, any profound historical reasons?

I boldly think maybe online/offline should be integrated into
real plug/unplug process of CPU.

Thanks!

 
 
 
  so, offline is just like eject, but eject attribute seems not
  available since recent kernel(can't figure out when), with
  this driver, if allowed, it will trigger a eject cpu process.
  and for automatically online, it was said there are objections,
  don't know the reason, so, send this patch-set boldly.
 
  of course, this approach is for QEMU 's hotplug cpu emulation
  only, but not limited, if someone like to explore ec space to
  implment cpu hot-plug/unplug for real platform please
  feel free to continue.
 
  Li Guang (3)
 drivers/platform/x86: add cpu physically hotplug driver
 ec: add ec space notifier
 cpu_physic_hotplug: register handler for ec space notifier
 
  drivers/acpi/ec.c | 32 
  
  drivers/platform/x86/Kconfig  |  8 
  drivers/platform/x86/Makefile |  1 +
  drivers/platform/x86/cpu_physic_hotplug.c | 90 
  +
  include/linux/acpi.h  |  2 ++
  5 files changed, 130 insertions(+), 3 deletions(-)
create mode 100644 drivers/platform/x86/cpu_physic_hotplug.c
 
 
  --
  To unsubscribe from this list: send the line unsubscribe linux-kernel in
  the body of a message to majord...@vger.kernel.org
  More majordomo info at  http://vger.kernel.org/majordomo-info.html
  Please read the FAQ at  http://www.tux.org/lkml/
 
 
 


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC v2 1/3] drivers/platform/x86: add cpu physically hotplug driver

2013-06-05 Thread li guang
在 2013-06-06四的 10:24 +0800,Gu Zheng写道:
> On 06/06/2013 09:40 AM, liguang wrote:
> 
> > this driver will support cpu phyical add/removal automatically
> > after online/offline. if cpu hotpluged, cpu will not
> > online automatically, and for cpu offline, we try to
> > do actually eject if allowed for cpu like
> > "echo 1 > /sys/bus/acpi/devices/LNXCPU\:0X/eject"
> > this "echo ..." is only present for recent kernel
> > (sorry, can't figure out since when), for a little
> > older kernel, there's not such approach AFAICS.
> > 
> > Signed-off-by: liguang 
> > ---
> >  drivers/platform/x86/Kconfig  |8 
> >  drivers/platform/x86/Makefile |1 +
> >  drivers/platform/x86/cpu_physic_hotplug.c |   60 
> > +
> >  3 files changed, 69 insertions(+), 0 deletions(-)
> >  create mode 100644 drivers/platform/x86/cpu_physic_hotplug.c
> > 
> > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> > index 8577261..39b2392 100644
> > --- a/drivers/platform/x86/Kconfig
> > +++ b/drivers/platform/x86/Kconfig
> > @@ -789,4 +789,12 @@ config PVPANIC
> >   a paravirtualized device provided by QEMU; it lets a virtual machine
> >   (guest) communicate panic events to the host.
> >  
> > +config QEMU_CPU_PHYSIC_HOTPLUG
> > +   tristate "physically add/remove cpu after cpu onlined/offlined"
> > +   depends on ACPI_HOTPLUG_CPU
> > +   ---help---
> > + This driver will support physically remove a cpu after
> > + it offlined for QEMU automatically. someone may require this feature
> > + to do a physically removal for a cpu.
> > +
> >  endif # X86_PLATFORM_DEVICES
> > diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
> > index ef0ec74..2e669b0 100644
> > --- a/drivers/platform/x86/Makefile
> > +++ b/drivers/platform/x86/Makefile
> > @@ -53,3 +53,4 @@ obj-$(CONFIG_APPLE_GMUX)  += apple-gmux.o
> >  obj-$(CONFIG_CHROMEOS_LAPTOP)  += chromeos_laptop.o
> >  
> >  obj-$(CONFIG_PVPANIC)   += pvpanic.o
> > +obj-$(CONFIG_QEMU_CPU_PHYSIC_HOTPLUG)  += cpu_physic_hotplug.o
> > diff --git a/drivers/platform/x86/cpu_physic_hotplug.c 
> > b/drivers/platform/x86/cpu_physic_hotplug.c
> > new file mode 100644
> > index 000..a52c042
> > --- /dev/null
> > +++ b/drivers/platform/x86/cpu_physic_hotplug.c
> > @@ -0,0 +1,60 @@
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +MODULE_AUTHOR("Li Guang");
> > +MODULE_DESCRIPTION("CPU physically hot-plug/unplug Driver");
> > +MODULE_LICENSE("GPL");
> > +
> > +static int cpu_logic_hotplug_notify(struct notifier_block *nfb,
> > +   unsigned long action, void *hcpu)
> > +{
> > +   unsigned int cpu = (unsigned long)hcpu;
> > +   struct acpi_processor *pr = per_cpu(processors, cpu);
> > +
> > +   if (pr) {
> > +   switch (action) {
> > +   case CPU_ONLINE:
> > +   break;
> > +   case CPU_DEAD:
> > +   break;
> > +   default:
> > +   break;
> > +   }
> > +   }
> > +   return NOTIFY_OK;
> > +}
> > +
> > +static struct notifier_block cpu_logic_hotplug_notifier =
> > +{
> > +   .notifier_call = cpu_logic_hotplug_notify,
> > +};
> > +
> > +static int cpu_physic_hotplug_notify(struct notifier_block *nfb,
> > +unsigned char *s)
> > +{
> > +}
> 
> Hi guang,
>   Maybe you need to define the callback function in the right format at 
> the beginning,
> if so, no need to correct it later.:)
> 

right,

Thanks!

> 
> > +
> > +static struct notifier_block cpu_physic_hotplug_notifier =
> > +{
> > +   .notifier_call = cpu_physic_hotplug_notify,
> > +};
> > +
> > +static int __init cpu_qemu_hotplug_init(void)
> > +{
> > +   register_hotcpu_notifier(_logic_hotplug_notifier);
> > +   register_ec_gpe_notifier(_physic_hotplug_notifier);
> 
> 
> As the [PATCH 2/3] has no dependence on this one, so you can set [PATCH 2/3] 
> to [PATCH 1/3] and this one
> to [PATCH 2/3]. Then you can use the xxx_ec_space_notifier directly here.
> 
> > +   return 0;
> > +}
> > +
> > +static void __exit cpu_qemu_hotplug_exit(void)
> > +{
> > +   unregister_hotcpu_notifier(_logic_hotplug_notifier);
> > +   unregister_ec_gpe_notifier(_physic_hotplug_notifier);
> > +}
> > +
> > +module_init(cpu_qemu_hotplug_init);
> > +module_exit(cpu_qemu_hotplug_exit);
> 
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC v2 1/3] drivers/platform/x86: add cpu physically hotplug driver

2013-06-05 Thread li guang
在 2013-06-06四的 10:24 +0800,Gu Zheng写道:
 On 06/06/2013 09:40 AM, liguang wrote:
 
  this driver will support cpu phyical add/removal automatically
  after online/offline. if cpu hotpluged, cpu will not
  online automatically, and for cpu offline, we try to
  do actually eject if allowed for cpu like
  echo 1  /sys/bus/acpi/devices/LNXCPU\:0X/eject
  this echo ... is only present for recent kernel
  (sorry, can't figure out since when), for a little
  older kernel, there's not such approach AFAICS.
  
  Signed-off-by: liguang lig.f...@cn.fujitsu.com
  ---
   drivers/platform/x86/Kconfig  |8 
   drivers/platform/x86/Makefile |1 +
   drivers/platform/x86/cpu_physic_hotplug.c |   60 
  +
   3 files changed, 69 insertions(+), 0 deletions(-)
   create mode 100644 drivers/platform/x86/cpu_physic_hotplug.c
  
  diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
  index 8577261..39b2392 100644
  --- a/drivers/platform/x86/Kconfig
  +++ b/drivers/platform/x86/Kconfig
  @@ -789,4 +789,12 @@ config PVPANIC
a paravirtualized device provided by QEMU; it lets a virtual machine
(guest) communicate panic events to the host.
   
  +config QEMU_CPU_PHYSIC_HOTPLUG
  +   tristate physically add/remove cpu after cpu onlined/offlined
  +   depends on ACPI_HOTPLUG_CPU
  +   ---help---
  + This driver will support physically remove a cpu after
  + it offlined for QEMU automatically. someone may require this feature
  + to do a physically removal for a cpu.
  +
   endif # X86_PLATFORM_DEVICES
  diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
  index ef0ec74..2e669b0 100644
  --- a/drivers/platform/x86/Makefile
  +++ b/drivers/platform/x86/Makefile
  @@ -53,3 +53,4 @@ obj-$(CONFIG_APPLE_GMUX)  += apple-gmux.o
   obj-$(CONFIG_CHROMEOS_LAPTOP)  += chromeos_laptop.o
   
   obj-$(CONFIG_PVPANIC)   += pvpanic.o
  +obj-$(CONFIG_QEMU_CPU_PHYSIC_HOTPLUG)  += cpu_physic_hotplug.o
  diff --git a/drivers/platform/x86/cpu_physic_hotplug.c 
  b/drivers/platform/x86/cpu_physic_hotplug.c
  new file mode 100644
  index 000..a52c042
  --- /dev/null
  +++ b/drivers/platform/x86/cpu_physic_hotplug.c
  @@ -0,0 +1,60 @@
  +#include linux/kernel.h
  +#include linux/module.h
  +#include linux/acpi.h
  +#include linux/cpu.h
  +#include linux/notifier.h
  +#include acpi/processor.h
  +
  +MODULE_AUTHOR(Li Guang);
  +MODULE_DESCRIPTION(CPU physically hot-plug/unplug Driver);
  +MODULE_LICENSE(GPL);
  +
  +static int cpu_logic_hotplug_notify(struct notifier_block *nfb,
  +   unsigned long action, void *hcpu)
  +{
  +   unsigned int cpu = (unsigned long)hcpu;
  +   struct acpi_processor *pr = per_cpu(processors, cpu);
  +
  +   if (pr) {
  +   switch (action) {
  +   case CPU_ONLINE:
  +   break;
  +   case CPU_DEAD:
  +   break;
  +   default:
  +   break;
  +   }
  +   }
  +   return NOTIFY_OK;
  +}
  +
  +static struct notifier_block cpu_logic_hotplug_notifier =
  +{
  +   .notifier_call = cpu_logic_hotplug_notify,
  +};
  +
  +static int cpu_physic_hotplug_notify(struct notifier_block *nfb,
  +unsigned char *s)
  +{
  +}
 
 Hi guang,
   Maybe you need to define the callback function in the right format at 
 the beginning,
 if so, no need to correct it later.:)
 

right,

Thanks!

 
  +
  +static struct notifier_block cpu_physic_hotplug_notifier =
  +{
  +   .notifier_call = cpu_physic_hotplug_notify,
  +};
  +
  +static int __init cpu_qemu_hotplug_init(void)
  +{
  +   register_hotcpu_notifier(cpu_logic_hotplug_notifier);
  +   register_ec_gpe_notifier(cpu_physic_hotplug_notifier);
 
 
 As the [PATCH 2/3] has no dependence on this one, so you can set [PATCH 2/3] 
 to [PATCH 1/3] and this one
 to [PATCH 2/3]. Then you can use the xxx_ec_space_notifier directly here.
 
  +   return 0;
  +}
  +
  +static void __exit cpu_qemu_hotplug_exit(void)
  +{
  +   unregister_hotcpu_notifier(cpu_logic_hotplug_notifier);
  +   unregister_ec_gpe_notifier(cpu_physic_hotplug_notifier);
  +}
  +
  +module_init(cpu_qemu_hotplug_init);
  +module_exit(cpu_qemu_hotplug_exit);
 
 


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/2] xen: remove bm_rld_set of xen_processor_flags

2013-06-04 Thread li guang
在 2013-06-04二的 09:13 +0100,Jan Beulich写道:
> >>> On 04.06.13 at 10:05, liguang  wrote:
> > bm_rld_set seems obsolete now
> > 
> > Signed-off-by: liguang 
> > ---
> >  include/xen/interface/platform.h |1 -
> >  1 files changed, 0 insertions(+), 1 deletions(-)
> > 
> > diff --git a/include/xen/interface/platform.h 
> > b/include/xen/interface/platform.h
> > index c57d5f6..733 100644
> > --- a/include/xen/interface/platform.h
> > +++ b/include/xen/interface/platform.h
> > @@ -240,7 +240,6 @@ struct xen_processor_flags {
> > uint32_t bm_check:1;
> > uint32_t has_cst:1;
> > uint32_t power_setup_done:1;
> > -   uint32_t bm_rld_set:1;
> >  };
> >  
> >  struct xen_processor_power {
> 
> Any such patch would need to be submitted against the master copy
> of the header (in the Xen repo), and by recognizing that you'd also
> notice that this is part of a public ABI, and hence can't be removed,
> but at best can be documented as obsolete. Of course you'd first
> need to check whether the hypervisor makes any use of that bit
> when passed down from Dom0.
> 

Right, this patch mostly likes a reminder,
hoping someone will try it and verify it's fail or OK,
since I can't test this for now.

Sorry and Thanks!


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/2] xen: remove bm_rld_set of xen_processor_flags

2013-06-04 Thread li guang
在 2013-06-04二的 09:13 +0100,Jan Beulich写道:
  On 04.06.13 at 10:05, liguang lig.f...@cn.fujitsu.com wrote:
  bm_rld_set seems obsolete now
  
  Signed-off-by: liguang lig.f...@cn.fujitsu.com
  ---
   include/xen/interface/platform.h |1 -
   1 files changed, 0 insertions(+), 1 deletions(-)
  
  diff --git a/include/xen/interface/platform.h 
  b/include/xen/interface/platform.h
  index c57d5f6..733 100644
  --- a/include/xen/interface/platform.h
  +++ b/include/xen/interface/platform.h
  @@ -240,7 +240,6 @@ struct xen_processor_flags {
  uint32_t bm_check:1;
  uint32_t has_cst:1;
  uint32_t power_setup_done:1;
  -   uint32_t bm_rld_set:1;
   };
   
   struct xen_processor_power {
 
 Any such patch would need to be submitted against the master copy
 of the header (in the Xen repo), and by recognizing that you'd also
 notice that this is part of a public ABI, and hence can't be removed,
 but at best can be documented as obsolete. Of course you'd first
 need to check whether the hypervisor makes any use of that bit
 when passed down from Dom0.
 

Right, this patch mostly likes a reminder,
hoping someone will try it and verify it's fail or OK,
since I can't test this for now.

Sorry and Thanks!


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 3/4] sys/reboot: boolize C_A_D

2013-06-02 Thread li guang
在 2013-05-31五的 16:02 -0700,Andrew Morton写道:
> On Thu, 30 May 2013 15:58:05 +0800 liguang  wrote:
> 
> > --- a/include/linux/reboot.h
> > +++ b/include/linux/reboot.h
> > @@ -35,7 +35,7 @@ extern void kernel_restart(char *cmd);
> >  extern void kernel_halt(void);
> >  extern void kernel_power_off(void);
> >  
> > -extern int C_A_D; /* for sysctl */
> > +extern bool C_A_D; /* for sysctl */
> >  void ctrl_alt_del(void);
> 
> This means that the pointer in kernel/sysctl.c:kern_table.data now
> points at a bool but is declared to have size sizeof(int).
> 
> That happens to work with current gcc verions, but there's no rule
> which states that sizeof(bool) must equal sizeof(int).
> 
> And I'm not sure that changing kern_table to use sizeof(bool) is really
> worth all the bother.

OK, got it,

Thanks!

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 3/4] sys/reboot: boolize C_A_D

2013-06-02 Thread li guang
在 2013-05-31五的 16:02 -0700,Andrew Morton写道:
 On Thu, 30 May 2013 15:58:05 +0800 liguang lig.f...@cn.fujitsu.com wrote:
 
  --- a/include/linux/reboot.h
  +++ b/include/linux/reboot.h
  @@ -35,7 +35,7 @@ extern void kernel_restart(char *cmd);
   extern void kernel_halt(void);
   extern void kernel_power_off(void);
   
  -extern int C_A_D; /* for sysctl */
  +extern bool C_A_D; /* for sysctl */
   void ctrl_alt_del(void);
 
 This means that the pointer in kernel/sysctl.c:kern_table.data now
 points at a bool but is declared to have size sizeof(int).
 
 That happens to work with current gcc verions, but there's no rule
 which states that sizeof(bool) must equal sizeof(int).
 
 And I'm not sure that changing kern_table to use sizeof(bool) is really
 worth all the bother.

OK, got it,

Thanks!

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] powerpc/pseries: use 'true' instead of '1' for orderly_poweroff

2013-05-30 Thread li guang
在 2013-05-30四的 00:14 -0700,Joe Perches写道:
> On Thu, 2013-05-30 at 15:07 +0800, liguang wrote:
> > orderly_poweroff is expecting a bool parameter, so
> > use 'ture' instead '1'
> []
> > diff --git a/arch/powerpc/platforms/pseries/ras.c 
> > b/arch/powerpc/platforms/pseries/ras.c
> []
> > @@ -162,7 +162,7 @@ void rtas_parse_epow_errlog(struct rtas_error_log *log)
> >  
> > case EPOW_SYSTEM_HALT:
> > pr_emerg("Firmware initiated power off");
> > -   orderly_poweroff(1);
> > +   orderly_poweroff(ture);
> > break;
> 
> Compile your patches _before_ submitting them please.
> 
> "true" not "ture" here and in the commit message.
> 

right, thanks!


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] powerpc/pseries: use 'true' instead of '1' for orderly_poweroff

2013-05-30 Thread li guang
在 2013-05-30四的 00:14 -0700,Joe Perches写道:
 On Thu, 2013-05-30 at 15:07 +0800, liguang wrote:
  orderly_poweroff is expecting a bool parameter, so
  use 'ture' instead '1'
 []
  diff --git a/arch/powerpc/platforms/pseries/ras.c 
  b/arch/powerpc/platforms/pseries/ras.c
 []
  @@ -162,7 +162,7 @@ void rtas_parse_epow_errlog(struct rtas_error_log *log)
   
  case EPOW_SYSTEM_HALT:
  pr_emerg(Firmware initiated power off);
  -   orderly_poweroff(1);
  +   orderly_poweroff(ture);
  break;
 
 Compile your patches _before_ submitting them please.
 
 true not ture here and in the commit message.
 

right, thanks!


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] drivers: idle: comment DEBUG

2013-05-22 Thread li guang
ping ...

在 2013-04-25四的 15:00 +0800,liguang写道:
> seems we should comment DEBUG as above comment said:
> "/* un-comment DEBUG to enable pr_debug() statements */"
> 
> now, pr_debug is already enabled as DEBUG been defined.
> 
> Signed-off-by: liguang 
> ---
>  drivers/idle/intel_idle.c |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
> index 5d66750..9618586 100644
> --- a/drivers/idle/intel_idle.c
> +++ b/drivers/idle/intel_idle.c
> @@ -51,7 +51,7 @@
>   */
>  
>  /* un-comment DEBUG to enable pr_debug() statements */
> -#define DEBUG
> +/* #define DEBUG */
>  
>  #include 
>  #include 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [rebased-again][PATCH 0/4] acpi: do some changes for numa info

2013-05-22 Thread li guang
ping ...

again.

在 2013-02-25一的 10:38 +0800,liguang写道:
> just do some trivial changes to make acpi's numa info
> operation more cleaner.
> 
> ChangeLog
> 
> v3->v4
>  1.fix srat_disabled function
>  spotted by Yasuaki Ishimatsu 
> 
> v2->v3
>  1. rebase on linux-next
>  2. bring back lost Makefile changes
>  spotted by David Rientjes 
>  spotted by Yasuaki Ishimatsu 
> 
> v1->v2
>  1. fix-up several coding issues
>  2. finish srat.c change
>  spotted by David Rientjes 
> 
> Li Guang (4)
>acpi: move x86/mm/srat.c to x86/kernel/acpi/srat.c
>numa: avoid export acpi_numa variable
>acpi: add clock_domain field to acpi_srat_cpu_affinity
>remove include asm/acpi.h in process_driver.c
> 
> arch/x86/include/asm/acpi.h | 2 +-
> arch/x86/kernel/acpi/Makefile   | 1 +
> arch/x86/kernel/acpi/srat.c | 299 
> +
> arch/x86/mm/Makefile| 1 -
> arch/x86/mm/numa.c  | 2 +-
> arch/x86/mm/srat.c  | 278 
> -
> arch/x86/xen/enlighten.c| 2 +-
> drivers/acpi/processor_driver.c | 1 -
> include/acpi/actbl1.h   | 2 +-
> 9 files changed, 296 insertions(+), 292 deletions(-)
>  create mode 100644 arch/x86/kernel/acpi/srat.c
>  delete mode 100644 arch/x86/mm/srat.c
>  
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [rebased-again][PATCH 0/4] acpi: do some changes for numa info

2013-05-22 Thread li guang
ping ...

again.

在 2013-02-25一的 10:38 +0800,liguang写道:
 just do some trivial changes to make acpi's numa info
 operation more cleaner.
 
 ChangeLog
 
 v3-v4
  1.fix srat_disabled function
  spotted by Yasuaki Ishimatsu isimatu.yasu...@jp.fujitsu.com
 
 v2-v3
  1. rebase on linux-next
  2. bring back lost Makefile changes
  spotted by David Rientjes rient...@google.com
  spotted by Yasuaki Ishimatsu isimatu.yasu...@jp.fujitsu.com
 
 v1-v2
  1. fix-up several coding issues
  2. finish srat.c change
  spotted by David Rientjes rient...@google.com
 
 Li Guang (4)
acpi: move x86/mm/srat.c to x86/kernel/acpi/srat.c
numa: avoid export acpi_numa variable
acpi: add clock_domain field to acpi_srat_cpu_affinity
remove include asm/acpi.h in process_driver.c
 
 arch/x86/include/asm/acpi.h | 2 +-
 arch/x86/kernel/acpi/Makefile   | 1 +
 arch/x86/kernel/acpi/srat.c | 299 
 +
 arch/x86/mm/Makefile| 1 -
 arch/x86/mm/numa.c  | 2 +-
 arch/x86/mm/srat.c  | 278 
 -
 arch/x86/xen/enlighten.c| 2 +-
 drivers/acpi/processor_driver.c | 1 -
 include/acpi/actbl1.h   | 2 +-
 9 files changed, 296 insertions(+), 292 deletions(-)
  create mode 100644 arch/x86/kernel/acpi/srat.c
  delete mode 100644 arch/x86/mm/srat.c
  
 --
 To unsubscribe from this list: send the line unsubscribe linux-kernel in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 Please read the FAQ at  http://www.tux.org/lkml/


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] drivers: idle: comment DEBUG

2013-05-22 Thread li guang
ping ...

在 2013-04-25四的 15:00 +0800,liguang写道:
 seems we should comment DEBUG as above comment said:
 /* un-comment DEBUG to enable pr_debug() statements */
 
 now, pr_debug is already enabled as DEBUG been defined.
 
 Signed-off-by: liguang lig.f...@cn.fujitsu.com
 ---
  drivers/idle/intel_idle.c |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)
 
 diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
 index 5d66750..9618586 100644
 --- a/drivers/idle/intel_idle.c
 +++ b/drivers/idle/intel_idle.c
 @@ -51,7 +51,7 @@
   */
  
  /* un-comment DEBUG to enable pr_debug() statements */
 -#define DEBUG
 +/* #define DEBUG */
  
  #include linux/kernel.h
  #include linux/cpuidle.h


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] cpu remove CONFIG_INIT_ALL_POSSIBLE check

2013-05-15 Thread li guang
在 2013-05-15三的 13:58 +0530,Srivatsa S. Bhat写道:
> On 05/15/2013 01:20 PM, liguang wrote:
> > seems CONFIG_INIT_ALL_POSSIBLE is obsolete now.
> > 
> 
> Um? A simple grep showed me this:
> 
> arch/s390/Kconfig:  select INIT_ALL_POSSIBLE
> arch/m32r/Kconfig:  select INIT_ALL_POSSIBLE
> arch/parisc/Kconfig:select INIT_ALL_POSSIBLE
> init/Kconfig:config INIT_ALL_POSSIBLE
> 

Oh, yes, 
I was fooled by a script which is not aiming to do this check.

Thanks!

> > Signed-off-by: liguang 
> > ---
> >  kernel/cpu.c |5 -
> >  1 files changed, 0 insertions(+), 5 deletions(-)
> > 
> > diff --git a/kernel/cpu.c b/kernel/cpu.c
> > index cd166d3..2697d1a 100644
> > --- a/kernel/cpu.c
> > +++ b/kernel/cpu.c
> > @@ -662,12 +662,7 @@ EXPORT_SYMBOL_GPL(cpu_bit_bitmap);
> >  const DECLARE_BITMAP(cpu_all_bits, NR_CPUS) = CPU_BITS_ALL;
> >  EXPORT_SYMBOL(cpu_all_bits);
> > 
> > -#ifdef CONFIG_INIT_ALL_POSSIBLE
> > -static DECLARE_BITMAP(cpu_possible_bits, CONFIG_NR_CPUS) __read_mostly
> > -   = CPU_BITS_ALL;
> > -#else
> >  static DECLARE_BITMAP(cpu_possible_bits, CONFIG_NR_CPUS) __read_mostly;
> > -#endif
> >  const struct cpumask *const cpu_possible_mask = 
> > to_cpumask(cpu_possible_bits);
> >  EXPORT_SYMBOL(cpu_possible_mask);
> > 
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] cpu remove CONFIG_INIT_ALL_POSSIBLE check

2013-05-15 Thread li guang
在 2013-05-15三的 13:58 +0530,Srivatsa S. Bhat写道:
 On 05/15/2013 01:20 PM, liguang wrote:
  seems CONFIG_INIT_ALL_POSSIBLE is obsolete now.
  
 
 Um? A simple grep showed me this:
 
 arch/s390/Kconfig:  select INIT_ALL_POSSIBLE
 arch/m32r/Kconfig:  select INIT_ALL_POSSIBLE
 arch/parisc/Kconfig:select INIT_ALL_POSSIBLE
 init/Kconfig:config INIT_ALL_POSSIBLE
 

Oh, yes, 
I was fooled by a script which is not aiming to do this check.

Thanks!

  Signed-off-by: liguang lig.f...@cn.fujitsu.com
  ---
   kernel/cpu.c |5 -
   1 files changed, 0 insertions(+), 5 deletions(-)
  
  diff --git a/kernel/cpu.c b/kernel/cpu.c
  index cd166d3..2697d1a 100644
  --- a/kernel/cpu.c
  +++ b/kernel/cpu.c
  @@ -662,12 +662,7 @@ EXPORT_SYMBOL_GPL(cpu_bit_bitmap);
   const DECLARE_BITMAP(cpu_all_bits, NR_CPUS) = CPU_BITS_ALL;
   EXPORT_SYMBOL(cpu_all_bits);
  
  -#ifdef CONFIG_INIT_ALL_POSSIBLE
  -static DECLARE_BITMAP(cpu_possible_bits, CONFIG_NR_CPUS) __read_mostly
  -   = CPU_BITS_ALL;
  -#else
   static DECLARE_BITMAP(cpu_possible_bits, CONFIG_NR_CPUS) __read_mostly;
  -#endif
   const struct cpumask *const cpu_possible_mask = 
  to_cpumask(cpu_possible_bits);
   EXPORT_SYMBOL(cpu_possible_mask);
  
 


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] cpu: rid cpu_hotplug_disabled check for cpu_down()

2013-04-29 Thread li guang
在 2013-04-29一的 10:29 +0530,Srivatsa S. Bhat写道:
> On 04/29/2013 10:12 AM, li guang wrote:
> > 在 2013-04-29一的 10:00 +0530,Srivatsa S. Bhat写道:
> >> On 04/29/2013 08:19 AM, liguang wrote:
> >>> in cpu_down(), _cpu_down() will do
> >>> "
> >>> if (num_online_cpus() == 1)
> >>>  return -EBUSY;
> >>> "
> >>> when cpu_hotplug_disabled was set, num_online_cpus
> >>> will return 1 for there's only 1 boot cpu.
> >>> so, it's unnecessary to check cpu_hotplug_disabled
> >>> here.
> >>>
> >>
> >> The 2 checks serve very different purposes; they are not the same!
> > 
> > purposes are different, but I think effects are same for this case,
> > the statement 'if ((num_online_cpus() == 1)' seems
> > have same effect with cpu_hotplug_disabled here,
> > because when cpu_hotplug_disabled, only boot cpu is online
> > 
> 
> Why do you keep saying that? They are *two* *different* checks!
> Whether they happen to have the same effect or not completely depends
> on the particular *usecase*. And for example, in the suspend/resume case,
> when the flag 'cpu_hotplug_disabled' is set, *all* CPUs are online!
> Only much later do we actually call disable_nonboot_cpus() to offline
> the non-boot CPUs. Take a look at the following functions, then you'll
> see what scenario I'm referring to:
> cpu_hotplug_disable_before_freeze(), cpu_hotplug_disable_after_thaw()
> 
> Also, recently, there was another usecase for the cpu_hotplug_disabled
> flag, in the reboot code. Here is the link to that discussion:
> http://thread.gmane.org/gmane.linux.kernel/1480380
> 

OK, Thanks!

> 
> >>
> >> The num_online_cpus() check is to ensure that the user doesn't do
> >> something insane like trying to offline the last online CPU in the
> >> system.
> >>
> >> Whereas, the flag 'cpu_hotplug_disabled' is used to prevent user-
> >> triggered CPU hotplug (such as those initiated through sysfs).
> >> This is useful in cases where the system itself wants to initiate CPU
> >> hotplug and it doesn't want annoying races with CPU hotplug going
> >> on in parallel due to other reasons. One such case is suspend/resume.
> >> That's why, if you have noticed, the suspend/resume code invokes the
> >> _cpu_down() version, in order to bypass the flag and get its job done.
> >>
> >> So, no, I think the check needs to stay.
> >>
> >> Regards,
> >> Srivatsa S. Bhat
> >>
> >>> Signed-off-by: liguang 
> >>> ---
> >>>  kernel/cpu.c |6 --
> >>>  1 files changed, 0 insertions(+), 6 deletions(-)
> >>>
> >>> diff --git a/kernel/cpu.c b/kernel/cpu.c
> >>> index b5e4ab2..cd166d3 100644
> >>> --- a/kernel/cpu.c
> >>> +++ b/kernel/cpu.c
> >>> @@ -330,14 +330,8 @@ int __ref cpu_down(unsigned int cpu)
> >>>
> >>>   cpu_maps_update_begin();
> >>>
> >>> - if (cpu_hotplug_disabled) {
> >>> - err = -EBUSY;
> >>> - goto out;
> >>> - }
> >>> -
> >>>   err = _cpu_down(cpu, 0);
> >>>
> >>> -out:
> >>>   cpu_maps_update_done();
> >>>   return err;
> >>>  }
> >>>
> >>
> > 
> > 
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] cpu: rid cpu_hotplug_disabled check for cpu_down()

2013-04-29 Thread li guang
在 2013-04-29一的 10:29 +0530,Srivatsa S. Bhat写道:
 On 04/29/2013 10:12 AM, li guang wrote:
  在 2013-04-29一的 10:00 +0530,Srivatsa S. Bhat写道:
  On 04/29/2013 08:19 AM, liguang wrote:
  in cpu_down(), _cpu_down() will do
  
  if (num_online_cpus() == 1)
   return -EBUSY;
  
  when cpu_hotplug_disabled was set, num_online_cpus
  will return 1 for there's only 1 boot cpu.
  so, it's unnecessary to check cpu_hotplug_disabled
  here.
 
 
  The 2 checks serve very different purposes; they are not the same!
  
  purposes are different, but I think effects are same for this case,
  the statement 'if ((num_online_cpus() == 1)' seems
  have same effect with cpu_hotplug_disabled here,
  because when cpu_hotplug_disabled, only boot cpu is online
  
 
 Why do you keep saying that? They are *two* *different* checks!
 Whether they happen to have the same effect or not completely depends
 on the particular *usecase*. And for example, in the suspend/resume case,
 when the flag 'cpu_hotplug_disabled' is set, *all* CPUs are online!
 Only much later do we actually call disable_nonboot_cpus() to offline
 the non-boot CPUs. Take a look at the following functions, then you'll
 see what scenario I'm referring to:
 cpu_hotplug_disable_before_freeze(), cpu_hotplug_disable_after_thaw()
 
 Also, recently, there was another usecase for the cpu_hotplug_disabled
 flag, in the reboot code. Here is the link to that discussion:
 http://thread.gmane.org/gmane.linux.kernel/1480380
 

OK, Thanks!

 
 
  The num_online_cpus() check is to ensure that the user doesn't do
  something insane like trying to offline the last online CPU in the
  system.
 
  Whereas, the flag 'cpu_hotplug_disabled' is used to prevent user-
  triggered CPU hotplug (such as those initiated through sysfs).
  This is useful in cases where the system itself wants to initiate CPU
  hotplug and it doesn't want annoying races with CPU hotplug going
  on in parallel due to other reasons. One such case is suspend/resume.
  That's why, if you have noticed, the suspend/resume code invokes the
  _cpu_down() version, in order to bypass the flag and get its job done.
 
  So, no, I think the check needs to stay.
 
  Regards,
  Srivatsa S. Bhat
 
  Signed-off-by: liguang lig.f...@cn.fujitsu.com
  ---
   kernel/cpu.c |6 --
   1 files changed, 0 insertions(+), 6 deletions(-)
 
  diff --git a/kernel/cpu.c b/kernel/cpu.c
  index b5e4ab2..cd166d3 100644
  --- a/kernel/cpu.c
  +++ b/kernel/cpu.c
  @@ -330,14 +330,8 @@ int __ref cpu_down(unsigned int cpu)
 
cpu_maps_update_begin();
 
  - if (cpu_hotplug_disabled) {
  - err = -EBUSY;
  - goto out;
  - }
  -
err = _cpu_down(cpu, 0);
 
  -out:
cpu_maps_update_done();
return err;
   }
 
 
  
  
 


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] cpu: rid cpu_hotplug_disabled check for cpu_down()

2013-04-28 Thread li guang
在 2013-04-29一的 10:00 +0530,Srivatsa S. Bhat写道:
> On 04/29/2013 08:19 AM, liguang wrote:
> > in cpu_down(), _cpu_down() will do
> > "
> > if (num_online_cpus() == 1)
> >  return -EBUSY;
> > "
> > when cpu_hotplug_disabled was set, num_online_cpus
> > will return 1 for there's only 1 boot cpu.
> > so, it's unnecessary to check cpu_hotplug_disabled
> > here.
> >
> 
> The 2 checks serve very different purposes; they are not the same!

purposes are different, but I think effects are same for this case,
the statement 'if ((num_online_cpus() == 1)' seems
have same effect with cpu_hotplug_disabled here,
because when cpu_hotplug_disabled, only boot cpu is online

> 
> The num_online_cpus() check is to ensure that the user doesn't do
> something insane like trying to offline the last online CPU in the
> system.
> 
> Whereas, the flag 'cpu_hotplug_disabled' is used to prevent user-
> triggered CPU hotplug (such as those initiated through sysfs).
> This is useful in cases where the system itself wants to initiate CPU
> hotplug and it doesn't want annoying races with CPU hotplug going
> on in parallel due to other reasons. One such case is suspend/resume.
> That's why, if you have noticed, the suspend/resume code invokes the
> _cpu_down() version, in order to bypass the flag and get its job done.
> 
> So, no, I think the check needs to stay.
> 
> Regards,
> Srivatsa S. Bhat
> 
> > Signed-off-by: liguang 
> > ---
> >  kernel/cpu.c |6 --
> >  1 files changed, 0 insertions(+), 6 deletions(-)
> > 
> > diff --git a/kernel/cpu.c b/kernel/cpu.c
> > index b5e4ab2..cd166d3 100644
> > --- a/kernel/cpu.c
> > +++ b/kernel/cpu.c
> > @@ -330,14 +330,8 @@ int __ref cpu_down(unsigned int cpu)
> > 
> > cpu_maps_update_begin();
> > 
> > -   if (cpu_hotplug_disabled) {
> > -   err = -EBUSY;
> > -   goto out;
> > -   }
> > -
> > err = _cpu_down(cpu, 0);
> > 
> > -out:
> > cpu_maps_update_done();
> > return err;
> >  }
> > 
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] cpu: rid cpu_hotplug_disabled check for cpu_down()

2013-04-28 Thread li guang
在 2013-04-29一的 10:00 +0530,Srivatsa S. Bhat写道:
 On 04/29/2013 08:19 AM, liguang wrote:
  in cpu_down(), _cpu_down() will do
  
  if (num_online_cpus() == 1)
   return -EBUSY;
  
  when cpu_hotplug_disabled was set, num_online_cpus
  will return 1 for there's only 1 boot cpu.
  so, it's unnecessary to check cpu_hotplug_disabled
  here.
 
 
 The 2 checks serve very different purposes; they are not the same!

purposes are different, but I think effects are same for this case,
the statement 'if ((num_online_cpus() == 1)' seems
have same effect with cpu_hotplug_disabled here,
because when cpu_hotplug_disabled, only boot cpu is online

 
 The num_online_cpus() check is to ensure that the user doesn't do
 something insane like trying to offline the last online CPU in the
 system.
 
 Whereas, the flag 'cpu_hotplug_disabled' is used to prevent user-
 triggered CPU hotplug (such as those initiated through sysfs).
 This is useful in cases where the system itself wants to initiate CPU
 hotplug and it doesn't want annoying races with CPU hotplug going
 on in parallel due to other reasons. One such case is suspend/resume.
 That's why, if you have noticed, the suspend/resume code invokes the
 _cpu_down() version, in order to bypass the flag and get its job done.
 
 So, no, I think the check needs to stay.
 
 Regards,
 Srivatsa S. Bhat
 
  Signed-off-by: liguang lig.f...@cn.fujitsu.com
  ---
   kernel/cpu.c |6 --
   1 files changed, 0 insertions(+), 6 deletions(-)
  
  diff --git a/kernel/cpu.c b/kernel/cpu.c
  index b5e4ab2..cd166d3 100644
  --- a/kernel/cpu.c
  +++ b/kernel/cpu.c
  @@ -330,14 +330,8 @@ int __ref cpu_down(unsigned int cpu)
  
  cpu_maps_update_begin();
  
  -   if (cpu_hotplug_disabled) {
  -   err = -EBUSY;
  -   goto out;
  -   }
  -
  err = _cpu_down(cpu, 0);
  
  -out:
  cpu_maps_update_done();
  return err;
   }
  
 


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] smp: use '|=' for csd_lock

2013-04-23 Thread li guang
在 2013-04-23二的 15:40 -0700,Andrew Morton写道:
> On Mon, 22 Apr 2013 13:47:22 +0800 liguang  wrote:
> 
> > originally, 'data->flags = CSD_FLAG_LOCK',
> > and we use 'data->flags &= ~CSD_FLAG_LOCK'
> > for csd_unlock, they are not symmetrix operations
> > so use '|=' instead of '='.
> > though, now data->flags only hold CSD_FLAG_LOCK,
> > it's not so meaningful to use '|=' to set 1 bit,
> > and '&= ~' to clear 1 bit.
> > 
> > Signed-off-by: liguang 
> > ---
> >  kernel/smp.c |2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/kernel/smp.c b/kernel/smp.c
> > index 1818dc0..2d5deb4 100644
> > --- a/kernel/smp.c
> > +++ b/kernel/smp.c
> > @@ -109,7 +109,7 @@ static void csd_lock_wait(struct call_single_data *data)
> >  static void csd_lock(struct call_single_data *data)
> >  {
> > csd_lock_wait(data);
> > -   data->flags = CSD_FLAG_LOCK;
> > +   data->flags |= CSD_FLAG_LOCK;
> >  
> > /*
> 
> call_single_data.flags is in fact presently a boolean - we only use one
> bit in that word.  We could remove all the &=, |=, & and | operations
> on call_single_data.flags and treat it as a boolean.  That would
> probably result in faster and smaller code.
> 
> But leaving the other 31 bits alone and reserved-for-future-use is not
> a bad thing.  But if we're going to do that we should do it consistently.
> 
> I rewrote your changelog ;)

That's fine, Thanks!

> 
> 
> From: liguang 
> Subject: kernel/smp.c: use '|=' for csd_lock
> 
> csd_lock() uses assignment to data->flags rather than |=.  That is not
> buggy at present because only one bit (CSD_FLAG_LOCK) is defined in
> call_single_data.flags.
> 
> But it will become buggy if we later add another flag, so fix it now.
> 
> Signed-off-by: liguang 
> Cc: Peter Zijlstra 
> Cc: Oleg Nesterov 
> Cc: Ingo Molnar 
> Signed-off-by: Andrew Morton 
> ---
> 
>  kernel/smp.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff -puN kernel/smp.c~kernel-smpc-use-=-for-csd_lock kernel/smp.c
> --- a/kernel/smp.c~kernel-smpc-use-=-for-csd_lock
> +++ a/kernel/smp.c
> @@ -110,7 +110,7 @@ static void csd_lock_wait(struct call_si
>  static void csd_lock(struct call_single_data *data)
>  {
>   csd_lock_wait(data);
> - data->flags = CSD_FLAG_LOCK;
> + data->flags |= CSD_FLAG_LOCK;
>  
>   /*
>* prevent CPU from reordering the above assignment
> _
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] smp: use '|=' for csd_lock

2013-04-23 Thread li guang
在 2013-04-23二的 15:40 -0700,Andrew Morton写道:
 On Mon, 22 Apr 2013 13:47:22 +0800 liguang lig.f...@cn.fujitsu.com wrote:
 
  originally, 'data-flags = CSD_FLAG_LOCK',
  and we use 'data-flags = ~CSD_FLAG_LOCK'
  for csd_unlock, they are not symmetrix operations
  so use '|=' instead of '='.
  though, now data-flags only hold CSD_FLAG_LOCK,
  it's not so meaningful to use '|=' to set 1 bit,
  and '= ~' to clear 1 bit.
  
  Signed-off-by: liguang lig.f...@cn.fujitsu.com
  ---
   kernel/smp.c |2 +-
   1 files changed, 1 insertions(+), 1 deletions(-)
  
  diff --git a/kernel/smp.c b/kernel/smp.c
  index 1818dc0..2d5deb4 100644
  --- a/kernel/smp.c
  +++ b/kernel/smp.c
  @@ -109,7 +109,7 @@ static void csd_lock_wait(struct call_single_data *data)
   static void csd_lock(struct call_single_data *data)
   {
  csd_lock_wait(data);
  -   data-flags = CSD_FLAG_LOCK;
  +   data-flags |= CSD_FLAG_LOCK;
   
  /*
 
 call_single_data.flags is in fact presently a boolean - we only use one
 bit in that word.  We could remove all the =, |=,  and | operations
 on call_single_data.flags and treat it as a boolean.  That would
 probably result in faster and smaller code.
 
 But leaving the other 31 bits alone and reserved-for-future-use is not
 a bad thing.  But if we're going to do that we should do it consistently.
 
 I rewrote your changelog ;)

That's fine, Thanks!

 
 
 From: liguang lig.f...@cn.fujitsu.com
 Subject: kernel/smp.c: use '|=' for csd_lock
 
 csd_lock() uses assignment to data-flags rather than |=.  That is not
 buggy at present because only one bit (CSD_FLAG_LOCK) is defined in
 call_single_data.flags.
 
 But it will become buggy if we later add another flag, so fix it now.
 
 Signed-off-by: liguang lig.f...@cn.fujitsu.com
 Cc: Peter Zijlstra a.p.zijls...@chello.nl
 Cc: Oleg Nesterov o...@redhat.com
 Cc: Ingo Molnar mi...@elte.hu
 Signed-off-by: Andrew Morton a...@linux-foundation.org
 ---
 
  kernel/smp.c |2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff -puN kernel/smp.c~kernel-smpc-use-=-for-csd_lock kernel/smp.c
 --- a/kernel/smp.c~kernel-smpc-use-=-for-csd_lock
 +++ a/kernel/smp.c
 @@ -110,7 +110,7 @@ static void csd_lock_wait(struct call_si
  static void csd_lock(struct call_single_data *data)
  {
   csd_lock_wait(data);
 - data-flags = CSD_FLAG_LOCK;
 + data-flags |= CSD_FLAG_LOCK;
  
   /*
* prevent CPU from reordering the above assignment
 _
 


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] smp: use '|=' for csd_lock

2013-04-22 Thread li guang
在 2013-04-22一的 08:18 +0200,Sedat Dilek写道:
> On Mon, Apr 22, 2013 at 7:47 AM, liguang  wrote:
> > originally, 'data->flags = CSD_FLAG_LOCK',
> > and we use 'data->flags &= ~CSD_FLAG_LOCK'
> > for csd_unlock, they are not symmetrix operations
> > so use '|=' instead of '='.
> > though, now data->flags only hold CSD_FLAG_LOCK,
> > it's not so meaningful to use '|=' to set 1 bit,
> > and '&= ~' to clear 1 bit.
> >
> 
> Hi,
> 
> what's the reason I got CCed on this two patches? The ipc-sem-next
> issue I reported?

sorry,
just use the result of scripts/get_maintainer.pl

> 
> Against what tree are those patches?
> They are not compatible with Linux-Next (next-20130419).

main

> 
> Thanks.
> 
> Regards,
> - Sedat -
> 
> [1] http://marc.info/?t=13663145795=1=2
> 
> > Signed-off-by: liguang 
> > ---
> >  kernel/smp.c |2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/kernel/smp.c b/kernel/smp.c
> > index 1818dc0..2d5deb4 100644
> > --- a/kernel/smp.c
> > +++ b/kernel/smp.c
> > @@ -109,7 +109,7 @@ static void csd_lock_wait(struct call_single_data *data)
> >  static void csd_lock(struct call_single_data *data)
> >  {
> > csd_lock_wait(data);
> > -   data->flags = CSD_FLAG_LOCK;
> > +   data->flags |= CSD_FLAG_LOCK;
> >
> > /*
> >  * prevent CPU from reordering the above assignment
> > --
> > 1.7.2.5
> >


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] smp: use '|=' for csd_lock

2013-04-22 Thread li guang
在 2013-04-22一的 08:18 +0200,Sedat Dilek写道:
 On Mon, Apr 22, 2013 at 7:47 AM, liguang lig.f...@cn.fujitsu.com wrote:
  originally, 'data-flags = CSD_FLAG_LOCK',
  and we use 'data-flags = ~CSD_FLAG_LOCK'
  for csd_unlock, they are not symmetrix operations
  so use '|=' instead of '='.
  though, now data-flags only hold CSD_FLAG_LOCK,
  it's not so meaningful to use '|=' to set 1 bit,
  and '= ~' to clear 1 bit.
 
 
 Hi,
 
 what's the reason I got CCed on this two patches? The ipc-sem-next
 issue I reported?

sorry,
just use the result of scripts/get_maintainer.pl

 
 Against what tree are those patches?
 They are not compatible with Linux-Next (next-20130419).

main

 
 Thanks.
 
 Regards,
 - Sedat -
 
 [1] http://marc.info/?t=13663145795r=1w=2
 
  Signed-off-by: liguang lig.f...@cn.fujitsu.com
  ---
   kernel/smp.c |2 +-
   1 files changed, 1 insertions(+), 1 deletions(-)
 
  diff --git a/kernel/smp.c b/kernel/smp.c
  index 1818dc0..2d5deb4 100644
  --- a/kernel/smp.c
  +++ b/kernel/smp.c
  @@ -109,7 +109,7 @@ static void csd_lock_wait(struct call_single_data *data)
   static void csd_lock(struct call_single_data *data)
   {
  csd_lock_wait(data);
  -   data-flags = CSD_FLAG_LOCK;
  +   data-flags |= CSD_FLAG_LOCK;
 
  /*
   * prevent CPU from reordering the above assignment
  --
  1.7.2.5
 


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [rebased-again][PATCH 0/4] acpi: do some changes for numa info

2013-04-08 Thread li guang
ping ...

在 2013-03-04一的 09:21 +0800,li guang写道:
> Hi, Thomas, Peter, Ingo
> 
> can you help to merge these 4 patches?
> or do you have any other comments?
> 
> 
> 在 2013-02-25一的 10:38 +0800,liguang写道:
> > just do some trivial changes to make acpi's numa info
> > operation more cleaner.
> > 
> > ChangeLog
> > 
> > v3->v4
> >  1.fix srat_disabled function
> >  spotted by Yasuaki Ishimatsu 
> > 
> > v2->v3
> >  1. rebase on linux-next
> >  2. bring back lost Makefile changes
> >  spotted by David Rientjes 
> >  spotted by Yasuaki Ishimatsu 
> > 
> > v1->v2
> >  1. fix-up several coding issues
> >  2. finish srat.c change
> >  spotted by David Rientjes 
> > 
> > Li Guang (4)
> >  acpi: move x86/mm/srat.c to x86/kernel/acpi/srat.c
> >  numa: avoid export acpi_numa variable
> >  acpi: add clock_domain field to acpi_srat_cpu_affinity
> >  remove include asm/acpi.h in process_driver.c
> > 
> > arch/x86/include/asm/acpi.h | 2 +-
> > arch/x86/kernel/acpi/Makefile   | 1 +
> > arch/x86/kernel/acpi/srat.c | 299 
> > +
> > arch/x86/mm/Makefile| 1 -
> > arch/x86/mm/numa.c  | 2 +-
> > arch/x86/mm/srat.c  | 278 
> > -
> > arch/x86/xen/enlighten.c| 2 +-
> > drivers/acpi/processor_driver.c | 1 -
> > include/acpi/actbl1.h   | 2 +-
> > 9 files changed, 296 insertions(+), 292 deletions(-)
> >  create mode 100644 arch/x86/kernel/acpi/srat.c
> >  delete mode 100644 arch/x86/mm/srat.c
> >  
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majord...@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [rebased-again][PATCH 0/4] acpi: do some changes for numa info

2013-04-08 Thread li guang
ping ...

在 2013-03-04一的 09:21 +0800,li guang写道:
 Hi, Thomas, Peter, Ingo
 
 can you help to merge these 4 patches?
 or do you have any other comments?
 
 
 在 2013-02-25一的 10:38 +0800,liguang写道:
  just do some trivial changes to make acpi's numa info
  operation more cleaner.
  
  ChangeLog
  
  v3-v4
   1.fix srat_disabled function
   spotted by Yasuaki Ishimatsu isimatu.yasu...@jp.fujitsu.com
  
  v2-v3
   1. rebase on linux-next
   2. bring back lost Makefile changes
   spotted by David Rientjes rient...@google.com
   spotted by Yasuaki Ishimatsu isimatu.yasu...@jp.fujitsu.com
  
  v1-v2
   1. fix-up several coding issues
   2. finish srat.c change
   spotted by David Rientjes rient...@google.com
  
  Li Guang (4)
   acpi: move x86/mm/srat.c to x86/kernel/acpi/srat.c
   numa: avoid export acpi_numa variable
   acpi: add clock_domain field to acpi_srat_cpu_affinity
   remove include asm/acpi.h in process_driver.c
  
  arch/x86/include/asm/acpi.h | 2 +-
  arch/x86/kernel/acpi/Makefile   | 1 +
  arch/x86/kernel/acpi/srat.c | 299 
  +
  arch/x86/mm/Makefile| 1 -
  arch/x86/mm/numa.c  | 2 +-
  arch/x86/mm/srat.c  | 278 
  -
  arch/x86/xen/enlighten.c| 2 +-
  drivers/acpi/processor_driver.c | 1 -
  include/acpi/actbl1.h   | 2 +-
  9 files changed, 296 insertions(+), 292 deletions(-)
   create mode 100644 arch/x86/kernel/acpi/srat.c
   delete mode 100644 arch/x86/mm/srat.c
   
  --
  To unsubscribe from this list: send the line unsubscribe linux-kernel in
  the body of a message to majord...@vger.kernel.org
  More majordomo info at  http://vger.kernel.org/majordomo-info.html
  Please read the FAQ at  http://www.tux.org/lkml/
 


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Makefile: allow no update .config build

2013-03-28 Thread li guang
在 2013-03-28四的 10:13 -0700,Randy Dunlap写道:
> On 03/28/13 00:28, liguang wrote:
> > if we pull some commits from other git repo
> > which bring in a few CONFIG_* options, then
> > we have to build all again, but we do assure
> > these options are not interesting for us,
> > so the long waiting build will be offending.
> > this change help us to avoid all-build.
> > 
> > Signed-off-by: liguang 
> > ---
> >  Makefile |9 +
> >  1 files changed, 9 insertions(+), 0 deletions(-)
> > 
> > diff --git a/Makefile b/Makefile
> > index 54d2b2a..d10e505 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -106,6 +106,12 @@ ifeq ("$(origin W)", "command line")
> >export KBUILD_ENABLE_EXTRA_GCC_CHECKS := $(W)
> >  endif
> >  
> > +# KBUILD_NCU is set to do not update .config even
> > +# a few new CONFIG_* options appeared
> > +ifeq ("$(origin NCU)", "command line")
> > +  KBUILD_NCU := $(NCU)
> > +endif
> > +
> >  # That's our default target when none is given on the command line
> >  PHONY := _all
> >  _all:
> > @@ -540,8 +546,10 @@ $(KCONFIG_CONFIG) include/config/auto.conf.cmd: ;
> >  # with it and forgot to run make oldconfig.
> >  # if auto.conf.cmd is missing then we are probably in a cleaned tree so
> >  # we execute the config step to be sure to catch updated Kconfig files
> > +ifneq ($(KBUILD_NCU), 1)
> >  include/config/%.conf: $(KCONFIG_CONFIG) include/config/auto.conf.cmd
> > $(Q)$(MAKE) -f $(srctree)/Makefile silentoldconfig
> > +endif # $(KBUILD_NCU)
> >  else
> >  # external modules needs include/generated/autoconf.h and 
> > include/config/auto.conf
> >  # but do not care if they are up-to-date. Use auto.conf to trigger the test
> > @@ -1157,6 +1165,7 @@ help:
> > @echo  '2: warnings which occur quite often but may 
> > still be relevant'
> > @echo  '3: more obscure warnings, can most likely be 
> > ignored'
> > @echo  'Multiple levels can be combined with W=12 or 
> > W=123'
> > +   @echo  '  make NCU=1 [targets] no .config udate'
> 
> update

OK, I'll update.

Thanks!

> 
> > @echo  ''
> > @echo  'Execute "make" or "make all" to build all targets marked with 
> > [*] '
> > @echo  'For further info see the ./README file'
> > 
> 
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Makefile: allow no update .config build

2013-03-28 Thread li guang
在 2013-03-28四的 10:13 -0700,Randy Dunlap写道:
 On 03/28/13 00:28, liguang wrote:
  if we pull some commits from other git repo
  which bring in a few CONFIG_* options, then
  we have to build all again, but we do assure
  these options are not interesting for us,
  so the long waiting build will be offending.
  this change help us to avoid all-build.
  
  Signed-off-by: liguang lig.f...@cn.fujitsu.com
  ---
   Makefile |9 +
   1 files changed, 9 insertions(+), 0 deletions(-)
  
  diff --git a/Makefile b/Makefile
  index 54d2b2a..d10e505 100644
  --- a/Makefile
  +++ b/Makefile
  @@ -106,6 +106,12 @@ ifeq ($(origin W), command line)
 export KBUILD_ENABLE_EXTRA_GCC_CHECKS := $(W)
   endif
   
  +# KBUILD_NCU is set to do not update .config even
  +# a few new CONFIG_* options appeared
  +ifeq ($(origin NCU), command line)
  +  KBUILD_NCU := $(NCU)
  +endif
  +
   # That's our default target when none is given on the command line
   PHONY := _all
   _all:
  @@ -540,8 +546,10 @@ $(KCONFIG_CONFIG) include/config/auto.conf.cmd: ;
   # with it and forgot to run make oldconfig.
   # if auto.conf.cmd is missing then we are probably in a cleaned tree so
   # we execute the config step to be sure to catch updated Kconfig files
  +ifneq ($(KBUILD_NCU), 1)
   include/config/%.conf: $(KCONFIG_CONFIG) include/config/auto.conf.cmd
  $(Q)$(MAKE) -f $(srctree)/Makefile silentoldconfig
  +endif # $(KBUILD_NCU)
   else
   # external modules needs include/generated/autoconf.h and 
  include/config/auto.conf
   # but do not care if they are up-to-date. Use auto.conf to trigger the test
  @@ -1157,6 +1165,7 @@ help:
  @echo  '2: warnings which occur quite often but may 
  still be relevant'
  @echo  '3: more obscure warnings, can most likely be 
  ignored'
  @echo  'Multiple levels can be combined with W=12 or 
  W=123'
  +   @echo  '  make NCU=1 [targets] no .config udate'
 
 update

OK, I'll update.

Thanks!

 
  @echo  ''
  @echo  'Execute make or make all to build all targets marked with 
  [*] '
  @echo  'For further info see the ./README file'
  
 
 


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: PROBLEM: All CPUs in soft lockup

2013-03-26 Thread li guang
seems tasks are hogging your cpu/memory resource,
did you check status your servicing processes?

在 2013-03-27三的 12:55 +1100,Robert Norris写道:
> In the last two weeks we've had three servers (identical hardware,
> software and load) hang. The details in this report are from one that
> hung last night.
> 
> They're all IMAP servers servicing many hundreds of users, so several
> thousand processes and active connections. There's been two major
> application level changes in the last couple of weeks, corresponding to
> the time where these hangs started. One is that we now do mail event
> notifications directly to user clients, so more TCP connections. The
> other is that we're now maintaining live search indexes, so a lot more
> disk and tmpfs IO.
> 
> All that said, we're not under what we'd consider to be heavy load. When
> they're running, the servers are fast and responsive.
> 
> During the hang itself, the machine responds to pings, and TCP
> connections can be established, but the servicing processes never
> respond. The console shows a new "BUG: soft lockup" line every few
> seconds, and will not respond to keyboard input. It is a virtual console
> though, which may or may not make a difference, I'm not sure.
> 
> The kernel is 3.4.33 with AUFS patches applied. However there are no
> AUFS mounts on this machine; we use this elsewhere. If you think that's
> a problem I can rebuild for this machine without it.
> 
> Attached are various bits of information requested in REPORTING-BUGS.
> I'm not entirely sure what else is relevant. I'm happy to supply any
> other information and test things, just let me know.
> 
> Thanks,
> Rob.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: PROBLEM: All CPUs in soft lockup

2013-03-26 Thread li guang
seems tasks are hogging your cpu/memory resource,
did you check status your servicing processes?

在 2013-03-27三的 12:55 +1100,Robert Norris写道:
 In the last two weeks we've had three servers (identical hardware,
 software and load) hang. The details in this report are from one that
 hung last night.
 
 They're all IMAP servers servicing many hundreds of users, so several
 thousand processes and active connections. There's been two major
 application level changes in the last couple of weeks, corresponding to
 the time where these hangs started. One is that we now do mail event
 notifications directly to user clients, so more TCP connections. The
 other is that we're now maintaining live search indexes, so a lot more
 disk and tmpfs IO.
 
 All that said, we're not under what we'd consider to be heavy load. When
 they're running, the servers are fast and responsive.
 
 During the hang itself, the machine responds to pings, and TCP
 connections can be established, but the servicing processes never
 respond. The console shows a new BUG: soft lockup line every few
 seconds, and will not respond to keyboard input. It is a virtual console
 though, which may or may not make a difference, I'm not sure.
 
 The kernel is 3.4.33 with AUFS patches applied. However there are no
 AUFS mounts on this machine; we use this elsewhere. If you think that's
 a problem I can rebuild for this machine without it.
 
 Attached are various bits of information requested in REPORTING-BUGS.
 I'm not entirely sure what else is relevant. I'm happy to supply any
 other information and test things, just let me know.
 
 Thanks,
 Rob.


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/2] task_work: check callback if it's NULL

2013-03-14 Thread li guang
在 2013-03-15五的 09:43 +0800,Li Zefan写道:
> On 2013/3/15 9:26, li guang wrote:
> > 在 2013-03-15五的 09:01 +0800,Li Zefan写道:
> >> On 2013/3/15 8:20, li guang wrote:
> >>> 在 2013-03-14四的 15:43 +0100,Oleg Nesterov写道:
> >>>> On 03/14, liguang wrote:
> >>>>>
> >>>>> Signed-off-by: liguang 
> >>>>> ---
> >>>>>  kernel/task_work.c |3 ++-
> >>>>>  1 files changed, 2 insertions(+), 1 deletions(-)
> >>>>>
> >>>>> diff --git a/kernel/task_work.c b/kernel/task_work.c
> >>>>> index 0bf4258..f458b08 100644
> >>>>> --- a/kernel/task_work.c
> >>>>> +++ b/kernel/task_work.c
> >>>>> @@ -75,7 +75,8 @@ void task_work_run(void)
> >>>>>
> >>>>> do {
> >>>>> next = work->next;
> >>>>> -   work->func(work);
> >>>>> +   if (unlikely(work->func))
> >>>>> +   work->func(work);
> >>>>
> >>>> Why?
> >>>>
> >>>> Oleg.
> >>>>
> >>>
> >>> can we believe a callback always be call-able?
> >>> can it happened to be 0? e.g. wrong initialized.
> >>> of course, we can complain the caller, be why don't
> >>> we easily make it more safer?
> >>>
> >>
> >> Because you're not making things safer, but your're trying
> >> to cover up bugs...
> >>
> > 
> > Oh, that's a little harsh to a normal programmer like me :-)
> > for it seems you are asking me to be coding without any bug.
> > are you? or it is the theory of kernel coding?
> > 
> 
> And you make a bug, and you want the kernel to cover up the bug
> instead of crash on a null pointer deref so you'll know you've
> made a bug?
> 
> Why we check if a callback is NULL before calling it? Because
> it's allowed to be. Why we don't check if a callback is NULL?
> Because it's not supposed to be.
> 

OK, Thanks for your reminder.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/2] task_work: check callback if it's NULL

2013-03-14 Thread li guang
在 2013-03-15五的 09:01 +0800,Li Zefan写道:
> On 2013/3/15 8:20, li guang wrote:
> > 在 2013-03-14四的 15:43 +0100,Oleg Nesterov写道:
> >> On 03/14, liguang wrote:
> >>>
> >>> Signed-off-by: liguang 
> >>> ---
> >>>  kernel/task_work.c |3 ++-
> >>>  1 files changed, 2 insertions(+), 1 deletions(-)
> >>>
> >>> diff --git a/kernel/task_work.c b/kernel/task_work.c
> >>> index 0bf4258..f458b08 100644
> >>> --- a/kernel/task_work.c
> >>> +++ b/kernel/task_work.c
> >>> @@ -75,7 +75,8 @@ void task_work_run(void)
> >>>
> >>>   do {
> >>>   next = work->next;
> >>> - work->func(work);
> >>> + if (unlikely(work->func))
> >>> + work->func(work);
> >>
> >> Why?
> >>
> >> Oleg.
> >>
> > 
> > can we believe a callback always be call-able?
> > can it happened to be 0? e.g. wrong initialized.
> > of course, we can complain the caller, be why don't
> > we easily make it more safer?
> > 
> 
> Because you're not making things safer, but your're trying
> to cover up bugs...
> 

Oh, that's a little harsh to a normal programmer like me :-)
for it seems you are asking me to be coding without any bug.
are you? or it is the theory of kernel coding?



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/2] task_work: check callback if it's NULL

2013-03-14 Thread li guang
在 2013-03-14四的 15:43 +0100,Oleg Nesterov写道:
> On 03/14, liguang wrote:
> >
> > Signed-off-by: liguang 
> > ---
> >  kernel/task_work.c |3 ++-
> >  1 files changed, 2 insertions(+), 1 deletions(-)
> >
> > diff --git a/kernel/task_work.c b/kernel/task_work.c
> > index 0bf4258..f458b08 100644
> > --- a/kernel/task_work.c
> > +++ b/kernel/task_work.c
> > @@ -75,7 +75,8 @@ void task_work_run(void)
> >
> > do {
> > next = work->next;
> > -   work->func(work);
> > +   if (unlikely(work->func))
> > +   work->func(work);
> 
> Why?
> 
> Oleg.
> 

can we believe a callback always be call-able?
can it happened to be 0? e.g. wrong initialized.
of course, we can complain the caller, be why don't
we easily make it more safer?

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] task_work: make FIFO task_work list

2013-03-14 Thread li guang
在 2013-03-14四的 15:40 +0100,Oleg Nesterov写道:
> On 03/14, liguang wrote:
> >
> > Signed-off-by: liguang 
> 
> Changelog please...
> 

OK.

> > ---
> >  kernel/task_work.c |   15 +++
> >  1 files changed, 3 insertions(+), 12 deletions(-)
> > 
> > diff --git a/kernel/task_work.c b/kernel/task_work.c
> > index 65bd3c9..0bf4258 100644
> > --- a/kernel/task_work.c
> > +++ b/kernel/task_work.c
> > @@ -13,11 +13,12 @@ task_work_add(struct task_struct *task, struct 
> > callback_head *work, bool notify)
> > head = ACCESS_ONCE(task->task_works);
> > if (unlikely(head == _exited))
> > return -ESRCH;
> > -   work->next = head;
> > -   } while (cmpxchg(>task_works, head, work) != head);
> > +   head = head->next;
> > +   } while (cmpxchg(, NULL, work) == head);
> 
> I simply can't understand how this can work... The patch assumes
> that head->next == NULL after head = head->next, why? And then
> compares the result with head and succeeds if not equal.
> 

then ->next filed was not initialized, so I think it will
be 0'ed by compiler, is it unreliable?.

> Could you please explain how it was supposed to work? If nothing
> else, Suppose we have task->task_works -> W1 -> W2 -> W3. How this
> code can add W4 after W3?
> 

1. head = task_works
2. head = head->next
3. if head == NULL
/* it's next node of list tail (w3->next) */
head = work
   else
goto 1



> And cmpxchg() should be cmpxchg(>next)
> 
> 
> 
> Anyway, whatever I missed this is racy.
> 
>   head = head->next;
> 
> nothing protects "head" after this. Say, it can be task_work_cancel'ed
> and freed. So,
> 
>cmpxchg(, ...)
> 
> can modify the freed and reused memory.
> 
> Oleg.
> 

Thanks Oleg,
Hmm, at first, I think even it was changed, it can't happened to be
NULL, but ... maybe it need more deliberation.

The motivation it make the list FIFO at task_work_add, so you don't
need to reverse it at task_work_run, and it's a time-saver if the list
doesn't have too many nodes I think.



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] task_work: make FIFO task_work list

2013-03-14 Thread li guang
在 2013-03-14四的 15:40 +0100,Oleg Nesterov写道:
 On 03/14, liguang wrote:
 
  Signed-off-by: liguang lig.f...@cn.fujitsu.com
 
 Changelog please...
 

OK.

  ---
   kernel/task_work.c |   15 +++
   1 files changed, 3 insertions(+), 12 deletions(-)
  
  diff --git a/kernel/task_work.c b/kernel/task_work.c
  index 65bd3c9..0bf4258 100644
  --- a/kernel/task_work.c
  +++ b/kernel/task_work.c
  @@ -13,11 +13,12 @@ task_work_add(struct task_struct *task, struct 
  callback_head *work, bool notify)
  head = ACCESS_ONCE(task-task_works);
  if (unlikely(head == work_exited))
  return -ESRCH;
  -   work-next = head;
  -   } while (cmpxchg(task-task_works, head, work) != head);
  +   head = head-next;
  +   } while (cmpxchg(head, NULL, work) == head);
 
 I simply can't understand how this can work... The patch assumes
 that head-next == NULL after head = head-next, why? And then
 compares the result with head and succeeds if not equal.
 

then -next filed was not initialized, so I think it will
be 0'ed by compiler, is it unreliable?.

 Could you please explain how it was supposed to work? If nothing
 else, Suppose we have task-task_works - W1 - W2 - W3. How this
 code can add W4 after W3?
 

1. head = task_works
2. head = head-next
3. if head == NULL
/* it's next node of list tail (w3-next) */
head = work
   else
goto 1



 And cmpxchg(head) should be cmpxchg(head-next)
 
 
 
 Anyway, whatever I missed this is racy.
 
   head = head-next;
 
 nothing protects head after this. Say, it can be task_work_cancel'ed
 and freed. So,
 
cmpxchg(head, ...)
 
 can modify the freed and reused memory.
 
 Oleg.
 

Thanks Oleg,
Hmm, at first, I think even it was changed, it can't happened to be
NULL, but ... maybe it need more deliberation.

The motivation it make the list FIFO at task_work_add, so you don't
need to reverse it at task_work_run, and it's a time-saver if the list
doesn't have too many nodes I think.



--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/2] task_work: check callback if it's NULL

2013-03-14 Thread li guang
在 2013-03-14四的 15:43 +0100,Oleg Nesterov写道:
 On 03/14, liguang wrote:
 
  Signed-off-by: liguang lig.f...@cn.fujitsu.com
  ---
   kernel/task_work.c |3 ++-
   1 files changed, 2 insertions(+), 1 deletions(-)
 
  diff --git a/kernel/task_work.c b/kernel/task_work.c
  index 0bf4258..f458b08 100644
  --- a/kernel/task_work.c
  +++ b/kernel/task_work.c
  @@ -75,7 +75,8 @@ void task_work_run(void)
 
  do {
  next = work-next;
  -   work-func(work);
  +   if (unlikely(work-func))
  +   work-func(work);
 
 Why?
 
 Oleg.
 

can we believe a callback always be call-able?
can it happened to be 0? e.g. wrong initialized.
of course, we can complain the caller, be why don't
we easily make it more safer?

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/2] task_work: check callback if it's NULL

2013-03-14 Thread li guang
在 2013-03-15五的 09:01 +0800,Li Zefan写道:
 On 2013/3/15 8:20, li guang wrote:
  在 2013-03-14四的 15:43 +0100,Oleg Nesterov写道:
  On 03/14, liguang wrote:
 
  Signed-off-by: liguang lig.f...@cn.fujitsu.com
  ---
   kernel/task_work.c |3 ++-
   1 files changed, 2 insertions(+), 1 deletions(-)
 
  diff --git a/kernel/task_work.c b/kernel/task_work.c
  index 0bf4258..f458b08 100644
  --- a/kernel/task_work.c
  +++ b/kernel/task_work.c
  @@ -75,7 +75,8 @@ void task_work_run(void)
 
do {
next = work-next;
  - work-func(work);
  + if (unlikely(work-func))
  + work-func(work);
 
  Why?
 
  Oleg.
 
  
  can we believe a callback always be call-able?
  can it happened to be 0? e.g. wrong initialized.
  of course, we can complain the caller, be why don't
  we easily make it more safer?
  
 
 Because you're not making things safer, but your're trying
 to cover up bugs...
 

Oh, that's a little harsh to a normal programmer like me :-)
for it seems you are asking me to be coding without any bug.
are you? or it is the theory of kernel coding?



--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/2] task_work: check callback if it's NULL

2013-03-14 Thread li guang
在 2013-03-15五的 09:43 +0800,Li Zefan写道:
 On 2013/3/15 9:26, li guang wrote:
  在 2013-03-15五的 09:01 +0800,Li Zefan写道:
  On 2013/3/15 8:20, li guang wrote:
  在 2013-03-14四的 15:43 +0100,Oleg Nesterov写道:
  On 03/14, liguang wrote:
 
  Signed-off-by: liguang lig.f...@cn.fujitsu.com
  ---
   kernel/task_work.c |3 ++-
   1 files changed, 2 insertions(+), 1 deletions(-)
 
  diff --git a/kernel/task_work.c b/kernel/task_work.c
  index 0bf4258..f458b08 100644
  --- a/kernel/task_work.c
  +++ b/kernel/task_work.c
  @@ -75,7 +75,8 @@ void task_work_run(void)
 
  do {
  next = work-next;
  -   work-func(work);
  +   if (unlikely(work-func))
  +   work-func(work);
 
  Why?
 
  Oleg.
 
 
  can we believe a callback always be call-able?
  can it happened to be 0? e.g. wrong initialized.
  of course, we can complain the caller, be why don't
  we easily make it more safer?
 
 
  Because you're not making things safer, but your're trying
  to cover up bugs...
 
  
  Oh, that's a little harsh to a normal programmer like me :-)
  for it seems you are asking me to be coding without any bug.
  are you? or it is the theory of kernel coding?
  
 
 And you make a bug, and you want the kernel to cover up the bug
 instead of crash on a null pointer deref so you'll know you've
 made a bug?
 
 Why we check if a callback is NULL before calling it? Because
 it's allowed to be. Why we don't check if a callback is NULL?
 Because it's not supposed to be.
 

OK, Thanks for your reminder.


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [Qemu-devel] [PATCH v13 0/8] pv event interface between host and guest

2013-03-06 Thread li guang
在 2013-03-06三的 10:07 +0100,Paolo Bonzini写道:
> Il 06/03/2013 09:56, Hu Tao ha scritto:
> >> > 
> >> > Something like this should work (in SeaBIOS's src/acpi-dsdt-isa.dsl):
> >> > 
> >> > Device(PEVT) {
> >> > Name(_HID, EisaId("QEMU0001"))
> >> > OperationRegion(PEOR, SystemIO, 0x505, 0x01)
> >> > Field(PEOR, ByteAcc, NoLock, Preserve) {
> >> > PEPT,   8,
> >> > }
> >> > 
> >> > Method(_STA, 0, NotSerialized) {
> >> > Store(PEPT, Local0)
> >> > If (LEqual(Local0, Zero)) {
> >> > Return (0x00)
> >> > } Else {
> >> > Return (0x0F)
> >> > }
> >> > }
> > IIUC, here _STA reads from ioport 0x505, if the result is 0, then the
> > device is not present. Otherwise, the device is present. But as Gleb
> > said, ''the data you read from unassigned port is not guarantied to be
> > zero, it may depend on QEMU version''. What should I do to tell if the
> > device is present or not correctly?
> 
> The firmware is tied to the QEMU version, so you can rely on unassigned
> ports returning zero.
> 

but what if we happen to transfer data end by 0x0,
here, will this device(QEMU0001) disappear?
(by this asl code snippet, I think it will)
so, if we transfer data(not 0x0) device appear,
then, it will disappear(if come across 0x0),
can this happen?
or should we use another status io port e.g.
0x506 to handle this condition?

> Later we can change this to use fw-cfg.
> 
> Paolo
> 
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [Qemu-devel] [PATCH v13 0/8] pv event interface between host and guest

2013-03-06 Thread li guang
在 2013-03-06三的 10:07 +0100,Paolo Bonzini写道:
 Il 06/03/2013 09:56, Hu Tao ha scritto:
   
   Something like this should work (in SeaBIOS's src/acpi-dsdt-isa.dsl):
   
   Device(PEVT) {
   Name(_HID, EisaId(QEMU0001))
   OperationRegion(PEOR, SystemIO, 0x505, 0x01)
   Field(PEOR, ByteAcc, NoLock, Preserve) {
   PEPT,   8,
   }
   
   Method(_STA, 0, NotSerialized) {
   Store(PEPT, Local0)
   If (LEqual(Local0, Zero)) {
   Return (0x00)
   } Else {
   Return (0x0F)
   }
   }
  IIUC, here _STA reads from ioport 0x505, if the result is 0, then the
  device is not present. Otherwise, the device is present. But as Gleb
  said, ''the data you read from unassigned port is not guarantied to be
  zero, it may depend on QEMU version''. What should I do to tell if the
  device is present or not correctly?
 
 The firmware is tied to the QEMU version, so you can rely on unassigned
 ports returning zero.
 

but what if we happen to transfer data end by 0x0,
here, will this device(QEMU0001) disappear?
(by this asl code snippet, I think it will)
so, if we transfer data(not 0x0) device appear,
then, it will disappear(if come across 0x0),
can this happen?
or should we use another status io port e.g.
0x506 to handle this condition?

 Later we can change this to use fw-cfg.
 
 Paolo
 
 


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [rebased-again][PATCH 0/4] acpi: do some changes for numa info

2013-03-03 Thread li guang
Hi, Thomas, Peter, Ingo

can you help to merge these 4 patches?
or do you have any other comments?


在 2013-02-25一的 10:38 +0800,liguang写道:
> just do some trivial changes to make acpi's numa info
> operation more cleaner.
> 
> ChangeLog
> 
> v3->v4
>  1.fix srat_disabled function
>  spotted by Yasuaki Ishimatsu 
> 
> v2->v3
>  1. rebase on linux-next
>  2. bring back lost Makefile changes
>  spotted by David Rientjes 
>  spotted by Yasuaki Ishimatsu 
> 
> v1->v2
>  1. fix-up several coding issues
>  2. finish srat.c change
>  spotted by David Rientjes 
> 
> Li Guang (4)
>acpi: move x86/mm/srat.c to x86/kernel/acpi/srat.c
>numa: avoid export acpi_numa variable
>acpi: add clock_domain field to acpi_srat_cpu_affinity
>remove include asm/acpi.h in process_driver.c
> 
> arch/x86/include/asm/acpi.h | 2 +-
> arch/x86/kernel/acpi/Makefile   | 1 +
> arch/x86/kernel/acpi/srat.c | 299 
> +
> arch/x86/mm/Makefile| 1 -
> arch/x86/mm/numa.c  | 2 +-
> arch/x86/mm/srat.c  | 278 
> -
> arch/x86/xen/enlighten.c| 2 +-
> drivers/acpi/processor_driver.c | 1 -
> include/acpi/actbl1.h   | 2 +-
> 9 files changed, 296 insertions(+), 292 deletions(-)
>  create mode 100644 arch/x86/kernel/acpi/srat.c
>  delete mode 100644 arch/x86/mm/srat.c
>  
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [rebased-again][PATCH 0/4] acpi: do some changes for numa info

2013-03-03 Thread li guang
Hi, Thomas, Peter, Ingo

can you help to merge these 4 patches?
or do you have any other comments?


在 2013-02-25一的 10:38 +0800,liguang写道:
 just do some trivial changes to make acpi's numa info
 operation more cleaner.
 
 ChangeLog
 
 v3-v4
  1.fix srat_disabled function
  spotted by Yasuaki Ishimatsu isimatu.yasu...@jp.fujitsu.com
 
 v2-v3
  1. rebase on linux-next
  2. bring back lost Makefile changes
  spotted by David Rientjes rient...@google.com
  spotted by Yasuaki Ishimatsu isimatu.yasu...@jp.fujitsu.com
 
 v1-v2
  1. fix-up several coding issues
  2. finish srat.c change
  spotted by David Rientjes rient...@google.com
 
 Li Guang (4)
acpi: move x86/mm/srat.c to x86/kernel/acpi/srat.c
numa: avoid export acpi_numa variable
acpi: add clock_domain field to acpi_srat_cpu_affinity
remove include asm/acpi.h in process_driver.c
 
 arch/x86/include/asm/acpi.h | 2 +-
 arch/x86/kernel/acpi/Makefile   | 1 +
 arch/x86/kernel/acpi/srat.c | 299 
 +
 arch/x86/mm/Makefile| 1 -
 arch/x86/mm/numa.c  | 2 +-
 arch/x86/mm/srat.c  | 278 
 -
 arch/x86/xen/enlighten.c| 2 +-
 drivers/acpi/processor_driver.c | 1 -
 include/acpi/actbl1.h   | 2 +-
 9 files changed, 296 insertions(+), 292 deletions(-)
  create mode 100644 arch/x86/kernel/acpi/srat.c
  delete mode 100644 arch/x86/mm/srat.c
  
 --
 To unsubscribe from this list: send the line unsubscribe linux-kernel in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 Please read the FAQ at  http://www.tux.org/lkml/


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/2] x86: add e820 descriptor attribute field

2013-03-01 Thread li guang
can we fix if it breaks anything?
I think there are sound reasons that
ACPI add these extended attributes.

在 2013-02-28四的 21:28 -0800,H. Peter Anvin写道:
> NAK in the extreme.  Not only does this break the bootloader protocol, but 
> there are systems in the field that break if you give e820 anything other 
> than a 20-byte buffer.
> 
> liguang  wrote:
> 
> >according to ACPI 5.0 Table 15-273
> >Address Range Descriptor Structure,
> >offset 20 is 32-bit field of Extended
> >Attributes for Address Range Descriptor Structure.
> >
> >Signed-off-by: liguang 
> >---
> > arch/x86/include/uapi/asm/e820.h |7 ++-
> > 1 files changed, 6 insertions(+), 1 deletions(-)
> >
> >diff --git a/arch/x86/include/uapi/asm/e820.h
> >b/arch/x86/include/uapi/asm/e820.h
> >index 2d400b1..eb87284 100644
> >--- a/arch/x86/include/uapi/asm/e820.h
> >+++ b/arch/x86/include/uapi/asm/e820.h
> >@@ -38,6 +38,10 @@
> > #define E820_TYPE_NVS   4
> > #define E820_TYPE_UNUSABLE  5
> > 
> >+#define E820_ATTRIB_NV 0x2
> >+#define E820_ATTRIB_SLOW_ACCESS 0x4
> >+#define E820_ATTRIB_ERR_LOG 0x8
> >+
> > 
> > /*
> >  * reserved RAM used by kernel itself
> >@@ -53,7 +57,8 @@ struct e820entry {
> > __u64 addr; /* start of memory segment */
> > __u64 size; /* size of memory segment */
> > __u32 type; /* type of memory segment */
> >-} __attribute__((packed));
> >+__u32 attrib;
> >+};
> > 
> > struct e820map {
> > __u32 nr_map;
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] x86: change names of e820 memory map type

2013-03-01 Thread li guang
gratuitous?

It was inspired by gtk library's key definitions,
gtk original keys like GDK_1, GDK_2, GDK_p, GDK_q, GDK_tab ...
now they all be changed to GDK_KEY_1, GDK_KEY_2 ...
do you think it's reasonable?
or gratuitous?


在 2013-02-28四的 21:29 -0800,H. Peter Anvin写道:
> NAK.  Gratuitous pointless change.
> 
> liguang  wrote:
> 
> >E820_RAM -> E820_TYPE_RAM
> >E820_ACPI-> E820_TYPE_ACPI
> >...
> >
> >names like E820_RAM is conflict-prone,
> >because user is more likely to define
> >a macro like this if did not strongly
> >aware this name have been defined
> >by e820.h
> >
> >Signed-off-by: liguang 
> >---
> > arch/x86/boot/compressed/eboot.c   |   10 +++---
> > arch/x86/include/asm/gart.h|2 +-
> > arch/x86/include/uapi/asm/e820.h   |   12 
> > arch/x86/kernel/acpi/boot.c|2 +-
> > arch/x86/kernel/aperture_64.c  |4 +-
> > arch/x86/kernel/cpu/centaur.c  |2 +-
> > arch/x86/kernel/cpu/mtrr/cleanup.c |2 +-
> >arch/x86/kernel/e820.c |   52
> >
> > arch/x86/kernel/setup.c|   22 +++---
> > arch/x86/kernel/tboot.c|8 ++--
> > arch/x86/mm/init_64.c  |   12 
> > arch/x86/pci/mmconfig-shared.c |2 +-
> > arch/x86/platform/efi/efi.c|   14 
> > arch/x86/platform/visws/visws_quirks.c |6 ++--
> > arch/x86/xen/setup.c   |   16 +-
> > 15 files changed, 83 insertions(+), 83 deletions(-)
> >
> >diff --git a/arch/x86/boot/compressed/eboot.c
> >b/arch/x86/boot/compressed/eboot.c
> >index f8fa411..5bda487 100644
> >--- a/arch/x86/boot/compressed/eboot.c
> >+++ b/arch/x86/boot/compressed/eboot.c
> >@@ -1040,15 +1040,15 @@ again:
> > case EFI_MEMORY_MAPPED_IO:
> > case EFI_MEMORY_MAPPED_IO_PORT_SPACE:
> > case EFI_PAL_CODE:
> >-e820_type = E820_RESERVED;
> >+e820_type = E820_TYPE_RESERVED;
> > break;
> > 
> > case EFI_UNUSABLE_MEMORY:
> >-e820_type = E820_UNUSABLE;
> >+e820_type = E820_TYPE_UNUSABLE;
> > break;
> > 
> > case EFI_ACPI_RECLAIM_MEMORY:
> >-e820_type = E820_ACPI;
> >+e820_type = E820_TYPE_ACPI;
> > break;
> > 
> > case EFI_LOADER_CODE:
> >@@ -1056,11 +1056,11 @@ again:
> > case EFI_BOOT_SERVICES_CODE:
> > case EFI_BOOT_SERVICES_DATA:
> > case EFI_CONVENTIONAL_MEMORY:
> >-e820_type = E820_RAM;
> >+e820_type = E820_TYPE_RAM;
> > break;
> > 
> > case EFI_ACPI_MEMORY_NVS:
> >-e820_type = E820_NVS;
> >+e820_type = E820_TYPE_NVS;
> > break;
> > 
> > default:
> >diff --git a/arch/x86/include/asm/gart.h b/arch/x86/include/asm/gart.h
> >index 156cd5d..4d22bcc 100644
> >--- a/arch/x86/include/asm/gart.h
> >+++ b/arch/x86/include/asm/gart.h
> >@@ -97,7 +97,7 @@ static inline int aperture_valid(u64 aper_base, u32
> >aper_size, u32 min_size)
> > printk(KERN_INFO "Aperture beyond 4GB. Ignoring.\n");
> > return 0;
> > }
> >-if (e820_any_mapped(aper_base, aper_base + aper_size, E820_RAM)) {
> >+if (e820_any_mapped(aper_base, aper_base + aper_size, E820_TYPE_RAM))
> >{
> > printk(KERN_INFO "Aperture pointing to e820 RAM. Ignoring.\n");
> > return 0;
> > }
> >diff --git a/arch/x86/include/uapi/asm/e820.h
> >b/arch/x86/include/uapi/asm/e820.h
> >index bbae024..2d400b1 100644
> >--- a/arch/x86/include/uapi/asm/e820.h
> >+++ b/arch/x86/include/uapi/asm/e820.h
> >@@ -32,11 +32,11 @@
> > 
> > #define E820NR  0x1e8   /* # entries in E820MAP */
> > 
> >-#define E820_RAM1
> >-#define E820_RESERVED   2
> >-#define E820_ACPI   3
> >-#define E820_NVS4
> >-#define E820_UNUSABLE   5
> >+#define E820_TYPE_RAM   1
> >+#define E820_TYPE_RESERVED  2
> >+#define E820_TYPE_ACPI  3
> >+#define E820_TYPE_NVS   4
> >+#define E820_TYPE_UNUSABLE  5
> > 
> > 
> > /*
> >@@ -45,7 +45,7 @@
> >  * included in the S3 integrity calculation and so should not include
> >  * any memory that BIOS might alter over the S3 transition
> >  */
> >-#define E820_RESERVED_KERN128
> >+#define E820_TYPE_RESERVED_KERN128
> > 
> > #ifndef __ASSEMBLY__
> > #include 
> >diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
> >index 230c8ea..9595747 100644
> >--- a/arch/x86/kernel/acpi/boot.c
> >+++ b/arch/x86/kernel/acpi/boot.c
> >@@ -1712,6 +1712,6 @@ int __acpi_release_global_lock(unsigned int
> >*lock)
> > 
> >void __init arch_reserve_mem_area(acpi_physical_address addr, size_t
> >size)
> > {
> >-e820_add_region(addr, size, E820_ACPI);
> >+e820_add_region(addr, 

Re: [PATCH 1/2] x86: change names of e820 memory map type

2013-03-01 Thread li guang
gratuitous?

It was inspired by gtk library's key definitions,
gtk original keys like GDK_1, GDK_2, GDK_p, GDK_q, GDK_tab ...
now they all be changed to GDK_KEY_1, GDK_KEY_2 ...
do you think it's reasonable?
or gratuitous?


在 2013-02-28四的 21:29 -0800,H. Peter Anvin写道:
 NAK.  Gratuitous pointless change.
 
 liguang lig.f...@cn.fujitsu.com wrote:
 
 E820_RAM - E820_TYPE_RAM
 E820_ACPI- E820_TYPE_ACPI
 ...
 
 names like E820_RAM is conflict-prone,
 because user is more likely to define
 a macro like this if did not strongly
 aware this name have been defined
 by e820.h
 
 Signed-off-by: liguang lig.f...@cn.fujitsu.com
 ---
  arch/x86/boot/compressed/eboot.c   |   10 +++---
  arch/x86/include/asm/gart.h|2 +-
  arch/x86/include/uapi/asm/e820.h   |   12 
  arch/x86/kernel/acpi/boot.c|2 +-
  arch/x86/kernel/aperture_64.c  |4 +-
  arch/x86/kernel/cpu/centaur.c  |2 +-
  arch/x86/kernel/cpu/mtrr/cleanup.c |2 +-
 arch/x86/kernel/e820.c |   52
 
  arch/x86/kernel/setup.c|   22 +++---
  arch/x86/kernel/tboot.c|8 ++--
  arch/x86/mm/init_64.c  |   12 
  arch/x86/pci/mmconfig-shared.c |2 +-
  arch/x86/platform/efi/efi.c|   14 
  arch/x86/platform/visws/visws_quirks.c |6 ++--
  arch/x86/xen/setup.c   |   16 +-
  15 files changed, 83 insertions(+), 83 deletions(-)
 
 diff --git a/arch/x86/boot/compressed/eboot.c
 b/arch/x86/boot/compressed/eboot.c
 index f8fa411..5bda487 100644
 --- a/arch/x86/boot/compressed/eboot.c
 +++ b/arch/x86/boot/compressed/eboot.c
 @@ -1040,15 +1040,15 @@ again:
  case EFI_MEMORY_MAPPED_IO:
  case EFI_MEMORY_MAPPED_IO_PORT_SPACE:
  case EFI_PAL_CODE:
 -e820_type = E820_RESERVED;
 +e820_type = E820_TYPE_RESERVED;
  break;
  
  case EFI_UNUSABLE_MEMORY:
 -e820_type = E820_UNUSABLE;
 +e820_type = E820_TYPE_UNUSABLE;
  break;
  
  case EFI_ACPI_RECLAIM_MEMORY:
 -e820_type = E820_ACPI;
 +e820_type = E820_TYPE_ACPI;
  break;
  
  case EFI_LOADER_CODE:
 @@ -1056,11 +1056,11 @@ again:
  case EFI_BOOT_SERVICES_CODE:
  case EFI_BOOT_SERVICES_DATA:
  case EFI_CONVENTIONAL_MEMORY:
 -e820_type = E820_RAM;
 +e820_type = E820_TYPE_RAM;
  break;
  
  case EFI_ACPI_MEMORY_NVS:
 -e820_type = E820_NVS;
 +e820_type = E820_TYPE_NVS;
  break;
  
  default:
 diff --git a/arch/x86/include/asm/gart.h b/arch/x86/include/asm/gart.h
 index 156cd5d..4d22bcc 100644
 --- a/arch/x86/include/asm/gart.h
 +++ b/arch/x86/include/asm/gart.h
 @@ -97,7 +97,7 @@ static inline int aperture_valid(u64 aper_base, u32
 aper_size, u32 min_size)
  printk(KERN_INFO Aperture beyond 4GB. Ignoring.\n);
  return 0;
  }
 -if (e820_any_mapped(aper_base, aper_base + aper_size, E820_RAM)) {
 +if (e820_any_mapped(aper_base, aper_base + aper_size, E820_TYPE_RAM))
 {
  printk(KERN_INFO Aperture pointing to e820 RAM. Ignoring.\n);
  return 0;
  }
 diff --git a/arch/x86/include/uapi/asm/e820.h
 b/arch/x86/include/uapi/asm/e820.h
 index bbae024..2d400b1 100644
 --- a/arch/x86/include/uapi/asm/e820.h
 +++ b/arch/x86/include/uapi/asm/e820.h
 @@ -32,11 +32,11 @@
  
  #define E820NR  0x1e8   /* # entries in E820MAP */
  
 -#define E820_RAM1
 -#define E820_RESERVED   2
 -#define E820_ACPI   3
 -#define E820_NVS4
 -#define E820_UNUSABLE   5
 +#define E820_TYPE_RAM   1
 +#define E820_TYPE_RESERVED  2
 +#define E820_TYPE_ACPI  3
 +#define E820_TYPE_NVS   4
 +#define E820_TYPE_UNUSABLE  5
  
  
  /*
 @@ -45,7 +45,7 @@
   * included in the S3 integrity calculation and so should not include
   * any memory that BIOS might alter over the S3 transition
   */
 -#define E820_RESERVED_KERN128
 +#define E820_TYPE_RESERVED_KERN128
  
  #ifndef __ASSEMBLY__
  #include linux/types.h
 diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
 index 230c8ea..9595747 100644
 --- a/arch/x86/kernel/acpi/boot.c
 +++ b/arch/x86/kernel/acpi/boot.c
 @@ -1712,6 +1712,6 @@ int __acpi_release_global_lock(unsigned int
 *lock)
  
 void __init arch_reserve_mem_area(acpi_physical_address addr, size_t
 size)
  {
 -e820_add_region(addr, size, E820_ACPI);
 +e820_add_region(addr, size, E820_TYPE_ACPI);
  update_e820();
  }
 diff --git a/arch/x86/kernel/aperture_64.c
 b/arch/x86/kernel/aperture_64.c
 index d5fd66f..0210300 100644
 --- a/arch/x86/kernel/aperture_64.c
 +++ 

Re: [PATCH 2/2] x86: add e820 descriptor attribute field

2013-03-01 Thread li guang
can we fix if it breaks anything?
I think there are sound reasons that
ACPI add these extended attributes.

在 2013-02-28四的 21:28 -0800,H. Peter Anvin写道:
 NAK in the extreme.  Not only does this break the bootloader protocol, but 
 there are systems in the field that break if you give e820 anything other 
 than a 20-byte buffer.
 
 liguang lig.f...@cn.fujitsu.com wrote:
 
 according to ACPI 5.0 Table 15-273
 Address Range Descriptor Structure,
 offset 20 is 32-bit field of Extended
 Attributes for Address Range Descriptor Structure.
 
 Signed-off-by: liguang lig.f...@cn.fujitsu.com
 ---
  arch/x86/include/uapi/asm/e820.h |7 ++-
  1 files changed, 6 insertions(+), 1 deletions(-)
 
 diff --git a/arch/x86/include/uapi/asm/e820.h
 b/arch/x86/include/uapi/asm/e820.h
 index 2d400b1..eb87284 100644
 --- a/arch/x86/include/uapi/asm/e820.h
 +++ b/arch/x86/include/uapi/asm/e820.h
 @@ -38,6 +38,10 @@
  #define E820_TYPE_NVS   4
  #define E820_TYPE_UNUSABLE  5
  
 +#define E820_ATTRIB_NV 0x2
 +#define E820_ATTRIB_SLOW_ACCESS 0x4
 +#define E820_ATTRIB_ERR_LOG 0x8
 +
  
  /*
   * reserved RAM used by kernel itself
 @@ -53,7 +57,8 @@ struct e820entry {
  __u64 addr; /* start of memory segment */
  __u64 size; /* size of memory segment */
  __u32 type; /* type of memory segment */
 -} __attribute__((packed));
 +__u32 attrib;
 +};
  
  struct e820map {
  __u32 nr_map;
 


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/3] perf: sort command-list.txt by alphabet order

2013-02-28 Thread li guang
ping ...

在 2013-02-26二的 12:12 +0800,liguang写道:
> Signed-off-by: liguang 
> ---
>  tools/perf/command-list.txt |   14 +++---
>  1 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/perf/command-list.txt b/tools/perf/command-list.txt
> index 3e86bbd..a28e31b 100644
> --- a/tools/perf/command-list.txt
> +++ b/tools/perf/command-list.txt
> @@ -10,17 +10,17 @@ perf-buildid-list mainporcelain common
>  perf-diffmainporcelain common
>  perf-evlist  mainporcelain common
>  perf-inject  mainporcelain common
> +perf-kmemmainporcelain common
> +perf-kvm mainporcelain common
>  perf-listmainporcelain common
> -perf-sched   mainporcelain common
> +perf-lockmainporcelain common
> +perf-probe   mainporcelain full
>  perf-record  mainporcelain common
>  perf-report  mainporcelain common
> +perf-sched   mainporcelain common
> +perf-script  mainporcelain common
>  perf-statmainporcelain common
> +perf-testmainporcelain common
>  perf-timechart   mainporcelain common
>  perf-top mainporcelain common
>  perf-trace   mainporcelain common
> -perf-script  mainporcelain common
> -perf-probe   mainporcelain full
> -perf-kmemmainporcelain common
> -perf-lockmainporcelain common
> -perf-kvm mainporcelain common
> -perf-testmainporcelain common


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [rebased][PATCH 0/4] acpi: do some changes for numa info

2013-02-28 Thread li guang
can anyone help to merge these patches?
or any other comments?

在 2013-02-22五的 13:44 +0800,liguang写道:
> just do some trivial changes to make acpi's numa info
> operation more cleaner.
> 
> ChangeLog
> 
> v3->v4
>  1.fix srat_disabled function
>  spotted by Yasuaki Ishimatsu 
> 
> v2->v3
>  1. rebase on linux-next
>  2. bring back lost Makefile changes
>  spotted by David Rientjes 
>  spotted by Yasuaki Ishimatsu 
> 
> v1->v2
>  1. fix-up several coding issues
>  2. finish srat.c change
>  spotted by David Rientjes 
> 
> Li Guang (4)
>acpi: move x86/mm/srat.c to x86/kernel/acpi/srat.c
>numa: avoid export acpi_numa variable
>acpi: add clock_domain field to acpi_srat_cpu_affinity
>remove include asm/acpi.h in process_driver.c
> 
> arch/x86/include/asm/acpi.h | 2 +-
> arch/x86/kernel/acpi/Makefile   | 1 +
> arch/x86/kernel/acpi/srat.c | 299 
> +
> arch/x86/mm/Makefile| 1 -
> arch/x86/mm/numa.c  | 2 +-
> arch/x86/mm/srat.c  | 278 
> -
> arch/x86/xen/enlighten.c| 2 +-
> drivers/acpi/processor_driver.c | 1 -
> include/acpi/actbl1.h   | 2 +-
> 9 files changed, 296 insertions(+), 292 deletions(-)
>  create mode 100644 arch/x86/kernel/acpi/srat.c
>  delete mode 100644 arch/x86/mm/srat.c
>  
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [rebased][PATCH 0/4] acpi: do some changes for numa info

2013-02-28 Thread li guang
can anyone help to merge these patches?
or any other comments?

在 2013-02-22五的 13:44 +0800,liguang写道:
 just do some trivial changes to make acpi's numa info
 operation more cleaner.
 
 ChangeLog
 
 v3-v4
  1.fix srat_disabled function
  spotted by Yasuaki Ishimatsu isimatu.yasu...@jp.fujitsu.com
 
 v2-v3
  1. rebase on linux-next
  2. bring back lost Makefile changes
  spotted by David Rientjes rient...@google.com
  spotted by Yasuaki Ishimatsu isimatu.yasu...@jp.fujitsu.com
 
 v1-v2
  1. fix-up several coding issues
  2. finish srat.c change
  spotted by David Rientjes rient...@google.com
 
 Li Guang (4)
acpi: move x86/mm/srat.c to x86/kernel/acpi/srat.c
numa: avoid export acpi_numa variable
acpi: add clock_domain field to acpi_srat_cpu_affinity
remove include asm/acpi.h in process_driver.c
 
 arch/x86/include/asm/acpi.h | 2 +-
 arch/x86/kernel/acpi/Makefile   | 1 +
 arch/x86/kernel/acpi/srat.c | 299 
 +
 arch/x86/mm/Makefile| 1 -
 arch/x86/mm/numa.c  | 2 +-
 arch/x86/mm/srat.c  | 278 
 -
 arch/x86/xen/enlighten.c| 2 +-
 drivers/acpi/processor_driver.c | 1 -
 include/acpi/actbl1.h   | 2 +-
 9 files changed, 296 insertions(+), 292 deletions(-)
  create mode 100644 arch/x86/kernel/acpi/srat.c
  delete mode 100644 arch/x86/mm/srat.c
  
 --
 To unsubscribe from this list: send the line unsubscribe linux-kernel in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 Please read the FAQ at  http://www.tux.org/lkml/


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/3] perf: sort command-list.txt by alphabet order

2013-02-28 Thread li guang
ping ...

在 2013-02-26二的 12:12 +0800,liguang写道:
 Signed-off-by: liguang lig.f...@cn.fujitsu.com
 ---
  tools/perf/command-list.txt |   14 +++---
  1 files changed, 7 insertions(+), 7 deletions(-)
 
 diff --git a/tools/perf/command-list.txt b/tools/perf/command-list.txt
 index 3e86bbd..a28e31b 100644
 --- a/tools/perf/command-list.txt
 +++ b/tools/perf/command-list.txt
 @@ -10,17 +10,17 @@ perf-buildid-list mainporcelain common
  perf-diffmainporcelain common
  perf-evlist  mainporcelain common
  perf-inject  mainporcelain common
 +perf-kmemmainporcelain common
 +perf-kvm mainporcelain common
  perf-listmainporcelain common
 -perf-sched   mainporcelain common
 +perf-lockmainporcelain common
 +perf-probe   mainporcelain full
  perf-record  mainporcelain common
  perf-report  mainporcelain common
 +perf-sched   mainporcelain common
 +perf-script  mainporcelain common
  perf-statmainporcelain common
 +perf-testmainporcelain common
  perf-timechart   mainporcelain common
  perf-top mainporcelain common
  perf-trace   mainporcelain common
 -perf-script  mainporcelain common
 -perf-probe   mainporcelain full
 -perf-kmemmainporcelain common
 -perf-lockmainporcelain common
 -perf-kvm mainporcelain common
 -perf-testmainporcelain common


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] perf annotate: Fix build with NO_NEWT=1

2013-02-26 Thread li guang
在 2013-02-27三的 12:22 +1100,Michael Ellerman写道:
> On Wed, Feb 27, 2013 at 09:14:28AM +0800, li guang wrote:
> > 在 2013-02-26二的 18:02 +0900,Namhyung Kim写道:
> > > Hi Michael,
> > > 
> > > On Tue, 26 Feb 2013 16:02:02 +1100, Michael Ellerman wrote:
> > > > Commit 18c9e5c "Make it to be able to skip unannotatable symbols" broke
> > > > the build with NO_NEWT=1:
> > > >
> > > >CC builtin-annotate.o
> > > > builtin-annotate.c: In function 'hists__find_annotations':
> > > > builtin-annotate.c:161:4: error: duplicate case value
> > > > builtin-annotate.c:154:4: error: previously used here
> > > > make: *** [builtin-annotate.o] Error 1
> > > >
> > > > This is because without NEWT support K_LEFT is #defined to -1 in
> > > > utils/hist.h
> > > >
> > > > Fix it by shifting the K_LEFT/K_RIGHT #defines out of the likely range
> > > > of error values.
> > > 
> > > Argh, didn't check NO_NEWT build on this, sorry.
> > > 
> > > The hist_entry__tui_annotate() - hence, symbol__tui_annotate() - returns
> > > either key code or error code.  This is not good IMHO but not sure it's
> > > worth refactoring.  Maybe we can move the error check to under default
> > > case so that possible future error checks to be done?
> > 
> > or can we skip gui-code-snippet when there's no gui library?
> > anyway, for present fix-up, we can just temporarily change K_LEFT value
> > like Michael's or my patch, I think.
> 
> Sorry I didn't see your patch, but yes I agree, either is fine and fixes
> the build for now.
> 
> cheers

https://lkml.org/lkml/2013/2/25/486

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] perf annotate: Fix build with NO_NEWT=1

2013-02-26 Thread li guang
在 2013-02-27三的 12:22 +1100,Michael Ellerman写道:
> On Wed, Feb 27, 2013 at 09:14:28AM +0800, li guang wrote:
> > 在 2013-02-26二的 18:02 +0900,Namhyung Kim写道:
> > > Hi Michael,
> > > 
> > > On Tue, 26 Feb 2013 16:02:02 +1100, Michael Ellerman wrote:
> > > > Commit 18c9e5c "Make it to be able to skip unannotatable symbols" broke
> > > > the build with NO_NEWT=1:
> > > >
> > > >CC builtin-annotate.o
> > > > builtin-annotate.c: In function 'hists__find_annotations':
> > > > builtin-annotate.c:161:4: error: duplicate case value
> > > > builtin-annotate.c:154:4: error: previously used here
> > > > make: *** [builtin-annotate.o] Error 1
> > > >
> > > > This is because without NEWT support K_LEFT is #defined to -1 in
> > > > utils/hist.h
> > > >
> > > > Fix it by shifting the K_LEFT/K_RIGHT #defines out of the likely range
> > > > of error values.
> > > 
> > > Argh, didn't check NO_NEWT build on this, sorry.
> > > 
> > > The hist_entry__tui_annotate() - hence, symbol__tui_annotate() - returns
> > > either key code or error code.  This is not good IMHO but not sure it's
> > > worth refactoring.  Maybe we can move the error check to under default
> > > case so that possible future error checks to be done?
> > 
> > or can we skip gui-code-snippet when there's no gui library?
> > anyway, for present fix-up, we can just temporarily change K_LEFT value
> > like Michael's or my patch, I think.
> 
> Sorry I didn't see your patch, but yes I agree, either is fine and fixes
> the build for now.
> 
> cheers


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] perf annotate: Fix build with NO_NEWT=1

2013-02-26 Thread li guang
在 2013-02-26二的 18:02 +0900,Namhyung Kim写道:
> Hi Michael,
> 
> On Tue, 26 Feb 2013 16:02:02 +1100, Michael Ellerman wrote:
> > Commit 18c9e5c "Make it to be able to skip unannotatable symbols" broke
> > the build with NO_NEWT=1:
> >
> >CC builtin-annotate.o
> > builtin-annotate.c: In function 'hists__find_annotations':
> > builtin-annotate.c:161:4: error: duplicate case value
> > builtin-annotate.c:154:4: error: previously used here
> > make: *** [builtin-annotate.o] Error 1
> >
> > This is because without NEWT support K_LEFT is #defined to -1 in
> > utils/hist.h
> >
> > Fix it by shifting the K_LEFT/K_RIGHT #defines out of the likely range
> > of error values.
> 
> Argh, didn't check NO_NEWT build on this, sorry.
> 
> The hist_entry__tui_annotate() - hence, symbol__tui_annotate() - returns
> either key code or error code.  This is not good IMHO but not sure it's
> worth refactoring.  Maybe we can move the error check to under default
> case so that possible future error checks to be done?

or can we skip gui-code-snippet when there's no gui library?
anyway, for present fix-up, we can just temporarily change K_LEFT value
like Michael's or my patch, I think.

> 
> Thanks,
> Namhyung
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] perf annotate: Fix build with NO_NEWT=1

2013-02-26 Thread li guang
在 2013-02-26二的 18:02 +0900,Namhyung Kim写道:
 Hi Michael,
 
 On Tue, 26 Feb 2013 16:02:02 +1100, Michael Ellerman wrote:
  Commit 18c9e5c Make it to be able to skip unannotatable symbols broke
  the build with NO_NEWT=1:
 
 CC builtin-annotate.o
  builtin-annotate.c: In function 'hists__find_annotations':
  builtin-annotate.c:161:4: error: duplicate case value
  builtin-annotate.c:154:4: error: previously used here
  make: *** [builtin-annotate.o] Error 1
 
  This is because without NEWT support K_LEFT is #defined to -1 in
  utils/hist.h
 
  Fix it by shifting the K_LEFT/K_RIGHT #defines out of the likely range
  of error values.
 
 Argh, didn't check NO_NEWT build on this, sorry.
 
 The hist_entry__tui_annotate() - hence, symbol__tui_annotate() - returns
 either key code or error code.  This is not good IMHO but not sure it's
 worth refactoring.  Maybe we can move the error check to under default
 case so that possible future error checks to be done?

or can we skip gui-code-snippet when there's no gui library?
anyway, for present fix-up, we can just temporarily change K_LEFT value
like Michael's or my patch, I think.

 
 Thanks,
 Namhyung
 --
 To unsubscribe from this list: send the line unsubscribe linux-kernel in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 Please read the FAQ at  http://www.tux.org/lkml/


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] perf annotate: Fix build with NO_NEWT=1

2013-02-26 Thread li guang
在 2013-02-27三的 12:22 +1100,Michael Ellerman写道:
 On Wed, Feb 27, 2013 at 09:14:28AM +0800, li guang wrote:
  在 2013-02-26二的 18:02 +0900,Namhyung Kim写道:
   Hi Michael,
   
   On Tue, 26 Feb 2013 16:02:02 +1100, Michael Ellerman wrote:
Commit 18c9e5c Make it to be able to skip unannotatable symbols broke
the build with NO_NEWT=1:
   
   CC builtin-annotate.o
builtin-annotate.c: In function 'hists__find_annotations':
builtin-annotate.c:161:4: error: duplicate case value
builtin-annotate.c:154:4: error: previously used here
make: *** [builtin-annotate.o] Error 1
   
This is because without NEWT support K_LEFT is #defined to -1 in
utils/hist.h
   
Fix it by shifting the K_LEFT/K_RIGHT #defines out of the likely range
of error values.
   
   Argh, didn't check NO_NEWT build on this, sorry.
   
   The hist_entry__tui_annotate() - hence, symbol__tui_annotate() - returns
   either key code or error code.  This is not good IMHO but not sure it's
   worth refactoring.  Maybe we can move the error check to under default
   case so that possible future error checks to be done?
  
  or can we skip gui-code-snippet when there's no gui library?
  anyway, for present fix-up, we can just temporarily change K_LEFT value
  like Michael's or my patch, I think.
 
 Sorry I didn't see your patch, but yes I agree, either is fine and fixes
 the build for now.
 
 cheers


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] perf annotate: Fix build with NO_NEWT=1

2013-02-26 Thread li guang
在 2013-02-27三的 12:22 +1100,Michael Ellerman写道:
 On Wed, Feb 27, 2013 at 09:14:28AM +0800, li guang wrote:
  在 2013-02-26二的 18:02 +0900,Namhyung Kim写道:
   Hi Michael,
   
   On Tue, 26 Feb 2013 16:02:02 +1100, Michael Ellerman wrote:
Commit 18c9e5c Make it to be able to skip unannotatable symbols broke
the build with NO_NEWT=1:
   
   CC builtin-annotate.o
builtin-annotate.c: In function 'hists__find_annotations':
builtin-annotate.c:161:4: error: duplicate case value
builtin-annotate.c:154:4: error: previously used here
make: *** [builtin-annotate.o] Error 1
   
This is because without NEWT support K_LEFT is #defined to -1 in
utils/hist.h
   
Fix it by shifting the K_LEFT/K_RIGHT #defines out of the likely range
of error values.
   
   Argh, didn't check NO_NEWT build on this, sorry.
   
   The hist_entry__tui_annotate() - hence, symbol__tui_annotate() - returns
   either key code or error code.  This is not good IMHO but not sure it's
   worth refactoring.  Maybe we can move the error check to under default
   case so that possible future error checks to be done?
  
  or can we skip gui-code-snippet when there's no gui library?
  anyway, for present fix-up, we can just temporarily change K_LEFT value
  like Michael's or my patch, I think.
 
 Sorry I didn't see your patch, but yes I agree, either is fine and fixes
 the build for now.
 
 cheers

https://lkml.org/lkml/2013/2/25/486

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2] mm: break circular include from linux/mmzone.h

2013-02-25 Thread li guang
ping ...

did any one find build errors after applying this patch?

在 2013-02-18一的 14:27 +0800,liguang写道:
> linux/mmzone.h included linux/memory_hotplug.h,
> and linux/memory_hotplug.h also included
> linux/mmzone.h, so there's a bad cirlular.
> 
> these are quite mechanical changes by a simple
> script, I've tested for ARCH x86,arm,mips,
> may someone help to test more.
> 
> Signed-off-by: liguang 
> ---
> many thanks to Fengguang Wu 
> and Stephen Rothwell  who
> find build errors for v1,
> and also David Rientjes 
> who try to fix v1.
> I'm really regretful for the bold v1 which
> lack of consideration and test.
> 
>  arch/alpha/include/asm/pgalloc.h  |1 +
>  arch/alpha/include/asm/pgtable.h  |1 +
>  arch/avr32/mm/init.c  |1 +
>  arch/cris/arch-v10/mm/init.c  |1 +
>  arch/cris/arch-v32/mm/init.c  |1 +
>  arch/hexagon/kernel/setup.c   |1 +
>  arch/ia64/kernel/acpi.c   |1 +
>  arch/ia64/kernel/machine_kexec.c  |1 +
>  arch/ia64/mm/init.c   |1 +
>  arch/ia64/sn/kernel/setup.c   |1 +
>  arch/ia64/sn/kernel/sn2/sn2_smp.c |1 +
>  arch/m32r/mm/discontig.c  |1 +
>  arch/m68k/include/asm/virtconvert.h   |1 +
>  arch/mips/include/asm/pgtable.h   |1 +
>  arch/mips/include/asm/sn/mapped_kernel.h  |1 +
>  arch/mips/sgi-ip27/ip27-hubio.c   |1 +
>  arch/mips/sgi-ip27/ip27-klnuma.c  |1 +
>  arch/mips/sgi-ip27/ip27-memory.c  |1 +
>  arch/mips/sgi-ip27/ip27-nmi.c |1 +
>  arch/mips/sgi-ip27/ip27-reset.c   |1 +
>  arch/powerpc/mm/numa.c|1 +
>  arch/powerpc/platforms/ps3/spu.c  |1 +
>  arch/sh/kernel/setup.c|1 +
>  arch/sparc/mm/init_64.c   |1 +
>  arch/tile/gxio/kiorpc.c   |1 +
>  arch/tile/include/asm/pgalloc.h   |1 +
>  arch/tile/kernel/pci_gx.c |1 +
>  arch/tile/kernel/setup.c  |1 +
>  arch/tile/kernel/stack.c  |1 +
>  arch/x86/kernel/acpi/srat.c   |1 +
>  arch/x86/kernel/aperture_64.c |1 +
>  arch/x86/kernel/apic/numaq_32.c   |1 +
>  arch/x86/kernel/probe_roms.c  |1 +
>  arch/x86/kernel/setup.c   |1 +
>  arch/x86/kernel/topology.c|1 +
>  arch/x86/mm/numa.c|1 +
>  drivers/char/agp/amd64-agp.c  |1 +
>  drivers/edac/amd64_edac.h |1 +
>  drivers/edac/i5100_edac.c |1 +
>  drivers/edac/i5400_edac.c |1 +
>  drivers/edac/i7300_edac.c |1 +
>  drivers/edac/i7core_edac.c|1 +
>  drivers/edac/sb_edac.c|1 +
>  drivers/s390/char/sclp_cmd.c  |1 +
>  drivers/staging/tidspbridge/core/tiomap3430.c |1 +
>  drivers/video/vermilion/vermilion.c   |1 +
>  fs/file.c |1 +
>  fs/proc/meminfo.c |1 +
>  fs/proc/nommu.c   |1 +
>  fs/proc/page.c|1 +
>  include/linux/bootmem.h   |1 +
>  include/linux/gfp.h   |1 +
>  include/linux/memory_hotplug.h|1 -
>  include/linux/mempolicy.h |1 +
>  include/linux/mm.h|1 +
>  include/linux/mmzone.h|2 --
>  include/linux/swap.h  |1 +
>  include/linux/topology.h  |1 +
>  include/linux/vmstat.h|1 +
>  mm/kmemleak.c |1 +
>  mm/mlock.c|1 +
>  mm/mmzone.c   |1 +
>  mm/page_cgroup.c  |1 +
>  mm/quicklist.c|1 +
>  mm/sparse-vmemmap.c   |1 +
>  mm/sparse.c   |1 +
>  66 files changed, 64 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/alpha/include/asm/pgalloc.h 
> b/arch/alpha/include/asm/pgalloc.h
> index bc2a0da..ab9c10e 100644
> --- a/arch/alpha/include/asm/pgalloc.h
> +++ b/arch/alpha/include/asm/pgalloc.h
> @@ -3,6 +3,7 @@
>  
>  #include 
>  #include 
> +#include 
>  
>  /*  
>   * Allocate and free page tables. The xxx_kernel() versions are
> diff --git a/arch/alpha/include/asm/pgtable.h 
> b/arch/alpha/include/asm/pgtable.h
> index 81a4342..def3f86 100644
> --- 

Re: [rebased-again][PATCH 1/4] acpi: move x86/mm/srat.c to x86/kernel/acpi/srat.c

2013-02-25 Thread li guang
在 2013-02-25一的 14:51 -0800,David Rientjes写道:
> On Mon, 25 Feb 2013, liguang wrote:
> 
> > srat table should present only on acpi domain,
> > seems mm/ is not the right place for it.
> > 
> > Reviewed-by: Yasuaki Ishimatsu 
> > Signed-off-by: liguang 
> 
> You're still missing 01a178a94e8e ("acpi, memory-hotplug: support getting 
> hotplug info from SRAT") that was merged at the end of last week.  Instead 
> of continuing to rebase perhaps it would be better at this point to ask 
> the x86 maintainers to just do the patch for you?

maybe I have to ask them to help generate this patch,
I pulled latest since 25th, Feb, and re-generate this patch,
and it did have the commit you've pointed out.
Thanks!

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [rebased-again][PATCH 1/4] acpi: move x86/mm/srat.c to x86/kernel/acpi/srat.c

2013-02-25 Thread li guang
在 2013-02-25一的 14:51 -0800,David Rientjes写道:
 On Mon, 25 Feb 2013, liguang wrote:
 
  srat table should present only on acpi domain,
  seems mm/ is not the right place for it.
  
  Reviewed-by: Yasuaki Ishimatsu isimatu.yasu...@jp.fujitsu.com
  Signed-off-by: liguang lig.f...@cn.fujitsu.com
 
 You're still missing 01a178a94e8e (acpi, memory-hotplug: support getting 
 hotplug info from SRAT) that was merged at the end of last week.  Instead 
 of continuing to rebase perhaps it would be better at this point to ask 
 the x86 maintainers to just do the patch for you?

maybe I have to ask them to help generate this patch,
I pulled latest since 25th, Feb, and re-generate this patch,
and it did have the commit you've pointed out.
Thanks!

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2] mm: break circular include from linux/mmzone.h

2013-02-25 Thread li guang
ping ...

did any one find build errors after applying this patch?

在 2013-02-18一的 14:27 +0800,liguang写道:
 linux/mmzone.h included linux/memory_hotplug.h,
 and linux/memory_hotplug.h also included
 linux/mmzone.h, so there's a bad cirlular.
 
 these are quite mechanical changes by a simple
 script, I've tested for ARCH x86,arm,mips,
 may someone help to test more.
 
 Signed-off-by: liguang lig.f...@cn.fujitsu.com
 ---
 many thanks to Fengguang Wu fengguang...@intel.com
 and Stephen Rothwell s...@canb.auug.org.au who
 find build errors for v1,
 and also David Rientjes rient...@google.com
 who try to fix v1.
 I'm really regretful for the bold v1 which
 lack of consideration and test.
 
  arch/alpha/include/asm/pgalloc.h  |1 +
  arch/alpha/include/asm/pgtable.h  |1 +
  arch/avr32/mm/init.c  |1 +
  arch/cris/arch-v10/mm/init.c  |1 +
  arch/cris/arch-v32/mm/init.c  |1 +
  arch/hexagon/kernel/setup.c   |1 +
  arch/ia64/kernel/acpi.c   |1 +
  arch/ia64/kernel/machine_kexec.c  |1 +
  arch/ia64/mm/init.c   |1 +
  arch/ia64/sn/kernel/setup.c   |1 +
  arch/ia64/sn/kernel/sn2/sn2_smp.c |1 +
  arch/m32r/mm/discontig.c  |1 +
  arch/m68k/include/asm/virtconvert.h   |1 +
  arch/mips/include/asm/pgtable.h   |1 +
  arch/mips/include/asm/sn/mapped_kernel.h  |1 +
  arch/mips/sgi-ip27/ip27-hubio.c   |1 +
  arch/mips/sgi-ip27/ip27-klnuma.c  |1 +
  arch/mips/sgi-ip27/ip27-memory.c  |1 +
  arch/mips/sgi-ip27/ip27-nmi.c |1 +
  arch/mips/sgi-ip27/ip27-reset.c   |1 +
  arch/powerpc/mm/numa.c|1 +
  arch/powerpc/platforms/ps3/spu.c  |1 +
  arch/sh/kernel/setup.c|1 +
  arch/sparc/mm/init_64.c   |1 +
  arch/tile/gxio/kiorpc.c   |1 +
  arch/tile/include/asm/pgalloc.h   |1 +
  arch/tile/kernel/pci_gx.c |1 +
  arch/tile/kernel/setup.c  |1 +
  arch/tile/kernel/stack.c  |1 +
  arch/x86/kernel/acpi/srat.c   |1 +
  arch/x86/kernel/aperture_64.c |1 +
  arch/x86/kernel/apic/numaq_32.c   |1 +
  arch/x86/kernel/probe_roms.c  |1 +
  arch/x86/kernel/setup.c   |1 +
  arch/x86/kernel/topology.c|1 +
  arch/x86/mm/numa.c|1 +
  drivers/char/agp/amd64-agp.c  |1 +
  drivers/edac/amd64_edac.h |1 +
  drivers/edac/i5100_edac.c |1 +
  drivers/edac/i5400_edac.c |1 +
  drivers/edac/i7300_edac.c |1 +
  drivers/edac/i7core_edac.c|1 +
  drivers/edac/sb_edac.c|1 +
  drivers/s390/char/sclp_cmd.c  |1 +
  drivers/staging/tidspbridge/core/tiomap3430.c |1 +
  drivers/video/vermilion/vermilion.c   |1 +
  fs/file.c |1 +
  fs/proc/meminfo.c |1 +
  fs/proc/nommu.c   |1 +
  fs/proc/page.c|1 +
  include/linux/bootmem.h   |1 +
  include/linux/gfp.h   |1 +
  include/linux/memory_hotplug.h|1 -
  include/linux/mempolicy.h |1 +
  include/linux/mm.h|1 +
  include/linux/mmzone.h|2 --
  include/linux/swap.h  |1 +
  include/linux/topology.h  |1 +
  include/linux/vmstat.h|1 +
  mm/kmemleak.c |1 +
  mm/mlock.c|1 +
  mm/mmzone.c   |1 +
  mm/page_cgroup.c  |1 +
  mm/quicklist.c|1 +
  mm/sparse-vmemmap.c   |1 +
  mm/sparse.c   |1 +
  66 files changed, 64 insertions(+), 3 deletions(-)
 
 diff --git a/arch/alpha/include/asm/pgalloc.h 
 b/arch/alpha/include/asm/pgalloc.h
 index bc2a0da..ab9c10e 100644
 --- a/arch/alpha/include/asm/pgalloc.h
 +++ b/arch/alpha/include/asm/pgalloc.h
 @@ -3,6 +3,7 @@
  
  #include linux/mm.h
  #include linux/mmzone.h
 +#include linux/memory_hotplug.h
  
  /*  
   * Allocate and free page tables. The xxx_kernel() versions are
 diff --git a/arch/alpha/include/asm/pgtable.h 
 b/arch/alpha/include/asm/pgtable.h
 index 81a4342..def3f86 

Re: [rebased][PATCH 1/4] acpi: move x86/mm/srat.c to x86/kernel/acpi/srat.c

2013-02-24 Thread li guang
在 2013-02-22五的 12:57 -0800,David Rientjes写道:
> On Fri, 22 Feb 2013, liguang wrote:
> 
> > srat table should present only on acpi domain,
> > seems mm/ is not the right place for it.
> > 
> > Reviewed-by: Yasuaki Ishimatsu 
> > Signed-off-by: liguang 
> 
> This is not rebased, it does not have 4819e14ff31e ("acpi, movablemem_map: 
> Set numa_nodes_hotplug nodemask when using SRAT info.") but does have 
> f7c24b7e1c41 ("acpi, memory-hotplug: support getting hotplug info from 
> SRAT"), so I don't know how you're generating these patches.
> 
> You're also not sending this to any maintainer who would merge it, so 
> please run scripts/get_maintainer.pl on it.

my linux-next repo will not always be latest for inconvenient network
service. 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [rebased][PATCH 1/4] acpi: move x86/mm/srat.c to x86/kernel/acpi/srat.c

2013-02-24 Thread li guang
在 2013-02-22五的 12:57 -0800,David Rientjes写道:
 On Fri, 22 Feb 2013, liguang wrote:
 
  srat table should present only on acpi domain,
  seems mm/ is not the right place for it.
  
  Reviewed-by: Yasuaki Ishimatsu isimatu.yasu...@jp.fujitsu.com
  Signed-off-by: liguang lig.f...@cn.fujitsu.com
 
 This is not rebased, it does not have 4819e14ff31e (acpi, movablemem_map: 
 Set numa_nodes_hotplug nodemask when using SRAT info.) but does have 
 f7c24b7e1c41 (acpi, memory-hotplug: support getting hotplug info from 
 SRAT), so I don't know how you're generating these patches.
 
 You're also not sending this to any maintainer who would merge it, so 
 please run scripts/get_maintainer.pl on it.

my linux-next repo will not always be latest for inconvenient network
service. 


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 1/4] acpi: move x86/mm/srat.c to x86/kernel/acpi/srat.c

2013-02-19 Thread li guang
在 2013-02-19二的 23:00 -0800,David Rientjes写道:
> On Wed, 20 Feb 2013, li guang wrote:
> 
> > Yes, I know there's no new changes in my patch as I said before(not
> > based on lasted), but as I try to apply my patch(1/4), it will do
> > the right work to move current srat.c from arch/x86/mm/ to
> > arch/x86/kernel/acpi/ regardless of what I based is not latest.
> > 
> 
> Sorry, but that makes no sense.  Your patch cannot be used cleanly if 
> there have been subsequent changes to a hunk prior to applying it -- in 
> this case the hunk would be the entire file since you're removing it.
> 
> These patches would also be pushed by the x86 maintainers, who are not 
> cc'd on this patch, and I think it would be unfair to ask them to make up 
> for your inability to generate a bleeding edge patch with linux-next.  The 
> changes already cited in this thread have been in linux-next for almost 
> two weeks, yet you refuse to rebase on top of them.

OK, I'd like to rebase,
Thanks!


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 1/4] acpi: move x86/mm/srat.c to x86/kernel/acpi/srat.c

2013-02-19 Thread li guang
在 2013-02-19二的 19:04 -0800,David Rientjes写道:
> On Wed, 20 Feb 2013, li guang wrote:
> 
> > > > No, it doesn't.  From next-20130219, you're missing at least two 
> > > > patches:
> > > > 
> > > > 3795e4893203 ("acpi, memory-hotplug: extend movablemem_map ranges to 
> > > > the end of node")
> > > > 9a561f4dfd70 ("acpi, memory-hotplug: support getting hotplug info from 
> > > > SRAT")
> > > > 
> > > > I'm trying to be patient in reviewing your patches, but please double 
> > > > check your work.
> > > 
> > > I have to say my latest pull has these commit, and can apply
> > > successfully.
> > > really don't know why you can't apply.
> 
> You should not be pulling linux-next, you should be fetching.
> > 
> > I mean although my patches did not based on latest linux-next(miss
> > several commits), but as I can see, they can be applied to latest
> > linux-next (tag next-20130218) successfully.
> > 
> 
> next-20130218 is not the latest linux-next as I've already said.  Do this: 
> go to 
> http://git.kernel.org/?p=linux/kernel/git/next/linux-next-history.git;a=blob_plain;f=arch/x86/mm/srat.c;hb=HEAD
>  
> and search for handle_movablemem().  That's one of the functions added in 
> one of the commits I mentioned and was from February.  Now search your 
> patch for handle_movablemem().  See a problem?

Yes, I know there's no new changes in my patch as I said before(not
based on lasted), but as I try to apply my patch(1/4), it will do
the right work to move current srat.c from arch/x86/mm/ to
arch/x86/kernel/acpi/ regardless of what I based is not latest.



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 1/4] acpi: move x86/mm/srat.c to x86/kernel/acpi/srat.c

2013-02-19 Thread li guang
在 2013-02-20三的 10:21 +0800,li guang写道:
> 在 2013-02-19二的 18:08 -0800,David Rientjes写道:
> > On Wed, 20 Feb 2013, li guang wrote:
> > 
> > > > > This doesn't apply to Linus' tree nor does it apply to linux-next.  
> > > > > Which 
> > > > > tree are you basing this on?
> > > > 
> > > > I did based on linux-next, 
> > > > but it seems without commit 479a99a8e510c8839e0d3d3de8391f8bc61b9760
> > > > which you may want to see.
> > > > let me see why it missed.
> > > 
> > > I pulled latest for linux-next
> > > and can apply my patches smoothly.
> > > 
> > 
> > No, it doesn't.  From next-20130219, you're missing at least two patches:
> > 
> > 3795e4893203 ("acpi, memory-hotplug: extend movablemem_map ranges to the 
> > end of node")
> > 9a561f4dfd70 ("acpi, memory-hotplug: support getting hotplug info from 
> > SRAT")
> > 
> > I'm trying to be patient in reviewing your patches, but please double 
> > check your work.
> 
> I have to say my latest pull has these commit, and can apply
> successfully.
> really don't know why you can't apply.
> 

I mean although my patches did not based on latest linux-next(miss
several commits), but as I can see, they can be applied to latest
linux-next (tag next-20130218) successfully.



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 1/4] acpi: move x86/mm/srat.c to x86/kernel/acpi/srat.c

2013-02-19 Thread li guang
在 2013-02-19二的 18:08 -0800,David Rientjes写道:
> On Wed, 20 Feb 2013, li guang wrote:
> 
> > > > This doesn't apply to Linus' tree nor does it apply to linux-next.  
> > > > Which 
> > > > tree are you basing this on?
> > > 
> > > I did based on linux-next, 
> > > but it seems without commit 479a99a8e510c8839e0d3d3de8391f8bc61b9760
> > > which you may want to see.
> > > let me see why it missed.
> > 
> > I pulled latest for linux-next
> > and can apply my patches smoothly.
> > 
> 
> No, it doesn't.  From next-20130219, you're missing at least two patches:
> 
> 3795e4893203 ("acpi, memory-hotplug: extend movablemem_map ranges to the end 
> of node")
> 9a561f4dfd70 ("acpi, memory-hotplug: support getting hotplug info from SRAT")
> 
> I'm trying to be patient in reviewing your patches, but please double 
> check your work.

I have to say my latest pull has these commit, and can apply
successfully.
really don't know why you can't apply.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 1/4] acpi: move x86/mm/srat.c to x86/kernel/acpi/srat.c

2013-02-19 Thread li guang
在 2013-02-20三的 09:39 +0800,li guang写道:
> 在 2013-02-19二的 13:21 -0800,David Rientjes写道:
> > On Fri, 15 Feb 2013, liguang wrote:
> > 
> > > srat table should present only on acpi domain,
> > > seems mm/ is not the right place for it.
> > > 
> > > Reviewed-by: Yasuaki Ishimatsu 
> > > Signed-off-by: liguang 
> > 
> > This doesn't apply to Linus' tree nor does it apply to linux-next.  Which 
> > tree are you basing this on?
> 
> I did based on linux-next, 
> but it seems without commit 479a99a8e510c8839e0d3d3de8391f8bc61b9760
> which you may want to see.
> let me see why it missed.

I pulled latest for linux-next
and can apply my patches smoothly.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 1/4] acpi: move x86/mm/srat.c to x86/kernel/acpi/srat.c

2013-02-19 Thread li guang
在 2013-02-19二的 13:21 -0800,David Rientjes写道:
> On Fri, 15 Feb 2013, liguang wrote:
> 
> > srat table should present only on acpi domain,
> > seems mm/ is not the right place for it.
> > 
> > Reviewed-by: Yasuaki Ishimatsu 
> > Signed-off-by: liguang 
> 
> This doesn't apply to Linus' tree nor does it apply to linux-next.  Which 
> tree are you basing this on?

I did based on linux-next, 
but it seems without commit 479a99a8e510c8839e0d3d3de8391f8bc61b9760
which you may want to see.
let me see why it missed.



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 6/6] Input: Add ChromeOS EC keyboard driver

2013-02-19 Thread li guang
在 2013-02-15五的 20:16 -0800,Simon Glass写道:
> Use the key-matrix layer to interpret key scan information from the EC
> and inject input based on the FDT-supplied key map. This driver registers
> itself with the ChromeOS EC driver to perform communications.

[snip ...]
> +/*
> + * Returns true when there is at least one combination of pressed keys that
> + * results in ghosting.
> + */
> +static bool cros_ec_keyb_has_ghosting(struct cros_ec_keyb *ckdev, uint8_t 
> *buf)
> +{
> + int row;
> +
> + /*
> +  * Ghosting happens if for any pressed key X there are other keys
> +  * pressed both in the same row and column of X as, for instance,
> +  * in the following diagram:
> +  *
> +  * . . Y . g .
> +  * . . . . . .
> +  * . . . . . .
> +  * . . X . Z .
> +  *
> +  * In this case only X, Y, and Z are pressed, but g appears to be
> +  * pressed too (see Wikipedia).
> +  *
> +  * We can detect ghosting in a single pass (*) over the keyboard state
> +  * by maintaining two arrays.  pressed_in_row counts how many pressed
> +  * keys we have found in a row.  row_has_teeth is true if any of the
> +  * pressed keys for this row has other pressed keys in its column.  If
> +  * at any point of the scan we find that a row has multiple pressed
> +  * keys, and at least one of them is at the intersection with a column
> +  * with multiple pressed keys, we're sure there is ghosting.
> +  * Conversely, if there is ghosting, we will detect such situation for
> +  * at least one key during the pass.
> +  *
> +  * (*) This looks linear in the number of keys, but it's not.  We can
> +  * cheat because the number of rows is small.
> +  */
> + for (row = 0; row < ckdev->rows; row++) {
> + if (cros_ec_keyb_row_has_ghosting(ckdev, buf, row))
> + return true;
> + }
> +
> + return false;
> +}

are you sure your EC's firmware did not do ghost-key detection?
or, did you test ghost-key with/without your own ghost-key detection?
as far as I know, ghost-key should be take care either by keyboard
designer or firmware.

> +
> +/*
> + * Compares the new keyboard state to the old one and produces key
> + * press/release events accordingly.  The keyboard state is 13 bytes (one 
> byte
> + * per column)
> + */
> +static void cros_ec_keyb_process(struct cros_ec_keyb *ckdev,
> +  uint8_t *kb_state, int len)
> +{
> + int col, row;
> + int new_state;
> + int num_cols;
> +
> + num_cols = len;
> +
> + if (ckdev->ghost_filter && cros_ec_keyb_has_ghosting(ckdev, kb_state)) {
> + /*
> +  * Simple-minded solution: ignore this state. The obvious
> +  * improvement is to only ignore changes to keys involved in
> +  * the ghosting, but process the other changes.
> +  */
> + dev_dbg(ckdev->dev, "ghosting found\n");
> + return;
> + }
> +
> + for (col = 0; col < ckdev->cols; col++) {
> + for (row = 0; row < ckdev->rows; row++) {
> + int code = MATRIX_SCAN_CODE(row, col, ckdev->row_shift);
> +
> + new_state = kb_state[col] & (1 << row);
> + if (!!new_state != test_bit(code, ckdev->idev->key)) {
> + dev_dbg(ckdev->dev,
> + "changed: [r%d c%d]: byte %02x\n",
> + row, col, new_state);
> +
> + input_report_key(ckdev->idev, code, new_state);
> + }
> + }
> + }
> + input_sync(ckdev->idev);
> +}
> +
> +static int cros_ec_keyb_open(struct input_dev *dev)
> +{
> + struct cros_ec_keyb *ckdev = input_get_drvdata(dev);
> +
> + return blocking_notifier_chain_register(>ec->event_notifier,
> + >notifier);
> +}
> +
> +static void cros_ec_keyb_close(struct input_dev *dev)
> +{
> + struct cros_ec_keyb *ckdev = input_get_drvdata(dev);
> +
> + blocking_notifier_chain_unregister(>ec->event_notifier,
> +>notifier);
> +}
> +
> +static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, uint8_t 
> *kb_state)
> +{
> + return ckdev->ec->command_recv(ckdev->ec, EC_CMD_MKBP_STATE,
> +   kb_state, ckdev->cols);
> +}
> +
> +static int cros_ec_keyb_work(struct notifier_block *nb,
> +  unsigned long state, void *_notify)
> +{
> + int ret;
> + struct cros_ec_keyb *ckdev = container_of(nb, struct cros_ec_keyb,
> + notifier);
> + uint8_t kb_state[ckdev->cols];
> +
> + ret = cros_ec_keyb_get_state(ckdev, kb_state);
> + if (ret >= 0)
> + cros_ec_keyb_process(ckdev, kb_state, ret);
> +
> + return NOTIFY_DONE;
> +}
> +
> +/* Clear any keys in the 

Re: [PATCH v4 6/6] Input: Add ChromeOS EC keyboard driver

2013-02-19 Thread li guang
在 2013-02-15五的 20:16 -0800,Simon Glass写道:
 Use the key-matrix layer to interpret key scan information from the EC
 and inject input based on the FDT-supplied key map. This driver registers
 itself with the ChromeOS EC driver to perform communications.

[snip ...]
 +/*
 + * Returns true when there is at least one combination of pressed keys that
 + * results in ghosting.
 + */
 +static bool cros_ec_keyb_has_ghosting(struct cros_ec_keyb *ckdev, uint8_t 
 *buf)
 +{
 + int row;
 +
 + /*
 +  * Ghosting happens if for any pressed key X there are other keys
 +  * pressed both in the same row and column of X as, for instance,
 +  * in the following diagram:
 +  *
 +  * . . Y . g .
 +  * . . . . . .
 +  * . . . . . .
 +  * . . X . Z .
 +  *
 +  * In this case only X, Y, and Z are pressed, but g appears to be
 +  * pressed too (see Wikipedia).
 +  *
 +  * We can detect ghosting in a single pass (*) over the keyboard state
 +  * by maintaining two arrays.  pressed_in_row counts how many pressed
 +  * keys we have found in a row.  row_has_teeth is true if any of the
 +  * pressed keys for this row has other pressed keys in its column.  If
 +  * at any point of the scan we find that a row has multiple pressed
 +  * keys, and at least one of them is at the intersection with a column
 +  * with multiple pressed keys, we're sure there is ghosting.
 +  * Conversely, if there is ghosting, we will detect such situation for
 +  * at least one key during the pass.
 +  *
 +  * (*) This looks linear in the number of keys, but it's not.  We can
 +  * cheat because the number of rows is small.
 +  */
 + for (row = 0; row  ckdev-rows; row++) {
 + if (cros_ec_keyb_row_has_ghosting(ckdev, buf, row))
 + return true;
 + }
 +
 + return false;
 +}

are you sure your EC's firmware did not do ghost-key detection?
or, did you test ghost-key with/without your own ghost-key detection?
as far as I know, ghost-key should be take care either by keyboard
designer or firmware.

 +
 +/*
 + * Compares the new keyboard state to the old one and produces key
 + * press/release events accordingly.  The keyboard state is 13 bytes (one 
 byte
 + * per column)
 + */
 +static void cros_ec_keyb_process(struct cros_ec_keyb *ckdev,
 +  uint8_t *kb_state, int len)
 +{
 + int col, row;
 + int new_state;
 + int num_cols;
 +
 + num_cols = len;
 +
 + if (ckdev-ghost_filter  cros_ec_keyb_has_ghosting(ckdev, kb_state)) {
 + /*
 +  * Simple-minded solution: ignore this state. The obvious
 +  * improvement is to only ignore changes to keys involved in
 +  * the ghosting, but process the other changes.
 +  */
 + dev_dbg(ckdev-dev, ghosting found\n);
 + return;
 + }
 +
 + for (col = 0; col  ckdev-cols; col++) {
 + for (row = 0; row  ckdev-rows; row++) {
 + int code = MATRIX_SCAN_CODE(row, col, ckdev-row_shift);
 +
 + new_state = kb_state[col]  (1  row);
 + if (!!new_state != test_bit(code, ckdev-idev-key)) {
 + dev_dbg(ckdev-dev,
 + changed: [r%d c%d]: byte %02x\n,
 + row, col, new_state);
 +
 + input_report_key(ckdev-idev, code, new_state);
 + }
 + }
 + }
 + input_sync(ckdev-idev);
 +}
 +
 +static int cros_ec_keyb_open(struct input_dev *dev)
 +{
 + struct cros_ec_keyb *ckdev = input_get_drvdata(dev);
 +
 + return blocking_notifier_chain_register(ckdev-ec-event_notifier,
 + ckdev-notifier);
 +}
 +
 +static void cros_ec_keyb_close(struct input_dev *dev)
 +{
 + struct cros_ec_keyb *ckdev = input_get_drvdata(dev);
 +
 + blocking_notifier_chain_unregister(ckdev-ec-event_notifier,
 +ckdev-notifier);
 +}
 +
 +static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, uint8_t 
 *kb_state)
 +{
 + return ckdev-ec-command_recv(ckdev-ec, EC_CMD_MKBP_STATE,
 +   kb_state, ckdev-cols);
 +}
 +
 +static int cros_ec_keyb_work(struct notifier_block *nb,
 +  unsigned long state, void *_notify)
 +{
 + int ret;
 + struct cros_ec_keyb *ckdev = container_of(nb, struct cros_ec_keyb,
 + notifier);
 + uint8_t kb_state[ckdev-cols];
 +
 + ret = cros_ec_keyb_get_state(ckdev, kb_state);
 + if (ret = 0)
 + cros_ec_keyb_process(ckdev, kb_state, ret);
 +
 + return NOTIFY_DONE;
 +}
 +
 +/* Clear any keys in the buffer */
 +static void cros_ec_keyb_clear_keyboard(struct cros_ec_keyb *ckdev)
 +{
 + uint8_t old_state[ckdev-cols];
 + uint8_t 

Re: [PATCH v4 1/4] acpi: move x86/mm/srat.c to x86/kernel/acpi/srat.c

2013-02-19 Thread li guang
在 2013-02-19二的 13:21 -0800,David Rientjes写道:
 On Fri, 15 Feb 2013, liguang wrote:
 
  srat table should present only on acpi domain,
  seems mm/ is not the right place for it.
  
  Reviewed-by: Yasuaki Ishimatsu isimatu.yasu...@jp.fujitsu.com
  Signed-off-by: liguang lig.f...@cn.fujitsu.com
 
 This doesn't apply to Linus' tree nor does it apply to linux-next.  Which 
 tree are you basing this on?

I did based on linux-next, 
but it seems without commit 479a99a8e510c8839e0d3d3de8391f8bc61b9760
which you may want to see.
let me see why it missed.



--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 1/4] acpi: move x86/mm/srat.c to x86/kernel/acpi/srat.c

2013-02-19 Thread li guang
在 2013-02-20三的 09:39 +0800,li guang写道:
 在 2013-02-19二的 13:21 -0800,David Rientjes写道:
  On Fri, 15 Feb 2013, liguang wrote:
  
   srat table should present only on acpi domain,
   seems mm/ is not the right place for it.
   
   Reviewed-by: Yasuaki Ishimatsu isimatu.yasu...@jp.fujitsu.com
   Signed-off-by: liguang lig.f...@cn.fujitsu.com
  
  This doesn't apply to Linus' tree nor does it apply to linux-next.  Which 
  tree are you basing this on?
 
 I did based on linux-next, 
 but it seems without commit 479a99a8e510c8839e0d3d3de8391f8bc61b9760
 which you may want to see.
 let me see why it missed.

I pulled latest for linux-next
and can apply my patches smoothly.


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 1/4] acpi: move x86/mm/srat.c to x86/kernel/acpi/srat.c

2013-02-19 Thread li guang
在 2013-02-19二的 18:08 -0800,David Rientjes写道:
 On Wed, 20 Feb 2013, li guang wrote:
 
This doesn't apply to Linus' tree nor does it apply to linux-next.  
Which 
tree are you basing this on?
   
   I did based on linux-next, 
   but it seems without commit 479a99a8e510c8839e0d3d3de8391f8bc61b9760
   which you may want to see.
   let me see why it missed.
  
  I pulled latest for linux-next
  and can apply my patches smoothly.
  
 
 No, it doesn't.  From next-20130219, you're missing at least two patches:
 
 3795e4893203 (acpi, memory-hotplug: extend movablemem_map ranges to the end 
 of node)
 9a561f4dfd70 (acpi, memory-hotplug: support getting hotplug info from SRAT)
 
 I'm trying to be patient in reviewing your patches, but please double 
 check your work.

I have to say my latest pull has these commit, and can apply
successfully.
really don't know why you can't apply.


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 1/4] acpi: move x86/mm/srat.c to x86/kernel/acpi/srat.c

2013-02-19 Thread li guang
在 2013-02-20三的 10:21 +0800,li guang写道:
 在 2013-02-19二的 18:08 -0800,David Rientjes写道:
  On Wed, 20 Feb 2013, li guang wrote:
  
 This doesn't apply to Linus' tree nor does it apply to linux-next.  
 Which 
 tree are you basing this on?

I did based on linux-next, 
but it seems without commit 479a99a8e510c8839e0d3d3de8391f8bc61b9760
which you may want to see.
let me see why it missed.
   
   I pulled latest for linux-next
   and can apply my patches smoothly.
   
  
  No, it doesn't.  From next-20130219, you're missing at least two patches:
  
  3795e4893203 (acpi, memory-hotplug: extend movablemem_map ranges to the 
  end of node)
  9a561f4dfd70 (acpi, memory-hotplug: support getting hotplug info from 
  SRAT)
  
  I'm trying to be patient in reviewing your patches, but please double 
  check your work.
 
 I have to say my latest pull has these commit, and can apply
 successfully.
 really don't know why you can't apply.
 

I mean although my patches did not based on latest linux-next(miss
several commits), but as I can see, they can be applied to latest
linux-next (tag next-20130218) successfully.



--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 1/4] acpi: move x86/mm/srat.c to x86/kernel/acpi/srat.c

2013-02-19 Thread li guang
在 2013-02-19二的 19:04 -0800,David Rientjes写道:
 On Wed, 20 Feb 2013, li guang wrote:
 
No, it doesn't.  From next-20130219, you're missing at least two 
patches:

3795e4893203 (acpi, memory-hotplug: extend movablemem_map ranges to 
the end of node)
9a561f4dfd70 (acpi, memory-hotplug: support getting hotplug info from 
SRAT)

I'm trying to be patient in reviewing your patches, but please double 
check your work.
   
   I have to say my latest pull has these commit, and can apply
   successfully.
   really don't know why you can't apply.
 
 You should not be pulling linux-next, you should be fetching.
  
  I mean although my patches did not based on latest linux-next(miss
  several commits), but as I can see, they can be applied to latest
  linux-next (tag next-20130218) successfully.
  
 
 next-20130218 is not the latest linux-next as I've already said.  Do this: 
 go to 
 http://git.kernel.org/?p=linux/kernel/git/next/linux-next-history.git;a=blob_plain;f=arch/x86/mm/srat.c;hb=HEAD
  
 and search for handle_movablemem().  That's one of the functions added in 
 one of the commits I mentioned and was from February.  Now search your 
 patch for handle_movablemem().  See a problem?

Yes, I know there's no new changes in my patch as I said before(not
based on lasted), but as I try to apply my patch(1/4), it will do
the right work to move current srat.c from arch/x86/mm/ to
arch/x86/kernel/acpi/ regardless of what I based is not latest.



--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 1/4] acpi: move x86/mm/srat.c to x86/kernel/acpi/srat.c

2013-02-19 Thread li guang
在 2013-02-19二的 23:00 -0800,David Rientjes写道:
 On Wed, 20 Feb 2013, li guang wrote:
 
  Yes, I know there's no new changes in my patch as I said before(not
  based on lasted), but as I try to apply my patch(1/4), it will do
  the right work to move current srat.c from arch/x86/mm/ to
  arch/x86/kernel/acpi/ regardless of what I based is not latest.
  
 
 Sorry, but that makes no sense.  Your patch cannot be used cleanly if 
 there have been subsequent changes to a hunk prior to applying it -- in 
 this case the hunk would be the entire file since you're removing it.
 
 These patches would also be pushed by the x86 maintainers, who are not 
 cc'd on this patch, and I think it would be unfair to ask them to make up 
 for your inability to generate a bleeding edge patch with linux-next.  The 
 changes already cited in this thread have been in linux-next for almost 
 two weeks, yet you refuse to rebase on top of them.

OK, I'd like to rebase,
Thanks!


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Input: psmouse - retry getid command in psmouse_probe()

2013-02-18 Thread li guang
seems a special change for a special device
so, you may need to place this change with
corresponding CONFIG_xxx_xxx.
generally, if command F2 failed, we will assume
there's no ps2 device, it's normal, 
or do you have some materials(SPEC) to specify
the change you have made?



在 2012-10-31三的 18:11 +0800,Chung-yih Wang写道:
> As the synaptics device may not respond to the first command in psmouse_probe
> when a machine is booting up, the patch gives a second probe if the first
> one fails.
> 
> Signed-off-by: Chung-yih Wang 
> ---
>  drivers/input/mouse/psmouse-base.c |   13 +++--
>  1 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/input/mouse/psmouse-base.c 
> b/drivers/input/mouse/psmouse-base.c
> index 22fe254..c4fc5ad 100644
> --- a/drivers/input/mouse/psmouse-base.c
> +++ b/drivers/input/mouse/psmouse-base.c
> @@ -1053,8 +1053,17 @@ static int psmouse_probe(struct psmouse *psmouse)
>   */
>  
>   param[0] = 0xa5;
> - if (ps2_command(ps2dev, param, PSMOUSE_CMD_GETID))
> - return -1;
> + if (ps2_command(ps2dev, param, PSMOUSE_CMD_GETID)) {
> + /*
> +  * Reprobe the device if it did not respond to the GETID
> +  * command. Before retry, additional dummy command is sent
> +  * to clear the 'RESEND' response if exists.
> +  */
> + psmouse_warn(psmouse, "GETID probe failed, retrying...\n");
> + ps2_command(ps2dev, NULL, PSMOUSE_CMD_SETSCALE11);
> + if (ps2_command(ps2dev, param, PSMOUSE_CMD_GETID))
> + return -1;
> + }
>  
>   if (param[0] != 0x00 && param[0] != 0x03 &&
>   param[0] != 0x04 && param[0] != 0xff)


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Input: psmouse - retry getid command in psmouse_probe()

2013-02-18 Thread li guang
seems a special change for a special device
so, you may need to place this change with
corresponding CONFIG_xxx_xxx.
generally, if command F2 failed, we will assume
there's no ps2 device, it's normal, 
or do you have some materials(SPEC) to specify
the change you have made?



在 2012-10-31三的 18:11 +0800,Chung-yih Wang写道:
 As the synaptics device may not respond to the first command in psmouse_probe
 when a machine is booting up, the patch gives a second probe if the first
 one fails.
 
 Signed-off-by: Chung-yih Wang cyw...@chromium.org
 ---
  drivers/input/mouse/psmouse-base.c |   13 +++--
  1 files changed, 11 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/input/mouse/psmouse-base.c 
 b/drivers/input/mouse/psmouse-base.c
 index 22fe254..c4fc5ad 100644
 --- a/drivers/input/mouse/psmouse-base.c
 +++ b/drivers/input/mouse/psmouse-base.c
 @@ -1053,8 +1053,17 @@ static int psmouse_probe(struct psmouse *psmouse)
   */
  
   param[0] = 0xa5;
 - if (ps2_command(ps2dev, param, PSMOUSE_CMD_GETID))
 - return -1;
 + if (ps2_command(ps2dev, param, PSMOUSE_CMD_GETID)) {
 + /*
 +  * Reprobe the device if it did not respond to the GETID
 +  * command. Before retry, additional dummy command is sent
 +  * to clear the 'RESEND' response if exists.
 +  */
 + psmouse_warn(psmouse, GETID probe failed, retrying...\n);
 + ps2_command(ps2dev, NULL, PSMOUSE_CMD_SETSCALE11);
 + if (ps2_command(ps2dev, param, PSMOUSE_CMD_GETID))
 + return -1;
 + }
  
   if (param[0] != 0x00  param[0] != 0x03 
   param[0] != 0x04  param[0] != 0xff)


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v3 2/4] numa: avoid export acpi_numa variable

2013-02-05 Thread li guang
在 2013-02-05二的 16:55 +0900,Yasuaki Ishimatsu写道:
> 2013/02/05 16:36, liguang wrote:
> > acpi_numa is used to prevent srat table
> > being parsed, seems a little miss-named,
> > if 'noacpi' was specified by cmdline and
> > CONFIG_ACPI_NUMA was enabled, acpi_numa
> > will be operated directly from everywhere
> > it needed to disable/enable numa in acpi
> > mode which was a bad thing, so, try to
> > export a fuction to get srat table
> > enable/disable info.
> > 
> > Signed-off-by: liguang 
> > ---
> 
> Hmm. Did you test the patch?
> By the patch, srat_disable() returns false(0) or 1. As a result,
> acpi_numa_x2apic_affinity_init(), acpi_numa_processor_affinity_init()
> and acpi_numa_memory_affinity_init() go wrong at following if sentence.
> 
> ---
>   if (srat_disable())
>   return -1;
> ---
> 
> When you change a return value of function, you should check othe
> functions which use it carefully.
> 
> And if you use acpi_numa as bool, you should use "acpi_numa = true"
> instead of "acpi_numa = 1"

Good observation!
maybe I'm a little hurry to go for dining, :)
Thanks! will fix.

> 
> Thanks,
> Yasuaki Ishimatsu
> 
> >   arch/x86/include/asm/acpi.h |2 +-
> >   arch/x86/kernel/acpi/srat.c |   17 +++--
> >   arch/x86/mm/numa.c  |2 +-
> >   arch/x86/xen/enlighten.c|2 +-
> >   4 files changed, 14 insertions(+), 9 deletions(-)
> > 
> > diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h
> > index b31bf97..449e12a 100644
> > --- a/arch/x86/include/asm/acpi.h
> > +++ b/arch/x86/include/asm/acpi.h
> > @@ -177,7 +177,7 @@ static inline void disable_acpi(void) { }
> >   #define ARCH_HAS_POWER_INIT   1
> >   
> >   #ifdef CONFIG_ACPI_NUMA
> > -extern int acpi_numa;
> > +extern void disable_acpi_numa(void);
> >   extern int x86_acpi_numa_init(void);
> >   #endif /* CONFIG_ACPI_NUMA */
> >   
> > diff --git a/arch/x86/kernel/acpi/srat.c b/arch/x86/kernel/acpi/srat.c
> > index cdd0da9..8d7f3f8 100644
> > --- a/arch/x86/kernel/acpi/srat.c
> > +++ b/arch/x86/kernel/acpi/srat.c
> > @@ -24,22 +24,27 @@
> >   #include 
> >   #include 
> >   
> > -int acpi_numa __initdata;
> > +static bool acpi_numa __initdata;
> >   
> >   static __init int setup_node(int pxm)
> >   {
> > return acpi_map_pxm_to_node(pxm);
> >   }
> >   
> > -static __init void bad_srat(void)
> > +void __init disable_acpi_numa(void)
> >   {
> > -   printk(KERN_ERR "SRAT: SRAT not used.\n");
> > -   acpi_numa = -1;
> > +   acpi_numa = false;
> >   }
> >   
> > -static __init inline int srat_disabled(void)
> > +static void __init bad_srat(void)
> >   {
> > -   return acpi_numa < 0;
> > +   disable_acpi_numa();
> > +   printk(KERN_ERR "SRAT: SRAT will not be used.\n");
> > +}
> > +
> > +static bool __init srat_disabled(void)
> > +{
> > +   return acpi_numa;
> >   }
> >   
> >   /* Callback for SLIT parsing */
> > diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c
> > index 245a4ba..1ebc907 100644
> > --- a/arch/x86/mm/numa.c
> > +++ b/arch/x86/mm/numa.c
> > @@ -47,7 +47,7 @@ static __init int numa_setup(char *opt)
> >   #endif
> >   #ifdef CONFIG_ACPI_NUMA
> > if (!strncmp(opt, "noacpi", 6))
> > -   acpi_numa = -1;
> > +   disable_acpi_numa();
> >   #endif
> > return 0;
> >   }
> > diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> > index 657eca9..3636bc6 100644
> > --- a/arch/x86/xen/enlighten.c
> > +++ b/arch/x86/xen/enlighten.c
> > @@ -1447,7 +1447,7 @@ asmlinkage void __init xen_start_kernel(void)
> >  * any NUMA information the kernel tries to get from ACPI will
> >  * be meaningless.  Prevent it from trying.
> >  */
> > -   acpi_numa = -1;
> > +   disable_acpi_numa();
> >   #endif
> >   
> > /* Don't do the full vcpu_info placement stuff until we have a
> > 
> 
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v3 2/4] numa: avoid export acpi_numa variable

2013-02-05 Thread li guang
在 2013-02-05二的 16:55 +0900,Yasuaki Ishimatsu写道:
 2013/02/05 16:36, liguang wrote:
  acpi_numa is used to prevent srat table
  being parsed, seems a little miss-named,
  if 'noacpi' was specified by cmdline and
  CONFIG_ACPI_NUMA was enabled, acpi_numa
  will be operated directly from everywhere
  it needed to disable/enable numa in acpi
  mode which was a bad thing, so, try to
  export a fuction to get srat table
  enable/disable info.
  
  Signed-off-by: liguang lig.f...@cn.fujitsu.com
  ---
 
 Hmm. Did you test the patch?
 By the patch, srat_disable() returns false(0) or 1. As a result,
 acpi_numa_x2apic_affinity_init(), acpi_numa_processor_affinity_init()
 and acpi_numa_memory_affinity_init() go wrong at following if sentence.
 
 ---
   if (srat_disable())
   return -1;
 ---
 
 When you change a return value of function, you should check othe
 functions which use it carefully.
 
 And if you use acpi_numa as bool, you should use acpi_numa = true
 instead of acpi_numa = 1

Good observation!
maybe I'm a little hurry to go for dining, :)
Thanks! will fix.

 
 Thanks,
 Yasuaki Ishimatsu
 
arch/x86/include/asm/acpi.h |2 +-
arch/x86/kernel/acpi/srat.c |   17 +++--
arch/x86/mm/numa.c  |2 +-
arch/x86/xen/enlighten.c|2 +-
4 files changed, 14 insertions(+), 9 deletions(-)
  
  diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h
  index b31bf97..449e12a 100644
  --- a/arch/x86/include/asm/acpi.h
  +++ b/arch/x86/include/asm/acpi.h
  @@ -177,7 +177,7 @@ static inline void disable_acpi(void) { }
#define ARCH_HAS_POWER_INIT   1

#ifdef CONFIG_ACPI_NUMA
  -extern int acpi_numa;
  +extern void disable_acpi_numa(void);
extern int x86_acpi_numa_init(void);
#endif /* CONFIG_ACPI_NUMA */

  diff --git a/arch/x86/kernel/acpi/srat.c b/arch/x86/kernel/acpi/srat.c
  index cdd0da9..8d7f3f8 100644
  --- a/arch/x86/kernel/acpi/srat.c
  +++ b/arch/x86/kernel/acpi/srat.c
  @@ -24,22 +24,27 @@
#include asm/apic.h
#include asm/uv/uv.h

  -int acpi_numa __initdata;
  +static bool acpi_numa __initdata;

static __init int setup_node(int pxm)
{
  return acpi_map_pxm_to_node(pxm);
}

  -static __init void bad_srat(void)
  +void __init disable_acpi_numa(void)
{
  -   printk(KERN_ERR SRAT: SRAT not used.\n);
  -   acpi_numa = -1;
  +   acpi_numa = false;
}

  -static __init inline int srat_disabled(void)
  +static void __init bad_srat(void)
{
  -   return acpi_numa  0;
  +   disable_acpi_numa();
  +   printk(KERN_ERR SRAT: SRAT will not be used.\n);
  +}
  +
  +static bool __init srat_disabled(void)
  +{
  +   return acpi_numa;
}

/* Callback for SLIT parsing */
  diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c
  index 245a4ba..1ebc907 100644
  --- a/arch/x86/mm/numa.c
  +++ b/arch/x86/mm/numa.c
  @@ -47,7 +47,7 @@ static __init int numa_setup(char *opt)
#endif
#ifdef CONFIG_ACPI_NUMA
  if (!strncmp(opt, noacpi, 6))
  -   acpi_numa = -1;
  +   disable_acpi_numa();
#endif
  return 0;
}
  diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
  index 657eca9..3636bc6 100644
  --- a/arch/x86/xen/enlighten.c
  +++ b/arch/x86/xen/enlighten.c
  @@ -1447,7 +1447,7 @@ asmlinkage void __init xen_start_kernel(void)
   * any NUMA information the kernel tries to get from ACPI will
   * be meaningless.  Prevent it from trying.
   */
  -   acpi_numa = -1;
  +   disable_acpi_numa();
#endif

  /* Don't do the full vcpu_info placement stuff until we have a
  
 
 


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] mm: break circular include from linux/mmzone.h

2013-02-04 Thread li guang
在 2013-02-04一的 21:20 -0800,David Rientjes写道:
> On Tue, 5 Feb 2013, liguang wrote:
> 
> > linux/mmzone.h included linux/memory_hotplug.h,
> > and linux/memory_hotplug.h also included
> > linux/mmzone.h, so there's a bad cirlular.
> > 
> 
> And both of these are protected by _LINUX_MMZONE_H and 
> __LINUX_MEMORY_HOTPLUG_H, respectively, so what's the problem?

obviously, It's a logical error,
and It has no more effect other than
combination of these 2 header files.
so, why don't we separate them?





--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2 1/4] acpi: move x86/mm/srat.c to x86/kernel/acpi/srat.c

2013-02-04 Thread li guang
在 2013-02-04一的 21:35 -0800,David Rientjes写道:
> On Tue, 5 Feb 2013, liguang wrote:
> 
> > srat table should present only on acpi domain,
> > seems mm/ is not the right place for it.
> > 
> > Reviewed-by: David Rientjes 
> 
> I certainly didn't review this, please read 
> Documentation/SubmittingPatches.
> 
> > Signed-off-by: liguang 
> 
> Couple of issues with this:
> 
>  - it doesn't include your followup change "acpi: change Makefile for
>srat.c building" which is required for this to build, please fold it in 
>and repost,
> 
>  - there is whitespace damage, we use tabs for indentation in Makefiles,
> 
>  - you're basing your patches on Linus' tree when arch/x86/mm/srat has
>changed in linux-next (15 insertions(+), 14 deletions(-)), please 
>rebase this on the linux-next tree at git.kernel.org.
> 
> Otherwise, looks good.

OK, I'll rebase on linux-next.
Thanks!


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2 2/4] numa: avoid export acpi_numa variable

2013-02-04 Thread li guang
在 2013-02-04一的 21:39 -0800,David Rientjes写道:
> On Tue, 5 Feb 2013, liguang wrote:
> 
> > acpi_numa is used to prevent srat table
> > being parsed, seems a little miss-named,
> > if 'noacpi' was specified by cmdline and
> > CONFIG_ACPI_NUMA was enabled, acpi_numa
> > will be operated directly from everywhere
> > it needed to disable/enable numa in acpi
> > mode which was a bad thing, so, try to
> > export a fuction to get srat table
> > enable/disable info.
> > 
> > Reviewed-by: David Rientjes 
> 
> Again, this was never reviewed by me, please learn how to use these tags 
> appropriately by reading Documentation/SubmittingPatches.

OK.

> 
> > Signed-off-by: liguang 
> > ---
> >  arch/x86/include/asm/acpi.h |2 +-
> >  arch/x86/kernel/acpi/srat.c |   15 ++-
> >  arch/x86/mm/numa.c  |2 +-
> >  arch/x86/xen/enlighten.c|2 +-
> >  4 files changed, 13 insertions(+), 8 deletions(-)
> > 
> > diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h
> > index 0c44630..6b9c861 100644
> > --- a/arch/x86/include/asm/acpi.h
> > +++ b/arch/x86/include/asm/acpi.h
> > @@ -181,7 +181,7 @@ static inline void disable_acpi(void) { }
> >  #define ARCH_HAS_POWER_INIT1
> >  
> >  #ifdef CONFIG_ACPI_NUMA
> > -extern int acpi_numa;
> > +extern void disable_acpi_numa(void);
> >  extern int x86_acpi_numa_init(void);
> >  #endif /* CONFIG_ACPI_NUMA */
> >  
> > diff --git a/arch/x86/kernel/acpi/srat.c b/arch/x86/kernel/acpi/srat.c
> > index 4ddf497..0a4d7ee 100644
> > --- a/arch/x86/kernel/acpi/srat.c
> > +++ b/arch/x86/kernel/acpi/srat.c
> > @@ -24,22 +24,27 @@
> >  #include 
> >  #include 
> >  
> > -int acpi_numa __initdata;
> > +static bool acpi_numa __initdata;
> >  
> >  static __init int setup_node(int pxm)
> >  {
> > return acpi_map_pxm_to_node(pxm);
> >  }
> >  
> > -static __init void bad_srat(void)
> > +void __init disable_acpi_numa(void)
> > +{
> > +   acpi_numa = false;
> > +}
> > +
> > +static void __init bad_srat(void)
> >  {
> > -   printk(KERN_ERR "SRAT: SRAT not used.\n");
> > acpi_numa = -1;
> > +   printk(KERN_ERR "SRAT: SRAT will not be used.\n");
> >  }
> >  
> > -static __init inline int srat_disabled(void)
> > +static bool __init srat_disabled(void)
> >  {
> > -   return acpi_numa < 0;
> > +   return acpi_numa;
> >  }
> >  
> >  /* Callback for SLIT parsing */
> > diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c
> > index 2d125be..870ca6b 100644
> > --- a/arch/x86/mm/numa.c
> > +++ b/arch/x86/mm/numa.c
> > @@ -47,7 +47,7 @@ static __init int numa_setup(char *opt)
> >  #endif
> >  #ifdef CONFIG_ACPI_NUMA
> > if (!strncmp(opt, "noacpi", 6))
> > -   acpi_numa = -1;
> > +   disable_acpi_numa();
> >  #endif
> > return 0;
> >  }
> > diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> > index 138e566..a5f6353 100644
> > --- a/arch/x86/xen/enlighten.c
> > +++ b/arch/x86/xen/enlighten.c
> > @@ -1415,7 +1415,7 @@ asmlinkage void __init xen_start_kernel(void)
> >  * any NUMA information the kernel tries to get from ACPI will
> >  * be meaningless.  Prevent it from trying.
> >  */
> > -   acpi_numa = -1;
> > +   disable_acpi_numa();
> >  #endif
> >  
> > /* Don't do the full vcpu_info placement stuff until we have a
> 
> You've declared disable_acpi_numa() but never defined it so this will 
> fail at link time.  Please compile and test all of your changes with care 
> before posting them or people will start to ignore your contributions.

It did pass build.



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2 1/4] acpi: move x86/mm/srat.c to x86/kernel/acpi/srat.c

2013-02-04 Thread li guang
在 2013-02-05二的 13:44 +0900,Yasuaki Ishimatsu写道:
> Hi Liguang,
> 
> 2013/02/05 11:37, liguang wrote:
> > srat table should present only on acpi domain,
> > seems mm/ is not the right place for it.
> > 
> > Reviewed-by: David Rientjes 
> > Signed-off-by: liguang 
> > ---
> >
> >   arch/x86/kernel/acpi/srat.c |  197 
> > +++
> >   arch/x86/mm/srat.c  |  197 
> > ---
> >   2 files changed, 197 insertions(+), 197 deletions(-)
> >   create mode 100644 arch/x86/kernel/acpi/srat.c
> >   delete mode 100644 arch/x86/mm/srat.c
> 
> Changes of Makefile disappeared.
> 
> Thanks,
> Yasuaki Ishimatsu

Oh, fogot it, 
Thank you so much for reminding me!

> 
> > 
> > diff --git a/arch/x86/kernel/acpi/srat.c b/arch/x86/kernel/acpi/srat.c
> > new file mode 100644
> > index 000..4ddf497
> > --- /dev/null
> > +++ b/arch/x86/kernel/acpi/srat.c
> > @@ -0,0 +1,197 @@
> > +/*
> > + * ACPI 3.0 based NUMA setup
> > + * Copyright 2004 Andi Kleen, SuSE Labs.
> > + *
> > + * Reads the ACPI SRAT table to figure out what memory belongs to which 
> > CPUs.
> > + *
> > + * Called from acpi_numa_init while reading the SRAT and SLIT tables.
> > + * Assumes all memory regions belonging to a single proximity domain
> > + * are in one chunk. Holes between them will be included in the node.
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +int acpi_numa __initdata;
> > +
> > +static __init int setup_node(int pxm)
> > +{
> > +   return acpi_map_pxm_to_node(pxm);
> > +}
> > +
> > +static __init void bad_srat(void)
> > +{
> > +   printk(KERN_ERR "SRAT: SRAT not used.\n");
> > +   acpi_numa = -1;
> > +}
> > +
> > +static __init inline int srat_disabled(void)
> > +{
> > +   return acpi_numa < 0;
> > +}
> > +
> > +/* Callback for SLIT parsing */
> > +void __init acpi_numa_slit_init(struct acpi_table_slit *slit)
> > +{
> > +   int i, j;
> > +
> > +   for (i = 0; i < slit->locality_count; i++)
> > +   for (j = 0; j < slit->locality_count; j++)
> > +   numa_set_distance(pxm_to_node(i), pxm_to_node(j),
> > +   slit->entry[slit->locality_count * i + j]);
> > +}
> > +
> > +/* Callback for Proximity Domain -> x2APIC mapping */
> > +void __init
> > +acpi_numa_x2apic_affinity_init(struct acpi_srat_x2apic_cpu_affinity *pa)
> > +{
> > +   int pxm, node;
> > +   int apic_id;
> > +
> > +   if (srat_disabled())
> > +   return;
> > +   if (pa->header.length < sizeof(struct acpi_srat_x2apic_cpu_affinity)) {
> > +   bad_srat();
> > +   return;
> > +   }
> > +   if ((pa->flags & ACPI_SRAT_CPU_ENABLED) == 0)
> > +   return;
> > +   pxm = pa->proximity_domain;
> > +   apic_id = pa->apic_id;
> > +   if (!apic->apic_id_valid(apic_id)) {
> > +   printk(KERN_INFO "SRAT: PXM %u -> X2APIC 0x%04x ignored\n",
> > +pxm, apic_id);
> > +   return;
> > +   }
> > +   node = setup_node(pxm);
> > +   if (node < 0) {
> > +   printk(KERN_ERR "SRAT: Too many proximity domains %x\n", pxm);
> > +   bad_srat();
> > +   return;
> > +   }
> > +
> > +   if (apic_id >= MAX_LOCAL_APIC) {
> > +   printk(KERN_INFO "SRAT: PXM %u -> APIC 0x%04x -> Node %u 
> > skipped apicid that is too big\n", pxm, apic_id, node);
> > +   return;
> > +   }
> > +   set_apicid_to_node(apic_id, node);
> > +   node_set(node, numa_nodes_parsed);
> > +   acpi_numa = 1;
> > +   printk(KERN_INFO "SRAT: PXM %u -> APIC 0x%04x -> Node %u\n",
> > +  pxm, apic_id, node);
> > +}
> > +
> > +/* Callback for Proximity Domain -> LAPIC mapping */
> > +void __init
> > +acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa)
> > +{
> > +   int pxm, node;
> > +   int apic_id;
> > +
> > +   if (srat_disabled())
> > +   return;
> > +   if (pa->header.length != sizeof(struct acpi_srat_cpu_affinity)) {
> > +   bad_srat();
> > +   return;
> > +   }
> > +   if ((pa->flags & ACPI_SRAT_CPU_ENABLED) == 0)
> > +   return;
> > +   pxm = pa->proximity_domain_lo;
> > +   if (acpi_srat_revision >= 2)
> > +   pxm |= *((unsigned int*)pa->proximity_domain_hi) << 8;
> > +   node = setup_node(pxm);
> > +   if (node < 0) {
> > +   printk(KERN_ERR "SRAT: Too many proximity domains %x\n", pxm);
> > +   bad_srat();
> > +   return;
> > +   }
> > +
> > +   if (get_uv_system_type() >= UV_X2APIC)
> > +   apic_id = (pa->apic_id << 8) | pa->local_sapic_eid;
> > +   else
> > +   apic_id = pa->apic_id;
> > +
> > +   if (apic_id >= MAX_LOCAL_APIC) {
> > +   printk(KERN_INFO "SRAT: PXM %u -> APIC 0x%02x -> Node %u 
> > skipped apicid that is too big\n", pxm, apic_id, node);
> > +   return;
> > +   }
> > +
> > +   

Re: [PATCH 3/5] acpi: move x86/mm/srat.c to x86/kernel/acpi/srat.c

2013-02-04 Thread li guang
在 2013-02-04一的 11:44 -0800,David Rientjes写道:
> On Mon, 4 Feb 2013, liguang wrote:
> 
> > srat table should present only on acpi domain,
> > seems mm/ is not the right place for it.
> > 
> > Signed-off-by: liguang 
> > ---
> >  arch/x86/kernel/acpi/Makefile |1 +
> >  arch/x86/mm/Makefile  |1 -
> >  arch/x86/mm/srat.c|  191 
> > -
> >  3 files changed, 1 insertions(+), 192 deletions(-)
> >  delete mode 100644 arch/x86/mm/srat.c
> > 
> 
> I was excited for a moment because this would have been one great cleanup, 
> but you never readded the code to arch/x86/kernel/acpi.  (You probably 
> need to do "git add " before generating the patch file.)  I do 
> like the idea of moving it, though, if it's build tested thoroughly.

OK, will git add it.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/5] acpi/numa: check if parsing acpi numa info disabled earlier

2013-02-04 Thread li guang
在 2013-02-04一的 11:41 -0800,David Rientjes写道:
> On Mon, 4 Feb 2013, liguang wrote:
> 
> > Signed-off-by: liguang 
> 
> Because there's no changelog, I have to read the patch to figure out what 
> it's doing since the title isn't that helpful either.  Please provide a 
> description of what problem you're trying to fix or what improvement 
> you're trying to make so it's clear.
> 
> > ---
> >  arch/x86/mm/srat.c  |6 --
> >  drivers/acpi/numa.c |2 ++
> >  2 files changed, 2 insertions(+), 6 deletions(-)
> > 
> > diff --git a/arch/x86/mm/srat.c b/arch/x86/mm/srat.c
> > index a837c95..78c67bd 100644
> > --- a/arch/x86/mm/srat.c
> > +++ b/arch/x86/mm/srat.c
> > @@ -60,8 +60,6 @@ acpi_numa_x2apic_affinity_init(struct 
> > acpi_srat_x2apic_cpu_affinity *pa)
> > int pxm, node;
> > int apic_id;
> >  
> > -   if (srat_disabled())
> > -   return;
> > if (pa->header.length < sizeof(struct acpi_srat_x2apic_cpu_affinity)) {
> > bad_srat();
> > return;
> > @@ -100,8 +98,6 @@ acpi_numa_processor_affinity_init(struct 
> > acpi_srat_cpu_affinity *pa)
> > int pxm, node;
> > int apic_id;
> >  
> > -   if (srat_disabled())
> > -   return;
> > if (pa->header.length != sizeof(struct acpi_srat_cpu_affinity)) {
> > bad_srat();
> > return;
> > @@ -148,8 +144,6 @@ acpi_numa_memory_affinity_init(struct 
> > acpi_srat_mem_affinity *ma)
> > u64 start, end;
> > int node, pxm;
> >  
> > -   if (srat_disabled())
> > -   return -1;
> > if (ma->header.length != sizeof(struct acpi_srat_mem_affinity)) {
> > bad_srat();
> > return -1;
> > diff --git a/drivers/acpi/numa.c b/drivers/acpi/numa.c
> > index cb31298..1f51222 100644
> > --- a/drivers/acpi/numa.c
> > +++ b/drivers/acpi/numa.c
> > @@ -262,6 +262,8 @@ static int __init acpi_parse_srat(struct 
> > acpi_table_header *table)
> > struct acpi_table_srat *srat;
> > if (!table)
> > return -EINVAL;
> > +   if (srat_disabled())
> > +   return -EACCES;
> >  
> > srat = (struct acpi_table_srat *)table;
> > acpi_srat_revision = srat->header.revision;
> 
> Nack, this isn't helpful since SRAT is only for x86 and other 
> architectures use this code.  It would break the build on ia64 since it's 
> obviously not going to have a function called srat_disabled().
> 
> And -EACCES would not be the appropriate return value, this has nothing to 
> do with permissions.

Yes, you're right, will drop this change.



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/5] numa: avoid export acpi_numa variable

2013-02-04 Thread li guang
在 2013-02-04一的 11:31 -0800,David Rientjes写道:
> On Mon, 4 Feb 2013, liguang wrote:
> 
> > diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h
> > index 0c44630..c4e0875 100644
> > --- a/arch/x86/include/asm/acpi.h
> > +++ b/arch/x86/include/asm/acpi.h
> > @@ -181,7 +181,8 @@ static inline void disable_acpi(void) { }
> >  #define ARCH_HAS_POWER_INIT1
> >  
> >  #ifdef CONFIG_ACPI_NUMA
> > -extern int acpi_numa;
> > +extern void bad_srat(void);
> > +extern bool srat_disabled(void);
> 
> So anything that wants to disable acpi_numa, such as xen, must call a 
> function named bad_srat()?  That seems a little misnamed itself.

I would assume every caller have the sense of what SRAT is.
(may be a little bold assumption)

> 
> >  extern int x86_acpi_numa_init(void);
> >  #endif /* CONFIG_ACPI_NUMA */
> >  
> > diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c
> > index 2d125be..92986d8 100644
> > --- a/arch/x86/mm/numa.c
> > +++ b/arch/x86/mm/numa.c
> > @@ -47,7 +47,7 @@ static __init int numa_setup(char *opt)
> >  #endif
> >  #ifdef CONFIG_ACPI_NUMA
> > if (!strncmp(opt, "noacpi", 6))
> > -   acpi_numa = -1;
> > +   bad_srat();
> 
> This will print an error line saying "SRAT: SRAT not used." which wasn't 
> emitted before.  I don't think that's the desire.

yes, you're right.

> 
> >  #endif
> > return 0;
> >  }
> > diff --git a/arch/x86/mm/srat.c b/arch/x86/mm/srat.c
> > index 4ddf497..a837c95 100644
> > --- a/arch/x86/mm/srat.c
> > +++ b/arch/x86/mm/srat.c
> > @@ -24,22 +24,22 @@
> >  #include 
> >  #include 
> >  
> > -int acpi_numa __initdata;
> > +static bool acpi_numa __initdata;
> >  
> 
> You've changed this to be type bool but didn't update the existing code 
> that still does "acpi_numa = 1".

good catch!

> 
> >  static __init int setup_node(int pxm)
> >  {
> > return acpi_map_pxm_to_node(pxm);
> >  }
> >  
> > -static __init void bad_srat(void)
> > +void __init bad_srat(void)
> >  {
> > printk(KERN_ERR "SRAT: SRAT not used.\n");
> > -   acpi_numa = -1;
> > +   acpi_numa = false;
> >  }
> >  
> > -static __init inline int srat_disabled(void)
> > +bool __init srat_disabled(void)
> >  {
> > -   return acpi_numa < 0;
> > +   return acpi_numa;
> >  }
> >  
> >  /* Callback for SLIT parsing */
> > diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> > index 138e566..94c8c70 100644
> > --- a/arch/x86/xen/enlighten.c
> > +++ b/arch/x86/xen/enlighten.c
> > @@ -1415,7 +1415,7 @@ asmlinkage void __init xen_start_kernel(void)
> >  * any NUMA information the kernel tries to get from ACPI will
> >  * be meaningless.  Prevent it from trying.
> >  */
> > -   acpi_numa = -1;
> > +   bad_srat();
> 
> Same as before.  It seems like you want to introduce a new function 
> specifically to disable it:
> 
>   void acpi_numa_disable(void)
>   {
>   acpi_numa = false;
>   }
> 
> and then bad_srat() should call into it?

good idea, Thanks!




--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/5] numa: avoid export acpi_numa variable

2013-02-04 Thread li guang
在 2013-02-04一的 11:31 -0800,David Rientjes写道:
 On Mon, 4 Feb 2013, liguang wrote:
 
  diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h
  index 0c44630..c4e0875 100644
  --- a/arch/x86/include/asm/acpi.h
  +++ b/arch/x86/include/asm/acpi.h
  @@ -181,7 +181,8 @@ static inline void disable_acpi(void) { }
   #define ARCH_HAS_POWER_INIT1
   
   #ifdef CONFIG_ACPI_NUMA
  -extern int acpi_numa;
  +extern void bad_srat(void);
  +extern bool srat_disabled(void);
 
 So anything that wants to disable acpi_numa, such as xen, must call a 
 function named bad_srat()?  That seems a little misnamed itself.

I would assume every caller have the sense of what SRAT is.
(may be a little bold assumption)

 
   extern int x86_acpi_numa_init(void);
   #endif /* CONFIG_ACPI_NUMA */
   
  diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c
  index 2d125be..92986d8 100644
  --- a/arch/x86/mm/numa.c
  +++ b/arch/x86/mm/numa.c
  @@ -47,7 +47,7 @@ static __init int numa_setup(char *opt)
   #endif
   #ifdef CONFIG_ACPI_NUMA
  if (!strncmp(opt, noacpi, 6))
  -   acpi_numa = -1;
  +   bad_srat();
 
 This will print an error line saying SRAT: SRAT not used. which wasn't 
 emitted before.  I don't think that's the desire.

yes, you're right.

 
   #endif
  return 0;
   }
  diff --git a/arch/x86/mm/srat.c b/arch/x86/mm/srat.c
  index 4ddf497..a837c95 100644
  --- a/arch/x86/mm/srat.c
  +++ b/arch/x86/mm/srat.c
  @@ -24,22 +24,22 @@
   #include asm/apic.h
   #include asm/uv/uv.h
   
  -int acpi_numa __initdata;
  +static bool acpi_numa __initdata;
   
 
 You've changed this to be type bool but didn't update the existing code 
 that still does acpi_numa = 1.

good catch!

 
   static __init int setup_node(int pxm)
   {
  return acpi_map_pxm_to_node(pxm);
   }
   
  -static __init void bad_srat(void)
  +void __init bad_srat(void)
   {
  printk(KERN_ERR SRAT: SRAT not used.\n);
  -   acpi_numa = -1;
  +   acpi_numa = false;
   }
   
  -static __init inline int srat_disabled(void)
  +bool __init srat_disabled(void)
   {
  -   return acpi_numa  0;
  +   return acpi_numa;
   }
   
   /* Callback for SLIT parsing */
  diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
  index 138e566..94c8c70 100644
  --- a/arch/x86/xen/enlighten.c
  +++ b/arch/x86/xen/enlighten.c
  @@ -1415,7 +1415,7 @@ asmlinkage void __init xen_start_kernel(void)
   * any NUMA information the kernel tries to get from ACPI will
   * be meaningless.  Prevent it from trying.
   */
  -   acpi_numa = -1;
  +   bad_srat();
 
 Same as before.  It seems like you want to introduce a new function 
 specifically to disable it:
 
   void acpi_numa_disable(void)
   {
   acpi_numa = false;
   }
 
 and then bad_srat() should call into it?

good idea, Thanks!




--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


  1   2   >