inotify: simplify the inotify idr handling
This patch moves all of the idr editing operations into their own idr functions. It makes it easier to prove locking correctness and to to understand the code flow. Signed-off-by: Eric Paris <eparis@redhat.com>
This commit is contained in:
Eric Paris
parent
fc0f5ac8fe
commit
b7ba837153
1 changed files with 144 additions and 60 deletions
|
|
@ -357,6 +357,77 @@ static int inotify_find_inode(const char __user *dirname, struct path *path, uns
|
|||
return error;
|
||||
}
|
||||
|
||||
static int inotify_add_to_idr(struct idr *idr, spinlock_t *idr_lock,
|
||||
int last_wd,
|
||||
struct inotify_inode_mark_entry *ientry)
|
||||
{
|
||||
int ret;
|
||||
|
||||
do {
|
||||
if (unlikely(!idr_pre_get(idr, GFP_KERNEL)))
|
||||
return -ENOMEM;
|
||||
|
||||
spin_lock(idr_lock);
|
||||
ret = idr_get_new_above(idr, ientry, last_wd + 1,
|
||||
&ientry->wd);
|
||||
/* we added the mark to the idr, take a reference */
|
||||
if (!ret)
|
||||
fsnotify_get_mark(&ientry->fsn_entry);
|
||||
spin_unlock(idr_lock);
|
||||
} while (ret == -EAGAIN);
|
||||
|
||||
return ret;
|
||||
}
|
||||
|
||||
static struct inotify_inode_mark_entry *inotify_idr_find_locked(struct fsnotify_group *group,
|
||||
int wd)
|
||||
{
|
||||
struct idr *idr = &group->inotify_data.idr;
|
||||
spinlock_t *idr_lock = &group->inotify_data.idr_lock;
|
||||
struct inotify_inode_mark_entry *ientry;
|
||||
|
||||
assert_spin_locked(idr_lock);
|
||||
|
||||
ientry = idr_find(idr, wd);
|
||||
if (ientry) {
|
||||
struct fsnotify_mark_entry *fsn_entry = &ientry->fsn_entry;
|
||||
|
||||
fsnotify_get_mark(fsn_entry);
|
||||
/* One ref for being in the idr, one ref we just took */
|
||||
BUG_ON(atomic_read(&fsn_entry->refcnt) < 2);
|
||||
}
|
||||
|
||||
return ientry;
|
||||
}
|
||||
|
||||
static struct inotify_inode_mark_entry *inotify_idr_find(struct fsnotify_group *group,
|
||||
int wd)
|
||||
{
|
||||
struct inotify_inode_mark_entry *ientry;
|
||||
spinlock_t *idr_lock = &group->inotify_data.idr_lock;
|
||||
|
||||
spin_lock(idr_lock);
|
||||
ientry = inotify_idr_find_locked(group, wd);
|
||||
spin_unlock(idr_lock);
|
||||
|
||||
return ientry;
|
||||
}
|
||||
|
||||
static void do_inotify_remove_from_idr(struct fsnotify_group *group,
|
||||
struct inotify_inode_mark_entry *ientry)
|
||||
{
|
||||
struct idr *idr = &group->inotify_data.idr;
|
||||
spinlock_t *idr_lock = &group->inotify_data.idr_lock;
|
||||
int wd = ientry->wd;
|
||||
|
||||
assert_spin_locked(idr_lock);
|
||||
|
||||
idr_remove(idr, wd);
|
||||
|
||||
/* removed from the idr, drop that ref */
|
||||
fsnotify_put_mark(&ientry->fsn_entry);
|
||||
}
|
||||
|
||||
/*
|
||||
* Remove the mark from the idr (if present) and drop the reference
|
||||
* on the mark because it was in the idr.
|
||||
|
|
@ -364,42 +435,72 @@ static int inotify_find_inode(const char __user *dirname, struct path *path, uns
|
|||
static void inotify_remove_from_idr(struct fsnotify_group *group,
|
||||
struct inotify_inode_mark_entry *ientry)
|
||||
{
|
||||
struct idr *idr;
|
||||
struct fsnotify_mark_entry *entry;
|
||||
struct inotify_inode_mark_entry *found_ientry;
|
||||
spinlock_t *idr_lock = &group->inotify_data.idr_lock;
|
||||
struct inotify_inode_mark_entry *found_ientry = NULL;
|
||||
int wd;
|
||||
|
||||
spin_lock(&group->inotify_data.idr_lock);
|
||||
idr = &group->inotify_data.idr;
|
||||
spin_lock(idr_lock);
|
||||
wd = ientry->wd;
|
||||
|
||||
if (wd == -1)
|
||||
goto out;
|
||||
|
||||
entry = idr_find(&group->inotify_data.idr, wd);
|
||||
if (unlikely(!entry))
|
||||
goto out;
|
||||
|
||||
found_ientry = container_of(entry, struct inotify_inode_mark_entry, fsn_entry);
|
||||
if (unlikely(found_ientry != ientry)) {
|
||||
/* We found an entry in the idr with the right wd, but it's
|
||||
* not the entry we were told to remove. eparis seriously
|
||||
* fucked up somewhere. */
|
||||
/*
|
||||
* does this ientry think it is in the idr? we shouldn't get called
|
||||
* if it wasn't....
|
||||
*/
|
||||
if (wd == -1) {
|
||||
printk(KERN_WARNING "%s: ientry=%p ientry->wd=%d ientry->group=%p"
|
||||
" ientry->inode=%p\n", __func__, ientry, ientry->wd,
|
||||
ientry->fsn_entry.group, ientry->fsn_entry.inode);
|
||||
WARN_ON(1);
|
||||
ientry->wd = -1;
|
||||
goto out;
|
||||
}
|
||||
|
||||
/* One ref for being in the idr, one ref held by the caller */
|
||||
BUG_ON(atomic_read(&entry->refcnt) < 2);
|
||||
/* Lets look in the idr to see if we find it */
|
||||
found_ientry = inotify_idr_find_locked(group, wd);
|
||||
if (unlikely(!found_ientry)) {
|
||||
printk(KERN_WARNING "%s: ientry=%p ientry->wd=%d ientry->group=%p"
|
||||
" ientry->inode=%p\n", __func__, ientry, ientry->wd,
|
||||
ientry->fsn_entry.group, ientry->fsn_entry.inode);
|
||||
WARN_ON(1);
|
||||
goto out;
|
||||
}
|
||||
|
||||
idr_remove(idr, wd);
|
||||
ientry->wd = -1;
|
||||
/*
|
||||
* We found an entry in the idr at the right wd, but it's
|
||||
* not the entry we were told to remove. eparis seriously
|
||||
* fucked up somewhere.
|
||||
*/
|
||||
if (unlikely(found_ientry != ientry)) {
|
||||
WARN_ON(1);
|
||||
printk(KERN_WARNING "%s: ientry=%p ientry->wd=%d ientry->group=%p "
|
||||
"entry->inode=%p found_ientry=%p found_ientry->wd=%d "
|
||||
"found_ientry->group=%p found_ientry->inode=%p\n",
|
||||
__func__, ientry, ientry->wd, ientry->fsn_entry.group,
|
||||
ientry->fsn_entry.inode, found_ientry, found_ientry->wd,
|
||||
found_ientry->fsn_entry.group,
|
||||
found_ientry->fsn_entry.inode);
|
||||
goto out;
|
||||
}
|
||||
|
||||
/* removed from the idr, drop that ref */
|
||||
fsnotify_put_mark(entry);
|
||||
/*
|
||||
* One ref for being in the idr
|
||||
* one ref held by the caller trying to kill us
|
||||
* one ref grabbed by inotify_idr_find
|
||||
*/
|
||||
if (unlikely(atomic_read(&ientry->fsn_entry.refcnt) < 3)) {
|
||||
printk(KERN_WARNING "%s: ientry=%p ientry->wd=%d ientry->group=%p"
|
||||
" ientry->inode=%p\n", __func__, ientry, ientry->wd,
|
||||
ientry->fsn_entry.group, ientry->fsn_entry.inode);
|
||||
/* we can't really recover with bad ref cnting.. */
|
||||
BUG();
|
||||
}
|
||||
|
||||
do_inotify_remove_from_idr(group, ientry);
|
||||
out:
|
||||
spin_unlock(&group->inotify_data.idr_lock);
|
||||
/* match the ref taken by inotify_idr_find_locked() */
|
||||
if (found_ientry)
|
||||
fsnotify_put_mark(&found_ientry->fsn_entry);
|
||||
ientry->wd = -1;
|
||||
spin_unlock(idr_lock);
|
||||
}
|
||||
|
||||
/*
|
||||
|
|
@ -524,6 +625,8 @@ static int inotify_new_watch(struct fsnotify_group *group,
|
|||
struct inotify_inode_mark_entry *tmp_ientry;
|
||||
__u32 mask;
|
||||
int ret;
|
||||
struct idr *idr = &group->inotify_data.idr;
|
||||
spinlock_t *idr_lock = &group->inotify_data.idr_lock;
|
||||
|
||||
/* don't allow invalid bits: we don't want flags set */
|
||||
mask = inotify_arg_to_mask(arg);
|
||||
|
|
@ -541,29 +644,12 @@ static int inotify_new_watch(struct fsnotify_group *group,
|
|||
ret = -ENOSPC;
|
||||
if (atomic_read(&group->inotify_data.user->inotify_watches) >= inotify_max_user_watches)
|
||||
goto out_err;
|
||||
retry:
|
||||
ret = -ENOMEM;
|
||||
if (unlikely(!idr_pre_get(&group->inotify_data.idr, GFP_KERNEL)))
|
||||
|
||||
ret = inotify_add_to_idr(idr, idr_lock, group->inotify_data.last_wd,
|
||||
tmp_ientry);
|
||||
if (ret)
|
||||
goto out_err;
|
||||
|
||||
/* we are putting the mark on the idr, take a reference */
|
||||
fsnotify_get_mark(&tmp_ientry->fsn_entry);
|
||||
|
||||
spin_lock(&group->inotify_data.idr_lock);
|
||||
ret = idr_get_new_above(&group->inotify_data.idr, &tmp_ientry->fsn_entry,
|
||||
group->inotify_data.last_wd+1,
|
||||
&tmp_ientry->wd);
|
||||
spin_unlock(&group->inotify_data.idr_lock);
|
||||
if (ret) {
|
||||
/* we didn't get on the idr, drop the idr reference */
|
||||
fsnotify_put_mark(&tmp_ientry->fsn_entry);
|
||||
|
||||
/* idr was out of memory allocate and try again */
|
||||
if (ret == -EAGAIN)
|
||||
goto retry;
|
||||
goto out_err;
|
||||
}
|
||||
|
||||
/* we are on the idr, now get on the inode */
|
||||
ret = fsnotify_add_mark(&tmp_ientry->fsn_entry, group, inode);
|
||||
if (ret) {
|
||||
|
|
@ -726,7 +812,7 @@ SYSCALL_DEFINE3(inotify_add_watch, int, fd, const char __user *, pathname,
|
|||
SYSCALL_DEFINE2(inotify_rm_watch, int, fd, __s32, wd)
|
||||
{
|
||||
struct fsnotify_group *group;
|
||||
struct fsnotify_mark_entry *entry;
|
||||
struct inotify_inode_mark_entry *ientry;
|
||||
struct file *filp;
|
||||
int ret = 0, fput_needed;
|
||||
|
||||
|
|
@ -735,25 +821,23 @@ SYSCALL_DEFINE2(inotify_rm_watch, int, fd, __s32, wd)
|
|||
return -EBADF;
|
||||
|
||||
/* verify that this is indeed an inotify instance */
|
||||
if (unlikely(filp->f_op != &inotify_fops)) {
|
||||
ret = -EINVAL;
|
||||
ret = -EINVAL;
|
||||
if (unlikely(filp->f_op != &inotify_fops))
|
||||
goto out;
|
||||
}
|
||||
|
||||
group = filp->private_data;
|
||||
|
||||
spin_lock(&group->inotify_data.idr_lock);
|
||||
entry = idr_find(&group->inotify_data.idr, wd);
|
||||
if (unlikely(!entry)) {
|
||||
spin_unlock(&group->inotify_data.idr_lock);
|
||||
ret = -EINVAL;
|
||||
ret = -EINVAL;
|
||||
ientry = inotify_idr_find(group, wd);
|
||||
if (unlikely(!ientry))
|
||||
goto out;
|
||||
}
|
||||
fsnotify_get_mark(entry);
|
||||
spin_unlock(&group->inotify_data.idr_lock);
|
||||
|
||||
fsnotify_destroy_mark_by_entry(entry);
|
||||
fsnotify_put_mark(entry);
|
||||
ret = 0;
|
||||
|
||||
fsnotify_destroy_mark_by_entry(&ientry->fsn_entry);
|
||||
|
||||
/* match ref taken by inotify_idr_find */
|
||||
fsnotify_put_mark(&ientry->fsn_entry);
|
||||
|
||||
out:
|
||||
fput_light(filp, fput_needed);
|
||||
|
|
|
|||
Loading…
Reference in a new issue