From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1030819AbXDVTuS (ORCPT ); Sun, 22 Apr 2007 15:50:18 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1030824AbXDVTuR (ORCPT ); Sun, 22 Apr 2007 15:50:17 -0400 Received: from pentafluge.infradead.org ([213.146.154.40]:39424 "EHLO pentafluge.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1030819AbXDVTuP (ORCPT ); Sun, 22 Apr 2007 15:50:15 -0400 Date: Sun, 22 Apr 2007 20:50:01 +0100 From: Christoph Hellwig To: Andrew Morton Cc: Simon Horman , "Eric W. Biederman" , containers@lists.osdl.org, Oleg Nesterov , Christoph Hellwig , linux-kernel@vger.kernel.org, Wensong Zhang , Julian Anastasov Subject: Re: [PATCH] ipv4/ipvs: Convert to kthread API Message-ID: <20070422195001.GB18561@infradead.org> Mail-Followup-To: Christoph Hellwig , Andrew Morton , Simon Horman , "Eric W. Biederman" , containers@lists.osdl.org, Oleg Nesterov , linux-kernel@vger.kernel.org, Wensong Zhang , Julian Anastasov References: <11769696262330-git-send-email-ebiederm@xmission.com> <20070419090435.GB26050@verge.net.au> <20070419155944.e5fb8e66.akpm@linux-foundation.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20070419155944.e5fb8e66.akpm@linux-foundation.org> User-Agent: Mutt/1.4.2.2i X-SRS-Rewrite: SMTP reverse-path rewritten from by pentafluge.infradead.org See http://www.infradead.org/rpr.html Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Apr 19, 2007 at 03:59:44PM -0700, Andrew Morton wrote: > There still seems to be quite a lot of complexity in this driver's > thread handling which could be removed if we did a full conversion > to the kthread API. > > It all looks.... surprisingly complex in there. It is. There quite a few interesting oddities in this code: - creation of a forker thread. This is superflous when using the kthread infrastructure as a thread created by kthread_create always comes from our dedicated forker thread. - the infinite retry on failure looks very bogus, the system doesn't recover very well if you try to fork forever in a loop :) - a lot of very overlapping state variables. My reading of the code suggests that both a 'master' and 'backup' thread can run at the same time. I think the code would benefit a lot from totally separating these codepathes. - start_sync_thread and stop_sync_thread are called with unchecked user supplied arguments and bug if they don't match the expected values. While all this is under capable(CAP_NET_ADMIN) it still sounds like something to fix. - and the usual removal of semaphores and completions for startup/shutdown would benefit the code a lot, as for most thread users.