Date: Mon, 19 Jul 2010 22:27:01 GMT From: Gleb Kurtsou <gk@FreeBSD.org> To: Perforce Change Reviews <perforce@FreeBSD.org> Subject: PERFORCE change 181205 for review Message-ID: <201007192227.o6JMR1Yt023292@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://p4web.freebsd.org/@@181205?ac=10 Change 181205 by gk@gk_h1 on 2010/07/19 22:26:26 Cleanup dircache_ref refactoring Merge dircache_add and dicache_addnegative funcs Fix integer overflows in *_cmp() Affected files ... .. //depot/projects/soc2010/gk_namecache/sys/kern/subr_witness.c#3 edit .. //depot/projects/soc2010/gk_namecache/sys/kern/vfs_dircache.c#7 edit .. //depot/projects/soc2010/gk_namecache/sys/sys/dircache.h#7 edit Differences ... ==== //depot/projects/soc2010/gk_namecache/sys/kern/subr_witness.c#3 (text+ko) ==== @@ -614,16 +614,12 @@ { "vnode interlock", &lock_class_mtx_sleep }, { NULL, NULL }, /* - * dircache pool locks/vnode interlock + * dircache locks */ - { "dircache lock 0", &lock_class_mtx_sleep }, - { "dircache lock 1", &lock_class_mtx_sleep }, - { "dircache lock 2", &lock_class_mtx_sleep }, - { "dircache lock 3", &lock_class_mtx_sleep }, - { "dircache lock 4", &lock_class_mtx_sleep }, - { "dircache lock 5", &lock_class_mtx_sleep }, - { "dircache lock 6", &lock_class_mtx_sleep }, - { "dircache lock 7", &lock_class_mtx_sleep }, + { "dircache ref", &lock_class_mtx_sleep }, + { "dircache entry", &lock_class_mtx_sleep }, + { "dircache mount", &lock_class_mtx_sleep }, + { "dircache pool", &lock_class_mtx_sleep }, { "vnode interlock", &lock_class_mtx_sleep }, { NULL, NULL }, /* ==== //depot/projects/soc2010/gk_namecache/sys/kern/vfs_dircache.c#7 (text+ko) ==== @@ -80,9 +80,10 @@ struct dircache_mount { struct mtx dm_mtx; - LIST_HEAD(, dircache_ref) dm_inohead; + struct dircache_inotree dm_inohead; struct dircache_ref *dm_rootref; struct dircache_ref *dm_negativeref; + u_long dm_inocnt; }; static struct dircache * dc_use(struct dircache *dc); @@ -93,6 +94,8 @@ static void dp_unused_lazyclear(void); static void dp_invalid_lazyclear(void); +static int ds_mountstats(SYSCTL_HANDLER_ARGS); + static int dircache_debug = 1; static struct dircache_pool pool; static struct taskqueue *dc_tq; @@ -120,23 +123,24 @@ SYSCTL_ULONG(_vfs_dircache_stats, OID_AUTO, unused, CTLFLAG_RD, &pool.dp_unused_cnt, 0, "Unused entries"); +SYSCTL_PROC(_vfs_dircache_stats, OID_AUTO, inoderefs, + CTLFLAG_RD | CTLTYPE_ULONG | CTLFLAG_MPSAFE, NULL, + __offsetof(struct dircache_mount, dm_inocnt), ds_mountstats, "LU", ""); + enum { ds_hit, ds_hit_negative, ds_miss, ds_add, ds_add_error, + ds_add_negative, ds_remove, ds_remove_error, ds_rename, ds_rename_realloc, ds_rename_error, - ds_setnegative, - ds_setnegative_error, - ds_setvnode, - ds_setvnode_hit, - ds_setvnode_error, ds_allocvnode, + ds_allocvnode_inode_hit, ds_reclaimvnode, ds_alloc, ds_free, @@ -173,17 +177,14 @@ DC_STAT_DEFINE(miss, ""); DC_STAT_DEFINE(add, ""); DC_STAT_DEFINE(add_error, ""); +DC_STAT_DEFINE(add_negative, ""); DC_STAT_DEFINE(remove, ""); DC_STAT_DEFINE(remove_error, ""); DC_STAT_DEFINE(rename, ""); DC_STAT_DEFINE(rename_realloc, ""); DC_STAT_DEFINE(rename_error, ""); -DC_STAT_DEFINE(setnegative, ""); -DC_STAT_DEFINE(setnegative_error, ""); -DC_STAT_DEFINE(setvnode, ""); -DC_STAT_DEFINE(setvnode_hit, ""); -DC_STAT_DEFINE(setvnode_error, ""); DC_STAT_DEFINE(allocvnode, ""); +DC_STAT_DEFINE(allocvnode_inode_hit, ""); DC_STAT_DEFINE(reclaimvnode, ""); DC_STAT_DEFINE(alloc, ""); DC_STAT_DEFINE(free, ""); @@ -246,16 +247,52 @@ } SYSUNINIT(dircache, SI_SUB_VFS, SI_ORDER_SECOND, dircache_sysuninit, NULL); +static int +ds_mountstats(SYSCTL_HANDLER_ARGS) +{ + struct mount *mp; + char *ptr; + u_long res; + + if (arg2 < 0 || arg2 + sizeof(u_long) > sizeof(struct dircache_mount)) + return (EINVAL); + + res = 0; + mtx_lock(&mountlist_mtx); + TAILQ_FOREACH(mp, &mountlist, mnt_list) { + ptr = (char *)mp->mnt_dircache; + if (ptr == NULL) + continue; + + res += *(u_long *)(ptr + arg2); + } + mtx_unlock(&mountlist_mtx); + + return (SYSCTL_OUT(req, &res, sizeof(res))); +} + static __inline struct dircache_mount * dm_get(struct vnode *vp) { return (vp->v_mount->mnt_dircache); } +static __inline int +dr_cmp(struct dircache_ref *a, struct dircache_ref *b) +{ + if (a->dr_ino > b->dr_ino) + return (1); + else if (a->dr_ino < b->dr_ino) + return (-1); + return (0); +} + +RB_GENERATE_STATIC(dircache_inotree, dircache_ref, dr_inotree, dr_cmp); + static struct dircache_ref * dr_alloc(struct dircache_mount *dm, struct vnode *vp, ino_t ino) { - struct dircache_ref *dr; + struct dircache_ref *dr, *col; dr = uma_zalloc(dircache_ref_zone, M_WAITOK | M_ZERO); mtx_init(&dr->dr_mtx, "dircache ref", NULL, MTX_DEF); @@ -267,7 +304,9 @@ MPASS(dm_get(vp) == dr->dr_mount); if (ino != 0) { dm_lock(dm); - LIST_INSERT_HEAD(&dm->dm_inohead, dr, dr_inolist); + col = RB_INSERT(dircache_inotree, &dm->dm_inohead, dr); + dm->dm_inocnt++; + MPASS(col == NULL); dm_unlock(dm); } @@ -285,7 +324,8 @@ dr != dm->dm_rootref && dr != dm->dm_negativeref) { dr_unlock(dr); dm_lock(dm); - LIST_REMOVE(dr, dr_inolist); + RB_REMOVE(dircache_inotree, &dm->dm_inohead, dr); + dm->dm_inocnt--; dm_unlock(dm); uma_zfree(dircache_ref_zone, dr); } else @@ -350,7 +390,7 @@ r = dc->dc_namelen - namelen; if (r != 0) return (r); - r = bcmp(dc->dc_name, name, namelen); + r = memcmp(dc->dc_name, name, namelen); return (r); } @@ -365,7 +405,7 @@ r = a->dc_namelen - b->dc_namelen; if (r != 0) return (r); - r = bcmp(a->dc_name, b->dc_name, a->dc_namelen); + r = memcmp(a->dc_name, b->dc_name, a->dc_namelen); return (r); } @@ -376,7 +416,7 @@ { dc->dc_name = name; dc->dc_namelen = namelen; - dc->dc_namehash = hash32_buf(name, namelen, HASHINIT * namelen); + dc->dc_namehash = hash32_buf(name, namelen, HASHINIT * namelen) >> 1; } static __inline u_int @@ -432,8 +472,7 @@ } static struct dircache * -dc_alloc(struct dircache_ref *dr, enum dircache_type type, - char *name, u_int namelen) +dc_alloc(enum dircache_type type, char *name, u_int namelen) { struct dircache *dc; @@ -447,13 +486,6 @@ if (name != NULL && namelen != 0) dc_setname(dc, name, namelen, NULL); - dr_lock(dr); - dc_lock(dc); - dp_unused_insert(dc); - dr_add(dr, dc); - dc_unlock(dc); - dr_unlock(dr); - DC_STAT_INC(ds_alloc); return (dc); } @@ -646,6 +678,7 @@ } dc_unlock(dc->dc_parent); dc_unlock(dc); + dr_lock(dr); } if (dc == NULL) { dr_unlock(dr); @@ -842,7 +875,8 @@ { struct dircache *col; - DCDEBUG("insert: parent=%p name=%s\n", pdc, pdc->dc_name); + DCDEBUG("insert: parent=%p name=%s dc=%p\n", + pdc, pdc->dc_name, dc); restart: dc_assertlock(dc, MA_OWNED); @@ -853,8 +887,13 @@ if (dc->dc_type == col->dc_type) { DCDEBUG("insert: warn: same entry added: %s\n", dc->dc_name); - MPASS(col->dc_ref == dc->dc_ref); + /* TODO + KASSERT(col->dc_ref == dr, + ("dircache: entry already exists: %s %p %p\n", + dc->dc_name, col->dc_ref, dc->dc_ref)); + */ dc_unlock(pdc); + dc_unlock(dc); dc_drop(dc); return (NULL); } else if (col->dc_type == DT_NEGATIVE) { @@ -882,9 +921,12 @@ dc->dc_type, dc->dc_name); } else { dc->dc_parent = pdc; + dp_unused_insert(dc); dc_use(pdc); dc_hold(dc); dc_unlock(pdc); + dc_updategen(dc); + dc_unlock(dc); } return (dc); } @@ -931,7 +973,7 @@ DC_STAT_INC(ds_clearunused); dc = TAILQ_FIRST(&pool.dp_unused); while (dc != NULL && pool.dp_unused_cnt > pool.dp_unused_max) { - if (dc_trylock(dc) != 0) { + if (dc_trylock(dc) == 0) { dc = TAILQ_NEXT(dc, dc_list); shift++; if (shift >= pool.dp_unused_threshold) @@ -940,7 +982,7 @@ continue; } if (dc->dc_parent != NULL) { - if (dc_trylock(dc->dc_parent) != 0) { + if (dc_trylock(dc->dc_parent) == 0) { dc_unlock(dc); dc = TAILQ_NEXT(dc, dc_list); shift++; @@ -986,7 +1028,7 @@ if (dc_dropsafe(dc) == 0) { dc_assertlock(dc, MA_OWNED); dc_hold(dc); - MPASS(dc->dc_ref->dr_vnode == NULL); + MPASS(dc->dc_ref == NULL); if (dc->dc_parent != NULL) { dc_lock(dc->dc_parent); dc_removeentry(dc); @@ -1030,12 +1072,20 @@ dm = malloc(sizeof(struct dircache_mount), M_DIRCACHE, M_WAITOK | M_ZERO); mtx_init(&dm->dm_mtx, "dircache mount", NULL, MTX_DEF); - LIST_INIT(&dm->dm_inohead); + RB_INIT(&dm->dm_inohead); dm->dm_negativeref = dr_alloc(dm, NULL, 0); dm->dm_rootref = dr_alloc(dm, NULL, inode); MPASS(mp->mnt_dircache == NULL); - dc = dc_alloc(dm->dm_rootref, DT_ROOT, NULL, 0); + dc = dc_alloc(DT_ROOT, NULL, 0); + + dr_lock(dm->dm_rootref); + dc_lock(dc); + dp_unused_insert(dc); + dr_add(dm->dm_rootref, dc); + dc_use(dc); + dc_unlock(dc); + dr_unlock(dm->dm_rootref); MNT_ILOCK(mp); mp->mnt_dircache = dm; @@ -1119,7 +1169,14 @@ dp_invalid_clear(); mtx_unlock(&pool.dp_mtx); - MPASS(LIST_EMPTY(&dm->dm_inohead)); + dm_lock(dm); + RB_REMOVE(dircache_inotree, &dm->dm_inohead, dm->dm_rootref); + dm->dm_inocnt--; + dm_unlock(dm); + uma_zfree(dircache_ref_zone, dm->dm_rootref); + uma_zfree(dircache_ref_zone, dm->dm_negativeref); + + MPASS(RB_EMPTY(&dm->dm_inohead)); free(dm, M_DIRCACHE); } @@ -1289,52 +1346,37 @@ { struct dircache *pdc; struct dircache *ndc; + struct dircache_ref *dr; + + MPASS(type == DT_STRONG || type == DT_WEAK || type == DT_NEGATIVE); - MPASS(type == DT_STRONG || type == DT_WEAK); - MPASS(vp->v_dircache != NULL); + ndc = dc_alloc(type, cnp->cn_nameptr, cnp->cn_namelen); DCDEBUG("add: %s; vp=%p\n", cnp->cn_nameptr, vp); - ndc = dc_alloc(vp->v_dircache, type, - cnp->cn_nameptr, cnp->cn_namelen); - dc_lock(ndc); pdc = dc_getentry(dvp, NULL, NULL); if (pdc == NULL) { dc_drop(ndc); DC_STAT_INC(ds_add_error); return (ENOENT); } + dc_lock(ndc); ndc = dc_insertentry(pdc, ndc); if (ndc != NULL) { - dc_updategen(ndc); + if (type == DT_NEGATIVE) { + MPASS(vp == NULL); + dr = dm_get(dvp)->dm_negativeref; + dr_lock(dr); + } else { + dr = dr_get(vp); + } + MPASS(dr != NULL); + dc_lock(ndc); + dr_add(dr, ndc); dc_unlock(ndc); - } - DC_STAT_INC(ds_add); - return (0); -} + dr_unlock(dr); -int -dircache_addnegative(struct vnode *dvp, struct componentname *cnp) -{ - struct dircache *pdc; - struct dircache *ndc; - - if (cnp->cn_nameptr[0] == '.' && (cnp->cn_namelen == 1 || - (cnp->cn_namelen == 2 && cnp->cn_nameptr[1] == '.'))) - panic("dircache: set negative for '.' or '..'"); - - ndc = dc_alloc(dm_get(dvp)->dm_negativeref, DT_NEGATIVE, - cnp->cn_nameptr, cnp->cn_namelen); - dc_lock(ndc); - pdc = dc_getentry(dvp, NULL, NULL); - if (pdc == NULL) { - dc_drop(ndc); - DC_STAT_INC(ds_setnegative_error); - return (ENOENT); } - ndc = dc_insertentry(pdc, ndc); - if (ndc != NULL) - dc_unlock(ndc); - DC_STAT_INC(ds_setnegative); + DC_STAT_INC(type == DT_NEGATIVE ? ds_add_negative : ds_add); return (0); } @@ -1467,22 +1509,23 @@ { struct dircache *dc; struct dircache_mount *dm; - struct dircache_ref *dr; + struct dircache_ref *dr, key; MPASS(vp->v_type != VNON && vp->v_type != VBAD); MPASS(vp->v_dircache == NULL); + MPASS(ino != 0); dm = dm_get(vp); dm_lock(dm); - LIST_FOREACH(dr, &dm->dm_inohead, dr_inolist) { - if (dr->dr_ino == ino) - break; - } + key.dr_ino = ino; + dr = RB_FIND(dircache_inotree, &dm->dm_inohead, &key); dm_unlock(dm); if (dr == NULL) dr = dr_alloc(dm_get(vp), vp, ino); + else + DC_STAT_INC(ds_allocvnode_inode_hit); DCDEBUG("alloc vnode: vp=%p; ino=%d; dr=%p\n", vp, ino, dr); ==== //depot/projects/soc2010/gk_namecache/sys/sys/dircache.h#7 (text+ko) ==== @@ -71,12 +71,14 @@ struct dircache_ref { struct mtx dr_mtx; LIST_HEAD(, dircache) dr_entries; - LIST_ENTRY(dircache_ref) dr_inolist; + RB_ENTRY(dircache_ref) dr_inotree; struct vnode *dr_vnode; struct dircache_mount *dr_mount; ino_t dr_ino; }; +RB_HEAD(dircache_inotree, dircache_ref); + void dircache_init(struct mount *mp, ino_t inode); void dircache_uninit(struct mount *mp); void dircache_purge_negative(struct vnode *dvp); @@ -85,7 +87,6 @@ struct componentname *cnp); int dircache_add(struct vnode *dvp, struct vnode *vp, struct componentname *cnp, enum dircache_type type); -int dircache_addnegative(struct vnode *dvp, struct componentname *cnp); int dircache_remove(struct vnode *dvp, struct vnode *vp, struct componentname *cnp); int dircache_rename(struct vnode *fdvp, struct componentname *fcnp, @@ -105,7 +106,7 @@ if (vp != NULL) error = dircache_add(dvp, vp, cnp, DT_STRONG); else - error = dircache_addnegative(dvp, cnp); + error = dircache_add(dvp, NULL, cnp, DT_NEGATIVE); return (error); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201007192227.o6JMR1Yt023292>