Skip to content

Commit

Permalink
inotify: simplify the inotify idr handling
Browse files Browse the repository at this point in the history 
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>
  • Loading branch information
Eric Paris committed Jul 28, 2010
1 parent fc0f5ac commit b7ba837
Showing 1 changed file with 139 additions and 55 deletions.
194 changes: 139 additions & 55 deletions fs/notify/inotify/inotify_user.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -357,49 +357,150 @@ 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.
*/
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)
/*
* 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);
goto out;
}

entry = idr_find(&group->inotify_data.idr, wd);
if (unlikely(!entry))
/* 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;
}

found_ientry = container_of(entry, struct inotify_inode_mark_entry, fsn_entry);
/*
* 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)) {
/* 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. */
WARN_ON(1);
ientry->wd = -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;
}

/* One ref for being in the idr, one ref held by the caller */
BUG_ON(atomic_read(&entry->refcnt) < 2);

idr_remove(idr, wd);
ientry->wd = -1;
/*
* 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();
}

/* removed from the idr, drop that ref */
fsnotify_put_mark(entry);
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);
}

/*
Expand Down Expand Up @@ -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);
Expand All @@ -541,28 +644,11 @@ 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)))
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;
ret = inotify_add_to_idr(idr, idr_lock, group->inotify_data.last_wd,
tmp_ientry);
if (ret)
goto out_err;
}

/* we are on the idr, now get on the inode */
ret = fsnotify_add_mark(&tmp_ientry->fsn_entry, group, inode);
Expand Down Expand Up @@ -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;

Expand All @@ -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);
Expand Down

0 comments on commit b7ba837

Please sign in to comment.