[PATCH] kthread: Export kthread functions

[PATCH] kthread: Export kthread functions

[David Kershner]

The s-Par visornic driver, currently in staging, processes a queue
being serviced by the an s-Par service partition. We can get a message
that something has happened with the Service Partition, when that
happens, we must not access the channel until we get a message that the
service partition is back again.

The visornic driver has a thread for processing the channel, when we
get the message, we need to be able to park the thread and then resume
it when the problem clears.

We can do this with kthread_park and unpark but they are not exported
from the kernel, this patch exports the needed functions.

Signed-off-by: David Kershner <david.kershner@unisys.com>
---
 kernel/kthread.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/kthread.c b/kernel/kthread.c
index 10e489c..bad80c1 100644
--- a/kernel/kthread.c
+++ b/kernel/kthread.c
@@ -97,6 +97,7 @@ bool kthread_should_park(void)
 {
 	return test_bit(KTHREAD_SHOULD_PARK, &to_kthread(current)->flags);
 }
+EXPORT_SYMBOL(kthread_should_park);
 
 /**
  * kthread_freezable_should_stop - should this freezable kthread return now?
@@ -171,6 +172,7 @@ void kthread_parkme(void)
 {
 	__kthread_parkme(to_kthread(current));
 }
+EXPORT_SYMBOL(kthread_parkme);
 
 static int kthread(void *_create)
 {
@@ -411,6 +413,7 @@ void kthread_unpark(struct task_struct *k)
 	if (kthread)
 		__kthread_unpark(k, kthread);
 }
+EXPORT_SYMBOL(kthread_unpark);
 
 /**
  * kthread_park - park a thread created by kthread_create().
@@ -441,6 +444,7 @@ int kthread_park(struct task_struct *k)
 	}
 	return ret;
 }
+EXPORT_SYMBOL(kthread_park);
 
 /**
  * kthread_stop - stop a thread created by kthread_create().
-- 
1.9.1

Re: [PATCH] kthread: Export kthread functions

[Neil Horman]

On Fri, Jul 24, 2015 at 06:45:20PM -0400, David Kershner wrote:
> The s-Par visornic driver, currently in staging, processes a queue
> being serviced by the an s-Par service partition. We can get a message
> that something has happened with the Service Partition, when that
> happens, we must not access the channel until we get a message that the
> service partition is back again.
> 
> The visornic driver has a thread for processing the channel, when we
> get the message, we need to be able to park the thread and then resume
> it when the problem clears.
> 
> We can do this with kthread_park and unpark but they are not exported
> from the kernel, this patch exports the needed functions.
> 
> Signed-off-by: David Kershner <david.kershner@unisys.com>

Given that these functions are visible in the header and part of the
kthread_api, I think it makes good sense to do this

Acked-by: Neil Horman <nhorman@tuxdriver.com>

> ---
>  kernel/kthread.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/kernel/kthread.c b/kernel/kthread.c
> index 10e489c..bad80c1 100644
> --- a/kernel/kthread.c
> +++ b/kernel/kthread.c
> @@ -97,6 +97,7 @@ bool kthread_should_park(void)
>  {
>  	return test_bit(KTHREAD_SHOULD_PARK, &to_kthread(current)->flags);
>  }
> +EXPORT_SYMBOL(kthread_should_park);
>  
>  /**
>   * kthread_freezable_should_stop - should this freezable kthread return now?
> @@ -171,6 +172,7 @@ void kthread_parkme(void)
>  {
>  	__kthread_parkme(to_kthread(current));
>  }
> +EXPORT_SYMBOL(kthread_parkme);
>  
>  static int kthread(void *_create)
>  {
> @@ -411,6 +413,7 @@ void kthread_unpark(struct task_struct *k)
>  	if (kthread)
>  		__kthread_unpark(k, kthread);
>  }
> +EXPORT_SYMBOL(kthread_unpark);
>  
>  /**
>   * kthread_park - park a thread created by kthread_create().
> @@ -441,6 +444,7 @@ int kthread_park(struct task_struct *k)
>  	}
>  	return ret;
>  }
> +EXPORT_SYMBOL(kthread_park);
>  
>  /**
>   * kthread_stop - stop a thread created by kthread_create().
> -- 
> 1.9.1
> 

Re: [PATCH] kthread: Export kthread functions

[Richard Weinberger]

On Sat, Jul 25, 2015 at 12:45 AM, David Kershner
<david.kershner@unisys.com> wrote:
> The s-Par visornic driver, currently in staging, processes a queue
> being serviced by the an s-Par service partition. We can get a message
> that something has happened with the Service Partition, when that
> happens, we must not access the channel until we get a message that the
> service partition is back again.
>
> The visornic driver has a thread for processing the channel, when we
> get the message, we need to be able to park the thread and then resume
> it when the problem clears.
>
> We can do this with kthread_park and unpark but they are not exported
> from the kernel, this patch exports the needed functions.

Are you sure that you need these function?
You would be the first user.
Please see: https://lkml.org/lkml/2015/7/8/1150

-- 
Thanks,
//richard

RE: [PATCH] kthread: Export kthread functions

[Kershner, David A]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 6091 bytes --]



> -----Original Message-----
> From: Richard Weinberger [mailto:richard.weinberger@gmail.com]
> Sent: Saturday, July 25, 2015 8:05 AM
> To: Kershner, David A
> Cc: Andrew Morton; Tejun Heo; laijs@cn.fujitsu.com;
> nacc@linux.vnet.ibm.com; nhorman@redhat.com; Thomas Gleixner; Ingo
> Molnar; LKML; jes.sorensen@redhat.com; *S-Par-Maintainer
> Subject: Re: [PATCH] kthread: Export kthread functions
> 
> On Sat, Jul 25, 2015 at 12:45 AM, David Kershner
> <david.kershner@unisys.com> wrote:
> > The s-Par visornic driver, currently in staging, processes a queue
> > being serviced by the an s-Par service partition. We can get a message
> > that something has happened with the Service Partition, when that
> > happens, we must not access the channel until we get a message that the
> > service partition is back again.
> >
> > The visornic driver has a thread for processing the channel, when we
> > get the message, we need to be able to park the thread and then resume
> > it when the problem clears.
> >
> > We can do this with kthread_park and unpark but they are not exported
> > from the kernel, this patch exports the needed functions.
> 
> Are you sure that you need these function?
> You would be the first user.
> Please see: https://lkml.org/lkml/2015/7/8/1150
> 

The driver is in staging, and this is the patch that requested it. I'll defer to Neil
logistics of it, but this is why we are attempting this. 

From: Neil Horman <nhorman@redhat.com>

Missed this in my initial review.  The usage counter that guards against
kthread task is horribly racy.  The atomic is self consistent, but theres
nothing that prevents the service_resp_queue function from re-incrementing
it immediately after the check in disable_with_timeout is complete.  Its
just a improper usage of atomics as a serialization mechanism.

Instead use kthread_park to pause the thread in its activity so that
buffers can be properly freed without racing against usage in
service_resp_queue

Signed-off-by: Neil Horman <nhorman@redhat.com>
Signed-off-by: Benjamin Romer <benjamin.romer@unisys.com>
---
 drivers/staging/unisys/visornic/visornic_main.c | 23 ++++++-----------------
 kernel/kthread.c                                |  4 ++++
 2 files changed, 10 insertions(+), 17 deletions(-)

diff --git a/drivers/staging/unisys/visornic/visornic_main.c b/drivers/staging/unisys/visornic/visornic_main.c
index 4d49937..aeecb14 100644
--- a/drivers/staging/unisys/visornic/visornic_main.c
+++ b/drivers/staging/unisys/visornic/visornic_main.c
@@ -126,7 +126,6 @@ struct visornic_devdata {
 	unsigned short old_flags;	/* flags as they were prior to
 					 * set_multicast_list
 					 */
-	atomic_t usage;			/* count of users */
 	int num_rcv_bufs;		/* indicates how many rcv buffers
 					 * the vnic will post
 					 */
@@ -565,19 +564,7 @@ visornic_disable_with_timeout(struct net_device *netdev, const int timeout)
 		spin_lock_irqsave(&devdata->priv_lock, flags);
 	}
 
-	/* Wait for usage to go to 1 (no other users) before freeing
-	 * rcv buffers
-	 */
-	if (atomic_read(&devdata->usage) > 1) {
-		while (1) {
-			set_current_state(TASK_INTERRUPTIBLE);
-			spin_unlock_irqrestore(&devdata->priv_lock, flags);
-			schedule_timeout(msecs_to_jiffies(10));
-			spin_lock_irqsave(&devdata->priv_lock, flags);
-			if (atomic_read(&devdata->usage))
-				break;
-		}
-	}
+	kthread_park(devdata->threadinfo.task);
 
 	/* we've set enabled to 0, so we can give up the lock. */
 	spin_unlock_irqrestore(&devdata->priv_lock, flags);
@@ -594,6 +581,7 @@ visornic_disable_with_timeout(struct net_device *netdev, const int timeout)
 		}
 	}
 
+	kthread_unpark(devdata->threadinfo.task);
 	return 0;
 }
 
@@ -1622,7 +1610,7 @@ send_rcv_posts_if_needed(struct visornic_devdata *devdata)
  *	Returns when response queue is empty or when the threadd stops.
  */
 static void
-drain_queue(struct uiscmdrsp *cmdrsp, struct visornic_devdata *devdata)
+service_resp_queue(struct uiscmdrsp *cmdrsp, struct visornic_devdata *devdata)
 {
 	unsigned long flags;
 	struct net_device *netdev;
@@ -1742,6 +1730,8 @@ process_incoming_rsps(void *v)
 			devdata->rsp_queue, (atomic_read(
 					     &devdata->interrupt_rcvd) == 1),
 				msecs_to_jiffies(devdata->thread_wait_ms));
+		if (kthread_should_park())
+			kthread_parkme();
 
 		/* periodically check to see if there are any rcf bufs which
 		 * need to get sent to the IOSP. This can only happen if
@@ -1749,7 +1739,7 @@ process_incoming_rsps(void *v)
 		 */
 		atomic_set(&devdata->interrupt_rcvd, 0);
 		send_rcv_posts_if_needed(devdata);
-		drain_queue(cmdrsp, devdata);
+		service_resp_queue(cmdrsp, devdata);
 	}
 
 	kfree(cmdrsp);
@@ -1809,7 +1799,6 @@ static int visornic_probe(struct visor_device *dev)
 	init_waitqueue_head(&devdata->rsp_queue);
 	spin_lock_init(&devdata->priv_lock);
 	devdata->enabled = 0; /* not yet */
-	atomic_set(&devdata->usage, 1);
 
 	/* Setup rcv bufs */
 	channel_offset = offsetof(struct spar_io_channel_protocol,
diff --git a/kernel/kthread.c b/kernel/kthread.c
index 10e489c..bad80c1 100644
--- a/kernel/kthread.c
+++ b/kernel/kthread.c
@@ -97,6 +97,7 @@ bool kthread_should_park(void)
 {
 	return test_bit(KTHREAD_SHOULD_PARK, &to_kthread(current)->flags);
 }
+EXPORT_SYMBOL(kthread_should_park);
 
 /**
  * kthread_freezable_should_stop - should this freezable kthread return now?
@@ -171,6 +172,7 @@ void kthread_parkme(void)
 {
 	__kthread_parkme(to_kthread(current));
 }
+EXPORT_SYMBOL(kthread_parkme);
 
 static int kthread(void *_create)
 {
@@ -411,6 +413,7 @@ void kthread_unpark(struct task_struct *k)
 	if (kthread)
 		__kthread_unpark(k, kthread);
 }
+EXPORT_SYMBOL(kthread_unpark);
 
 /**
  * kthread_park - park a thread created by kthread_create().
@@ -441,6 +444,7 @@ int kthread_park(struct task_struct *k)
 	}
 	return ret;
 }
+EXPORT_SYMBOL(kthread_park);
 
 /**
  * kthread_stop - stop a thread created by kthread_create().
-- 
2.1.4

> --
> Thanks,
> //richard
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH] kthread: Export kthread functions

[Thomas Gleixner]

On Sat, 25 Jul 2015, Richard Weinberger wrote:

> On Sat, Jul 25, 2015 at 12:45 AM, David Kershner
> <david.kershner@unisys.com> wrote:
> > The s-Par visornic driver, currently in staging, processes a queue
> > being serviced by the an s-Par service partition. We can get a message
> > that something has happened with the Service Partition, when that
> > happens, we must not access the channel until we get a message that the
> > service partition is back again.
> >
> > The visornic driver has a thread for processing the channel, when we
> > get the message, we need to be able to park the thread and then resume
> > it when the problem clears.
> >
> > We can do this with kthread_park and unpark but they are not exported
> > from the kernel, this patch exports the needed functions.
> 
> Are you sure that you need these function?
> You would be the first user.

And a reasonable one. The use case is sensible and it's preferrable
that people reuse known to work core facilities instead of creating
their own variants.

> Please see: https://lkml.org/lkml/2015/7/8/1150

Please do not use lkml.org links. lkml.org sucks.

Thanks,

	tglx

Re: [PATCH] kthread: Export kthread functions

[Ingo Molnar]

* David Kershner <david.kershner@unisys.com> wrote:

> The s-Par visornic driver, currently in staging, processes a queue
> being serviced by the an s-Par service partition. We can get a message
> that something has happened with the Service Partition, when that
> happens, we must not access the channel until we get a message that the
> service partition is back again.
> 
> The visornic driver has a thread for processing the channel, when we
> get the message, we need to be able to park the thread and then resume
> it when the problem clears.
> 
> We can do this with kthread_park and unpark but they are not exported
> from the kernel, this patch exports the needed functions.
> 
> Signed-off-by: David Kershner <david.kershner@unisys.com>
> ---
>  kernel/kthread.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/kernel/kthread.c b/kernel/kthread.c
> index 10e489c..bad80c1 100644
> --- a/kernel/kthread.c
> +++ b/kernel/kthread.c
> @@ -97,6 +97,7 @@ bool kthread_should_park(void)
>  {
>  	return test_bit(KTHREAD_SHOULD_PARK, &to_kthread(current)->flags);
>  }
> +EXPORT_SYMBOL(kthread_should_park);
>  
>  /**
>   * kthread_freezable_should_stop - should this freezable kthread return now?
> @@ -171,6 +172,7 @@ void kthread_parkme(void)
>  {
>  	__kthread_parkme(to_kthread(current));
>  }
> +EXPORT_SYMBOL(kthread_parkme);
>  
>  static int kthread(void *_create)
>  {
> @@ -411,6 +413,7 @@ void kthread_unpark(struct task_struct *k)
>  	if (kthread)
>  		__kthread_unpark(k, kthread);
>  }
> +EXPORT_SYMBOL(kthread_unpark);
>  
>  /**
>   * kthread_park - park a thread created by kthread_create().
> @@ -441,6 +444,7 @@ int kthread_park(struct task_struct *k)
>  	}
>  	return ret;
>  }
> +EXPORT_SYMBOL(kthread_park);
>  
>  /**
>   * kthread_stop - stop a thread created by kthread_create().

Please make these EXPORT_SYMBOL_GPL(), as kthread_freezable_should_stop() is 
already _GPL(), and generally new kthread APIs are exported via 
EXPORT_SYMBOL_GPL().

With that change:

  Acked-by: Ingo Molnar <mingo@kernel.org>

Thanks,

	Ingo

Re: [PATCH] kthread: Export kthread functions

[Neil Horman]

On Mon, Jul 27, 2015 at 05:45:50PM +0200, Ingo Molnar wrote:
> 
> * David Kershner <david.kershner@unisys.com> wrote:
> 
> > The s-Par visornic driver, currently in staging, processes a queue
> > being serviced by the an s-Par service partition. We can get a message
> > that something has happened with the Service Partition, when that
> > happens, we must not access the channel until we get a message that the
> > service partition is back again.
> > 
> > The visornic driver has a thread for processing the channel, when we
> > get the message, we need to be able to park the thread and then resume
> > it when the problem clears.
> > 
> > We can do this with kthread_park and unpark but they are not exported
> > from the kernel, this patch exports the needed functions.
> > 
> > Signed-off-by: David Kershner <david.kershner@unisys.com>
> > ---
> >  kernel/kthread.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/kernel/kthread.c b/kernel/kthread.c
> > index 10e489c..bad80c1 100644
> > --- a/kernel/kthread.c
> > +++ b/kernel/kthread.c
> > @@ -97,6 +97,7 @@ bool kthread_should_park(void)
> >  {
> >  	return test_bit(KTHREAD_SHOULD_PARK, &to_kthread(current)->flags);
> >  }
> > +EXPORT_SYMBOL(kthread_should_park);
> >  
> >  /**
> >   * kthread_freezable_should_stop - should this freezable kthread return now?
> > @@ -171,6 +172,7 @@ void kthread_parkme(void)
> >  {
> >  	__kthread_parkme(to_kthread(current));
> >  }
> > +EXPORT_SYMBOL(kthread_parkme);
> >  
> >  static int kthread(void *_create)
> >  {
> > @@ -411,6 +413,7 @@ void kthread_unpark(struct task_struct *k)
> >  	if (kthread)
> >  		__kthread_unpark(k, kthread);
> >  }
> > +EXPORT_SYMBOL(kthread_unpark);
> >  
> >  /**
> >   * kthread_park - park a thread created by kthread_create().
> > @@ -441,6 +444,7 @@ int kthread_park(struct task_struct *k)
> >  	}
> >  	return ret;
> >  }
> > +EXPORT_SYMBOL(kthread_park);
> >  
> >  /**
> >   * kthread_stop - stop a thread created by kthread_create().
> 
> Please make these EXPORT_SYMBOL_GPL(), as kthread_freezable_should_stop() is 
> already _GPL(), and generally new kthread APIs are exported via 
> EXPORT_SYMBOL_GPL().
> 
Im ok with that.  David?
Neil

> With that change:
> 
>   Acked-by: Ingo Molnar <mingo@kernel.org>
> 
> Thanks,
> 
> 	Ingo

RE: [PATCH] kthread: Export kthread functions

[Kershner, David A]

> -----Original Message-----
> From: Neil Horman [mailto:nhorman@redhat.com]
> Sent: Monday, July 27, 2015 11:49 AM
> To: Ingo Molnar
> Cc: Kershner, David A; akpm@linux-foundation.org; tj@kernel.org;
> laijs@cn.fujitsu.com; nacc@linux.vnet.ibm.com; tglx@linutronix.de;
> mingo@redhat.com; linux-kernel@vger.kernel.org;
> jes.sorensen@redhat.com; *S-Par-Maintainer
> Subject: Re: [PATCH] kthread: Export kthread functions
> 
> On Mon, Jul 27, 2015 at 05:45:50PM +0200, Ingo Molnar wrote:
> >
> > * David Kershner <david.kershner@unisys.com> wrote:
> >
> > > The s-Par visornic driver, currently in staging, processes a queue
> > > being serviced by the an s-Par service partition. We can get a message
> > > that something has happened with the Service Partition, when that
> > > happens, we must not access the channel until we get a message that the
> > > service partition is back again.
> > >
> > > The visornic driver has a thread for processing the channel, when we
> > > get the message, we need to be able to park the thread and then resume
> > > it when the problem clears.
> > >
> > > We can do this with kthread_park and unpark but they are not exported
> > > from the kernel, this patch exports the needed functions.
> > >
> > > Signed-off-by: David Kershner <david.kershner@unisys.com>
> > > ---
> > >  kernel/kthread.c | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/kernel/kthread.c b/kernel/kthread.c
> > > index 10e489c..bad80c1 100644
> > > --- a/kernel/kthread.c
> > > +++ b/kernel/kthread.c
> > > @@ -97,6 +97,7 @@ bool kthread_should_park(void)
> > >  {
> > >  	return test_bit(KTHREAD_SHOULD_PARK, &to_kthread(current)-
> >flags);
> > >  }
> > > +EXPORT_SYMBOL(kthread_should_park);
> > >
> > >  /**
> > >   * kthread_freezable_should_stop - should this freezable kthread return
> now?
> > > @@ -171,6 +172,7 @@ void kthread_parkme(void)
> > >  {
> > >  	__kthread_parkme(to_kthread(current));
> > >  }
> > > +EXPORT_SYMBOL(kthread_parkme);
> > >
> > >  static int kthread(void *_create)
> > >  {
> > > @@ -411,6 +413,7 @@ void kthread_unpark(struct task_struct *k)
> > >  	if (kthread)
> > >  		__kthread_unpark(k, kthread);
> > >  }
> > > +EXPORT_SYMBOL(kthread_unpark);
> > >
> > >  /**
> > >   * kthread_park - park a thread created by kthread_create().
> > > @@ -441,6 +444,7 @@ int kthread_park(struct task_struct *k)
> > >  	}
> > >  	return ret;
> > >  }
> > > +EXPORT_SYMBOL(kthread_park);
> > >
> > >  /**
> > >   * kthread_stop - stop a thread created by kthread_create().
> >
> > Please make these EXPORT_SYMBOL_GPL(), as
> kthread_freezable_should_stop() is
> > already _GPL(), and generally new kthread APIs are exported via
> > EXPORT_SYMBOL_GPL().
> >
> Im ok with that.  David?
> Neil
> 

Fine with that as well.

David 
> > With that change:
> >
> >   Acked-by: Ingo Molnar <mingo@kernel.org>
> >
> > Thanks,
> >
> > 	Ingo

[PATCH v2] kthread: Export kthread functions

[David Kershner]

The s-Par visornic driver, currently in staging, processes a queue
being serviced by the an s-Par service partition. We can get a message
that something has happened with the Service Partition, when that
happens, we must not access the channel until we get a message that the
service partition is back again.

The visornic driver has a thread for processing the channel, when we
get the message, we need to be able to park the thread and then resume
it when the problem clears.

We can do this with kthread_park and unpark but they are not exported
from the kernel, this patch exports the needed functions.

Signed-off-by: David Kershner <david.kershner@unisys.com>
---
 kernel/kthread.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/kthread.c b/kernel/kthread.c
index 10e489c..fdea0be 100644
--- a/kernel/kthread.c
+++ b/kernel/kthread.c
@@ -97,6 +97,7 @@ bool kthread_should_park(void)
 {
 	return test_bit(KTHREAD_SHOULD_PARK, &to_kthread(current)->flags);
 }
+EXPORT_SYMBOL_GPL(kthread_should_park);
 
 /**
  * kthread_freezable_should_stop - should this freezable kthread return now?
@@ -171,6 +172,7 @@ void kthread_parkme(void)
 {
 	__kthread_parkme(to_kthread(current));
 }
+EXPORT_SYMBOL_GPL(kthread_parkme);
 
 static int kthread(void *_create)
 {
@@ -411,6 +413,7 @@ void kthread_unpark(struct task_struct *k)
 	if (kthread)
 		__kthread_unpark(k, kthread);
 }
+EXPORT_SYMBOL_GPL(kthread_unpark);
 
 /**
  * kthread_park - park a thread created by kthread_create().
@@ -441,6 +444,7 @@ int kthread_park(struct task_struct *k)
 	}
 	return ret;
 }
+EXPORT_SYMBOL_GPL(kthread_park);
 
 /**
  * kthread_stop - stop a thread created by kthread_create().
-- 
1.9.1

Re: [PATCH v2] kthread: Export kthread functions

[Andrew Morton]

On Tue, 28 Jul 2015 11:59:01 -0400 David Kershner <david.kershner@unisys.com> wrote:

> The s-Par visornic driver, currently in staging, processes a queue
> being serviced by the an s-Par service partition. We can get a message
> that something has happened with the Service Partition, when that
> happens, we must not access the channel until we get a message that the
> service partition is back again.
> 
> The visornic driver has a thread for processing the channel, when we
> get the message, we need to be able to park the thread and then resume
> it when the problem clears.
> 
> We can do this with kthread_park and unpark but they are not exported
> from the kernel, this patch exports the needed functions.
> 
> Signed-off-by: David Kershner <david.kershner@unisys.com>

Please accumulate the acked-by's and reviewed-by's in the changelog as
they are received.   I presently have

Acked-by: Ingo Molnar <mingo@kernel.org>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Richard Weinberger <richard.weinberger@gmail.com>
Cc: Tejun Heo <tj@kernel.org>


I'll scoot this into mainline probably this week to make life simpler
for the various trees.

Re: [PATCH v2] kthread: Export kthread functions

[Thomas Gleixner]

On Tue, 28 Jul 2015, Andrew Morton wrote:

> On Tue, 28 Jul 2015 11:59:01 -0400 David Kershner <david.kershner@unisys.com> wrote:
> 
> > The s-Par visornic driver, currently in staging, processes a queue
> > being serviced by the an s-Par service partition. We can get a message
> > that something has happened with the Service Partition, when that
> > happens, we must not access the channel until we get a message that the
> > service partition is back again.
> > 
> > The visornic driver has a thread for processing the channel, when we
> > get the message, we need to be able to park the thread and then resume
> > it when the problem clears.
> > 
> > We can do this with kthread_park and unpark but they are not exported
> > from the kernel, this patch exports the needed functions.
> > 
> > Signed-off-by: David Kershner <david.kershner@unisys.com>
> 
> Please accumulate the acked-by's and reviewed-by's in the changelog as
> they are received.   I presently have
> 
> Acked-by: Ingo Molnar <mingo@kernel.org>
> Acked-by: Neil Horman <nhorman@tuxdriver.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Richard Weinberger <richard.weinberger@gmail.com>
> Cc: Tejun Heo <tj@kernel.org>
> 
> 
> I'll scoot this into mainline probably this week to make life simpler
> for the various trees.

Acked-by: Thomas Gleixner <tglx@linutronix.de>

Re: [PATCH v2] kthread: Export kthread functions

[yalin wang]

> On Jul 29, 2015, at 18:34, Thomas Gleixner <tglx@linutronix.de> wrote:
> 
> On Tue, 28 Jul 2015, Andrew Morton wrote:
> 
>> On Tue, 28 Jul 2015 11:59:01 -0400 David Kershner <david.kershner@unisys.com> wrote:
>> 
>>> The s-Par visornic driver, currently in staging, processes a queue
>>> being serviced by the an s-Par service partition. We can get a message
>>> that something has happened with the Service Partition, when that
>>> happens, we must not access the channel until we get a message that the
>>> service partition is back again.
>>> 
>>> The visornic driver has a thread for processing the channel, when we
>>> get the message, we need to be able to park the thread and then resume
>>> it when the problem clears.
>>> 
>>> We can do this with kthread_park and unpark but they are not exported
>>> from the kernel, this patch exports the needed functions.
>>> 
>>> Signed-off-by: David Kershner <david.kershner@unisys.com>
>> 
>> Please accumulate the acked-by's and reviewed-by's in the changelog as
>> they are received.   I presently have
>> 
>> Acked-by: Ingo Molnar <mingo@kernel.org>
>> Acked-by: Neil Horman <nhorman@tuxdriver.com>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Richard Weinberger <richard.weinberger@gmail.com>
>> Cc: Tejun Heo <tj@kernel.org>
>> 
>> 
>> I'll scoot this into mainline probably this week to make life simpler
>> for the various trees.
> 
i am curious why not make some tiny functions to be inline ?
so that don’t need EXPORT_SYMOBLS , shrink the kernel size.
Thanks

Re: [PATCH v2] kthread: Export kthread functions

[Neil Horman]

On Thu, Jul 30, 2015 at 11:48:17AM +0800, yalin wang wrote:
> 
> > On Jul 29, 2015, at 18:34, Thomas Gleixner <tglx@linutronix.de> wrote:
> > 
> > On Tue, 28 Jul 2015, Andrew Morton wrote:
> > 
> >> On Tue, 28 Jul 2015 11:59:01 -0400 David Kershner <david.kershner@unisys.com> wrote:
> >> 
> >>> The s-Par visornic driver, currently in staging, processes a queue
> >>> being serviced by the an s-Par service partition. We can get a message
> >>> that something has happened with the Service Partition, when that
> >>> happens, we must not access the channel until we get a message that the
> >>> service partition is back again.
> >>> 
> >>> The visornic driver has a thread for processing the channel, when we
> >>> get the message, we need to be able to park the thread and then resume
> >>> it when the problem clears.
> >>> 
> >>> We can do this with kthread_park and unpark but they are not exported
> >>> from the kernel, this patch exports the needed functions.
> >>> 
> >>> Signed-off-by: David Kershner <david.kershner@unisys.com>
> >> 
> >> Please accumulate the acked-by's and reviewed-by's in the changelog as
> >> they are received.   I presently have
> >> 
> >> Acked-by: Ingo Molnar <mingo@kernel.org>
> >> Acked-by: Neil Horman <nhorman@tuxdriver.com>
> >> Cc: Thomas Gleixner <tglx@linutronix.de>
> >> Cc: Richard Weinberger <richard.weinberger@gmail.com>
> >> Cc: Tejun Heo <tj@kernel.org>
> >> 
> >> 
> >> I'll scoot this into mainline probably this week to make life simpler
> >> for the various trees.
> > 
> i am curious why not make some tiny functions to be inline ?
> so that don’t need EXPORT_SYMOBLS , shrink the kernel size.
> Thanks

Because exporting symbols isn't a big deal, and the compiler can decide when its
best to inline these functions.  As it is, they aren't that small, if you expand
all their internals

Neil

Re: [PATCH v2] kthread: Export kthread functions

[yalin wang]

> On Jul 30, 2015, at 20:02, Neil Horman <nhorman@redhat.com> wrote:
> 
> On Thu, Jul 30, 2015 at 11:48:17AM +0800, yalin wang wrote:
>> 
>>> On Jul 29, 2015, at 18:34, Thomas Gleixner <tglx@linutronix.de> wrote:
>>> 
>>> On Tue, 28 Jul 2015, Andrew Morton wrote:
>>> 
>>>> On Tue, 28 Jul 2015 11:59:01 -0400 David Kershner <david.kershner@unisys.com> wrote:
>>>> 
>>>>> The s-Par visornic driver, currently in staging, processes a queue
>>>>> being serviced by the an s-Par service partition. We can get a message
>>>>> that something has happened with the Service Partition, when that
>>>>> happens, we must not access the channel until we get a message that the
>>>>> service partition is back again.
>>>>> 
>>>>> The visornic driver has a thread for processing the channel, when we
>>>>> get the message, we need to be able to park the thread and then resume
>>>>> it when the problem clears.
>>>>> 
>>>>> We can do this with kthread_park and unpark but they are not exported
>>>>> from the kernel, this patch exports the needed functions.
>>>>> 
>>>>> Signed-off-by: David Kershner <david.kershner@unisys.com>
>>>> 
>>>> Please accumulate the acked-by's and reviewed-by's in the changelog as
>>>> they are received.   I presently have
>>>> 
>>>> Acked-by: Ingo Molnar <mingo@kernel.org>
>>>> Acked-by: Neil Horman <nhorman@tuxdriver.com>
>>>> Cc: Thomas Gleixner <tglx@linutronix.de>
>>>> Cc: Richard Weinberger <richard.weinberger@gmail.com>
>>>> Cc: Tejun Heo <tj@kernel.org>
>>>> 
>>>> 
>>>> I'll scoot this into mainline probably this week to make life simpler
>>>> for the various trees.
>>> 
>> i am curious why not make some tiny functions to be inline ?
>> so that don’t need EXPORT_SYMOBLS , shrink the kernel size.
>> Thanks
> 
> Because exporting symbols isn't a big deal, and the compiler can decide when its
> best to inline these functions.  As it is, they aren't that small, if you expand
> all their internals
> 
> Neil
> 

this is my test on arm64 arch,
 i think kthread_should_stop() kthread_should_park() kthread_data() should be inline :


without inline:
ffffffc0000d265c <smpboot_thread_fn>:
                                 ……….
ffffffc0000d26a0:       b9001a61        str     w1, [x19,#24]
ffffffc0000d26a4:       97fff260        bl      ffffffc0000cf024 <kthread_should_stop>
ffffffc0000d26a8:       53001c00        uxtb    w0, w0
ffffffc0000d26ac:       35000c20        cbnz    w0, ffffffc0000d2830 <smpboot_thread_fn+0x1d4>
ffffffc0000d26b0:       97fff4aa        bl      ffffffc0000cf958 <kthread_should_park>
ffffffc0000d26b4:       53001c00        uxtb    w0, w0
ffffffc0000d26b8:       34000320        cbz     w0, ffffffc0000d271c <smpboot_thread_fn+0xc0>
ffffffc0000d26bc:       f9400a60        ldr     x0, [x19,#16]
ffffffc0000d26c0:       f900001f        str     xzr, [x0]


ffffffc0000cf958 <kthread_should_park>:
ffffffc0000cf958:       910003e0        mov     x0, sp
ffffffc0000cf95c:       9272c400        and     x0, x0, #0xffffffffffffc000
ffffffc0000cf960:       f9400800        ldr     x0, [x0,#16]
ffffffc0000cf964:       f9420400        ldr     x0, [x0,#1032]
ffffffc0000cf968:       f85c8000        ldr     x0, [x0,#-56]
ffffffc0000cf96c:       d3420800        ubfx    x0, x0, #2, #1
ffffffc0000cf970:       d65f03c0        ret






if i mark kthread_should_park to be inline :

ffffffc0000d19ec <smpboot_thread_fn>:
ffffffc0000d19ec:       a9bc7bfd        stp     x29, x30, [sp,#-64]!
ffffffc0000d19f0:       910003fd        mov     x29, sp
ffffffc0000d19f4:       a90153f3        stp     x19, x20, [sp,#16]
ffffffc0000d19f8:       aa0003f4        mov     x20, x0
ffffffc0000d19fc:       910003e0        mov     x0, sp
ffffffc0000d1a00:       a90363f7        stp     x23, x24, [sp,#48]
ffffffc0000d1a04:       a9025bf5        stp     x21, x22, [sp,#32]
ffffffc0000d1a08:       d2800035        mov     x21, #0x1                       // #1
ffffffc0000d1a0c:       9272c413        and     x19, x0, #0xffffffffffffc000
ffffffc0000d1a10:       f9400696        ldr     x22, [x20,#8]
ffffffc0000d1a14:       2a1503f7        mov     w23, w21
ffffffc0000d1a18:       52800058        mov     w24, #0x2                       // #2
ffffffc0000d1a1c:       f9400a60        ldr     x0, [x19,#16]
ffffffc0000d1a20:       f9000015        str     x21, [x0]
ffffffc0000d1a24:       d5033bbf        dmb     ish
ffffffc0000d1a28:       b9401a61        ldr     w1, [x19,#24]
ffffffc0000d1a2c:       11000421        add     w1, w1, #0x1
ffffffc0000d1a30:       b9001a61        str     w1, [x19,#24]
ffffffc0000d1a34:       f9400a62        ldr     x2, [x19,#16]
ffffffc0000d1a38:       f9420441        ldr     x1, [x2,#1032]
ffffffc0000d1a3c:       f85c8020        ldr     x0, [x1,#-56]
ffffffc0000d1a40:       37080a60        tbnz    w0, #1, ffffffc0000d1b8c <smpboot_thread_fn+0x1a0>
ffffffc0000d1a44:       f85c8020        ldr     x0, [x1,#-56]    									// kthread_should_park  line
ffffffc0000d1a48:       36100300        tbz     w0, #2, ffffffc0000d1aa8 <smpboot_thread_fn+0xbc>   // kthread_should_park  line
ffffffc0000d1a4c:       f900005f        str     xzr, [x2]
ffffffc0000d1a50:       b9401a60        ldr     w0, [x19,#24]

it is optimised to 2 instructions ,

this is my patch, hope can be merged :

diff --git a/include/linux/kthread.h b/include/linux/kthread.h
diff --git a/include/linux/kthread.h b/include/linux/kthread.h
index 3e6773e..9210030 100644
--- a/include/linux/kthread.h
+++ b/include/linux/kthread.h
@@ -4,6 +4,70 @@
 #include <linux/err.h>
 #include <linux/sched.h>
 
+struct kthread {
+       unsigned long flags;
+       unsigned int cpu;
+       void *data;
+       struct completion parked;
+       struct completion exited;
+};
+
+enum KTHREAD_BITS {
+       KTHREAD_IS_PER_CPU = 0,
+       KTHREAD_SHOULD_STOP,
+       KTHREAD_SHOULD_PARK,
+       KTHREAD_IS_PARKED,
+};
+
+#define __to_kthread(vfork)    \
+       container_of(vfork, struct kthread, exited)
+
+static inline struct kthread *to_kthread(struct task_struct *k)
+{
+       return __to_kthread(k->vfork_done);
+}
+
+/**
+ * kthread_should_stop - should this kthread return now?
+ *
+ * When someone calls kthread_stop() on your kthread, it will be woken
+ * and this will return true.  You should then return, and your return
+ * value will be passed through to kthread_stop().
+ */
+static inline bool kthread_should_stop(void)
+{
+       return test_bit(KTHREAD_SHOULD_STOP, &to_kthread(current)->flags);
+}
+
+/**
+ * kthread_should_park - should this kthread park now?
+ *
+ * When someone calls kthread_park() on your kthread, it will be woken
+ * and this will return true.  You should then do the necessary
+ * cleanup and call kthread_parkme()
+ *
+ * Similar to kthread_should_stop(), but this keeps the thread alive
+ * and in a park position. kthread_unpark() "restarts" the thread and
+ * calls the thread function again.
+ */
+static inline bool kthread_should_park(void)
+{
+       return test_bit(KTHREAD_SHOULD_PARK, &to_kthread(current)->flags);
+}
+
+/**
+ * kthread_data - return data value specified on kthread creation
+ * @task: kthread task in question
+ *
+ * Return the data value specified when kthread @task was created.
+ * The caller is responsible for ensuring the validity of @task when
+ * calling this function.
+ */
+static inline void *kthread_data(struct task_struct *task)
+{
+       return to_kthread(task)->data;
+}
+
 __printf(4, 5)
 struct task_struct *kthread_create_on_node(int (*threadfn)(void *data),
                                           void *data,
@@ -39,10 +103,7 @@ struct task_struct *kthread_create_on_cpu(int (*threadfn)(void *data),
 
 void kthread_bind(struct task_struct *k, unsigned int cpu);
 int kthread_stop(struct task_struct *k);
-bool kthread_should_stop(void);
-bool kthread_should_park(void);
 bool kthread_freezable_should_stop(bool *was_frozen);
-void *kthread_data(struct task_struct *k);
 void *probe_kthread_data(struct task_struct *k);
 int kthread_park(struct task_struct *k);
 void kthread_unpark(struct task_struct *k);
diff --git a/kernel/kthread.c b/kernel/kthread.c
index baf3673..e036019 100644
--- a/kernel/kthread.c
+++ b/kernel/kthread.c
@@ -38,29 +38,6 @@ struct kthread_create_info
        struct list_head list;
 };
 
-struct kthread {
-       unsigned long flags;
-       unsigned int cpu;
-       void *data;
-       struct completion parked;
-       struct completion exited;
-};
-
-enum KTHREAD_BITS {
-       KTHREAD_IS_PER_CPU = 0,
-       KTHREAD_SHOULD_STOP,
-       KTHREAD_SHOULD_PARK,
-       KTHREAD_IS_PARKED,
-};
-
-#define __to_kthread(vfork)    \
-       container_of(vfork, struct kthread, exited)
-
-static inline struct kthread *to_kthread(struct task_struct *k)
-{
-       return __to_kthread(k->vfork_done);
-}
-
 static struct kthread *to_live_kthread(struct task_struct *k)
 {
        struct completion *vfork = ACCESS_ONCE(k->vfork_done);
@@ -70,36 +47,6 @@ static struct kthread *to_live_kthread(struct task_struct *k)
 }
 
 /**
...skipping...
- * When someone calls kthread_stop() on your kthread, it will be woken
- * and this will return true.  You should then return, and your return
- * value will be passed through to kthread_stop().
- */
-bool kthread_should_stop(void)
-{
-       return test_bit(KTHREAD_SHOULD_STOP, &to_kthread(current)->flags);
-}
-EXPORT_SYMBOL(kthread_should_stop);
-
-/**
- * kthread_should_park - should this kthread park now?
- *
- * When someone calls kthread_park() on your kthread, it will be woken
- * and this will return true.  You should then do the necessary
- * cleanup and call kthread_parkme()
- *
- * Similar to kthread_should_stop(), but this keeps the thread alive
- * and in a park position. kthread_unpark() "restarts" the thread and
- * calls the thread function again.
- */
-bool kthread_should_park(void)
-{
-       return test_bit(KTHREAD_SHOULD_PARK, &to_kthread(current)->flags);
-}
-EXPORT_SYMBOL_GPL(kthread_should_park);
-
-/**
  * kthread_freezable_should_stop - should this freezable kthread return now?
  * @was_frozen: optional out parameter, indicates whether %current was frozen
  *
@@ -125,19 +72,6 @@ bool kthread_freezable_should_stop(bool *was_frozen)
 EXPORT_SYMBOL_GPL(kthread_freezable_should_stop);
 
 /**
- * kthread_data - return data value specified on kthread creation
- * @task: kthread task in question
- *
- * Return the data value specified when kthread @task was created.
- * The caller is responsible for ensuring the validity of @task when
- * calling this function.
- */
-void *kthread_data(struct task_struct *task)
-{
-       return to_kthread(task)->data;
-}
-
-/**
  * probe_kthread_data - speculative version of kthread_data()
  * @task: possible kthread task in question
  *
---
(END)
Thanks

Re: [PATCH v2] kthread: Export kthread functions

[Thomas Gleixner]

On Fri, 31 Jul 2015, yalin wang wrote:
> it is optimised to 2 instructions ,
> 
> this is my patch, hope can be merged :

We are not exposing the internals of kthread management. Period.

Thanks,

	tglx

Re: [PATCH v2] kthread: Export kthread functions

[Thomas Gleixner]

On Fri, 31 Jul 2015, Thomas Gleixner wrote:

> On Fri, 31 Jul 2015, yalin wang wrote:
> > it is optimised to 2 instructions ,
> > 
> > this is my patch, hope can be merged :
> 
> We are not exposing the internals of kthread management. Period.

And your 'optimization' is completely bogus:

Before your modification:

size kernel/built-in.o

   text	   data	    bss	    dec	    hex	filename
1091514	 141498  341928 1574940  18081c	../build/kernel/built-in.o

After:

   text	   data	    bss	    dec	    hex	filename
1091664  141498  341928 1575090  1808b2	../build/kernel/built-in.o

That's an increase of text size by 150 byte. Interesting optimization.

Thanks,

	tglx

Re: [PATCH v2] kthread: Export kthread functions

[yalin wang]

> 在 2015年7月31日，22:14，Thomas Gleixner <tglx@linutronix.de> 写道：
> 
> 
> On Fri, 31 Jul 2015, Thomas Gleixner wrote:
> 
>> On Fri, 31 Jul 2015, yalin wang wrote:
>>> it is optimised to 2 instructions ,
>>> 
>>> this is my patch, hope can be merged :
>> 
>> We are not exposing the internals of kthread management. Period.
> 
> And your 'optimization' is completely bogus:
> 
> Before your modification:
> 
> size kernel/built-in.o
> 
>   text	   data	    bss	    dec	    hex	filename
> 1091514	 141498  341928 1574940  18081c	../build/kernel/built-in.o
> 
> After:
> 
>   text	   data	    bss	    dec	    hex	filename
> 1091664  141498  341928 1575090  1808b2	../build/kernel/built-in.o
> 
> That's an increase of text size by 150 byte. Interesting optimization.
> 
> Thanks,
> 
> 	tglx
> 
> 
strange,  this is my test result:

size   built-in.o*
  text	   data	    bss	    dec	    hex	filename
743937	  50786	  56008	 850731	  cfb2b	built-in.o        // with the patch
744069	  50786	  56008	 850863	  cfbaf	built-in.o_old  // with out the patch

you can see text size is reduced.
in addition, because we don’t use EXPORT_SYMBOLS() for inline function,
some data can also be removed,

[27] __ksymtab_strings PROGBITS         0000000000000000  000a7fe0
      0000000000003907  0000000000000000   A       0     0     1
0x3907

[27] __ksymtab_strings PROGBITS         0000000000000000  000a8020
      000000000000392f  0000000000000000   A       0     0     1
0x392f

you can see after apply the patch, this string section is also reduced,
but size command output seems not calculate these special section,
there are lots of special sections in linux kernel image,
i think size command is not suitable to calculate kernel image, maybe it just
calculate formal section which name like   .data  .text   etc..

Thanks

Re: [PATCH v2] kthread: Export kthread functions

[Thomas Gleixner]

On Sat, 1 Aug 2015, yalin wang wrote:
> size   built-in.o*
>   text	   data	    bss	    dec	    hex	filename
> 743937	  50786	  56008	 850731	  cfb2b	built-in.o        // with the patch
> 744069	  50786	  56008	 850863	  cfbaf	built-in.o_old  // with out the patch

Not all architectures/compilers are giving the same results.

> i think size command is not suitable to calculate kernel image, maybe it just
> calculate formal section which name like   .data  .text   etc..

Nonsense. .text is .text no matter what.

Anyway, it's not going to change.

Thanks,

	tglx

Re: [PATCH v2] kthread: Export kthread functions

[Neil Horman]

On Sat, Aug 01, 2015 at 03:12:42PM +0800, yalin wang wrote:
> 
> > 在 2015年7月31日，22:14，Thomas Gleixner <tglx@linutronix.de> 写道：
> > 
> > 
> > On Fri, 31 Jul 2015, Thomas Gleixner wrote:
> > 
> >> On Fri, 31 Jul 2015, yalin wang wrote:
> >>> it is optimised to 2 instructions ,
> >>> 
> >>> this is my patch, hope can be merged :
> >> 
> >> We are not exposing the internals of kthread management. Period.
> > 
> > And your 'optimization' is completely bogus:
> > 
> > Before your modification:
> > 
> > size kernel/built-in.o
> > 
> >   text	   data	    bss	    dec	    hex	filename
> > 1091514	 141498  341928 1574940  18081c	../build/kernel/built-in.o
> > 
> > After:
> > 
> >   text	   data	    bss	    dec	    hex	filename
> > 1091664  141498  341928 1575090  1808b2	../build/kernel/built-in.o
> > 
> > That's an increase of text size by 150 byte. Interesting optimization.
> > 
> > Thanks,
> > 
> > 	tglx
> > 
> > 
> strange,  this is my test result:
> 
> size   built-in.o*
>   text	   data	    bss	    dec	    hex	filename
> 743937	  50786	  56008	 850731	  cfb2b	built-in.o        // with the patch
> 744069	  50786	  56008	 850863	  cfbaf	built-in.o_old  // with out the patch
> 
So you're willing to expose the internals of kthread_park in exchange for the
hope of saving 132 bytes of text.

Thats just dumb.  I agree with tglx, this shouldn't change.

Neil

Re: [PATCH v2] kthread: Export kthread functions

[yalin wang]

> On Aug 1, 2015, at 21:32, Neil Horman <nhorman@redhat.com> wrote:
> 
> On Sat, Aug 01, 2015 at 03:12:42PM +0800, yalin wang wrote:
>> 
>>> 在 2015年7月31日，22:14，Thomas Gleixner <tglx@linutronix.de> 写道：
>>> 
>>> 
>>> On Fri, 31 Jul 2015, Thomas Gleixner wrote:
>>> 
>>>> On Fri, 31 Jul 2015, yalin wang wrote:
>>>>> it is optimised to 2 instructions ,
>>>>> 
>>>>> this is my patch, hope can be merged :
>>>> 
>>>> We are not exposing the internals of kthread management. Period.
>>> 
>>> And your 'optimization' is completely bogus:
>>> 
>>> Before your modification:
>>> 
>>> size kernel/built-in.o
>>> 
>>>  text	   data	    bss	    dec	    hex	filename
>>> 1091514	 141498  341928 1574940  18081c	../build/kernel/built-in.o
>>> 
>>> After:
>>> 
>>>  text	   data	    bss	    dec	    hex	filename
>>> 1091664  141498  341928 1575090  1808b2	../build/kernel/built-in.o
>>> 
>>> That's an increase of text size by 150 byte. Interesting optimization.
>>> 
>>> Thanks,
>>> 
>>> 	tglx
>>> 
>>> 
>> strange,  this is my test result:
>> 
>> size   built-in.o*
>>  text	   data	    bss	    dec	    hex	filename
>> 743937	  50786	  56008	 850731	  cfb2b	built-in.o        // with the patch
>> 744069	  50786	  56008	 850863	  cfbaf	built-in.o_old  // with out the patch
>> 
> So you're willing to expose the internals of kthread_park in exchange for the
> hope of saving 132 bytes of text.
> 
> Thats just dumb.  I agree with tglx, this shouldn't change.
> 
> Neil
not just size, mainly for performance,
without inline:

ffffffc0000d26b0:       97fff4aa        bl      ffffffc0000cf958 <kthread_should_park>
ffffffc0000d26b4:       53001c00        uxtb    w0, w0

if kthread_should_park() inline:
ffffffc0000d1a44:       f85c8020        ldr     x0, [x1,#-56]    									// kthread_should_park line
ffffffc0000d1a48:       36100300        tbz     w0, #2, ffffffc0000d1aa8 <smpboot_thread_fn+0xbc>   // kthread_should_park  line

still use 2 instructions, but don’t need a function call,
maybe can do more optimisation by gcc sometimes .
Anyway, this is just a suggest,
it is up to you apply it or not. :) 

Thanks

Re: [PATCH v2] kthread: Export kthread functions

[Jes Sorensen]

yalin wang <yalin.wang2010@gmail.com> writes:
>> On Aug 1, 2015, at 21:32, Neil Horman <nhorman@redhat.com> wrote:
>>> strange,  this is my test result:
>>> 
>>> size   built-in.o*
>>>  text	   data	    bss	    dec	    hex	filename
>>> 743937 50786 56008 850731 cfb2b built-in.o // with the patch
>>> 744069 50786 56008 850863 cfbaf built-in.o_old // with out the
>>> patch
>>> 
>> So you're willing to expose the internals of kthread_park in exchange for the
>> hope of saving 132 bytes of text.
>> 
>> Thats just dumb.  I agree with tglx, this shouldn't change.
>> 
>> Neil
> not just size, mainly for performance,
> without inline:
>
> ffffffc0000d26b0: 97fff4aa bl ffffffc0000cf958 <kthread_should_park>
> ffffffc0000d26b4:       53001c00        uxtb    w0, w0
>
> if kthread_should_park() inline:
> ffffffc0000d1a44: f85c8020 ldr x0, [x1,#-56] // kthread_should_park
> line
> ffffffc0000d1a48: 36100300 tbz w0, #2, ffffffc0000d1aa8
> <smpboot_thread_fn+0xbc> // kthread_should_park line
>
> still use 2 instructions, but don’t need a function call,
> maybe can do more optimisation by gcc sometimes .
> Anyway, this is just a suggest,
> it is up to you apply it or not. :) 

kthread_park() isn't exactly a performance critical function call.
Saving two instructions does not outway the cost of exposing the
internals of the kthread API.

Jes