From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932278AbbGJN5e (ORCPT ); Fri, 10 Jul 2015 09:57:34 -0400 Received: from mail-wi0-f173.google.com ([209.85.212.173]:36710 "EHLO mail-wi0-f173.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754078AbbGJN51 (ORCPT ); Fri, 10 Jul 2015 09:57:27 -0400 Date: Fri, 10 Jul 2015 15:57:22 +0200 From: Frederic Weisbecker To: Oleg Nesterov Cc: LKML , Christoph Lameter , Rik van Riel , Andrew Morton Subject: Re: [RFC PATCH 5/5] kmod: Handle UMH_WAIT_PROC from system unbound workqueue Message-ID: <20150710135720.GB25078@lerouge> References: <1436465237-22031-1-git-send-email-fweisbec@gmail.com> <1436465237-22031-6-git-send-email-fweisbec@gmail.com> <20150709225117.GB17528@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150709225117.GB17528@redhat.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Jul 10, 2015 at 12:51:17AM +0200, Oleg Nesterov wrote: > On 07/09, Frederic Weisbecker wrote: > > > > The UMH_WAIT_PROC handler runs in its own thread for obsolete reasons. > > We couldn't launch and then wait for the exec kernel thread completion > > without blocking other usermodehelper queued jobs since khelper was > > implemented as a singlthread ordered workqueue. > > > > But now we replaced khelper with generic system unbound workqueues which > > can handle concurrent blocking jobs. > > > > So lets run it from the workqueue. > > Probably this is fine, but I am a bit worried... > > WQ_MAX_ACTIVE == 512, this should be enough "in practice". But nothing > protects us from creative driver(s) which spawns 512 long-living user > space tasks... > > Note also that userpace can ptrace these task and "block" sys_wait() > forever. > > I am worried ;) I am too. And it depends a lot on which workqueue we rely on. If we can't rely on the existing ones, we'll to create a new one with a high value for max active and thus potentially a lot of tasks created just for that usermodehelper thing... Then again if we can't know, all we can do is stay conservative. At least we can update the comments to tell about those doubts. > > > CHECK: I'm just worried about the signal handler that gets tweaked > > and also the call to sys_wait() that might fiddle with internals. The > > system workqueue must continue to work without surprise for other > > works. > > Yes. This means that this patch is wrong without disallow_signal() > at the end. Yeah. At least that's fixable :-)