Type info can be recovered from the .o compiled with -g, using dwarf dump utilities, which we do not have in base.

When we discussed inlining critical_enter(9) with mjg, my opinion was that struct thread_lite only adds complications. It is good enough to only have the offsets to the members auto-generated, and manually calculate the addresses of the td_critnest and td_owepreempt. It seems that I am the only one who thinks so, everybody else prefer thread_lite. genoffset.h is the good illustration of what I mean. BTW, what are the restrictions on the structure definitions which are processed by the script ?

In D16078#340860, @kib wrote:

Type info can be recovered from the .o compiled with -g, using dwarf dump utilities, which we do not have in base.

When we discussed inlining critical_enter(9) with mjg, my opinion was that struct thread_lite only adds complications. It is good enough to only have the offsets to the members auto-generated, and manually calculate the addresses of the td_critnest and td_owepreempt. It seems that I am the only one who thinks so, everybody else prefer thread_lite. genoffset.h is the good illustration of what I mean. BTW, what are the restrictions on the structure definitions which are processed by the script ?

That ends up being quite ugly. The use of macros here is the least of the issues here:

#define CRITICAL_ENTER(td)										\
	do {														\
		int *td_critnest = (int *)(((char *)td) + TD_CRITNEST);	\
		(*td_critnest)++;										\
		__compiler_membar();									\
	} while (0)
#define CRITICAL_EXIT(td)												\
	do {																\
	int *td_critnest = (int *)(((char *)td) + TD_CRITNEST);				\
	u_char *td_owepreempt = (u_char *)(((char *)td) + TD_OWEPREEMPT);	\
	MPASS(*td_critnest > 0);											\
	__compiler_membar();												\
	(*td_critnest)--;													\
	__compiler_membar();												\
	if (__predict_false(*td_owepreempt != 0))							\
		critical_exit_preempt();										\
} while (0)
#define SCHED_PIN(td)											\
	do {														\
		int *td_pinned = (int *)(((char *)td) + TD_PINNED);		\
		(*td_pinned)++;											\
		__compiler_membar();									\
	} while (0)

#define SCHED_UNPIN(td)											\
	do {														\
		int *td_pinned = (int *)(((char *)td) + TD_PINNED);		\
		__compiler_membar();									\
		MPASS(*td_pinned > 0);									\
		(*td_pinned)--;											\
		__compiler_membar();									\
	} while (0)

static __inline void
epoch_enter_preempt(epoch_t epoch, epoch_tracker_t et)
{
	struct epoch_record *er;
	struct epoch_thread *etd;
	caddr_t ptd;
	u_char *td_priority, *td_pre_epoch_prio;
	u_char *td_epochnest;
	MPASS(cold || epoch != NULL);
	INIT_CHECK(epoch);
	etd = (void *)et;
#ifdef INVARIANTS
	MPASS(epoch->e_flags & EPOCH_PREEMPT);
	etd->et_magic_pre = EPOCH_MAGIC0;
	etd->et_magic_post = EPOCH_MAGIC1;
#endif
	ptd = (caddr_t)curthread;
	etd->et_td = (void*)ptd;
	td_epochnest = ptd + TD_EPOCHNEST;
	(*td_epochnest)++;
	CRITICAL_ENTER(ptd);
	SCHED_PIN(ptd);

	td_priority = (u_char *)ptd + TD_PRIORITY;
	td_pre_epoch_prio = (u_char *)ptd + TD_PRE_EPOCH_PRIO;
	*td_pre_epoch_prio = *td_priority;
	er = epoch->e_pcpu[curcpu];
	TAILQ_INSERT_TAIL(&er->er_tdlist, etd, et_link);
	ck_epoch_begin(&er->er_record, (ck_epoch_section_t *)&etd->et_section);
	CRITICAL_EXIT(ptd);
}

Most would agree this is much nicer:

/*
 * Standalone (_sa) routines for thread state manipulation
 */
static __inline void
critical_enter_sa(void *tdarg)
{
	struct thread_global *td;

	td = tdarg;
	td->td_critnest++;
	__compiler_membar();
}

static __inline void
critical_exit_sa(void *tdarg)
{
	struct thread_global *td;

	td = tdarg;
	MPASS(td->td_critnest > 0);
	__compiler_membar();
	td->td_critnest--;
	__compiler_membar();
	if (__predict_false(td->td_owepreempt != 0))
		critical_exit_preempt();
}

static __inline void
sched_pin_sa(void *tdarg)
{
	struct thread_global *td;

	td = tdarg;
	td->td_pinned++;
	__compiler_membar();
}

static __inline void
sched_unpin_sa(void *tdarg)
{
	struct thread_global *td;

	td = tdarg;
	MPASS(td->td_pinned > 0);
	__compiler_membar();
	td->td_pinned--;
	__compiler_membar();
}

static __inline void
epoch_enter_preempt(epoch_t epoch, epoch_tracker_t et)
{
	struct epoch_record *er;
	struct epoch_thread *etd;
	struct thread_global *td;
	MPASS(cold || epoch != NULL);
	INIT_CHECK(epoch);
	etd = (void *)et;
#ifdef INVARIANTS
	MPASS(epoch->e_flags & EPOCH_PREEMPT);
	etd->et_magic_pre = EPOCH_MAGIC0;
	etd->et_magic_post = EPOCH_MAGIC1;
#endif
	td = (struct thread_global *)curthread;
	etd->et_td = (void*)td;
	td->td_epochnest++;
	critical_enter_sa(td);
	sched_pin_sa(td);

	td->td_pre_epoch_prio = td->td_priority;
	er = epoch->e_pcpu[curcpu];
	TAILQ_INSERT_TAIL(&er->er_tdlist, etd, et_link);
	ck_epoch_begin(&er->er_record, (ck_epoch_section_t *)&etd->et_section);
	critical_exit_sa(td);
}

Rather than (int *)((char *)td + offset), you might need to do (int *)((uinptr_t)td + offset) to get around alignment changed warnings.

so you have both thread_lite and offset generator... Why?

In D16078#340968, @imp wrote:

so you have both thread_lite and offset generator... Why?

thread_lite is generated by the awk.

I would like it if we either generated a verification file that is built into the kernel that asserts typeof and offsetof matches. Or if we can easily extract type information as kib suggested. I like this approach in general but I just want to avoid the inevitable bugs that will come from any mismatch.

In D16078#340860, @kib wrote:

Type info can be recovered from the .o compiled with -g, using dwarf dump utilities, which we do not have in base.

When we discussed inlining critical_enter(9) with mjg, my opinion was that struct thread_lite only adds complications. It is good enough to only have the offsets to the members auto-generated, and manually calculate the addresses of the td_critnest and td_owepreempt. It seems that I am the only one who thinks so, everybody else prefer thread_lite. genoffset.h is the good illustration of what I mean. BTW, what are the restrictions on the structure definitions which are processed by the script ?

I understand your hesitation. I like it if we can make it more strongly verified. Managing offsets is really only palatable if we just use it for this one feature. I suspect this will give us opportunities to unwind some headers a bit and it may see more use.

The awk script is pretty dumb. It just takes the two types as strings. If the member type is complex it will need its header included in the generated file which somewhat defeats the purpose. The code uses sizeof to calculate the padding.

In D16078#340965, @mmacy wrote:

When we discussed inlining critical_enter(9) with mjg, my opinion was that struct thread_lite only adds complications. It is good enough to only have the offsets to the members auto-generated, and manually calculate the addresses of the td_critnest and td_owepreempt. It seems that I am the only one who thinks so, everybody else prefer thread_lite. genoffset.h is the good illustration of what I mean. BTW, what are the restrictions on the structure definitions which are processed by the script ?

That ends up being quite ugly. The use of macros here is the least of the issues here:

...

Most would agree this is much nicer:

Yes, I understand the prevailing opinion, I just wanted to express mine. It is clear that thread_lite version is going to be used.

The awk script is pretty dumb. It just takes the two types as strings. If the member type is complex it will need its header included in the generated file which somewhat defeats the purpose. The code uses sizeof to calculate the padding.

genoffset.c will fail to compile if the size of the declared doesn't match. It doesn't address sign issues. But I think it provides the safety guarantees we need.