Re: Filepaths in VM map for tmpfs files

On Thursday, February 05, 2015 10:37:55 AM Konstantin Belousov wrote:
> On Wed, Feb 04, 2015 at 10:15:04AM -0500, John Baldwin wrote:
> > On Tuesday, February 03, 2015 10:33:36 PM Konstantin Belousov wrote:
> > > On Mon, Feb 02, 2015 at 09:50:22PM -0600, Eric Badger wrote:
> > > > On 02/02/2015 03:30 AM, Konstantin Belousov wrote:
> > > > > On Sun, Feb 01, 2015 at 08:38:29PM -0600, Eric Badger wrote:
> > > > >> On 01/31/2015 09:36 AM, Konstantin Belousov wrote:
> > > > >>> First, shouldn't the kve_type changed to KVME_TYPE_VNODE as well ?
> > > > >> 
> > > > >> My thinking is no, because KVME_TYPE_SWAP is in fact the correct
> > > > >> type;
> > > > >> I'd opine that it is better to be transparent than make it look
> > > > >> like
> > > > >> there is an OBJT_VNODE object there. It may be that some programs
> > > > >> would
> > > > >> be confused by VNODE info returned on a SWAP type mapping, though I
> > > > >> know
> > > > >> that dtrace handles it OK.
> > > > > 
> > > > > kve_vn_* and kve_path fields are defined only for KVME_TYPE_VNODE
> > > > > kve_type.
> > > > > So this is in fact a bug in whatever used the API to access kve_path
> > > > > for KVE_TYPE_SWAP.
> > > > 
> > > > Hmm, is that documented anywhere? I think it's fair to assume that
> > > > kve_vn* applies only to the VNODE type,
> > > > but I know there are several in-tree users that reference kve_path
> > > > regardless of type (ostensibly relying
> > > > on the default of an empty string). Maybe one could determine the
> > > > validity of the kve_vn* fields by
> > > > inspecting the kve_vn_type (not sure of all the consequences of that)?
> > > > Or change it to KVME_TYPE_VNODE
> > > > and deal with the below problem...
> > > 
> > > There is no useful documentation for the kern.proc. sysctls.
> > > My word (and statements from other involved developers) could be
> > > considered as close to the truth as it can be.
> > > Somebody taking the efforts to document the stuff would make very
> > > valuable contribution.
> > 
> > I think that kve_path should be valid for all types (e.g. shm_open()
> > is not a vnode but has a pathname, and that should be fixed to display
> > if possible). In the equivalent for files (kinfo_file), the pathname
> > is type-independent and always valid.
> 
> Well, this means that it should be valid for vnodes and shm.  My point
> is that kvme_vn_path should be used only after the check for type.
> We can and do set it to nul string, but using the path unconditionally
> is a bug in the user code.

The problem is that shm's can have different types (DEFAULT vs SWAP vs PHYS). 
:)  For kinfo_file, tools like fstat always print kf_path regardless of type.  
I do think it would be more consistent if the path in a kvme worked the same 
way.  Then you don't have to update all the tools each time a type starts 
populating the path.

> > That said, I think tmpfs nodes should be exposed as files. It is an
> > implementation detail of tmpfs that they are swap-backed, but from a
> > user's perspective these are files, and if you want to expose other
> > vnode-specific fields than just the path, KVME_TYPE_VNODE would be
> > more correct.
> 
> I agree, but doing it is not easy, since there might be no vnode
> to get the required information from.  We do know that this swap
> object is for tmpfs node, but currently we only store pointer to
> object in the node, not pointer to node from the object.  When the
> vnode exists, pointer to vnode is stored in the object.
> 
> To fix the issue, we should store pointer to node. Code was not done
> this way, because VM code  which handles special-case for OBJT_TMPFS,
> would need to know tmpfs internals. Right now, code knows about vnodes
> anyway, so object->vnode does not bring tmpfs internals into vm.

I'm more arguing in support of your original proposal.  Doing a best effort if 
the vnode exists would certainly be an improvement over what we have now.

-- 
John Baldwin