From 4e94fafc4fb53b5bbdda750a2d85cb058a6b8de2 Mon Sep 17 00:00:00 2001 From: Poul-Henning Kamp Date: Mon, 14 Mar 2005 12:29:39 +0000 Subject: [PATCH] Use vfs_hash instead of home-rolled. Correct locking around g_vfs_close() --- sys/fs/udf/udf.h | 6 -- sys/fs/udf/udf_vfsops.c | 128 ++++++++++++++++++++++------------------ sys/fs/udf/udf_vnops.c | 74 +---------------------- 3 files changed, 74 insertions(+), 134 deletions(-) diff --git a/sys/fs/udf/udf.h b/sys/fs/udf/udf.h index 820a4868777..1437e3b5b4b 100644 --- a/sys/fs/udf/udf.h +++ b/sys/fs/udf/udf.h @@ -53,9 +53,6 @@ struct udf_mnt { uint32_t part_len; uint64_t root_id; struct long_ad root_icb; - LIST_HEAD(udf_hash_lh, udf_node) *hashtbl; - u_long hashsz; - struct mtx hash_mtx; int p_sectors; int s_table_entries; struct udf_sparing_table *s_table; @@ -130,9 +127,6 @@ udf_getid(struct long_ad *icb) } int udf_allocv(struct mount *, struct vnode **, struct thread *); -int udf_hashlookup(struct udf_mnt *, ino_t, int, struct vnode **); -int udf_hashins(struct udf_node *); -int udf_hashrem(struct udf_node *); int udf_checktag(struct desc_tag *, uint16_t); int udf_vget(struct mount *, ino_t, int, struct vnode **); diff --git a/sys/fs/udf/udf_vfsops.c b/sys/fs/udf/udf_vfsops.c index 6733b781588..6a9f9e71c6d 100644 --- a/sys/fs/udf/udf_vfsops.c +++ b/sys/fs/udf/udf_vfsops.c @@ -474,9 +474,6 @@ udf_mountfs(struct vnode *devvp, struct mount *mp, struct thread *td) { brelse(bp); bp = NULL; - mtx_init(&udfmp->hash_mtx, "udf_hash", NULL, MTX_DEF); - udfmp->hashtbl = phashinit(UDF_HASHTBLSIZE, M_UDFMOUNT, &udfmp->hashsz); - return 0; bail: @@ -515,15 +512,16 @@ udf_unmount(struct mount *mp, int mntflags, struct thread *td) #endif } + DROP_GIANT(); + g_topology_lock(); g_vfs_close(udfmp->im_cp, td); + g_topology_unlock(); + PICKUP_GIANT(); vrele(udfmp->im_devvp); if (udfmp->s_table != NULL) FREE(udfmp->s_table, M_UDFMOUNT); - if (udfmp->hashtbl != NULL) - FREE(udfmp->hashtbl, M_UDFMOUNT); - FREE(udfmp, M_UDFMOUNT); mp->mnt_data = (qaddr_t)0; @@ -583,58 +581,16 @@ udf_vget(struct mount *mp, ino_t ino, int flags, struct vnode **vpp) struct file_entry *fe; int error, sector, size; + error = vfs_hash_get(mp, ino, flags, curthread, vpp); + if (error) + return (error); + if (*vpp != NULL) + return (0); + td = curthread; udfmp = VFSTOUDFFS(mp); - /* See if we already have this in the cache */ - if ((error = udf_hashlookup(udfmp, ino, flags, vpp)) != 0) - return (error); - if (*vpp != NULL) { - return (0); - } - - /* - * Allocate memory and check the tag id's before grabbing a new - * vnode, since it's hard to roll back if there is a problem. - */ - unode = uma_zalloc(udf_zone_node, M_WAITOK); - if (unode == NULL) { - printf("Cannot allocate udf node\n"); - return (ENOMEM); - } - - /* - * Copy in the file entry. Per the spec, the size can only be 1 block. - */ - sector = ino + udfmp->part_start; - devvp = udfmp->im_devvp; - if ((error = RDSECTOR(devvp, sector, udfmp->bsize, &bp)) != 0) { - printf("Cannot read sector %d\n", sector); - uma_zfree(udf_zone_node, unode); - return (error); - } - - fe = (struct file_entry *)bp->b_data; - if (udf_checktag(&fe->tag, TAGID_FENTRY)) { - printf("Invalid file entry!\n"); - uma_zfree(udf_zone_node, unode); - brelse(bp); - return (ENOMEM); - } - size = UDF_FENTRY_SIZE + le32toh(fe->l_ea) + le32toh(fe->l_ad); - MALLOC(unode->fentry, struct file_entry *, size, M_UDFFENTRY, - M_NOWAIT | M_ZERO); - if (unode->fentry == NULL) { - printf("Cannot allocate file entry block\n"); - uma_zfree(udf_zone_node, unode); - brelse(bp); - return (ENOMEM); - } - - bcopy(bp->b_data, unode->fentry, size); - - brelse(bp); - bp = NULL; + unode = uma_zalloc(udf_zone_node, M_WAITOK | M_ZERO); if ((error = udf_allocv(mp, &vp, td))) { printf("Error from udf_allocv\n"); @@ -648,8 +604,68 @@ udf_vget(struct mount *mp, ino_t ino, int flags, struct vnode **vpp) unode->i_dev = udfmp->im_dev; unode->udfmp = udfmp; vp->v_data = unode; + + /* + * Exclusively lock the vnode before adding to hash. Note, that we + * must not release nor downgrade the lock (despite flags argument + * says) till it is fully initialized. + */ + lockmgr(vp->v_vnlock, LK_EXCLUSIVE, (struct mtx *)0, td); + + /* + * Atomicaly (in terms of vfs_hash operations) check the hash for + * duplicate of vnode being created and add it to the hash. If a + * duplicate vnode was found, it will be vget()ed from hash for us. + */ + if ((error = vfs_hash_insert(vp, ino, flags, curthread, vpp)) != 0) { + vput(vp); + *vpp = NULL; + return (error); + } + + /* We lost the race, then throw away our vnode and return existing */ + if (*vpp != NULL) { + vput(vp); + return (0); + } + + /* + * Copy in the file entry. Per the spec, the size can only be 1 block. + */ + sector = ino + udfmp->part_start; + devvp = udfmp->im_devvp; + if ((error = RDSECTOR(devvp, sector, udfmp->bsize, &bp)) != 0) { + printf("Cannot read sector %d\n", sector); + vput(vp); + brelse(bp); + *vpp = NULL; + return (error); + } + + fe = (struct file_entry *)bp->b_data; + if (udf_checktag(&fe->tag, TAGID_FENTRY)) { + printf("Invalid file entry!\n"); + vput(vp); + brelse(bp); + *vpp = NULL; + return (ENOMEM); + } + size = UDF_FENTRY_SIZE + le32toh(fe->l_ea) + le32toh(fe->l_ad); + MALLOC(unode->fentry, struct file_entry *, size, M_UDFFENTRY, + M_NOWAIT | M_ZERO); + if (unode->fentry == NULL) { + printf("Cannot allocate file entry block\n"); + vput(vp); + brelse(bp); + *vpp = NULL; + return (ENOMEM); + } + VREF(udfmp->im_devvp); - udf_hashins(unode); + bcopy(bp->b_data, unode->fentry, size); + + brelse(bp); + bp = NULL; switch (unode->fentry->icbtag.file_type) { default: diff --git a/sys/fs/udf/udf_vnops.c b/sys/fs/udf/udf_vnops.c index d702629a8b2..7a99b1eee45 100644 --- a/sys/fs/udf/udf_vnops.c +++ b/sys/fs/udf/udf_vnops.c @@ -36,6 +36,7 @@ #include #include #include +#include #include #include #include @@ -92,77 +93,6 @@ MALLOC_DEFINE(M_UDFDS, "UDF DS", "UDF Dirstream structure"); #define UDF_INVALID_BMAP -1 -/* Look up a udf_node based on the ino_t passed in and return it's vnode */ -int -udf_hashlookup(struct udf_mnt *udfmp, ino_t id, int flags, struct vnode **vpp) -{ - struct udf_node *node; - struct udf_hash_lh *lh; - int error; - - *vpp = NULL; - -loop: - mtx_lock(&udfmp->hash_mtx); - lh = &udfmp->hashtbl[id % udfmp->hashsz]; - if (lh == NULL) - return (ENOENT); - LIST_FOREACH(node, lh, le) { - if (node->hash_id == id) { - VI_LOCK(node->i_vnode); - mtx_unlock(&udfmp->hash_mtx); - error = vget(node->i_vnode, flags | LK_INTERLOCK, - curthread); - if (error == ENOENT) - goto loop; - if (error) - return (error); - *vpp = node->i_vnode; - return (0); - } - } - - mtx_unlock(&udfmp->hash_mtx); - return (0); -} - -int -udf_hashins(struct udf_node *node) -{ - struct udf_mnt *udfmp; - struct udf_hash_lh *lh; - - udfmp = node->udfmp; - - vn_lock(node->i_vnode, LK_EXCLUSIVE | LK_RETRY, curthread); - mtx_lock(&udfmp->hash_mtx); - lh = &udfmp->hashtbl[node->hash_id % udfmp->hashsz]; - if (lh == NULL) - LIST_INIT(lh); - LIST_INSERT_HEAD(lh, node, le); - mtx_unlock(&udfmp->hash_mtx); - - return (0); -} - -int -udf_hashrem(struct udf_node *node) -{ - struct udf_mnt *udfmp; - struct udf_hash_lh *lh; - - udfmp = node->udfmp; - - mtx_lock(&udfmp->hash_mtx); - lh = &udfmp->hashtbl[node->hash_id % udfmp->hashsz]; - if (lh == NULL) - panic("hash entry is NULL, node->hash_id= %d\n", node->hash_id); - LIST_REMOVE(node, le); - mtx_unlock(&udfmp->hash_mtx); - - return (0); -} - int udf_allocv(struct mount *mp, struct vnode **vpp, struct thread *td) { @@ -1037,7 +967,7 @@ udf_reclaim(struct vop_reclaim_args *a) unode = VTON(vp); if (unode != NULL) { - udf_hashrem(unode); + vfs_hash_remove(vp); if (unode->i_devvp) { vrele(unode->i_devvp); unode->i_devvp = 0;