Re: [PATCH] Let gcore use ptrace interface rather than the procfs

2009/11/19 Robert Watson <rwatson_at_freebsd.org>:
>
> On Thu, 19 Nov 2009, Attilio Rao wrote:
>
>> 2009/11/17 Robert N. M. Watson <rwatson_at_freebsd.org>:
>>>
>>> On 17 Nov 2009, at 14:17, Ed Maste wrote:
>>>
>>>> Our original motivation for doing this was to make gcore work with
>>>> threaded apps, not avoiding procfs, but that's a useful side-effect of the
>>>> work.  Note though that for that purpose it isn't complete; procfs is still
>>>> used in readmap to read the process' memory map.  It looks like we need to
>>>> find a way to implement readmap without procfs.
>>>
>>> Are the sysctls used for procstat -v sufficient for this purpose?
>>
>> This patch should address the arised concerns by both of you:
>> http://www.freebsd.org/~attilio/Sandvine/STABLE_8/gcore/gcore2.diff
>>
>> and additively fix elf_getstatus() to not use procfs, so that gcore is
>> completely procfs independent now. Comments, reviews and testing are
>> welcome.
>
> If you add the missing include of sys/wait.h, elfcore.c generates an error
> instead of a warning on this non-traditional use of wait(2):
>
> +       wait();
>
> Something like this may be preferred:
>
>        if (waitpid(pid, NULL, 0) < 0)
>                err(1, "waitpid");

I didn't get a warning neither an error but yes, the waitpid() is
preferred and should be used.

> This further persisting reference to procfs can be replaced with a
> sysctl/kvm interface:
>
> gcore.c:  asprintf(&binfile, "/proc/%d/file", pid);

Right, I just modified elfcore so I missed it.

> See the implementation of "procstat -b" which returns the path to the binary
> using the same underlying mechanism (vn_fullpath on the process image
> vnode).
>
> I think that kills the last of the procfs dependencies, in which case
> perhaps we can remove the procfs.h include from elfcore.c, which requires
> defining a local version of a summary data structure borrowed from procfs.
>  It's worth trying with procfs unmounted, however, to make sure they're
> really all gone (which is how I ran into the above problem).

I don't like the idea to replicate the structures because of code
maintence. IMHO is ok to have procfs header.

I will provide ASAP a new patch which addresses this concerns and
testing without procfs mounted.

Thanks,
Attilio


-- 
Peace can only be achieved by understanding - A. Einstein