subject:"\[linux\-yocto\] \[PATCH 16\/17\] kernel\/smp\: Allow smp_call_function_single\(\) to be called with a function that doesn't return."

Re: [linux-yocto] [PATCH 16/17] kernel/smp: Allow smp_call_function_single() to be called with a function that doesn't return.

2014-07-16 Thread Paul, Charlie

Bruce 

LSI decided to cancel this patch but wants the rest in.

Can I send you the pull request?

Charlie

-Original Message-
From: Bruce Ashfield [mailto:bruce.ashfi...@windriver.com] 
Sent: Friday, July 11, 2014 9:43 AM
To: Charlie Paul; linux-yocto@yoctoproject.org
Cc: Paul, Charlie
Subject: Re: [PATCH 16/17] kernel/smp: Allow smp_call_function_single() to be 
called with a function that doesn't return.

On 14-07-09 01:26 PM, Charlie Paul wrote:
 From: John Jacques john.jacq...@lsi.com

 When we do a reset (core, not chip or system), the core that is doing 
 the reset has to tell all the other cores to reset.  To do this, we call 
 smp_call_function_single().
 In this case the function specified in smp_call_function_single() doesn't 
 return.
 The wait parameter (third argument, described as '@wait: If true, 
 wait until function has completed on other CPUs.') doesn't work as described 
 without the patch.

 Without the patch, smp_call_function_single() doesn't return when the 
 function indicated resets the called core.  The comments indicate that it 
 should, but it does not.

 Signen-off-by: John Jacques john.jacq...@lsi.com
 ---
   kernel/smp.c |   24 +++-
   1 file changed, 19 insertions(+), 5 deletions(-)

 diff --git a/kernel/smp.c b/kernel/smp.c index d5f3238..040b2b1 100644
 --- a/kernel/smp.c
 +++ b/kernel/smp.c
 @@ -17,14 +17,14 @@
   static struct {
   struct list_headqueue;
   raw_spinlock_t  lock;
 -} call_function __cacheline_aligned_in_smp =
 - {
 +} call_function __cacheline_aligned_in_smp = {
   .queue  = LIST_HEAD_INIT(call_function.queue),
   .lock   = __RAW_SPIN_LOCK_UNLOCKED(call_function.lock),
   };

   enum {
   CSD_FLAG_LOCK   = 0x01,
 + CSD_FLAG_NOWAIT = 0x02,
   };

   struct call_function_data {
 @@ -268,6 +268,8 @@ void 
 generic_smp_call_function_single_interrupt(void)

   while (!list_empty(list)) {
   struct call_single_data *data;
 + void (*func)(void *);
 + void *info;

   data = list_entry(list.next, struct call_single_data, list);
   list_del(data-list);
 @@ -278,12 +280,21 @@ void generic_smp_call_function_single_interrupt(void)
* so save them away before making the call:
*/
   data_flags = data-flags;
 -
 - data-func(data-info);
 + func = data-func;
 + info = data-info;

   /*
 +  * Unlock before calling func so that func never has
 +  * to return.
 +  *
* Unlocked CSDs are valid through generic_exec_single():
*/
 + if ((data_flags  CSD_FLAG_LOCK) 
 + (data_flags  CSD_FLAG_NOWAIT))
 + csd_unlock(data);
 +
 + func(info);
 +
   if (data_flags  CSD_FLAG_LOCK)
   csd_unlock(data);
   }
 @@ -337,6 +348,9 @@ int smp_call_function_single(int cpu, 
 smp_call_func_t func, void *info,

   csd_lock(data);

 + if (!wait)
 + data-flags |= CSD_FLAG_NOWAIT;
 +
   data-func = func;
   data-info = info;
   generic_exec_single(cpu, data, wait); @@ -672,7 +686,7 
 @@ 
 EXPORT_SYMBOL(nr_cpu_ids);
   /* An arch may set nr_cpu_ids earlier if needed, so this would be redundant 
 */
   void __init setup_nr_cpu_ids(void)
   {
 - nr_cpu_ids = find_last_bit(cpumask_bits(cpu_possible_mask),NR_CPUS) + 1;
 + nr_cpu_ids = find_last_bit(cpumask_bits(cpu_possible_mask), NR_CPUS) 
 ++ 1;

I had some structural comments about this patch, that aren't addressed. 
The rest of the series looks ok though.

Bruce

   }

   /* Called by boot processor to activate the rest. */


-- 
___
linux-yocto mailing list
linux-yocto@yoctoproject.org
https://lists.yoctoproject.org/listinfo/linux-yocto

Re: [linux-yocto] [PATCH 16/17] kernel/smp: Allow smp_call_function_single() to be called with a function that doesn't return.

2014-07-16 Thread Bruce Ashfield


On 14-07-16 12:44 PM, Paul, Charlie wrote:

Bruce

LSI decided to cancel this patch but wants the rest in.

Can I send you the pull request?


Sure.

Bruce



Charlie

-Original Message-
From: Bruce Ashfield [mailto:bruce.ashfi...@windriver.com]
Sent: Friday, July 11, 2014 9:43 AM
To: Charlie Paul; linux-yocto@yoctoproject.org
Cc: Paul, Charlie
Subject: Re: [PATCH 16/17] kernel/smp: Allow smp_call_function_single() to be 
called with a function that doesn't return.

On 14-07-09 01:26 PM, Charlie Paul wrote:

From: John Jacques john.jacq...@lsi.com

When we do a reset (core, not chip or system), the core that is doing
the reset has to tell all the other cores to reset.  To do this, we call 
smp_call_function_single().
In this case the function specified in smp_call_function_single() doesn't 
return.
The wait parameter (third argument, described as '@wait: If true,
wait until function has completed on other CPUs.') doesn't work as described 
without the patch.

Without the patch, smp_call_function_single() doesn't return when the
function indicated resets the called core.  The comments indicate that it 
should, but it does not.

Signen-off-by: John Jacques john.jacq...@lsi.com
---
   kernel/smp.c |   24 +++-
   1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/kernel/smp.c b/kernel/smp.c index d5f3238..040b2b1 100644
--- a/kernel/smp.c
+++ b/kernel/smp.c
@@ -17,14 +17,14 @@
   static struct {
struct list_headqueue;
raw_spinlock_t  lock;
-} call_function __cacheline_aligned_in_smp =
-   {
+} call_function __cacheline_aligned_in_smp = {
.queue  = LIST_HEAD_INIT(call_function.queue),
.lock   = __RAW_SPIN_LOCK_UNLOCKED(call_function.lock),
};

   enum {
CSD_FLAG_LOCK   = 0x01,
+   CSD_FLAG_NOWAIT = 0x02,
   };

   struct call_function_data {
@@ -268,6 +268,8 @@ void
generic_smp_call_function_single_interrupt(void)

while (!list_empty(list)) {
struct call_single_data *data;
+   void (*func)(void *);
+   void *info;

data = list_entry(list.next, struct call_single_data, list);
list_del(data-list);
@@ -278,12 +280,21 @@ void generic_smp_call_function_single_interrupt(void)
 * so save them away before making the call:
 */
data_flags = data-flags;
-
-   data-func(data-info);
+   func = data-func;
+   info = data-info;

/*
+* Unlock before calling func so that func never has
+* to return.
+*
 * Unlocked CSDs are valid through generic_exec_single():
 */
+   if ((data_flags  CSD_FLAG_LOCK) 
+   (data_flags  CSD_FLAG_NOWAIT))
+   csd_unlock(data);
+
+   func(info);
+
if (data_flags  CSD_FLAG_LOCK)
csd_unlock(data);
}
@@ -337,6 +348,9 @@ int smp_call_function_single(int cpu,
smp_call_func_t func, void *info,

csd_lock(data);

+   if (!wait)
+   data-flags |= CSD_FLAG_NOWAIT;
+
data-func = func;
data-info = info;
generic_exec_single(cpu, data, wait); @@ -672,7 +686,7 
@@
EXPORT_SYMBOL(nr_cpu_ids);
   /* An arch may set nr_cpu_ids earlier if needed, so this would be redundant 
*/
   void __init setup_nr_cpu_ids(void)
   {
-   nr_cpu_ids = find_last_bit(cpumask_bits(cpu_possible_mask),NR_CPUS) + 1;
+   nr_cpu_ids = find_last_bit(cpumask_bits(cpu_possible_mask), NR_CPUS)
++ 1;


I had some structural comments about this patch, that aren't addressed.
The rest of the series looks ok though.

Bruce


   }

   /* Called by boot processor to activate the rest. */





--
___
linux-yocto mailing list
linux-yocto@yoctoproject.org
https://lists.yoctoproject.org/listinfo/linux-yocto

Re: [linux-yocto] [PATCH 16/17] kernel/smp: Allow smp_call_function_single() to be called with a function that doesn't return.

2014-07-11 Thread Bruce Ashfield


On 14-07-09 01:26 PM, Charlie Paul wrote:

From: John Jacques john.jacq...@lsi.com

When we do a reset (core, not chip or system), the core that is doing the reset 
has to
tell all the other cores to reset.  To do this, we call 
smp_call_function_single().
In this case the function specified in smp_call_function_single() doesn't 
return.
The wait parameter (third argument, described as '@wait: If true, wait until
function has completed on other CPUs.') doesn't work as described without the 
patch.

Without the patch, smp_call_function_single() doesn't return when the function
indicated resets the called core.  The comments indicate that it should, but it 
does not.

Signen-off-by: John Jacques john.jacq...@lsi.com
---
  kernel/smp.c |   24 +++-
  1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/kernel/smp.c b/kernel/smp.c
index d5f3238..040b2b1 100644
--- a/kernel/smp.c
+++ b/kernel/smp.c
@@ -17,14 +17,14 @@
  static struct {
struct list_headqueue;
raw_spinlock_t  lock;
-} call_function __cacheline_aligned_in_smp =
-   {
+} call_function __cacheline_aligned_in_smp = {
.queue  = LIST_HEAD_INIT(call_function.queue),
.lock   = __RAW_SPIN_LOCK_UNLOCKED(call_function.lock),
};

  enum {
CSD_FLAG_LOCK   = 0x01,
+   CSD_FLAG_NOWAIT = 0x02,
  };

  struct call_function_data {
@@ -268,6 +268,8 @@ void generic_smp_call_function_single_interrupt(void)

while (!list_empty(list)) {
struct call_single_data *data;
+   void (*func)(void *);
+   void *info;

data = list_entry(list.next, struct call_single_data, list);
list_del(data-list);
@@ -278,12 +280,21 @@ void generic_smp_call_function_single_interrupt(void)
 * so save them away before making the call:
 */
data_flags = data-flags;
-
-   data-func(data-info);
+   func = data-func;
+   info = data-info;

/*
+* Unlock before calling func so that func never has
+* to return.
+*
 * Unlocked CSDs are valid through generic_exec_single():
 */
+   if ((data_flags  CSD_FLAG_LOCK) 
+   (data_flags  CSD_FLAG_NOWAIT))
+   csd_unlock(data);
+
+   func(info);
+
if (data_flags  CSD_FLAG_LOCK)
csd_unlock(data);
}
@@ -337,6 +348,9 @@ int smp_call_function_single(int cpu, smp_call_func_t func, 
void *info,

csd_lock(data);

+   if (!wait)
+   data-flags |= CSD_FLAG_NOWAIT;
+
data-func = func;
data-info = info;
generic_exec_single(cpu, data, wait);
@@ -672,7 +686,7 @@ EXPORT_SYMBOL(nr_cpu_ids);
  /* An arch may set nr_cpu_ids earlier if needed, so this would be redundant */
  void __init setup_nr_cpu_ids(void)
  {
-   nr_cpu_ids = find_last_bit(cpumask_bits(cpu_possible_mask),NR_CPUS) + 1;
+   nr_cpu_ids = find_last_bit(cpumask_bits(cpu_possible_mask), NR_CPUS) + 
1;


I had some structural comments about this patch, that aren't addressed. 
The rest of the series looks ok though.


Bruce


  }

  /* Called by boot processor to activate the rest. */



--
___
linux-yocto mailing list
linux-yocto@yoctoproject.org
https://lists.yoctoproject.org/listinfo/linux-yocto

[linux-yocto] [PATCH 16/17] kernel/smp: Allow smp_call_function_single() to be called with a function that doesn't return.

2014-07-09 Thread Charlie Paul

From: John Jacques john.jacq...@lsi.com

When we do a reset (core, not chip or system), the core that is doing the reset 
has to
tell all the other cores to reset.  To do this, we call 
smp_call_function_single().
In this case the function specified in smp_call_function_single() doesn't 
return.
The wait parameter (third argument, described as '@wait: If true, wait until
function has completed on other CPUs.') doesn't work as described without the 
patch.

Without the patch, smp_call_function_single() doesn't return when the function
indicated resets the called core.  The comments indicate that it should, but it 
does not.

Signen-off-by: John Jacques john.jacq...@lsi.com
---
 kernel/smp.c |   24 +++-
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/kernel/smp.c b/kernel/smp.c
index d5f3238..040b2b1 100644
--- a/kernel/smp.c
+++ b/kernel/smp.c
@@ -17,14 +17,14 @@
 static struct {
struct list_headqueue;
raw_spinlock_t  lock;
-} call_function __cacheline_aligned_in_smp =
-   {
+} call_function __cacheline_aligned_in_smp = {
.queue  = LIST_HEAD_INIT(call_function.queue),
.lock   = __RAW_SPIN_LOCK_UNLOCKED(call_function.lock),
};
 
 enum {
CSD_FLAG_LOCK   = 0x01,
+   CSD_FLAG_NOWAIT = 0x02,
 };
 
 struct call_function_data {
@@ -268,6 +268,8 @@ void generic_smp_call_function_single_interrupt(void)
 
while (!list_empty(list)) {
struct call_single_data *data;
+   void (*func)(void *);
+   void *info;
 
data = list_entry(list.next, struct call_single_data, list);
list_del(data-list);
@@ -278,12 +280,21 @@ void generic_smp_call_function_single_interrupt(void)
 * so save them away before making the call:
 */
data_flags = data-flags;
-
-   data-func(data-info);
+   func = data-func;
+   info = data-info;
 
/*
+* Unlock before calling func so that func never has
+* to return.
+*
 * Unlocked CSDs are valid through generic_exec_single():
 */
+   if ((data_flags  CSD_FLAG_LOCK) 
+   (data_flags  CSD_FLAG_NOWAIT))
+   csd_unlock(data);
+
+   func(info);
+
if (data_flags  CSD_FLAG_LOCK)
csd_unlock(data);
}
@@ -337,6 +348,9 @@ int smp_call_function_single(int cpu, smp_call_func_t func, 
void *info,
 
csd_lock(data);
 
+   if (!wait)
+   data-flags |= CSD_FLAG_NOWAIT;
+
data-func = func;
data-info = info;
generic_exec_single(cpu, data, wait);
@@ -672,7 +686,7 @@ EXPORT_SYMBOL(nr_cpu_ids);
 /* An arch may set nr_cpu_ids earlier if needed, so this would be redundant */
 void __init setup_nr_cpu_ids(void)
 {
-   nr_cpu_ids = find_last_bit(cpumask_bits(cpu_possible_mask),NR_CPUS) + 1;
+   nr_cpu_ids = find_last_bit(cpumask_bits(cpu_possible_mask), NR_CPUS) + 
1;
 }
 
 /* Called by boot processor to activate the rest. */
-- 
1.7.9.5

-- 
___
linux-yocto mailing list
linux-yocto@yoctoproject.org
https://lists.yoctoproject.org/listinfo/linux-yocto

Re: [linux-yocto] [PATCH 16/17] kernel/smp: Allow smp_call_function_single() to be called with a function that doesn't return.

2014-07-08 Thread Bruce Ashfield


On 14-07-08 10:22 AM, Charlie Paul wrote:

From: John Jacques john.jacq...@lsi.com

When we do a reset (core, not chip or system), the core that is doing the reset 
has to
tell all the other cores to reset.  To do this, we call 
smp_call_function_single().
In this case the function specified in smp_call_function_single() doesn't 
return.
The wait parameter (third argument, described as '@wait: If true, wait until
function has completed on other CPUs.') doesn't work as described without the 
patch.


We need more explanation here. Why doesn't it work ? Deadlock ?
Something else ?



Signen-off-by: John Jacques john.jacq...@lsi.com
---
  kernel/smp.c |   24 +++-
  1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/kernel/smp.c b/kernel/smp.c
index d5f3238..040b2b1 100644
--- a/kernel/smp.c
+++ b/kernel/smp.c
@@ -17,14 +17,14 @@
  static struct {
struct list_headqueue;
raw_spinlock_t  lock;
-} call_function __cacheline_aligned_in_smp =
-   {
+} call_function __cacheline_aligned_in_smp = {


Cosmetic change.


.queue  = LIST_HEAD_INIT(call_function.queue),
.lock   = __RAW_SPIN_LOCK_UNLOCKED(call_function.lock),
};

  enum {
CSD_FLAG_LOCK   = 0x01,
+   CSD_FLAG_NOWAIT = 0x02,
  };

  struct call_function_data {
@@ -268,6 +268,8 @@ void generic_smp_call_function_single_interrupt(void)

while (!list_empty(list)) {
struct call_single_data *data;
+   void (*func)(void *);
+   void *info;

data = list_entry(list.next, struct call_single_data, list);
list_del(data-list);
@@ -278,12 +280,21 @@ void generic_smp_call_function_single_interrupt(void)
 * so save them away before making the call:
 */
data_flags = data-flags;
-
-   data-func(data-info);
+   func = data-func;
+   info = data-info;


Why create these local variables ? func is only used once, and
info in that very same call. Hardly worth modifying the original
code for the change. Keeping the patch footprint small is a
virtue.



/*
+* Unlock before calling func so that func never has
+* to return.
+*
 * Unlocked CSDs are valid through generic_exec_single():
 */
+   if ((data_flags  CSD_FLAG_LOCK) 
+   (data_flags  CSD_FLAG_NOWAIT))


To confirm that this is not changing semantics, you now must be both
lock and no wait to unlock, but is no wait the default ? i.e. I
don't see how an existing caller that sets the lock flag, can be
guaranteed to also be 'no wait'.



+   csd_unlock(data);
+
+   func(info);
+
if (data_flags  CSD_FLAG_LOCK)
csd_unlock(data);
}
@@ -337,6 +348,9 @@ int smp_call_function_single(int cpu, smp_call_func_t func, 
void *info,

csd_lock(data);

+   if (!wait)
+   data-flags |= CSD_FLAG_NOWAIT;
+
data-func = func;
data-info = info;
generic_exec_single(cpu, data, wait);
@@ -672,7 +686,7 @@ EXPORT_SYMBOL(nr_cpu_ids);
  /* An arch may set nr_cpu_ids earlier if needed, so this would be redundant */
  void __init setup_nr_cpu_ids(void)
  {
-   nr_cpu_ids = find_last_bit(cpumask_bits(cpu_possible_mask),NR_CPUS) + 1;
+   nr_cpu_ids = find_last_bit(cpumask_bits(cpu_possible_mask), NR_CPUS) + 
1;


Cosmetic change that should be dropped.

Bruce


  }

  /* Called by boot processor to activate the rest. */



--
___
linux-yocto mailing list
linux-yocto@yoctoproject.org
https://lists.yoctoproject.org/listinfo/linux-yocto

Re: [linux-yocto] [PATCH 16/17] kernel/smp: Allow smp_call_function_single() to be called with a function that doesn't return.

Re: [linux-yocto] [PATCH 16/17] kernel/smp: Allow smp_call_function_single() to be called with a function that doesn't return.

Re: [linux-yocto] [PATCH 16/17] kernel/smp: Allow smp_call_function_single() to be called with a function that doesn't return.

[linux-yocto] [PATCH 16/17] kernel/smp: Allow smp_call_function_single() to be called with a function that doesn't return.

Re: [linux-yocto] [PATCH 16/17] kernel/smp: Allow smp_call_function_single() to be called with a function that doesn't return.

5 matches

Site Navigation

Mail list logo

Footer information