Date: Thu, 25 Jun 2009 00:21:45 +0200 From: Ulf Lilleengen <ulf.lilleengen@gmail.com> To: Bruce Evans <brde@optusnet.com.au> Cc: svn-src-projects@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r194847 - projects/libprocstat/sys/sys Message-ID: <200906250021.46249.lulf@freebsd.org> In-Reply-To: <20090625052324.I33482@delplex.bde.org> References: <200906241544.n5OFi43Q019124@svn.freebsd.org> <200906241204.51117.jhb@freebsd.org> <20090625052324.I33482@delplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wednesday 24 June 2009 21:45:43 Bruce Evans wrote: > On Wed, 24 Jun 2009, John Baldwin wrote: > > On Wednesday 24 June 2009 11:44:04 am Ulf Lilleengen wrote: > > ========================================================================= > >===== > > > >> --- projects/libprocstat/sys/sys/user.h Wed Jun 24 15:41:21 > >> 2009 (r194846) +++ projects/libprocstat/sys/sys/user.h Wed Jun 24 > >> 15:44:04 2009 (r194847) @@ -312,6 +312,7 @@ struct kinfo_ofile { > >> > >> struct kinfo_file { > >> int kf_structsize; /* Variable size of record. */ > >> + uint16_t kf_status; /* Status flags. */ > >> int kf_type; /* Descriptor type. */ > >> int kf_fd; /* Array index. */ > >> int kf_ref_count; /* Reference count. */ *** SNIP *** > > >> int _kf_ispare[9]; /* Space for more stuff. */ > > Back to normal indentation. The indentation was fixed in the next commit, as i wanted to handle that in a separate commit. > > >> /* Truncated before copyout in sysctl */ > >> char kf_path[PATH_MAX]; /* Path to file, if any. */ > > > > You probably don't want to add kf_status where you did as it disturbs the > > ABI of all the fields after it. New fields should be added in the spare > > region. Given that mode_t is 16-bits I would just stick it next to > > kf_file_mode. > > Also, almost everything is disordered on size (really alignment). This > seems to break any possibility of the structs having the same size for > amd64 and i386 like the size macro still says they have: > - padding might be needed on amd64 only after "dev_t kf_file_fsid;", > depending on whether this field ends at a 32-bit or 64-bit boundary. > - "uint64_t kf_file_fileid;" gives maximal alignment on both amd64 and > i386, so the padding for the next few fields is clear: > - "mode_t kf_file_mode;" has size 16 bits and "off_t kf_file_size;" has > size 64 bits and maximal alignment requirements, so between these fields > there is 48 bits of padding on amd64 and 16 bits of padding on i386. > - "off_t kf_file_size;" has size 64 and gives maximal alignment again > - "dev_t kf_file_rdev;" has size 32 bits > - "mode_t kf_file_mode;" has size 16 bits and "int _kf_ispare[9];" has > alignment 32 bits, so between these fields there is 16 bits of padding > on both amd64 and i386. Unlike the previous padding, this is > sufficiently MD, but it is still wasteful. The mode_t's should be packed > together. > How about packing them like this: dev_t kf_file_fsid; /* Vnode filesystem id. */ dev_t kf_file_rdev; /* File device. */ 64 uint64_t kf_file_fileid; /* Global file id. */ 64 off_t kf_file_size; /* File size. */ 64 mode_t kf_file_mode; /* File mode. */ uint16_t kf_status; /* Status flags. */ 32 spare[9] Seems good? -- Ulf Lilleengen
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200906250021.46249.lulf>