Re: dentry bloat.

From: Andrew Morton
Date: Sat May 08 2004 - 14:03:44 EST


Linus Torvalds <torvalds@xxxxxxxx> wrote:
>
>
>
> On Sat, 8 May 2004, Andrew Morton wrote:
> > */
> > struct qstr {
> > const unsigned char *name;
> > - unsigned int len;
> > unsigned int hash;
> > -};
> > + unsigned short len;
> > +} __attribute__((packed));
>
> This would make me nervous.

yeah, that was just mucking about.

> Also, in your previous patch (which I'm not as convinced might be wrong),
> the d_qstr pointer removal makes me worry:
>
> - struct qstr * d_qstr; /* quick str ptr used in lockless lookup and concurrent d_move */
>
> I thought the point of d_qstr was that when we do the lockless lookup,
> we're guaranteed to always see "stable storage" in the sense that when we
> follow the d_qstr, we will always get a "char *" + "len" that match, and
> we could never see a partial update (ie len points to the old one, and
> "char *" points to the new one).

It looks that way.

> In particular, think about the "d_compare(parent, qstr, name)" /
> "memcmp(qstr->name, str, len)" part - what if "len" doesn't match str,
> because a concurrent d_move() is updating them, and maybe we will compare
> past the end of kernel mapped memory or something?
>
> (In other words, the "move_count" check should protect us from returning a
> wrong dentry, but I'd worry that we'd do something that could cause
> serious problems before we even get to the "move_count" check).
>
> Hmm?
>
> Btw, I'd love to be proven wrong, since I hate that d_qstr, and I think
> your d_qstr removal patch otherwise looked wonderful.

It looks very similar to 2.4 actually ;)

I think we can simply take ->d_lock a bit earlier in __d_lookup. That will
serialise against d_move(), fixing the problem which you mention, and also
makes d_movecount go away.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/