From: Al Viro <viro@ZenIV.linux.org.uk>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mikhail Efremov <sem@altlinux.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Miklos Szeredi <mszeredi@suse.cz>,
linux-fsdevel <linux-fsdevel@vger.kernel.org>,
stable <stable@vger.kernel.org>
Subject: Re: [PATCH v2] vfs: Don't exchange "short" filenames unconditionally.
Date: Sat, 27 Sep 2014 20:45:58 +0100 [thread overview]
Message-ID: <20140927194558.GV7996@ZenIV.linux.org.uk> (raw)
In-Reply-To: <20140927191657.GU7996@ZenIV.linux.org.uk>
On Sat, Sep 27, 2014 at 08:16:57PM +0100, Al Viro wrote:
> On Sat, Sep 27, 2014 at 07:31:39PM +0100, Al Viro wrote:
>
> > We can get the long name cases right, and I agree that it'll make the
> > things nicer, but it might take a couple of days to get right. The thing
> > I'm concerned about is not screwing DCACHE_RCUACCESS up.
>
> FWIW, I suspect that the right approach is to put refcount + rcu_head in
> front of external name and do the following:
> * __d_free() checks if we have an external name, gets its containing
> structure and does if (atomic_dec_and_test(&name->count)) kfree(name);
> * switch_names() in non-exchange case (I'd probably call it copy_name,
> not move_names, but anyway) sets DCACHE_RCUACCESS on target (source has
> already gotten it from __d_rehash()), increments refcount on target's name
> if external and, if the source old name is external, decrements its refcount
> and calls kfree_rcu() if it has hit zero.
>
> AFAICS, it guarantees that we'll schedule an RCU callback on name's rch_head
> at most once, that we won't free it while RCU callback on it is scheduled
> and we won't free it until a grace period has expired since the last time
> it had been referenced by observable dentries. Do you see any holes in that?
We probably want to put a union of refcount and rcu_head there, actually...
Gives the right alignment without padding. As in
struct ext_name {
union {
atomic_t count;
struct rcu_head head;
};
char name[0];
};
->count corresponds to the number of dentries that have ->d_name.name
pointing to the sucker's ->name. And we use ->head only when it reaches
zero in __d_move(). That's 2 words per external name; somewhat unpleasant
on 64bit, but I don't see how to avoid an rcu_head in there... The cutoff
for external names is 32 bytes on 64bit boxen. That way we get 16 bytes
of overhead per long-named dentry... OTOH, we allocate them with kmalloc(),
so it means that 32-character names lead to 64-bytes actual allocation.
Hmmm... So the old behaviour is
32--63 => 64 byte allocation
64--95 => 96
96--127 => 128
and the new one
32--47 => 64 byte allocation
48--79 => 96
80--111 => 128
112--127 => 192
(components longer than 128 characters are definitely too rare to worry about)
IOW, the main worry is about the names in range from 48 to 64 characters;
for those we push the allocation from size-64 to size-96...
Note, BTW, that git hits external name case on everything except 32-bit UP;
a _lot_ of 38-character names there. And IIRC there had been some plans for
possible replacement of SHA1 with something wider, right?
next prev parent reply other threads:[~2014-09-27 19:46 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-09-24 18:14 [PATCH v2] vfs: Don't exchange "short" filenames unconditionally Mikhail Efremov
2014-09-24 18:55 ` Al Viro
2014-09-24 19:20 ` Linus Torvalds
2014-09-24 19:27 ` Linus Torvalds
2014-09-24 20:18 ` Al Viro
2014-09-25 4:46 ` Al Viro
2014-09-26 16:44 ` Al Viro
2014-09-27 4:45 ` Al Viro
2014-09-27 17:56 ` Linus Torvalds
2014-09-27 18:31 ` Al Viro
2014-09-27 19:16 ` Al Viro
2014-09-27 19:37 ` Linus Torvalds
2014-09-27 19:39 ` Linus Torvalds
2014-09-27 19:49 ` Al Viro
2014-09-27 19:55 ` Linus Torvalds
2014-09-27 21:48 ` [git pull] vfs.git for 3.17-rc7 Al Viro
2014-09-27 19:45 ` Al Viro [this message]
2014-09-28 7:47 ` [PATCH v2] vfs: Don't exchange "short" filenames unconditionally Al Viro
2014-09-28 18:05 ` Al Viro
2014-09-28 21:51 ` Al Viro
2014-09-29 1:06 ` [PATCH] missing data dependency barrier in prepend_name() Al Viro
2014-09-29 15:15 ` [PATCH v2] vfs: Don't exchange "short" filenames unconditionally Linus Torvalds
2014-09-29 15:59 ` Al Viro
2014-09-29 16:07 ` Linus Torvalds
2014-09-29 16:27 ` Al Viro
2014-09-29 17:54 ` Linus Torvalds
2014-09-29 19:04 ` Al Viro
2014-09-29 20:45 ` Linus Torvalds
2014-09-29 18:42 ` Paul E. McKenney
2014-10-01 0:16 ` Al Viro
2014-10-02 5:38 ` Paul E. McKenney
2014-10-02 10:35 ` Chuck Ebbert
2014-10-03 2:11 ` Paul E. McKenney
2014-09-29 13:16 ` Paul E. McKenney
2014-09-29 15:04 ` Al Viro
2014-09-28 15:01 ` Mikhail Efremov
2014-09-26 20:23 ` 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=20140927194558.GV7996@ZenIV.linux.org.uk \
--to=viro@zeniv.linux.org.uk \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mszeredi@suse.cz \
--cc=sem@altlinux.org \
--cc=stable@vger.kernel.org \
--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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.