From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753613AbbKXKG4 (ORCPT ); Tue, 24 Nov 2015 05:06:56 -0500 Received: from mx2.suse.de ([195.135.220.15]:36967 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753191AbbKXKGw (ORCPT ); Tue, 24 Nov 2015 05:06:52 -0500 Date: Tue, 24 Nov 2015 11:06:50 +0100 From: Petr Mladek To: Tejun Heo Cc: Andrew Morton , Oleg Nesterov , Ingo Molnar , Peter Zijlstra , Steven Rostedt , "Paul E. McKenney" , Josh Triplett , Thomas Gleixner , Linus Torvalds , Jiri Kosina , Borislav Petkov , Michal Hocko , linux-mm@kvack.org, Vlastimil Babka , linux-api@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v3 07/22] kthread: Detect when a kthread work is used by more workers Message-ID: <20151124100650.GF10750@pathway.suse.cz> References: <1447853127-3461-1-git-send-email-pmladek@suse.com> <1447853127-3461-8-git-send-email-pmladek@suse.com> <20151123222703.GH19072@mtj.duckdns.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20151123222703.GH19072@mtj.duckdns.org> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon 2015-11-23 17:27:03, Tejun Heo wrote: > Hello, > > On Wed, Nov 18, 2015 at 02:25:12PM +0100, Petr Mladek wrote: > > @@ -610,6 +625,12 @@ repeat: > > if (work) { > > __set_current_state(TASK_RUNNING); > > work->func(work); > > + > > + spin_lock_irq(&worker->lock); > > + /* Allow to queue the work into another worker */ > > + if (!kthread_work_pending(work)) > > + work->worker = NULL; > > + spin_unlock_irq(&worker->lock); > > Doesn't this mean that the work item can't be freed from its callback? > That pattern tends to happen regularly. I am not sure if I understand your question. Do you mean switching work->func during the life time of the struct kthread_work? This should not be affected by the above code. The above code allows to queue an _unused_ kthread_work into any kthread_worker. For example, it is needed for khugepaged, see http://marc.info/?l=linux-kernel&m=144785344924871&w=2 The work is static but the worker can be started/stopped (allocated/freed) repeatedly. It means that the work need to be usable with many workers. But it is associated only with one worker when being used. If the work is in use (pending or being proceed), we must not touch work->worker. Otherwise there might be a race. Because all the operations with the work are synchronized using work->worker->lock. I hope that it makes sense. Thanks a lot for feedback, Petr From mboxrd@z Thu Jan 1 00:00:00 1970 From: Petr Mladek Subject: Re: [PATCH v3 07/22] kthread: Detect when a kthread work is used by more workers Date: Tue, 24 Nov 2015 11:06:50 +0100 Message-ID: <20151124100650.GF10750@pathway.suse.cz> References: <1447853127-3461-1-git-send-email-pmladek@suse.com> <1447853127-3461-8-git-send-email-pmladek@suse.com> <20151123222703.GH19072@mtj.duckdns.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <20151123222703.GH19072-qYNAdHglDFBN0TnZuCh8vA@public.gmane.org> Sender: linux-api-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Tejun Heo Cc: Andrew Morton , Oleg Nesterov , Ingo Molnar , Peter Zijlstra , Steven Rostedt , "Paul E. McKenney" , Josh Triplett , Thomas Gleixner , Linus Torvalds , Jiri Kosina , Borislav Petkov , Michal Hocko , linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org, Vlastimil Babka , linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-Id: linux-api@vger.kernel.org On Mon 2015-11-23 17:27:03, Tejun Heo wrote: > Hello, > > On Wed, Nov 18, 2015 at 02:25:12PM +0100, Petr Mladek wrote: > > @@ -610,6 +625,12 @@ repeat: > > if (work) { > > __set_current_state(TASK_RUNNING); > > work->func(work); > > + > > + spin_lock_irq(&worker->lock); > > + /* Allow to queue the work into another worker */ > > + if (!kthread_work_pending(work)) > > + work->worker = NULL; > > + spin_unlock_irq(&worker->lock); > > Doesn't this mean that the work item can't be freed from its callback? > That pattern tends to happen regularly. I am not sure if I understand your question. Do you mean switching work->func during the life time of the struct kthread_work? This should not be affected by the above code. The above code allows to queue an _unused_ kthread_work into any kthread_worker. For example, it is needed for khugepaged, see http://marc.info/?l=linux-kernel&m=144785344924871&w=2 The work is static but the worker can be started/stopped (allocated/freed) repeatedly. It means that the work need to be usable with many workers. But it is associated only with one worker when being used. If the work is in use (pending or being proceed), we must not touch work->worker. Otherwise there might be a race. Because all the operations with the work are synchronized using work->worker->lock. I hope that it makes sense. Thanks a lot for feedback, Petr From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f41.google.com (mail-wm0-f41.google.com [74.125.82.41]) by kanga.kvack.org (Postfix) with ESMTP id 0B1E96B0038 for ; Tue, 24 Nov 2015 05:06:53 -0500 (EST) Received: by wmww144 with SMTP id w144so131220709wmw.1 for ; Tue, 24 Nov 2015 02:06:52 -0800 (PST) Received: from mx2.suse.de (mx2.suse.de. [195.135.220.15]) by mx.google.com with ESMTPS id df7si25591136wjc.222.2015.11.24.02.06.51 for (version=TLS1 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Tue, 24 Nov 2015 02:06:51 -0800 (PST) Date: Tue, 24 Nov 2015 11:06:50 +0100 From: Petr Mladek Subject: Re: [PATCH v3 07/22] kthread: Detect when a kthread work is used by more workers Message-ID: <20151124100650.GF10750@pathway.suse.cz> References: <1447853127-3461-1-git-send-email-pmladek@suse.com> <1447853127-3461-8-git-send-email-pmladek@suse.com> <20151123222703.GH19072@mtj.duckdns.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20151123222703.GH19072@mtj.duckdns.org> Sender: owner-linux-mm@kvack.org List-ID: To: Tejun Heo Cc: Andrew Morton , Oleg Nesterov , Ingo Molnar , Peter Zijlstra , Steven Rostedt , "Paul E. McKenney" , Josh Triplett , Thomas Gleixner , Linus Torvalds , Jiri Kosina , Borislav Petkov , Michal Hocko , linux-mm@kvack.org, Vlastimil Babka , linux-api@vger.kernel.org, linux-kernel@vger.kernel.org On Mon 2015-11-23 17:27:03, Tejun Heo wrote: > Hello, > > On Wed, Nov 18, 2015 at 02:25:12PM +0100, Petr Mladek wrote: > > @@ -610,6 +625,12 @@ repeat: > > if (work) { > > __set_current_state(TASK_RUNNING); > > work->func(work); > > + > > + spin_lock_irq(&worker->lock); > > + /* Allow to queue the work into another worker */ > > + if (!kthread_work_pending(work)) > > + work->worker = NULL; > > + spin_unlock_irq(&worker->lock); > > Doesn't this mean that the work item can't be freed from its callback? > That pattern tends to happen regularly. I am not sure if I understand your question. Do you mean switching work->func during the life time of the struct kthread_work? This should not be affected by the above code. The above code allows to queue an _unused_ kthread_work into any kthread_worker. For example, it is needed for khugepaged, see http://marc.info/?l=linux-kernel&m=144785344924871&w=2 The work is static but the worker can be started/stopped (allocated/freed) repeatedly. It means that the work need to be usable with many workers. But it is associated only with one worker when being used. If the work is in use (pending or being proceed), we must not touch work->worker. Otherwise there might be a race. Because all the operations with the work are synchronized using work->worker->lock. I hope that it makes sense. Thanks a lot for feedback, Petr -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org