On Wednesday 23 February 2005 17:16, Chuck Silvers wrote:
> On Wed, Feb 16, 2005 at 06:45:02AM -0800, Chuck Silvers wrote:
[...]
> > could you post a summary of what major interfaces are changing?
[..]
>
> nick replied privately that he hasn't had time to do this, so I took a
> stab at it myself.  I looked through header files for changes to data
> fields or to function pointer arguments, and it appeared that almost all
> of the changes can be summarized that way.  here's the list:

[...]

Thanks for "summarising" this. ;)

> I think that uio_procp was originally used more to refer to the address
> space than the thread, so it would be good to take a look at which (if any)
> paths actually need the lwp pointer and see if they can get it some other
> way. I took a quick look at this and almost all of the uses are just to get
> at the proc.

So, I traced a few of the uio_lwp users and found the following

 - coda_{read,write} => coda_rdwr => VOP_{OPEN,CLOSE}
 - coda_readdir => VOP_{OPEN,CLOSE}

I'm still digging.

FWIW, FreeBSD has done s/struct proc *uio_procp/struct thread *uio_td/

> as for the layout changes, we appear to be reinterpreting some of the
> fields and bits in struct ktr_header.
>
> old:
>
> struct ktr_header {
>  int ktr_len;  /* length of ktr_buf */
>  short ktr_type;  /* trace record type */
>  pid_t ktr_pid;  /* process id */
>  char ktr_comm[MAXCOMLEN+1]; /* command name */
>  struct timeval ktr_time; /* timestamp */
>  const void *ktr_buf;
> };
>
>
> new:
>
> struct ktr_header {
>         int     ktr_len;                /* length of record minus length of
> old header */ #if BYTE_ORDER == LITTLE_ENDIAN
>         short   ktr_type;               /* trace record type */
>         short   ktr_version;            /* trace record version */
> #else
>         short   ktr_version;            /* trace record version */
>         short   ktr_type;               /* trace record type */
> #endif
>         pid_t   ktr_pid;                /* process id */
>         char    ktr_comm[MAXCOMLEN+1];  /* command name */
>         union {
>                 struct timeval _tv;     /* v0 timestamp */
>                 struct timespec _ts;    /* v1 timespec */
>         } _ktr_time;
>         union {
>                 const void *_buf;       /* v0 unused */
>                 lwpid_t _lid;           /* v1 lwp id */
>         } _ktr_id;
> };
>
>
> so we're re-using the implicit padding between ktr_type and ktr_pid to
> store the record version.  that looks fine.  but since we're changing the
> layout it's be nice to take the opportunity to make it common across
> platforms. the use of "struct timeval" (whose members are of type "long")
> makes the files different on different platforms. old kdump binaries won't 
> be able to read the new files correctly anyway, so there's no need to make
> the records the same size.  I listed some specifics under "suggestions"
> below.

The changes above mean that old kdump binaries can decode, as normal ,for the 
default mode (i.e. without timestamps) the new ktrace header records with only 
one exception. This being the new KTR_SAUPCALL record for which the old kdump 
will print hex output.

[...]
> so to summarize my suggestions:
>
>  - reconsider whether uio_procp really needs to change.

I'm still looking at this.

>  - investigate eliminating proc/lwp args from some APIs.

Shouldn't this be done on HEAD?

>  - consider using all fixed-size types for ktr_header (ie. uint*_t).
>    so that trace files are more portable.
>    - get rid of the pointer from the on-disk format

This is gone really and replaced with lwp id. Unless you mean remove it from 
the _ktr_id union in struct ktr_header?

>    - timeval/timespec have "long", so don't use them directly.
>      choose 32- or 64-bit and use them explicitly.
>    - if there's also some way to detect the endianness, then the v1 layout
>      should be completely MI after these changes.

I'm not sure I want to break old binaries in this way.

>  - fix copyright merge botches.

I think I've done this.

>  - if we stick with uio_lwp, fix the uio_procp references that were missed.

I've done this for now.

Nick