From owner-freebsd-arch@FreeBSD.ORG Sun May 3 23:59:07 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 4B9B4BB2; Sun, 3 May 2015 23:59:07 +0000 (UTC) Received: from mail104.syd.optusnet.com.au (mail104.syd.optusnet.com.au [211.29.132.246]) by mx1.freebsd.org (Postfix) with ESMTP id 0E0621C53; Sun, 3 May 2015 23:59:06 +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 mail104.syd.optusnet.com.au (Postfix) with ESMTPS id 1E55F424B6E; Mon, 4 May 2015 09:58:53 +1000 (AEST) Date: Mon, 4 May 2015 09:58:48 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Julian Elischer cc: Mateusz Guzik , Bruce Evans , freebsd-arch@freebsd.org Subject: Re: [PATCH 1/2] Generalised support for copy-on-write structures shared by threads. In-Reply-To: <55461CC3.2020306@freebsd.org> Message-ID: <20150504094742.C3339@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> <55461CC3.2020306@freebsd.org> 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=A5NVYcmG c=1 sm=1 tr=0 a=KA6XNC2GZCFrdESI5ZmdjQ==:117 a=PO7r1zJSAAAA:8 a=kj9zAlcOel0A:10 a=JzwRw_2MAAAA:8 a=bUfv3vSTku50TrIb1NEA: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:59:07 -0000 On Sun, 3 May 2015, Julian Elischer wrote: > On 5/2/15 12:56 AM, 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. >> >> Is _cowgen variant ok? > td_cowgen is much preferable with comment "(k) generation of c.o.w. > pointers." > + u_int td_cowgen; /* (k) generation of c.o.w. pointers. */ It is less preferable with such a comment. The change in the comment just adds 3 style bugs: - inconsistent capitalization. In this file, comments on struct members start with a capital letter - punctuation for COW. Acronyms are not punctuated. Perhaps this one should be in lower case (like vm is usually in lower case). - verboseness from the previous bug. It expands the line line to 80. But there is a problem with the comment. It doesn't do much more than echo the variable name with minor expansions and rearrangements. The only useful parts of it are "(k)" and "pointers". Bruce