linux-kernel - Re: vfs-scale, chroot

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 12 Jan 2011 22:55:10 -0500
From:	Trond Myklebust <Trond.Myklebust@...app.com>
To:	Nick Piggin <npiggin@...il.com>
Cc:	"J. R. Okajima" <hooanon05@...oo.co.jp>,
	linux-fsdevel <linux-fsdevel@...r.kernel.org>,
	linux-kernel@...r.kernel.org
Subject: Re: vfs-scale, chroot

On Thu, 2011-01-13 at 12:25 +1100, Nick Piggin wrote: 
> On Thu, Jan 13, 2011 at 7:04 AM, Trond Myklebust
> <Trond.Myklebust@...app.com> wrote:
> 
> > BTW, Nick: Given that some filesystems such as NFS are _always_ going to
> > reject LOOKUP_RCU,
> 
> That's not very optimistic of you... why is that a given, my I ask?
> Or do you just mean as-in the current code?

In the current code, the ECHILD is unconditional, so adding the
unlikely() is clearly pre-empting the reality on the ground.

In the longer run, we can convert the NFS d_revalidate() to accept
LOOKUP_RCU in the case where we believe we can trust the cache without
having to issue an RPC call. That will mainly be in the case where we
hold an NFSv4 delegation, or where we believe the parent directory mtime
has not changed. However that is not something I'm going to have the
cycles to do in this merge window. I assume there will be other
filesystems whose maintainers have similar constraints.

However, even if we were to make these changes, then we're not going to
be accepting LOOKUP_RCU in the 99.999% case that is usually a
requirement for unlikely() to be an acceptable optimisation. For a
number of common workload, directories change all the time, and in NFS,
that inevitably means we need to revalidate their contents with an RPC
call (unless we happen to hold an NFSv4 delegation).

> > it would appear to be completely out of place to use
> > the 'unlikely()' keyword when testing the results of path_walk_rcu() and
> > friends. In particular when the kernel is running with nfsroot, we're
> > saying that 100% of all cases are 'unlikely'...
> 
> Well that _is_ an accepted use of branch annotations. For example it
> is used when scheduling realtime tasks, because even if some systems
> will do 99.9% of their scheduling on realtime tasks, it is not the common
> case.
> 
> I'm not saying that applies here, but: if path walk performance is
> important, then we should use local caching and rcu-walk. If not, then
> why do we care about slightly slower branch?
> 
> The annotations really help to reduce icache penalty of added
> complexity which is why I like them, but I'm happy to remove them
> where they don't make sense of course.

You are adding overhead to existing common paths by doubling the calls
to d_revalidate() in ways which afaics will break branch prediction
(first setting, then clearing LOOKUP_RCU). You can at least try not to
maximise that impact by adding further branch prediction impediments.

Trond

-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@...app.com
www.netapp.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/