On Wed, Aug 27, 2014 at 01:06:02PM -0600, Eric Blake wrote:
> On 08/27/2014 10:15 AM, Pádraig Brady wrote:
> > On 08/27/2014 03:08 PM, Fridolin Pokorny wrote:
> >> diff --git a/lib/mountlist.c b/lib/mountlist.c
> >
> > Cool, this fits well.
> > It would be good to mention the functionality and
> > performance benefits in the changelog.
> >
> >> +#ifdef MOUNTED_PROC_MOUNTINFO
> >> +static const char *
> >> +mountinfo_path (void) {
> >> + static char filename[sizeof ("/proc//mountinfo") + 13]; /* 13 to hold a
> >> PID */
>
> The magic number 13 is gross, compared to using <intprops.h> from gnulib
> and using INT_STRLEN_BOUND(pid_t) (which should evaluate to 13) instead.
>
> >> +
> >> + sprintf (filename, "/proc/%u/mountinfo", getpid ());
> >> +
> >> + return filename;
> >> +}
> >
> > I dislike the above as it precludes concurrent usage.
> > You could avoid that issue by allocating on the heap,
>
> or by setting up a witness so that sprintf is called only once by the
> first caller, and all later callers wait until the first caller is complete.
>
> > but can't this be simplified by using /proc/self/mountinfo ?
>
> The argument here was that the kernel is slightly slower in order to
> resolve the symlink of /proc/self in all cases - but is that penalty
> going to be noticeable?
Well, we have /proc/self is everywhere, including libmount, findmnt,
mount, ... systemd sources:
$ git grep "/proc/self" | wc -l
51
please, don't try to be more smart then kernel, it's premature
optimization to care about such things in userspace.
BTW, there will be /proc/thread-self
(http://lwn.net/Articles/607422/)
Karel
--
Karel Zak <k...@redhat.com>
http://karelzak.blogspot.com
