[PATCH 1/2] Generalised support for copy-on-write structures shared by threads.

Bruce Evans brde at optusnet.com.au
Sun May 3 23:47:40 UTC 2015


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


More information about the freebsd-arch mailing list