Re: Infinite loop in task_dentry_path

[Henrik /KaarPoSoft <henrik at kaarposoft dot dk>]

On 05/09/2014 05:29 PM, David Smith wrote:
> On 05/08/2014 05:34 PM, Henrik /KaarPoSoft wrote:
> > Dear systemtap developers,
> >
> > I am using systemtap in a linux distro called KaarPux:
> > http://kaarpux.kaarposoft.dk/
> >
> > Systemtap version 2.5
> > Linux kernel version 3.14.2
> > Linux kernel config:
> > https://sourceforge.net/p/kaarpux/code/ci/31d3302a970318382f64c1e97b9c9df863040e43/tree/master/packages/l/linux.files/config.x86_64
> >
> > I have a simple stp script:
> > ========================================
> > global FSNOTIFY_EVENT_PATH    = 1;
> > probe kernel.function("fsnotify") {
> >      if (!target_set_pid(pid())) { next; }
> >      if ($data_is==FSNOTIFY_EVENT_PATH) {
> >          path = task_dentry_path(task_current(),
> >              @cast($data, "path")->dentry,
> >              @cast($data, "path")->mnt);
> >          println(path);
> > }    }
> > ========================================
> >
> >
> > If I am running this script on a simple shell script like
> > ========================================
> > KX_BASE="$(cd $(dirname ".")/../..; pwd -P)"
> > echo $KX_BASE
> > ========================================
>
> I'm a bit confused by your script. Doesn't 'dirname "."' just return "."
> (at least it did for me)?

The problem occurred in a much more complex system,
and the above script was just a simple one
which triggers the infinite loop.

In the actual case, the argument to dirname is not
"." but a shell variable.

And yes, dirname "." returns ".".

So, it IS a pathetic example, but it is simple
and provokes the same infinite loop as the
one that gives me problems in the complex system.

I think we should stick to the simple example,
but to get an idea of where it actually occurred to me, see:
http://kaarpux.kaarposoft.dk/dependency_tracking.html
https://sourceforge.net/p/kaarpux/code/ci/master/tree/master/chroot_scripts/kx_open.stp
https://sourceforge.net/p/kaarpux/code/ci/master/tree/master/chroot_scripts/install_kx_open_stp.sh
https://sourceforge.net/p/kaarpux/code/ci/master/tree/master/shinc/linux_functions.shinc

> > I get:
> > ========================================
> > ERROR: MAXACTION exceeded near keyword at
> > //share/systemtap/tapset/linux/dentry.stp:211:25
> > ERROR: MAXACTION exceeded near keyword at
> > //share/systemtap/tapset/linux/dentry.stp:211:25
> > ========================================
> >
> > The problem seems to be in task_dentry_path:
> > task_dentry_path goes into an infinite loop on
> > "special" file systems such as the pipe file system.
>
> I was following you up until this point. By "pipe file system" do you
> mean the RPC pipe file system or something else?

I mean pipes as in a shell script: "ls XXX | grep YYY".
Ie:
http://man7.org/linux/man-pages/man7/pipe.7.html

As far as I understand, such pipes have their own
file system implementation:
http://lxr.free-electrons.com/source/fs/pipe.c

> Can you give me more step by step instructions on how to duplicate this
> problem? If detailed enough, we might be able to create a testcase out
> of them. For instance, where were you when you run your shell script?

I am sorry for the lack of detail and clarity.
Here is how to reproduce:

===================
cd /tmp
mkdir T
cd T
cat > t.sh <<"EOF1"
KX_BASE="$(cd $(dirname ".")/../..; pwd -P)"
echo $KX_BASE
EOF1
cat > t.stp <<"EOF2"
global FSNOTIFY_EVENT_PATH    = 1;
probe kernel.function("fsnotify") {
    if (!target_set_pid(pid())) { next; }
    if ($data_is==FSNOTIFY_EVENT_PATH) {
        path = task_dentry_path(task_current(),
            @cast($data, "path")->dentry,
            @cast($data, "path")->mnt);
        println(path);
}    }
EOF2
stap -v -c "sh t.sh" t.stp
===================

> > This patch solves the problem for me:
> > https://sourceforge.net/p/kaarpux/code/ci/master/tree/master/packages/s/systemtap.files/dentry.patch
>
> I've looked at the patch, and I don't think it is quite correct. There
> are 2 paths through that code, and you just modified one of them. In
> addition, I'm still not sold on the need for the new code.
> task_dentry_path() is based on the kernel's dentry_path(), and I don't
> see a similar check of the one you'd like to add in it.

I am certainly no expert on neither systemtap nor the linux kernel.

I am positive that there IS a problem, though!

I see it in my more complex real life example,
as well as in the simple example in this mail.

I have instrumented task_dentry_path
with print statements, and I can see that
dentry = @cast(mnt, "mount")->mnt_mountpoint
vfsmnt = &@cast(mnt, "mount")->mnt_parent->mnt
leaves dentry and vfsmnt with the same values as before,
hence the infinite loop.

(I only modified one of the two code paths,
because it is in that path I see the problem).

The patch is probably wrong, and I hope that someone more
knowledgeable than me may come up with a better one.

However, if I look at the comment about task_dentry_path in
https://sourceware.org/git/gitweb.cgi?p=systemtap.git;a=blob;f=tapset/linux/dentry.stp
it seems to me that is based on the kernel's d_path().

And the comment in the top of
http://lxr.free-electrons.com/source/fs/dcache.c#L3052
says something about "synthetic filesystems",
and just maybe this is doing something to overcome the problem.

But again:
I am no expert on neither systemtap nor the linux kernel.
I am sure that there is a problem, but not at all that
my "solution"/patch is correct.