Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S964864AbWIJULi (ORCPT ); Sun, 10 Sep 2006 16:11:38 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S964863AbWIJULi (ORCPT ); Sun, 10 Sep 2006 16:11:38 -0400 Received: from ebiederm.dsl.xmission.com ([166.70.28.69]:14516 "EHLO ebiederm.dsl.xmission.com") by vger.kernel.org with ESMTP id S964866AbWIJULh (ORCPT ); Sun, 10 Sep 2006 16:11:37 -0400 From: ebiederm@xmission.com (Eric W. Biederman) To: Oleg Nesterov Cc: Andrew Morton , linux-kernel@vger.kernel.org, Linux Containers , Alan Cox Subject: Re: [PATCH] vt: Rework the console spawning variables. References: <20060910142942.GA7384@oleg> Date: Sun, 10 Sep 2006 14:10:37 -0600 In-Reply-To: <20060910142942.GA7384@oleg> (Oleg Nesterov's message of "Sun, 10 Sep 2006 18:29:42 +0400") Message-ID: User-Agent: Gnus/5.110004 (No Gnus v0.4) Emacs/21.4 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1531 Lines: 43 Oleg Nesterov writes: > On 09/09, Eric W. Biederman wrote: >> >> This patch does several things. >> - The variables used are moved into a structure and declared in vt_kern.h >> - A spinlock is added so we don't have SMP races updating the values. >> - Instead of raw pid_t value a struct_pid is used to guard against >> pid wrap around issues, if the daemon to spawn a new console dies. > > I am not arguing against this patch, but it's a pity we can't use 'struct pid' > lockless. What dou you think about this: Actually with xchg I can use a reference counted struct pid lockless. In the general case you have more then one variable you want to keep in sync and you need the lock for that. rcu is definitely not the solution in these cases as the struct pid is stored for a long time so we need the reference count. It might make sense to have some helper code makes that wraps the following line so it is obvious you can do this. put_pid(xchg(&vc->vt_pid, get_pid(task_pid(current)))); Perhaps: void update_pid(struct pid **ref, struct pid *new) { struct pid *old; get_pid(new); old = xchg(ref, new); put_pid(old); } But since I can write it as a moderately clear one liner in the case that matters I don't much care. Eric - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/