unregister path on devfs_remove
PATCH 1/1 unregister path on devfs_remove
The patch implements cleanup of device path on devfs_remove. It does not use extra counters as was suggested once - rather it simply removes all empty directories. Manually created directories (and connecting paths) are preserved and cleaned up on rmdir/unlink.
The creation/removing of path is not atomic; it will be addressed later. It apparently requires replacing per-directory locks with global one; using extra counters does not actually helps here.
This patch will cause devfs_remove on directories emit warning in most cases. Removing connecting directories makes no sense with this change so if it is agreed upon I send patch that removes all devfs_remove("directory") from kernel. It was never done consistently anyway. I would actually makedevfs_mk_dir noop except for two places - /dev/pts and /dev/shm.
regards
-andrey
fs/devfs/base.c renamed from tmp/linux-2.6.3-mm2/fs/devfs/base.c +68 -46
@@ -783,7 +783,6 @@ struct devfs_entry#ifdef CONFIG_DEVFS_DEBUG unsigned int magic_number;#endif -void *info;atomic_t refcount;/* When this drops to zero, it's unused */ union {
@@ -799,7 +798,7 @@ struct devfs_entrystruct devfs_inode inode;umode_t mode;unsigned short namelen;/* I think 64k+ filenames are a way off... */ - unsigned char vfs:1;/* Whether the VFS may delete the entry */ + unsigned char vfs:1; /* Entry was created by VFS */char name[1 ];/* This is just a dummy: the allocated array is bigger. This is NULL-terminated */ };
@@ -904,12 +903,13 @@ static void devfs_put (devfs_handle_t de {if (!de)return ;VERIFY_ENTRY (de); -if (de->info ==POISON_PTR )OOPS ("(%p): poisoned pointer\n" , de); +if (de->parent ==POISON_PTR )OOPS ("(%p): poisoned pointer\n" , de);if ( !atomic_dec_and_test (&de->refcount) )return ;if (de == root_entry)OOPS ("(%p): root entry being freed\n" , de);DPRINTK (DEBUG_FREE ,"(%s): de: %p, parent: %p \"%s\"\n" , de->name, de, de->parent, de->parent ? de->parent->name :"no parent" ); +devfs_put (de->parent);if (S_ISLNK (de->mode) )kfree (de->u.symlink.linkname);WRITE_ENTRY_MAGIC (de,0 );#ifdef CONFIG_DEVFS_DEBUG
@@ -919,7 +919,7 @@ static void devfs_put (devfs_handle_t deif (S_ISLNK (de->mode) ) stat_num_bytes -= de->u.symlink.length +1 ;spin_unlock (&stat_lock);#endif - de->info =POISON_PTR ; + de->parent =POISON_PTR ;kfree (de); }/* End Function devfs_put */
@@ -1032,7 +1032,7 @@ static int _devfs_append_entry (devfs_haelse devfs_put (old);if (old ==NULL ) { - de->parent = dir; + de->parent =devfs_get (dir); de->prev = dir->u.dir.last;/* Append to the directory's list of children */ if (dir->u.dir.first ==NULL ) dir->u.dir.first = de;
@@ -1368,8 +1368,8 @@ static int wait_for_devfsd_finished (str/** * devfsd_notify_de - Notify the devfsd daemon of a change. -* @de: The devfs entry that has changed. This and all parent entries will -* have their reference counts incremented if the event was queued. +* @de: The devfs entry that has changed. This will have reference +* count incremented if the event was queued. * @type: The type of change. * @mode: The mode of the entry. * @uid: The user ID.
@@ -1384,7 +1384,6 @@ static int devfsd_notify_de (struct devf uid_t uid, gid_t gid, struct fs_info *fs_info) { struct devfsd_buf_entry *entry; -struct devfs_entry *curr;if ( !( fs_info->devfsd_event_mask & (1 << type) ) )return (FALSE);if ( ( entry =kmem_cache_alloc (devfsd_buf_cache,SLAB_KERNEL ) ) ==NULL )
@@ -1392,8 +1391,7 @@ static int devfsd_notify_de (struct devfatomic_inc (&fs_info->devfsd_overrun_count);return (FALSE); } -for (curr = de; curr !=NULL ; curr = curr->parent)devfs_get (curr); - entry->de = de; + entry->de =devfs_get (de); entry->type = type; entry->mode = mode; entry->uid = uid;
@@ -1518,39 +1516,74 @@ static int _devfs_unhook (struct devfs_e/** -* _devfs_unregister - Unregister a device entry from its parent. +* _devfs_unregister - Unregister a device path starting from bottom * @dir: The parent directory. * @de: The entry to unregister. * * The caller must have a write lock on the parent directory, which is * unlocked by this function. +* +* Caller must devfs_get @de to ensure the path won't disappear +* while we traverse it from bottom up +* +* If @dir becomes empty we unregister it too recursively unless +* it has been created by user +* +* Returns true if entry was really unhooked, false otherwise. */ -static void _devfs_unregister (struct devfs_entry *dir,struct devfs_entry *de) +static int _devfs_unregister (struct devfs_entry *dir,struct devfs_entry *de) { -int unhooked =_devfs_unhook (de); +int unhooked; +struct devfs_entry *leaf = de; +int followup; -write_unlock (&dir->u.dir.lock); -if (!unhooked)return ; -devfs_get (dir); -devfsd_notify (de,DEVFSD_NOTIFY_UNREGISTERED ); -free_dentry (de); -devfs_put (dir); -if ( !S_ISDIR (de->mode) )return ; +repeat: + followup =0 ; + unhooked =_devfs_unhook (de); +if (dir->u.dir.first ==NULL && dir->u.dir.last ==NULL && + !dir->vfs && dir != root_entry && unhooked) { + dir->u.dir.no_more_additions = TRUE; + followup =1 ; + } +write_unlock (&dir->u.dir.lock); + +if (!unhooked) { +if (de != leaf) + goto kill_subtree; +else +return 0 ; + } + +free_dentry (de); +devfs_put (de); + +/* remove parent if it became empty */ +if (followup) { + de = dir; + dir = de->parent; +write_lock (&dir->u.dir.lock); + goto repeat; + } + +kill_subtree: +if ( !S_ISDIR (leaf->mode) )return 1 ;while (TRUE)/* Recursively unregister: this is a stack chomper */ {struct devfs_entry *child; -write_lock (&de->u.dir.lock); - de->u.dir.no_more_additions = TRUE; - child = de->u.dir.first; +write_lock (&leaf->u.dir.lock); + leaf->u.dir.no_more_additions = TRUE; + child =devfs_get (leaf->u.dir.first);VERIFY_ENTRY (child); -_devfs_unregister (de, child); +_devfs_unregister (leaf, child);if (!child)break ;DPRINTK (DEBUG_UNREGISTER ,"(%s): child: %p refcount: %d\n" , child->name, child,atomic_read (&child->refcount) );devfs_put (child); } + +return 1 ; }/* End Function _devfs_unregister */ static int devfs_do_symlink (devfs_handle_t dir, const char *name,
@@ -1584,7 +1617,6 @@ static int devfs_do_symlink (devfs_handlkfree (newlink);return -ENOTDIR ; } - de->info =NULL ; de->u.symlink.linkname = newlink; de->u.symlink.length = linklength;if ( ( err =_devfs_append_entry (dir, de,NULL ) ) !=0 )
@@ -1699,9 +1731,12 @@ void devfs_remove(const char *fmt, ...)return ; } -write_lock (&de->parent->u.dir.lock); -_devfs_unregister (de->parent, de); -devfs_put (de); +/* no way to return an error; should be EPERM */ +if (!de->vfs) { +write_lock (&de->parent->u.dir.lock); +if (_devfs_unregister (de->parent, de)) +devfsd_notify (de,DEVFSD_NOTIFY_UNREGISTERED ); + }devfs_put (de); } }
@@ -2330,7 +2365,6 @@ out:static int devfs_unlink (struct inode *dir,struct dentry *dentry) { -int unhooked;struct devfs_entry *de;struct inode *inode = dentry->d_inode;struct fs_info *fs_info = dir->i_sb->s_fs_info;
@@ -2340,14 +2374,11 @@ static int devfs_unlink (struct inode *dif (de ==NULL )return -ENOENT ;if (!de->vfs)return -EPERM ;write_lock (&de->parent->u.dir.lock); - unhooked =_devfs_unhook (de); -write_unlock (&de->parent->u.dir.lock); -if (!unhooked)return -ENOENT ; +if (!_devfs_unregister (de->parent, de)) +return -ENOENT ;if ( !is_devfsd_or_child (fs_info) )devfsd_notify_de (de,DEVFSD_NOTIFY_DELETE , inode->i_mode, inode->i_uid, inode->i_gid, fs_info); -free_dentry (de); -devfs_put (de);return 0 ; }/* End Function devfs_unlink */
@@ -2434,14 +2465,11 @@ static int devfs_rmdir (struct inode *diif (err)return err;/* Now unhook the directory from its parent */ write_lock (&de->parent->u.dir.lock); -if ( !_devfs_unhook (de) ) err = -ENOENT ; -write_unlock (&de->parent->u.dir.lock); -if (err)return err; +if (!_devfs_unregister (de->parent, de)) +return -ENOENT ;if ( !is_devfsd_or_child (fs_info) )devfsd_notify_de (de,DEVFSD_NOTIFY_DELETE , inode->i_mode, inode->i_uid, inode->i_gid, fs_info); -free_dentry (de); -devfs_put (de);return 0 ; }/* End Function devfs_rmdir */
@@ -2653,17 +2681,11 @@ static ssize_t devfsd_read (struct file tlen = rpos - *ppos;if (done) { -devfs_handle_t parent; -spin_lock (&fs_info->devfsd_buffer_lock); fs_info->devfsd_first_event = entry->next;if (entry->next ==NULL ) fs_info->devfsd_last_event =NULL ;spin_unlock (&fs_info->devfsd_buffer_lock); -for (; de !=NULL ; de = parent) - { - parent = de->parent; -devfs_put (de); - } +devfs_put (de);kmem_cache_free (devfsd_buf_cache, entry);if (ival >0 )atomic_sub (ival, &fs_info->devfsd_overrun_count); *ppos =0 ;
Quoted 6 lines · Andrey Borzenkov
The patch implements cleanup of device path on
devfs_remove. It does not use extra counters as was suggested once - rather it simply removes all empty directories. Manually created directories (and connecting paths) are preserved and cleaned up on rmdir/unlink.
Quoted 8 lines
The creation/removing of path is not atomic; it will be addressed later. It apparently requires replacing per-directory locks with global one; using extra counters does not actually helps here.
This patch will cause
devfs_removeon directories emit warning in most cases. Removing connecting directories makes no sense with this change so if it is agreed upon I send patch that removes alldevfs_remove("directory")from kernel.
Well could you please either send that patch or remove the warning? I get way too much mail when -mm kernels start spitting new warnings.