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()?

In D11866#246139, @smahadevan_freebsdfoundation.org wrote:

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.

In D11866#249639, @emaste wrote:

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.

In D11866#249643, @emaste wrote:

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

In D11866#309269, @emaste wrote:

Likely superceded by D14712

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

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.