From: Al Viro <viro@ZenIV.linux.org.uk>
To: NeilBrown <neilb@suse.de>
Cc: Christoph Hellwig <hch@infradead.org>,
Linus Torvalds <torvalds@linux-foundation.org>,
linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org
Subject: Re: [RFC][PATCHSET] non-recursive link_path_walk() and reducing stack footprint
Date: Thu, 23 Apr 2015 06:01:03 +0100 [thread overview]
Message-ID: <20150423050103.GZ889@ZenIV.linux.org.uk> (raw)
In-Reply-To: <20150421212007.GU889@ZenIV.linux.org.uk>
On Tue, Apr 21, 2015 at 10:20:07PM +0100, Al Viro wrote:
> I agree that unlazy_walk() attempted when walking a symlink ought to fail
> with -ECHILD; we can't legitimize the symlink itself, so once we are out
> of RCU mode, there's nothing to hold the inode of symlink (and its body)
> from getting freed. Solution is wrong though; for example, when
> nested symlink occurs in the middle of a trailing one, we should *not*
> remove the flag upon leaving the nested symlink.
>
> Another unpleasant thing is that ->follow_link() saying "can't do that in
> RCU mode" ends up with restart from scratch - that actually risks to be
> worse than the mainline; there we would attempt unlazy_walk() and normally
> it would've succeed.
>
> AFAICS, the real rule is "can't unlazy if nd->last.name points into a symlink
> body and we might still need to access it"...
Actually, I'm not sure anymore. What if we have unlazy_walk() legitimize
all the symlinks we are traversing? They are visible in nd->stack, after
all... It would mean more complex unlazy_walk(), but not terribly so -
succeeding legitimize_mnt() won't block and we already deal with the
possibility of having vfsmount legitimized, only to be dropped afterwards.
The real unpleasantness here is different - it's the need to keep ->d_seq
of those dentries to tell if they can be grabbed. That's 4 more bytes per
level plus the fun with alignment. OTOH, it both avoids the fun with getting
the logics of when to bail out right *and* avoids the guaranteed restarts
when running into a symlink we can't deal with in RCU mode - we could simply
unlazy and continue in such a situation.
Hell knows... it probably means going all the way wrt dynamic (on demand)
allocation, though. Say it, keeping a couple of levels on stack and allocating
when we need more; the interesting part is in not freeing that sucker too
early. At the very least, we don't want the progression through RCU/normal/
revalidate-everything modes to trigger allocation/freeing on each step; the
nesting depth is going to be the same every time. That's not hard to do...
I'm about to fall asleep right now, so all of the above might very well be
complete hogwash; I'll look into it when I wake up. If anyone has any
comments (including "Al, you are nuts", but something more specific would
be more interesting), please reply.
next prev parent reply other threads:[~2015-04-23 5:01 UTC|newest]
Thread overview: 53+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-04-20 18:12 [RFC][PATCHSET] non-recursive link_path_walk() and reducing stack footprint Al Viro
2015-04-20 18:12 ` [PATCH 01/24] lustre: rip the private symlink nesting limit out Al Viro
2015-04-20 19:08 ` Andreas Dilger
2015-04-20 19:22 ` Al Viro
2015-04-20 20:35 ` Al Viro
2015-04-20 18:12 ` [PATCH 02/24] VFS: replace {, total_}link_count in task_struct with pointer to nameidata Al Viro
2015-04-20 18:12 ` [PATCH 03/24] ovl: rearrange ovl_follow_link to it doesn't need to call ->put_link Al Viro
2015-04-20 18:12 ` [PATCH 04/24] VFS: replace nameidata arg to ->put_link with a char* Al Viro
2015-04-20 18:12 ` [PATCH 05/24] SECURITY: remove nameidata arg from inode_follow_link Al Viro
2015-04-20 18:12 ` [PATCH 06/24] VFS: remove nameidata args from ->follow_link Al Viro
2015-04-20 18:12 ` [PATCH 07/24] namei: expand nested_symlink() in its only caller Al Viro
2015-04-20 18:12 ` [PATCH 08/24] namei.c: separate the parts of follow_link() that find the link body Al Viro
2015-04-20 18:12 ` [PATCH 09/24] namei: fold follow_link() into link_path_walk() Al Viro
2015-04-20 18:12 ` [PATCH 10/24] link_path_walk: handle get_link() returning ERR_PTR() immediately Al Viro
2015-04-20 18:12 ` [PATCH 11/24] link_path_walk: don't bother with walk_component() after jumping link Al Viro
2015-04-20 18:12 ` [PATCH 12/24] link_path_walk: turn inner loop into explicit goto Al Viro
2015-04-20 18:12 ` [PATCH 13/24] link_path_walk: massage a bit more Al Viro
2015-04-20 18:12 ` [PATCH 14/24] link_path_walk: get rid of duplication Al Viro
2015-04-20 18:12 ` [PATCH 15/24] link_path_walk: final preparations to killing recursion Al Viro
2015-04-20 18:13 ` [PATCH 16/24] link_path_walk: kill the recursion Al Viro
2015-04-20 21:04 ` Linus Torvalds
2015-04-20 21:32 ` Al Viro
2015-04-20 21:39 ` Linus Torvalds
2015-04-20 21:51 ` Al Viro
2015-04-20 21:41 ` Linus Torvalds
2015-04-20 21:42 ` Linus Torvalds
2015-04-20 21:59 ` Al Viro
2015-04-20 21:52 ` Al Viro
2015-04-20 18:13 ` [PATCH 17/24] link_path_walk: split "return from recursive call" path Al Viro
2015-04-20 18:13 ` [PATCH 18/24] link_path_walk: cleanup - turn goto start; into continue; Al Viro
2015-04-20 18:13 ` [PATCH 19/24] namei: fold may_follow_link() into follow_link() Al Viro
2015-04-20 18:13 ` [PATCH 20/24] namei: introduce nameidata->stack Al Viro
2015-04-20 18:13 ` [PATCH 21/24] namei: regularize use of put_link() and follow_link(), trim arguments Al Viro
2015-04-20 18:13 ` [PATCH 22/24] namei: trim the arguments of get_link() Al Viro
2015-04-20 18:13 ` [PATCH 23/24] new ->follow_link() and ->put_link() calling conventions Al Viro
2015-04-20 18:13 ` [PATCH 24/24] uninline walk_component() Al Viro
2015-04-21 14:49 ` [RFC][PATCHSET] non-recursive link_path_walk() and reducing stack footprint Al Viro
2015-04-21 15:04 ` Christoph Hellwig
2015-04-21 15:12 ` Richard Weinberger
2015-04-21 15:45 ` Al Viro
2015-04-21 16:46 ` Boaz Harrosh
2015-04-21 21:20 ` Al Viro
2015-04-22 18:07 ` Al Viro
2015-04-22 20:12 ` Al Viro
2015-04-22 21:05 ` Al Viro
2015-04-23 7:45 ` NeilBrown
2015-04-23 18:07 ` Al Viro
2015-04-24 6:35 ` NeilBrown
2015-04-24 13:42 ` Al Viro
2015-05-04 5:11 ` Al Viro
2015-05-04 7:30 ` NeilBrown
2015-04-23 5:01 ` Al Viro [this message]
2015-04-21 14:51 ` [PATCH] logfs: fix a pagecache leak for symlinks Al Viro
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150423050103.GZ889@ZenIV.linux.org.uk \
--to=viro@zeniv.linux.org.uk \
--cc=hch@infradead.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=neilb@suse.de \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).