Page MenuHomeFreeBSD

Allow vt_timer() callout to run during mountroot input loop in vt(4)
AbandonedPublic

Authored by emaste on Aug 4 2017, 3:59 PM.

Details

Summary

Fixes bug 208239.

During mountroot in vt(4), the vt_timer() callout does not run after regular character key input (special key input causes a manual vd_flush()). Not sure if this is the right approach to enable the callout to run, because I haven't been able to figure out why the callouts are disabled in the first place.

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

Sorry, I don't feel myself enough knowledgeable to give sane answer here :)

usually a driver implement cngrab and cnungrab to deal with locking and other issues related to multiplexing with other devices. Why can't that be used here?

cngets() already calls cngrab() in sys/kern/kern_cons.c here:

void
cngets(char *cp, size_t size, int visible)
{
	char *lp, *end;
	int c;

	cngrab();
	...
}

Should each cnputc() be wrapped with cngrab() and cnungrab() instead of the entire body of cngets()?

cngets() already calls cngrab() in sys/kern/kern_cons.c here:

void
cngets(char *cp, size_t size, int visible)
{
	char *lp, *end;
	int c;

	cngrab();
	...
}

Should each cnputc() be wrapped with cngrab() and cnungrab() instead of the entire body of cngets()?

I was referring to the cn_grab() and cn_ungrab() function pointers for the driver.

However, we can't call DROP/PICKUP because they have to be in the same block.

However, we can't call DROP/PICKUP because they have to be in the same block.

I don't follow what you mean here.

kib edited edge metadata.EditedAug 15 2017, 9:02 PM

However, we can't call DROP/PICKUP because they have to be in the same block.

I don't follow what you mean here.

You cannot directly use DROP_GIANT() in one function, and pair it with PICKUP_GIANT() in another. You would need to inline them and save state (the counter of giant mtx recursion) in between. Look at the definitions of these macros.

You cannot directly use DROP_GIANT() in one function, and PICKUP_GIANT() in another.

I mean, I don't see what @imp is pointing out with that statement; unless I'm missing something I don't see anywhere in the patch or discussion that proposes doing so.

You cannot directly use DROP_GIANT() in one function, and PICKUP_GIANT() in another.

I mean, I don't see what @imp is pointing out with that statement; unless I'm missing something I don't see anywhere in the patch or discussion that proposes doing so.

I'm saying that if the driver needs a lock dropped, shouldn't the driver do that?

But the more important, why do we hold GIANT here at all?

Likely superceded by D14712

This inspired it. And down the rabbit hole to D14712 I went.

emaste edited reviewers, added: smahadevan_freebsdfoundation.org; removed: emaste.

I believe this can be closed as OBE

Or at least just directly committed to stable/{10,11}... It's still relevant there, and likely the right fix. After arguing against it, I've changed my mind and think it's the right code here in older branches.