[PATCH v6 00/20] kthread: Use kthread worker API more widely

classic Classic list List threaded Threaded
25 messages Options
12
Reply | Threaded
Open this post in threaded view
|

[PATCH v6 04/20] kthread: Add drain_kthread_worker()

Petr Mladek-2
flush_kthread_worker() returns when the currently queued works are proceed.
But some other works might have been queued in the meantime.

This patch adds drain_kthread_worker() that is inspired by
drain_workqueue(). It returns when the queue is completely
empty and warns when it takes too long.

The initial implementation does not block queuing new works when
draining. It makes things much easier. The blocking would be useful
to debug potential problems but it is not clear if it is worth
the complication at the moment.

Signed-off-by: Petr Mladek <[hidden email]>
---
 kernel/kthread.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/kernel/kthread.c b/kernel/kthread.c
index 76364374ff98..6051aa9d93c6 100644
--- a/kernel/kthread.c
+++ b/kernel/kthread.c
@@ -818,3 +818,37 @@ void flush_kthread_worker(struct kthread_worker *worker)
  wait_for_completion(&fwork.done);
 }
 EXPORT_SYMBOL_GPL(flush_kthread_worker);
+
+/**
+ * drain_kthread_worker - drain a kthread worker
+ * @worker: worker to be drained
+ *
+ * Wait until there is no work queued for the given kthread worker.
+ * @worker is flushed repeatedly until it becomes empty.  The number
+ * of flushing is determined by the depth of chaining and should
+ * be relatively short.  Whine if it takes too long.
+ *
+ * The caller is responsible for blocking all users of this kthread
+ * worker from queuing new works. Also it is responsible for blocking
+ * the already queued works from an infinite re-queuing!
+ */
+void drain_kthread_worker(struct kthread_worker *worker)
+{
+ int flush_cnt = 0;
+
+ spin_lock_irq(&worker->lock);
+
+ while (!list_empty(&worker->work_list)) {
+ spin_unlock_irq(&worker->lock);
+
+ flush_kthread_worker(worker);
+ WARN_ONCE(flush_cnt++ > 10,
+  "kthread worker %s: drain_kthread_worker() isn't complete after %u tries\n",
+  worker->task->comm, flush_cnt);
+
+ spin_lock_irq(&worker->lock);
+ }
+
+ spin_unlock_irq(&worker->lock);
+}
+EXPORT_SYMBOL(drain_kthread_worker);
--
1.8.5.6

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v6 00/20] kthread: Use kthread worker API more widely

Tejun Heo-2
In reply to this post by Petr Mladek-2
Hello, Petr.

On Thu, Apr 14, 2016 at 05:14:19PM +0200, Petr Mladek wrote:

> My intention is to make it easier to manipulate and maintain kthreads.
> Especially, I want to replace all the custom main cycles with a
> generic one. Also I want to make the kthreads sleep in a consistent
> state in a common place when there is no work.
>
> My first attempt was with a brand new API (iterant kthread), see
> http://thread.gmane.org/gmane.linux.kernel.api/11892 . But I was
> directed to improve the existing kthread worker API. This is
> the 4th iteration of the new direction.
>
> 1nd..10th patches: improve the existing kthread worker API

I glanced over them and they generally look good to me.  Let's see how
people respond to actual conversions.

Thanks.

--
tejun
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v6 00/20] kthread: Use kthread worker API more widely

Petr Mladek-2
On Fri 2016-04-22 14:30:40, Tejun Heo wrote:

> Hello, Petr.
>
> On Thu, Apr 14, 2016 at 05:14:19PM +0200, Petr Mladek wrote:
> > My intention is to make it easier to manipulate and maintain kthreads.
> > Especially, I want to replace all the custom main cycles with a
> > generic one. Also I want to make the kthreads sleep in a consistent
> > state in a common place when there is no work.
> >
> > My first attempt was with a brand new API (iterant kthread), see
> > http://thread.gmane.org/gmane.linux.kernel.api/11892 . But I was
> > directed to improve the existing kthread worker API. This is
> > the 4th iteration of the new direction.
> >
> > 1nd..10th patches: improve the existing kthread worker API
>
> I glanced over them and they generally look good to me.  Let's see how
> people respond to actual conversions.

The part improving the kthread worker API and the intel powerclamp
conversion seem to be ready for the mainline. But it is getting too late
for 4.7.

I am going to resend this part of the patch set separately after
the 4.7 merge window finishes with the aim for 4.8. The other
conversions are spread over many subsystems, so I will send
them separately.

Tejun, may I add your ack for some of the patches, please?
Or do you want to wait for the resend?

Andrew, I wonder if it could go via the -mm tree once I get
the acks.

Best Regards,
Petr
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v6 16/20] IB/fmr_pool: Convert the cleanup thread into kthread worker API

Doug Ledford
In reply to this post by Petr Mladek-2
On 04/14/2016 11:14 AM, Petr Mladek wrote:

> Kthreads are currently implemented as an infinite loop. Each
> has its own variant of checks for terminating, freezing,
> awakening. In many cases it is unclear to say in which state
> it is and sometimes it is done a wrong way.
>
> The plan is to convert kthreads into kthread_worker or workqueues
> API. It allows to split the functionality into separate operations.
> It helps to make a better structure. Also it defines a clean state
> where no locks are taken, IRQs blocked, the kthread might sleep
> or even be safely migrated.
>
> The kthread worker API is useful when we want to have a dedicated
> single thread for the work. It helps to make sure that it is
> available when needed. Also it allows a better control, e.g.
> define a scheduling priority.
>
> This patch converts the frm_pool kthread into the kthread worker
> API because I am not sure how busy the thread is. It is well
> possible that it does not need a dedicated kthread and workqueues
> would be perfectly fine. Well, the conversion between kthread
> worker API and workqueues is pretty trivial.
>
> The patch moves one iteration from the kthread into the work function.
> It preserves the check for a spurious queuing (wake up). Then it
> processes one request. Finally, it re-queues itself if more requests
> are pending.
>
> Otherwise, wake_up_process() is replaced by queuing the work.
>
> Important: The change is only compile tested. I did not find an easy
> way how to check it in a real life.
I had to do some digging myself to figure out how to move forward on
this patch.  The issue is that your conversion touches the fmr_pool code
as your target.  That code is slowly being phased out.  Right now, only
two drivers in the IB core support fmr: mthca and mlx4.  The generally
preferred method of mem management is fr instead of fmr.  The mlx4
driver support both fr and fmr, while the mthca driver is fmr only.  All
of the other drivers are fr only.  The only code that uses the fmr pools
are the upper layer iSER and SRP drivers.  So, if you have mthca
hardware, you can test fmr using either iSER or SRP clients.  If you
have mlx4 hardware, you can still test fmr by using the SRP client and
setting prefer_fr to false when you load the module.

Now that I know that, I can provide testing of this patch when the
overall patchset is ready to be submitted next.

--
Doug Ledford <[hidden email]>
              GPG KeyID: 0E572FDD



signature.asc (902 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v6 00/20] kthread: Use kthread worker API more widely

Tejun Heo-2
In reply to this post by Petr Mladek-2
On Wed, May 11, 2016 at 6:52 AM, Petr Mladek <[hidden email]> wrote:
> Tejun, may I add your ack for some of the patches, please?
> Or do you want to wait for the resend?

When you repost, I'll explicitly ack the patches.

Thanks!

--
tejun
12