From 89d10571dbd835a79c96ba9a6de5b8607c234295 Mon Sep 17 00:00:00 2001 From: Jaakko Heinonen Date: Wed, 15 Sep 2010 14:23:55 +0000 Subject: [PATCH] Remove empty devfs directories automatically. devfs_delete() now recursively removes empty parent directories unless the DEVFS_DEL_NORECURSE flag is specified. devfs_delete() can't be called anymore with a parent directory vnode lock held because the possible parent directory deletion needs to lock the vnode. Thus we unlock the parent directory vnode in devfs_remove() before calling devfs_delete(). Call devfs_populate_vp() from devfs_symlink() and devfs_vptocnp() as now directories can get removed. Add a check for DE_DOOMED flag to devfs_populate_vp() because devfs_delete() drops dm_lock before the VI_DOOMED vnode flag gets set. This ensures that devfs_populate_vp() returns an error for directories which are in progress of deletion. Reviewed by: kib Discussed on: freebsd-current (mostly silence) --- sys/fs/devfs/devfs.h | 5 ++- sys/fs/devfs/devfs_devs.c | 79 ++++++++++++++++++++++++++++++++++---- sys/fs/devfs/devfs_vnops.c | 39 +++++++++++++++---- 3 files changed, 106 insertions(+), 17 deletions(-) diff --git a/sys/fs/devfs/devfs.h b/sys/fs/devfs/devfs.h index ee5eafb31c8d..87549d222b06 100644 --- a/sys/fs/devfs/devfs.h +++ b/sys/fs/devfs/devfs.h @@ -170,12 +170,15 @@ extern unsigned devfs_rule_depth; #define DEVFS_DMP_HOLD(dmp) ((dmp)->dm_holdcnt++) #define DEVFS_DMP_DROP(dmp) (--(dmp)->dm_holdcnt == 0) +#define DEVFS_DEL_VNLOCKED 0x01 +#define DEVFS_DEL_NORECURSE 0x02 + void devfs_rules_apply(struct devfs_mount *dm, struct devfs_dirent *de); void devfs_rules_cleanup (struct devfs_mount *dm); int devfs_rules_ioctl(struct devfs_mount *dm, u_long cmd, caddr_t data, struct thread *td); int devfs_allocv(struct devfs_dirent *de, struct mount *mp, int lockmode, struct vnode **vpp); -void devfs_delete(struct devfs_mount *dm, struct devfs_dirent *de, int vp_locked); +void devfs_delete(struct devfs_mount *dm, struct devfs_dirent *de, int flags); void devfs_dirent_free(struct devfs_dirent *de); void devfs_populate (struct devfs_mount *dm); void devfs_cleanup (struct devfs_mount *dm); diff --git a/sys/fs/devfs/devfs_devs.c b/sys/fs/devfs/devfs_devs.c index a11544f80031..eaaea6876a86 100644 --- a/sys/fs/devfs/devfs_devs.c +++ b/sys/fs/devfs/devfs_devs.c @@ -274,18 +274,69 @@ devfs_dirent_free(struct devfs_dirent *de) free(de, M_DEVFS3); } +/* + * Removes a directory if it is empty. Also empty parent directories are + * removed recursively. + */ +static void +devfs_rmdir_empty(struct devfs_mount *dm, struct devfs_dirent *de) +{ + struct devfs_dirent *dd, *de_dot, *de_dotdot; + + sx_assert(&dm->dm_lock, SX_XLOCKED); + + for (;;) { + KASSERT(de->de_dirent->d_type == DT_DIR, + ("devfs_rmdir_empty: de is not a directory")); + + if ((de->de_flags & DE_DOOMED) != 0 || de == dm->dm_rootdir) + return; + + de_dot = TAILQ_FIRST(&de->de_dlist); + KASSERT(de_dot != NULL, ("devfs_rmdir_empty: . missing")); + de_dotdot = TAILQ_NEXT(de_dot, de_list); + KASSERT(de_dotdot != NULL, ("devfs_rmdir_empty: .. missing")); + /* Return if the directory is not empty. */ + if (TAILQ_NEXT(de_dotdot, de_list) != NULL) + return; + + dd = devfs_parent_dirent(de); + KASSERT(dd != NULL, ("devfs_rmdir_empty: NULL dd")); + TAILQ_REMOVE(&dd->de_dlist, de, de_list); + DEVFS_DE_HOLD(dd); + devfs_delete(dm, de, DEVFS_DEL_NORECURSE); + devfs_delete(dm, de_dot, DEVFS_DEL_NORECURSE); + devfs_delete(dm, de_dotdot, DEVFS_DEL_NORECURSE); + if (DEVFS_DE_DROP(dd)) { + devfs_dirent_free(dd); + return; + } + + de = dd; + } +} + /* * The caller needs to hold the dm for the duration of the call since * dm->dm_lock may be temporary dropped. */ void -devfs_delete(struct devfs_mount *dm, struct devfs_dirent *de, int vp_locked) +devfs_delete(struct devfs_mount *dm, struct devfs_dirent *de, int flags) { + struct devfs_dirent *dd; struct vnode *vp; KASSERT((de->de_flags & DE_DOOMED) == 0, ("devfs_delete doomed dirent")); de->de_flags |= DE_DOOMED; + + if ((flags & DEVFS_DEL_NORECURSE) == 0) { + dd = devfs_parent_dirent(de); + if (dd != NULL) + DEVFS_DE_HOLD(dd); + } else + dd = NULL; + mtx_lock(&devfs_de_interlock); vp = de->de_vnode; if (vp != NULL) { @@ -293,12 +344,12 @@ devfs_delete(struct devfs_mount *dm, struct devfs_dirent *de, int vp_locked) mtx_unlock(&devfs_de_interlock); vholdl(vp); sx_unlock(&dm->dm_lock); - if (!vp_locked) + if ((flags & DEVFS_DEL_VNLOCKED) == 0) vn_lock(vp, LK_EXCLUSIVE | LK_INTERLOCK | LK_RETRY); else VI_UNLOCK(vp); vgone(vp); - if (!vp_locked) + if ((flags & DEVFS_DEL_VNLOCKED) == 0) VOP_UNLOCK(vp, 0); vdrop(vp); sx_xlock(&dm->dm_lock); @@ -317,6 +368,13 @@ devfs_delete(struct devfs_mount *dm, struct devfs_dirent *de, int vp_locked) } if (DEVFS_DE_DROP(de)) devfs_dirent_free(de); + + if (dd != NULL) { + if (DEVFS_DE_DROP(dd)) + devfs_dirent_free(dd); + else + devfs_rmdir_empty(dm, dd); + } } /* @@ -331,19 +389,24 @@ devfs_purge(struct devfs_mount *dm, struct devfs_dirent *dd) struct devfs_dirent *de; sx_assert(&dm->dm_lock, SX_XLOCKED); + + DEVFS_DE_HOLD(dd); for (;;) { de = TAILQ_FIRST(&dd->de_dlist); if (de == NULL) break; TAILQ_REMOVE(&dd->de_dlist, de, de_list); - if (de->de_flags & (DE_DOT|DE_DOTDOT)) - devfs_delete(dm, de, 0); + if (de->de_flags & (DE_DOT | DE_DOTDOT)) + devfs_delete(dm, de, DEVFS_DEL_NORECURSE); else if (de->de_dirent->d_type == DT_DIR) devfs_purge(dm, de); - else - devfs_delete(dm, de, 0); + else + devfs_delete(dm, de, DEVFS_DEL_NORECURSE); } - devfs_delete(dm, dd, 0); + if (DEVFS_DE_DROP(dd)) + devfs_dirent_free(dd); + else if ((dd->de_flags & DE_DOOMED) == 0) + devfs_delete(dm, dd, DEVFS_DEL_NORECURSE); } /* diff --git a/sys/fs/devfs/devfs_vnops.c b/sys/fs/devfs/devfs_vnops.c index 943742e6fc81..2c27b7d8357e 100644 --- a/sys/fs/devfs/devfs_vnops.c +++ b/sys/fs/devfs/devfs_vnops.c @@ -36,7 +36,6 @@ /* * TODO: - * remove empty directories * mkdir: want it ? */ @@ -191,6 +190,7 @@ devfs_clear_cdevpriv(void) static int devfs_populate_vp(struct vnode *vp) { + struct devfs_dirent *de; struct devfs_mount *dmp; int locked; @@ -214,7 +214,14 @@ devfs_populate_vp(struct vnode *vp) devfs_unmount_final(dmp); return (EBADF); } - if (vp->v_iflag & VI_DOOMED) { + if ((vp->v_iflag & VI_DOOMED) != 0) { + sx_xunlock(&dmp->dm_lock); + return (EBADF); + } + de = vp->v_data; + KASSERT(de != NULL, + ("devfs_populate_vp: vp->v_data == NULL but vnode not doomed")); + if ((de->de_flags & DE_DOOMED) != 0) { sx_xunlock(&dmp->dm_lock); return (EBADF); } @@ -234,11 +241,13 @@ devfs_vptocnp(struct vop_vptocnp_args *ap) int i, error; dmp = VFSTODEVFS(vp->v_mount); + + error = devfs_populate_vp(vp); + if (error != 0) + return (error); + i = *buflen; dd = vp->v_data; - error = 0; - - sx_xlock(&dmp->dm_lock); if (vp->v_type == VCHR) { i -= strlen(dd->de_cdp->cdp_c.si_name); @@ -1271,11 +1280,15 @@ devfs_reclaim(struct vop_reclaim_args *ap) static int devfs_remove(struct vop_remove_args *ap) { + struct vnode *dvp = ap->a_dvp; struct vnode *vp = ap->a_vp; struct devfs_dirent *dd; struct devfs_dirent *de, *de_covered; struct devfs_mount *dmp = VFSTODEVFS(vp->v_mount); + ASSERT_VOP_ELOCKED(dvp, "devfs_remove"); + ASSERT_VOP_ELOCKED(vp, "devfs_remove"); + sx_xlock(&dmp->dm_lock); dd = ap->a_dvp->v_data; de = vp->v_data; @@ -1287,11 +1300,19 @@ devfs_remove(struct vop_remove_args *ap) if (de_covered != NULL) de_covered->de_flags &= ~DE_COVERED; } - devfs_delete(dmp, de, 1); + /* We need to unlock dvp because devfs_delete() may lock it. */ + VOP_UNLOCK(vp, 0); + if (dvp != vp) + VOP_UNLOCK(dvp, 0); + devfs_delete(dmp, de, 0); + sx_xunlock(&dmp->dm_lock); + if (dvp != vp) + vn_lock(dvp, LK_EXCLUSIVE | LK_RETRY); + vn_lock(vp, LK_EXCLUSIVE | LK_RETRY); } else { de->de_flags |= DE_WHITEOUT; + sx_xunlock(&dmp->dm_lock); } - sx_xunlock(&dmp->dm_lock); return (0); } @@ -1533,6 +1554,9 @@ devfs_symlink(struct vop_symlink_args *ap) if (error) return(error); dmp = VFSTODEVFS(ap->a_dvp->v_mount); + if (devfs_populate_vp(ap->a_dvp) != 0) + return (ENOENT); + dd = ap->a_dvp->v_data; de = devfs_newdirent(ap->a_cnp->cn_nameptr, ap->a_cnp->cn_namelen); de->de_uid = 0; @@ -1544,7 +1568,6 @@ devfs_symlink(struct vop_symlink_args *ap) i = strlen(ap->a_target) + 1; de->de_symlink = malloc(i, M_DEVFS, M_WAITOK); bcopy(ap->a_target, de->de_symlink, i); - sx_xlock(&dmp->dm_lock); #ifdef MAC mac_devfs_create_symlink(ap->a_cnp->cn_cred, dmp->dm_mount, dd, de); #endif