From owner-freebsd-arch@FreeBSD.ORG Sun May 3 23:47:40 2015 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 95C30A47 for ; Sun, 3 May 2015 23:47:40 +0000 (UTC) Received: from mail109.syd.optusnet.com.au (mail109.syd.optusnet.com.au [211.29.132.80]) by mx1.freebsd.org (Postfix) with ESMTP id 5B4E61B81 for ; Sun, 3 May 2015 23:47:40 +0000 (UTC) Received: from c211-30-166-197.carlnfd1.nsw.optusnet.com.au (c211-30-166-197.carlnfd1.nsw.optusnet.com.au [211.30.166.197]) by mail109.syd.optusnet.com.au (Postfix) with ESMTPS id E798CD64EE7; Mon, 4 May 2015 09:47:36 +1000 (AEST) Date: Mon, 4 May 2015 09:47:36 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Mateusz Guzik cc: freebsd-arch@freebsd.org Subject: Re: [PATCH 1/2] Generalised support for copy-on-write structures shared by threads. In-Reply-To: <20150501165633.GA7112@dft-labs.eu> Message-ID: <20150504092712.X3339@besplex.bde.org> References: <1430188443-19413-1-git-send-email-mjguzik@gmail.com> <1430188443-19413-2-git-send-email-mjguzik@gmail.com> <20150428181802.F1119@besplex.bde.org> <20150501165633.GA7112@dft-labs.eu> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.1 cv=QeFf4Krv c=1 sm=1 tr=0 a=KA6XNC2GZCFrdESI5ZmdjQ==:117 a=PO7r1zJSAAAA:8 a=kj9zAlcOel0A:10 a=JzwRw_2MAAAA:8 a=qFd6ZJhFGKGVsV3VXbMA:9 a=CjuIK1q_8ugA:10 X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 03 May 2015 23:47:40 -0000 On Fri, 1 May 2015, Mateusz Guzik wrote: > On Tue, Apr 28, 2015 at 06:45:01PM +1000, Bruce Evans wrote: >> On Tue, 28 Apr 2015, Mateusz Guzik wrote: >>> diff --git a/sys/sys/proc.h b/sys/sys/proc.h >>> index 64b99fc..f29d796 100644 >>> --- a/sys/sys/proc.h >>> +++ b/sys/sys/proc.h >>> @@ -225,6 +225,7 @@ struct thread { >>> /* Cleared during fork1() */ >>> #define td_startzero td_flags >>> int td_flags; /* (t) TDF_* flags. */ >>> + u_int td_cowgeneration;/* (k) Generation of COW pointers. */ >>> int td_inhibitors; /* (t) Why can not run. */ >>> int td_pflags; /* (k) Private thread (TDP_*) flags. */ >>> int td_dupfd; /* (k) Ret value from fdopen. XXX */ >> >> This name is so verbose that it messes up the comment indentation. > > Yeah, that's crap, but the naming is already inconsistent and verbose. > For instance there is td_generation alrady. td_generation is a much worse. It looks like it might be a generation counter for reused thread instances, but is actually just a thread preemption counter. > Is _cowgen variant ok? Yes. It is more verbose than _cowg, but easier to guess what it means. Everyone knows what a cow is, but not what a g is. However, since td_cowgen is not for threads generally, but just for a couple of things hung off threads, perhaps its name should give a hint about those things and not say anything about "cow". I didn't notice how you named these things. >>> @@ -830,6 +832,11 @@ extern pid_t pid_max; >>> KASSERT((p)->p_lock == 0, ("process held")); \ >>> } while (0) >>> >>> +#define PROC_UPDATE_COW(p) do { \ >>> + PROC_LOCK_ASSERT((p), MA_OWNED); \ >>> + p->p_cowgeneration++; \ >> >> Missing parentheses. > > Oops, fixed. Further naming problems. This macro doesn't update cow things, but just increases the generation count. > That said, I would say the patch below is ok enough. OK. Bruce