svn commit: r335690 - head/sys/kern

[Warner Losh]

On Wed, Jun 27, 2018 at 8:14 PM, Devin Teske <dteske at freebsd.org> wrote:

>
> 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> wrote:
>
>>
>> On Jun 27, 2018, at 5:59 PM, Warner Losh <imp at bsdimp.com> wrote:
>>
>>
>>
>> On Wed, Jun 27, 2018 at 5:52 PM, Gleb Smirnoff <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
>>> 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.
>

We don't have the data you do in vop_create anymore, just the vp at the
point in the code where we want to do the reverse translation... But we
also have a name that's been filled in from the template and cwd, which
gives us almost the same thing as we'd hoped to dig out of the name cache...

Warner