svn commit: r335690 - head/sys/kern

[Devin Teske]

> On Jun 27, 2018, at 6:58 PM, Warner Losh <imp at bsdimp.com> wrote:
> 
> 
> 
> On Wed, Jun 27, 2018 at 7:49 PM, Devin Teske <dteske at freebsd.org <mailto:dteske at freebsd.org>> wrote:
> 
>> On Jun 27, 2018, at 5:59 PM, Warner Losh <imp at bsdimp.com <mailto:imp at bsdimp.com>> wrote:
>> 
>> 
>> 
>> On Wed, Jun 27, 2018 at 5:52 PM, Gleb Smirnoff <glebius at freebsd.org <mailto:glebius at freebsd.org>> wrote:
>> On Wed, Jun 27, 2018 at 04:11:09AM +0000, Warner Losh wrote:
>> W> Author: imp
>> W> Date: Wed Jun 27 04:11:09 2018
>> W> New Revision: 335690
>> W> URL: https://svnweb.freebsd.org/changeset/base/335690 <https://svnweb.freebsd.org/changeset/base/335690>
>> W> 
>> W> Log:
>> W>   Fix devctl generation for core files.
>> W>   
>> W>   We have a problem with vn_fullpath_global when the file exists. Work
>> W>   around it by printing the full path if the core file name starts with /,
>> W>   or current working directory followed by the filename if not.
>> 
>> Is this going to work when a core is dumped not at current working directory,
>> but at absolute path? e.g. kern.corefile=/var/log/cores/%N.core
>> 
>> Yes. That works.
>>  
>> Looks like the vn_fullpath_global needs to be fixed rather than problem
>> workarounded.
>> 
>> It can't be fixed reliably. FreeBSD does not and cannot map a vnode to a name. The only reason we're able to at all is due to the name cache. And when we recreate a file, we invalidate the name cache. And even if we fixed that, there's no guarantee the name cache won't get flushed before we translate the name.... Linux can do this because it keeps the path name associated with the inode. FreeBSD simply doesn't.
>> 
> 
> They said it couldn't be done, but I personally have done it ...
> 
> I map vnodes to full paths in dwatch. It's not impossible, just implausibly hard.
> 
> I derived my formula by reading the C code which was very twisty-turny and rather hard to read at times (and I have been using C since 1998 on everything from AIX and OSF/1 to HP/UX, Cygwin, MinGW, FreeBSD, countless Linux-like things, IRIX, and a God-awful remainder of many others; the vnode code was an adventure to say the least).
> 
> You're welcome to see how it's done in /usr/libexec/dwatch/vop_create
> 
> I load up a clause-local variable named "path" with a path constructed from a pointer to a vnode structure argument to VOP_CREATE(9).
> 
> The D code could easily be rewritten back into C to walk the vnode to completion (compared to the D code which is bounded by depth-limit since DTrace doesn't provide loops; so you have to, as I have done, use a higher-level language wrapper to repeat the code to some desired limit; dwatch defaulting to 64 for directory depth limit).
> 
> Compared this, say, to vfssnoop.d from Chapter 5 of the DTrace book which utilizes the vfs:namei:lookup: probes. My approach in dwatch is far more accurate, produces full paths, and I've benchmarked it at lower overhead with better results.
> 
> Maybe some of this can be useful? If not, just ignore me.
> 
> IMHO, there's no benefit from the crazy hoops than the super simple heuristic I did: if the path starts with / print it. If the path doesn't print cwd / and then the path.
> 
> I look forward to seeing your conversion of the D to C that works, even when there's namespace cache pressure.
> 

I was looking at the output of "dwatch -dX vop_create" (the -d flag to dwatch causes the generated dwatch to be printed instead of executed) and thinking to myself:

I could easily convert this, as I can recognize the flattened loop structure. However, not many people will be able to do it. I wasn't really volunteering, but now I'm curious what other people see when they run "dwatch -dX vop_create". If it's half as bad as I think it is, then I'll gladly do it -- but will have to balance it against work priorities.
-- 
Devin