Date: Mon, 4 May 2015 09:47:36 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Mateusz Guzik <mjguzik@gmail.com> Cc: freebsd-arch@freebsd.org Subject: Re: [PATCH 1/2] Generalised support for copy-on-write structures shared by threads. Message-ID: <20150504092712.X3339@besplex.bde.org> In-Reply-To: <20150501165633.GA7112@dft-labs.eu> 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>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20150504092712.X3339>