svn commit: r346120 - head/sys/kern

[Konstantin Belousov]

On Fri, Apr 12, 2019 at 04:23:35PM +0100, Edward Napierala wrote:
> On Fri, 12 Apr 2019 at 16:20, Konstantin Belousov <kostikbel at gmail.com> wrote:
> >
> > On Fri, Apr 12, 2019 at 03:28:26PM +0100, Edward Napierala wrote:
> > > On Thu, 11 Apr 2019 at 17:26, Conrad Meyer <cem at freebsd.org> wrote:
> > > >
> > > > Hi Edward,
> > > >
> > > > I have a question about this change below.
> > > >
> > > > On Thu, Apr 11, 2019 at 4:22 AM Edward Tomasz Napierala
> > > > <trasz at freebsd.org> wrote:
> > > > >
> > > > > Author: trasz
> > > > > Date: Thu Apr 11 11:21:45 2019
> > > > > New Revision: 346120
> > > > > URL: https://svnweb.freebsd.org/changeset/base/346120
> > > > >
> > > > > Log:
> > > > >   Use shared vnode locks for the ELF interpreter.
> > >
> > > [..]
> > >
> > > > On the one hand, perhaps VOP_IS_TEXT() is rarely false for common
> > > > interpreters anyway.  On the other hand, there is sort of a
> > > > renaissance of static linking happening.  So maybe the thought is,
> > > > !VOP_IS_TEXT is likely to be rare, and LK_UPGRADE success even more
> > > > rare, so why bother writing additional code for it?
> > >
> > > Konstantin already answered to most of the points, but regarding
> > > this one: that's exactly the case.  In a typical case, the number of times
> > > this code path will be executed is zero.  I'd expect one - when running
> > > dynamically linked ELF binary for the first time - but for some reason in
> > > that case lookup() returns with the exclusive vnode lock already held.
> >
> > This is strange.  Which filesystem do you use ?
> 
> UFS.
I see. UFS has to use exclusive locking in ffs_vget() when the vnode
is created, i.e. de-facto when inode is read from the volume. See the
manipulations of flags in ffs_vgetf() right after vfs_hash_get() was
unable to find a cached vnode.

But if you just installed new /libexec/ld-elf.so.1, or the binary uses
some other interpreter which was written right before the execve(2)
call, then vnode most likely is still cached and is returned shared-locked.

Use this scenario to actually test the code.