Page MenuHomeFreeBSD
Differential D57442

vt: Do not lock request comming from terminal, only those originated by mouse. Because the terminal surrounds requests to vt(4) with locking.
ClosedPublic
Actions

Authored by ray on Thu, Jun 4, 3:16 PM.
Tags
None
Referenced Files
F159671146: D57442.id179634.diff
Tue, Jun 16, 10:00 PM
Unknown Object (File)
Mon, Jun 15, 7:34 PM
Unknown Object (File)
Mon, Jun 15, 7:34 PM
Unknown Object (File)
Mon, Jun 15, 7:34 PM
Unknown Object (File)
Mon, Jun 15, 7:34 PM
Unknown Object (File)
Mon, Jun 15, 7:34 PM
Unknown Object (File)
Sun, Jun 14, 10:18 PM
Unknown Object (File)
Fri, Jun 12, 9:27 PM
View All 17 Files
Subscribers
None

Details

Reviewers
imp
glebius
bz
adrian
Commits
rG1f68ca5802db: vt: Do not lock request comming from terminal
Summary

Reported by: bz, adrian
MFC after: 2 weeks

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 73819
Build 70702: arc lint + arc unit

Event Timeline

ray requested review of this revision.Thu, Jun 4, 3:16 PM
ray created this revision.
Harbormaster completed remote builds in B73670: Diff 179214.Thu, Jun 4, 3:16 PM
ray updated this revision to Diff 179215.Thu, Jun 4, 3:18 PM

Updating D57442: vt: Do not lock request comming from terminal, only those originated by

mouse. Because the terminal surrounds requests to vt(4) with locking.

Harbormaster completed remote builds in B73671: Diff 179215.Thu, Jun 4, 3:18 PM
adrian accepted this revision.Thu, Jun 4, 3:25 PM

ok, i see what's going on here. im' ok with this to unbrick -head, but personally:

  • have xxx_locked() versions of functions which mtx_assert(mtx, MA_OWNED)
  • have unlocked versions of functions that mtx_assert(mtx, MA_UNOWNED); grab lock; call _locked version; mtx_unlock();
  • callers would then either call the xxx() version if they don't own the lock, or xxx_locked() if they already hold the lock.

If this fixes the crash then I'm going to approve it to unbrick head, but it does tell me we should do a pass or two around locking in vt.

This revision is now accepted and ready to land.Thu, Jun 4, 3:25 PM
glebius added inline comments.Mon, Jun 8, 7:17 PM
sys/dev/vt/vt.h
248–251 ↗(On Diff #179215)

I'd suggest to just use bool locked. This is a very common style throughout the kernel in the last years. Or use Adrian's suggestion.

sys/dev/vt/vt_buf.c
744–758

Alternatively to this whole patch you can just use mtx_owned(9) and don't need to modify the function argument list:

#define VTBUF_LOCK_OWNED(vb) mtx_owned(&(vb)->vb_lock)
...
static void
vtbuf_flush_mark(...
	bool dounlock;
...
	if (!VTBUF_LOCK_OWNED(vb)) {
		dounlock = true;
		VTBUF_LOCK(vb);
	} else
		dounlock = false;
	vtbuf_dirty(vb, &area);
	if (dounlock)
		VTBUF_UNLOCK(vb);
ray updated this revision to Diff 179602.Thu, Jun 11, 1:36 PM

Check if lock owned.

Updating D57442: vt: Do not lock request comming from terminal, only those originated by

mouse. Because the terminal surrounds requests to vt(4) with locking.

This revision now requires review to proceed.Thu, Jun 11, 1:36 PM
Harbormaster completed remote builds in B73819: Diff 179602.Thu, Jun 11, 1:36 PM
glebius accepted this revision.Thu, Jun 11, 1:45 PM
This revision is now accepted and ready to land.Thu, Jun 11, 1:45 PM
adrian accepted this revision.Thu, Jun 11, 2:04 PM
Closed by commit rG1f68ca5802db: vt: Do not lock request comming from terminal (authored by ray). · Explain WhyThu, Jun 11, 11:57 PM
This revision was automatically updated to reflect the committed changes.
ray marked an inline comment as done.
ray added a commit: rG1f68ca5802db: vt: Do not lock request comming from terminal.

Revision Contents
Changeset List

PathSize
sys/
dev/
vt/
17 lines

Diff 179602

View Options

sys/dev/vt/vt_buf.c

Loading...