Add support for a copyoutmbuf() subroutine for DTrace.
Needs ReviewPublic

Authored by gnn on Mar 13 2017, 9:43 AM.

Details

Reviewers
None
Group Reviewers
DTrace
Summary

Diff Detail

Repository
rS FreeBSD src repository
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 15311
Build 15369: arc lint + arc unit
gnn created this revision.Mar 13 2017, 9:43 AM
gnn added a reviewer: DTrace.Mar 13 2017, 9:45 AM
bcr added a subscriber: bcr.Mar 14 2017, 6:24 PM

Fix some typos in comments.
It could be a conspiracy to gently introduce me into DTrace development, I'm not sure. ;-)

sys/cddl/contrib/opensolaris/uts/common/dtrace/dtrace.c
1341

s/functoin/function/
s/Copy/copy/

4621

s/respet/respect/

markj added a subscriber: markj.Mar 14 2017, 7:50 PM
markj added inline comments.
cddl/contrib/opensolaris/lib/libdtrace/common/dt_open.c
246

There should be a space after the first "void".

sys/cddl/contrib/opensolaris/uts/common/dtrace/dtrace.c
1351

Extra newline.

4601

This will break if fbt::m_length:entry is enabled. I think you also need to have NOFAULT set when walking the mbuf chain.

4607

"offset" is unsigned.

4616

dest == mstate->dtms_scratch_ptr here. Ditto below, so I'm confused by the subtraction here.

sys/cddl/contrib/opensolaris/uts/common/sys/dtrace.h
321

Why do the OS-specific subr indices need to be non-overlapping? Seems like every OS could just start at 100 or so for their own subroutines. And then we can use the same definition of VALID_SUBR() on every OS.

333

Maybe prefix this with "DIF_" too to make its purpose easier to guess?

gnn marked 6 inline comments as done.May 3 2017, 11:07 AM
gnn added inline comments.
sys/cddl/contrib/opensolaris/uts/common/sys/dtrace.h
321

If two different OSs have divergent numbers of subroutines (one has 5 the other has 3) I don't see how this can work.

gnn planned changes to this revision.Feb 28 2018, 4:49 AM
gnn marked 4 inline comments as done.
gnn updated this revision to Diff 39818.Feb 28 2018, 4:52 AM

Updates from bcr and markj

domagoj.stolfa_gmail.com added inline comments.
cddl/contrib/opensolaris/lib/libdtrace/common/dt_open.c
245

It's probably not a good idea for this to be DTrace version 1.0.0. Perhaps 1.13.0 would make sense -- or even a bump to 1.13.1?

sys/cddl/contrib/opensolaris/uts/common/dtrace/dtrace.c
4642

Unless anything other than the dtrace_bcopy() in dtrace_mbuf_copydata() can fault, which I can't see, this is not necessary. The DTRACE_LOADFUNC macro already sets CPU_DTRACE_NOFAULT.

sys/cddl/contrib/opensolaris/uts/common/dtrace/dtrace.c
4601

This could be circumvented by implementing this with a builtin DTrace-specific routine such as dtrace_m_length()...?

sys/cddl/contrib/opensolaris/uts/common/sys/dtrace.h
321

DIF_SUBR_OSDEP_MAX could be maintained as the last one added and the compiler could use that to verify it. However, to your point, if we want a portable codebase this would result in an ifdef forest that's a bit difficult to maintain.