This is the mail archive of the systemtap@sourceware.org mailing list for the systemtap project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Proposed task.stp additions


Stone, Joshua I wrote:
Mike Mason wrote:
Stone, Joshua I wrote:
It's unsafe to use d_path() because it takes locks.  Since we can
call from arbitrary contexts, this has the possibility to create a
deadlock. It also dereferences the pointers you give it, so you have
to be 100% sure that those pointers are valid.
I understand that now and should have earlier.  What are my options?
If I implement the d_path() functionality without locks and
protected by kreads, is that sufficient?  Also, if I limit cwd() to
the current task (put in context.stp) does that help?  I expect cwd()
to be needed in the context of current most of the time anyway.  I've
used it to construct a full pathname in a sys_open probe.

Actually, it looks like d_path() may only be valid for the current task, since it does work on "current->fs". Limiting cwd() to the current task means you don't have to worry about some of the dereferencing issues, since we trust that "current" is valid. But any of the data protected by locks could change dynamically, so if you rewrite d_path() without any locks, you're unsafe again.

Yes, the root part of the path is always in the context of the current task. That's probably fine for all anticipated uses.


I didn't mention that I'm trying to use this same general mechanism to get full pathnames when probing functions that operate on fd's and struct file's. The probes typically run in the context of current, but the same locking issues apply.


I don't have a nice solution for this. Perhaps we could somehow mark cwd() as a guru-only function, and then accept the potentially dangerous behavior with a warning. Or, you could submit a patch to the kernel to enable a lockless cwd query.

I'm not fond of the guru-only approach. We need a solution that allows all scripts to access actual filenames. fd's and struct file's have no meaning to humans.


Given that systemtap would be the only user, I doubt we could get a lockless cwd query into the kernel. Don't we already have some lockless versions of kernel functions in the runtime?

Just so I'm clear, probe handlers should not block in any way, correct? I know we've discussed that before, but I guess I just don't want to believe it :-) It sure limits what we can extract with systemtap.



BUT, as long as *everything* you do with the result of pid2task() is
protected by kread(), this is probably OK.  In the rare case that
the task disappears, you may get corrupt data or an error from an
invalid dereference, but this won't impact system stability.
That's true of all the task_* functions, isn't it?  If we cache
task_struct pointers and pass them to the task_* functions, there's
never a guarantee you'll get what you expect.

True, but in this case you may get bad results even if you use it right away.

I still don't see the difference. It doesn't matter how you obtained the task pointer. Once you pass it to one of the task_* functions, it's alway possible the task no longer exists unless it's the current task. What am I missing?


Mike



Josh


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]