Messages in this thread | | | Date | Sat, 8 May 2004 12:01:48 -0700 | From | Andrew Morton <> | Subject | Re: dentry bloat. |
| |
Linus Torvalds <torvalds@osdl.org> 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |