From: Al Viro <viro@ZenIV.linux.org.uk>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
linux-fsdevel <linux-fsdevel@vger.kernel.org>,
Neil Brown <neilb@suse.com>
Subject: Re: [PATCHSET v2] ->follow_link() without dropping from RCU mode
Date: Thu, 10 Dec 2015 00:10:12 +0000 [thread overview]
Message-ID: <20151210001012.GA20997@ZenIV.linux.org.uk> (raw)
In-Reply-To: <20151209182309.GZ20997@ZenIV.linux.org.uk>
On Wed, Dec 09, 2015 at 06:23:09PM +0000, Al Viro wrote:
> What's more, that dentry might very well have gone negative by that
> point. Think what happens if, during the symlink traversal, we run
> into the hard "restart from scratch in non-RCU mode". We'll need to
> do ->put_link() on everything we have in stack. Regardless of what
> might've happened to dentries/inodes of symlinks involved - all we
> are really promised is that RCU-delayed parts of their destruction hadn't
> been entered yet.
>
> Note, BTW, that *all* ->put_link() instances ignore the inode argument
> and I seriously considered dropping it completely.
PS: I toyed with the idea of replacing cookie a struct callback_head, to make
it really obvious what's going on - we are leaving a closure to be evaluated
once the caller is done with our return value. That way ->put_link() as a
method would've been gone and we would've had something like
init_callback(destructor, page_put_link, page);
instead of
*cookie = page;
plus
.put_link = page_put_link,
we have right now, etc.
The reason not to go that way (and not all that strong, at that) is that
we get an extra pointer in struct saved, i.e. slightly heavier nameidata
(there are two struct saved embedded into it). That, or play silly buggers
with
union {
struct callback_head callback;
struct inode *inode;
};
in struct saved - we need to stash a pointer to inode between pick_link()
and get_link().
Hell knows, still might make sense to go there... What we want is to set
a closure to be evaluated once the caller of ->get_link() is done with
the string ->get_link() returns. We end up picking the function to be
called at saved->inode->i_op->put_link and store its argument in
saved->cookie. Might as well just use the representation we use for such
stuff (rcu_head, etc.) and be done with that...
next prev parent reply other threads:[~2015-12-10 0:10 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-11-17 22:57 [PATCHSET] ->follow_link() without dropping from RCU mode Al Viro
2015-11-17 23:00 ` [PATCH 01/10] switch befs long symlinks to page_symlink_operations Al Viro
2015-11-17 23:00 ` [PATCH 02/10] logfs: don't duplicate page_symlink_inode_operations Al Viro
2015-11-17 23:00 ` [PATCH 03/10] udf: " Al Viro
2015-11-17 23:00 ` [PATCH 04/10] ufs: get rid of ->setattr() for symlinks Al Viro
2015-11-17 23:00 ` [PATCH 05/10] namei: page_getlink() and page_follow_link_light() are the same thing Al Viro
2015-11-17 23:00 ` [PATCH 06/10] [vfs] don't put symlink bodies in pagecache into highmem Al Viro
2015-11-19 23:02 ` Dave Chinner
2015-11-17 23:00 ` [PATCH 07/10] [vfs] replace ->follow_link() with new method that could stay in RCU mode Al Viro
2015-11-17 23:00 ` [PATCH 08/10] teach page_get_link() to work " Al Viro
2015-11-17 23:00 ` [PATCH 09/10] teach shmem_get_link() " Al Viro
2015-11-17 23:00 ` [PATCH 10/10] teach proc_self_get_link()/proc_thread_self_get_link() " Al Viro
2015-12-09 5:32 ` [PATCHSET v2] ->follow_link() without dropping from " Al Viro
2015-12-09 5:34 ` [PATCH v2 01/11] switch befs long symlinks to page_symlink_operations Al Viro
2015-12-09 5:34 ` [PATCH v2 02/11] logfs: don't duplicate page_symlink_inode_operations Al Viro
2015-12-09 5:34 ` [PATCH v2 03/11] udf: " Al Viro
2015-12-09 5:34 ` [PATCH v2 04/11] ufs: get rid of ->setattr() for symlinks Al Viro
2015-12-09 5:34 ` [PATCH v2 05/11] namei: page_getlink() and page_follow_link_light() are the same thing Al Viro
2015-12-09 5:34 ` [PATCH v2 06/11] don't put symlink bodies in pagecache into highmem Al Viro
2016-01-14 13:22 ` Tomeu Vizoso
2016-01-14 15:25 ` Al Viro
2016-01-14 15:58 ` Tomeu Vizoso
2016-01-14 16:23 ` Al Viro
2016-01-14 16:57 ` Tomeu Vizoso
2016-01-14 17:13 ` Al Viro
2016-01-14 19:15 ` Tomeu Vizoso
2016-01-14 21:02 ` Al Viro
2016-01-14 21:40 ` Linus Torvalds
2016-01-14 22:25 ` Al Viro
2016-01-14 23:33 ` Al Viro
2016-01-14 23:58 ` Linus Torvalds
2016-01-15 0:05 ` Al Viro
2015-12-09 5:34 ` [PATCH v2 07/11] replace ->follow_link() with new method that could stay in RCU mode Al Viro
2015-12-09 5:34 ` [PATCH v2 08/11] teach page_get_link() to work " Al Viro
2015-12-09 5:34 ` [PATCH v2 09/11] teach shmem_get_link() " Al Viro
2015-12-09 5:34 ` [PATCH v2 10/11] teach proc_self_get_link()/proc_thread_self_get_link() " Al Viro
2015-12-09 5:34 ` [PATCH v2 11/11] teach nfs_get_link() " Al Viro
2015-12-09 17:24 ` [PATCHSET v2] ->follow_link() without dropping from " Linus Torvalds
2015-12-09 18:23 ` Al Viro
2015-12-10 0:10 ` Al Viro [this message]
2015-12-10 2:40 ` Al Viro
2015-12-11 1:54 ` Al Viro
2015-12-11 7:49 ` Rasmus Villemoes
2015-12-11 23:16 ` Al Viro
2015-12-12 2:00 ` Al Viro
2015-12-13 18:43 ` Rasmus Villemoes
2015-12-13 3:47 ` Al Viro
2015-12-09 21:57 ` NeilBrown
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=20151210001012.GA20997@ZenIV.linux.org.uk \
--to=viro@zeniv.linux.org.uk \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=neilb@suse.com \
--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).