On Wed, 7 Mar 2007, Eric Dumazet wrote:
>
> OK no problem here is the patch without messing f_path.mnt
All right. I like it. Definitely worth putting into -mm, or just
re-sending to me after 2.6.21 is out (I'll forget all about it otherwise).
I have one more worry, namely this::
- char name[32];
- this.len = sprintf(name, "[%lu]", SOCK_INODE(sock)->i_ino);
- this.name = name;
+ this.len = 0;
+ this.name = NULL;
I think that's fine, and it *happens* to work, but it happens to work just
because then d_alloc() will do:
memcpy(dname, name->name, name->len);
dname[name->len] = 0;
and passing in NULL to memcpy() is generally ok when len is 0.
HOWEVER.
Not only might memcpy() do a "prefetch for read" on the source for some
architectures (which in turn may end up being slow for an address that
isn't in the TLB, like NULL), but you depend on a very much internal
detail, since it *could* have been using something like
memcpy(dname, name->name, name->len+1);
instead, and expected to get the final '\0' character from the source
string.
So I would actually much prefer it to be written as
this.len = 0;
this.name = "";
just because it's safer.
But other than that small detail, I think this is not only an
optimization, it's an actual cleanup, and we migth some day want to use
something like this for some other things too (eg maybe this kind of
approach is usable for /proc/<pid> entries too, to avoid instantiating
them).
As to avoiding the mntget(), I'm not violently opposed to it, but I do
think that it's a totally unrelated matter, so even if it's decided it's
worth it, it should be done as a separate patch regardless.
Linus
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
