Skip to content

Commit

Permalink
Merge branch 'for-next' of git://git.infradead.org/users/eparis/notify
Browse files Browse the repository at this point in the history
Pull filesystem notification updates from Eric Paris:
 "This pull mostly is about locking changes in the fsnotify system.  By
  switching the group lock from a spin_lock() to a mutex() we can now
  hold the lock across things like iput().  This fixes a problem
  involving unmounting a fs and having inodes be busy, first pointed out
  by FAT, but reproducible with tmpfs.

  This also restores signal driven I/O for inotify, which has been
  broken since about 2.6.32."

Ugh.  I *hate* the timing of this.  It was rebased after the merge
window opened, and then left to sit with the pull request coming the day
before the merge window closes.  That's just crap.  But apparently the
patches themselves have been around for over a year, just gathering
dust, so now it's suddenly critical.

Fixed up semantic conflict in fs/notify/fdinfo.c as per Stephen
Rothwell's fixes from -next.

* 'for-next' of git://git.infradead.org/users/eparis/notify:
  inotify: automatically restart syscalls
  inotify: dont skip removal of watch descriptor if creation of ignored event failed
  fanotify: dont merge permission events
  fsnotify: make fasync generic for both inotify and fanotify
  fsnotify: change locking order
  fsnotify: dont put marks on temporary list when clearing marks by group
  fsnotify: introduce locked versions of fsnotify_add_mark() and fsnotify_remove_mark()
  fsnotify: pass group to fsnotify_destroy_mark()
  fsnotify: use a mutex instead of a spinlock to protect a groups mark list
  fanotify: add an extra flag to mark_remove_from_mask that indicates wheather a mark should be destroyed
  fsnotify: take groups mark_lock before mark lock
  fsnotify: use reference counting for groups
  fsnotify: introduce fsnotify_get_group()
  inotify, fanotify: replace fsnotify_put_group() with fsnotify_destroy_group()
  • Loading branch information
torvalds committed Dec 21, 2012
2 parents 4c9a44a + 1ca39ab commit 96680d2
Show file tree
Hide file tree
Showing 14 changed files with 180 additions and 121 deletions.
4 changes: 2 additions & 2 deletions fs/notify/dnotify/dnotify.c
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ void dnotify_flush(struct file *filp, fl_owner_t id)

/* nothing else could have found us thanks to the dnotify_mark_mutex */
if (dn_mark->dn == NULL)
fsnotify_destroy_mark(fsn_mark);
fsnotify_destroy_mark(fsn_mark, dnotify_group);

mutex_unlock(&dnotify_mark_mutex);

Expand Down Expand Up @@ -385,7 +385,7 @@ int fcntl_dirnotify(int fd, struct file *filp, unsigned long arg)
spin_unlock(&fsn_mark->lock);

if (destroy)
fsnotify_destroy_mark(fsn_mark);
fsnotify_destroy_mark(fsn_mark, dnotify_group);

mutex_unlock(&dnotify_mark_mutex);
fsnotify_put_mark(fsn_mark);
Expand Down
6 changes: 6 additions & 0 deletions fs/notify/fanotify/fanotify.c
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@ static bool should_merge(struct fsnotify_event *old, struct fsnotify_event *new)
old->tgid == new->tgid) {
switch (old->data_type) {
case (FSNOTIFY_EVENT_PATH):
#ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS
/* dont merge two permission events */
if ((old->mask & FAN_ALL_PERM_EVENTS) &&
(new->mask & FAN_ALL_PERM_EVENTS))
return false;
#endif
if ((old->path.mnt == new->path.mnt) &&
(old->path.dentry == new->path.dentry))
return true;
Expand Down
37 changes: 25 additions & 12 deletions fs/notify/fanotify/fanotify_user.c
Original file line number Diff line number Diff line change
Expand Up @@ -397,8 +397,12 @@ static int fanotify_release(struct inode *ignored, struct file *file)

wake_up(&group->fanotify_data.access_waitq);
#endif

if (file->f_flags & FASYNC)
fsnotify_fasync(-1, file, 0);

/* matches the fanotify_init->fsnotify_alloc_group */
fsnotify_put_group(group);
fsnotify_destroy_group(group);

return 0;
}
Expand Down Expand Up @@ -493,7 +497,8 @@ static int fanotify_find_path(int dfd, const char __user *filename,

static __u32 fanotify_mark_remove_from_mask(struct fsnotify_mark *fsn_mark,
__u32 mask,
unsigned int flags)
unsigned int flags,
int *destroy)
{
__u32 oldmask;

Expand All @@ -507,8 +512,7 @@ static __u32 fanotify_mark_remove_from_mask(struct fsnotify_mark *fsn_mark,
}
spin_unlock(&fsn_mark->lock);

if (!(oldmask & ~mask))
fsnotify_destroy_mark(fsn_mark);
*destroy = !(oldmask & ~mask);

return mask & oldmask;
}
Expand All @@ -519,12 +523,17 @@ static int fanotify_remove_vfsmount_mark(struct fsnotify_group *group,
{
struct fsnotify_mark *fsn_mark = NULL;
__u32 removed;
int destroy_mark;

fsn_mark = fsnotify_find_vfsmount_mark(group, mnt);
if (!fsn_mark)
return -ENOENT;

removed = fanotify_mark_remove_from_mask(fsn_mark, mask, flags);
removed = fanotify_mark_remove_from_mask(fsn_mark, mask, flags,
&destroy_mark);
if (destroy_mark)
fsnotify_destroy_mark(fsn_mark, group);

fsnotify_put_mark(fsn_mark);
if (removed & real_mount(mnt)->mnt_fsnotify_mask)
fsnotify_recalc_vfsmount_mask(mnt);
Expand All @@ -538,12 +547,16 @@ static int fanotify_remove_inode_mark(struct fsnotify_group *group,
{
struct fsnotify_mark *fsn_mark = NULL;
__u32 removed;
int destroy_mark;

fsn_mark = fsnotify_find_inode_mark(group, inode);
if (!fsn_mark)
return -ENOENT;

removed = fanotify_mark_remove_from_mask(fsn_mark, mask, flags);
removed = fanotify_mark_remove_from_mask(fsn_mark, mask, flags,
&destroy_mark);
if (destroy_mark)
fsnotify_destroy_mark(fsn_mark, group);
/* matches the fsnotify_find_inode_mark() */
fsnotify_put_mark(fsn_mark);
if (removed & inode->i_fsnotify_mask)
Expand Down Expand Up @@ -710,13 +723,13 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
break;
default:
fd = -EINVAL;
goto out_put_group;
goto out_destroy_group;
}

if (flags & FAN_UNLIMITED_QUEUE) {
fd = -EPERM;
if (!capable(CAP_SYS_ADMIN))
goto out_put_group;
goto out_destroy_group;
group->max_events = UINT_MAX;
} else {
group->max_events = FANOTIFY_DEFAULT_MAX_EVENTS;
Expand All @@ -725,20 +738,20 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
if (flags & FAN_UNLIMITED_MARKS) {
fd = -EPERM;
if (!capable(CAP_SYS_ADMIN))
goto out_put_group;
goto out_destroy_group;
group->fanotify_data.max_marks = UINT_MAX;
} else {
group->fanotify_data.max_marks = FANOTIFY_DEFAULT_MAX_MARKS;
}

fd = anon_inode_getfd("[fanotify]", &fanotify_fops, group, f_flags);
if (fd < 0)
goto out_put_group;
goto out_destroy_group;

return fd;

out_put_group:
fsnotify_put_group(group);
out_destroy_group:
fsnotify_destroy_group(group);
return fd;
}

Expand Down
4 changes: 2 additions & 2 deletions fs/notify/fdinfo.c
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,13 @@ static int show_fdinfo(struct seq_file *m, struct file *f,
struct fsnotify_mark *mark;
int ret = 0;

spin_lock(&group->mark_lock);
mutex_lock(&group->mark_mutex);
list_for_each_entry(mark, &group->marks_list, g_list) {
ret = show(m, mark);
if (ret)
break;
}
spin_unlock(&group->mark_lock);
mutex_unlock(&group->mark_mutex);
return ret;
}

Expand Down
47 changes: 27 additions & 20 deletions fs/notify/group.c
Original file line number Diff line number Diff line change
Expand Up @@ -33,33 +33,37 @@
*/
void fsnotify_final_destroy_group(struct fsnotify_group *group)
{
/* clear the notification queue of all events */
fsnotify_flush_notify(group);

if (group->ops->free_group_priv)
group->ops->free_group_priv(group);

kfree(group);
}

/*
* Trying to get rid of a group. We need to first get rid of any outstanding
* allocations and then free the group. Remember that fsnotify_clear_marks_by_group
* could miss marks that are being freed by inode and those marks could still
* hold a reference to this group (via group->num_marks) If we get into that
* situtation, the fsnotify_final_destroy_group will get called when that final
* mark is freed.
* Trying to get rid of a group. Remove all marks, flush all events and release
* the group reference.
* Note that another thread calling fsnotify_clear_marks_by_group() may still
* hold a ref to the group.
*/
static void fsnotify_destroy_group(struct fsnotify_group *group)
void fsnotify_destroy_group(struct fsnotify_group *group)
{
/* clear all inode marks for this group */
fsnotify_clear_marks_by_group(group);

synchronize_srcu(&fsnotify_mark_srcu);

/* past the point of no return, matches the initial value of 1 */
if (atomic_dec_and_test(&group->num_marks))
fsnotify_final_destroy_group(group);
/* clear the notification queue of all events */
fsnotify_flush_notify(group);

fsnotify_put_group(group);
}

/*
* Get reference to a group.
*/
void fsnotify_get_group(struct fsnotify_group *group)
{
atomic_inc(&group->refcnt);
}

/*
Expand All @@ -68,7 +72,7 @@ static void fsnotify_destroy_group(struct fsnotify_group *group)
void fsnotify_put_group(struct fsnotify_group *group)
{
if (atomic_dec_and_test(&group->refcnt))
fsnotify_destroy_group(group);
fsnotify_final_destroy_group(group);
}

/*
Expand All @@ -84,21 +88,24 @@ struct fsnotify_group *fsnotify_alloc_group(const struct fsnotify_ops *ops)

/* set to 0 when there a no external references to this group */
atomic_set(&group->refcnt, 1);
/*
* hits 0 when there are no external references AND no marks for
* this group
*/
atomic_set(&group->num_marks, 1);
atomic_set(&group->num_marks, 0);

mutex_init(&group->notification_mutex);
INIT_LIST_HEAD(&group->notification_list);
init_waitqueue_head(&group->notification_waitq);
group->max_events = UINT_MAX;

spin_lock_init(&group->mark_lock);
mutex_init(&group->mark_mutex);
INIT_LIST_HEAD(&group->marks_list);

group->ops = ops;

return group;
}

int fsnotify_fasync(int fd, struct file *file, int on)
{
struct fsnotify_group *group = file->private_data;

return fasync_helper(fd, file, on, &group->fsn_fa) >= 0 ? 0 : -EIO;
}
14 changes: 11 additions & 3 deletions fs/notify/inode_mark.c
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ void fsnotify_destroy_inode_mark(struct fsnotify_mark *mark)
{
struct inode *inode = mark->i.inode;

BUG_ON(!mutex_is_locked(&mark->group->mark_mutex));
assert_spin_locked(&mark->lock);
assert_spin_locked(&mark->group->mark_lock);

spin_lock(&inode->i_lock);

Expand Down Expand Up @@ -99,8 +99,16 @@ void fsnotify_clear_marks_by_inode(struct inode *inode)
spin_unlock(&inode->i_lock);

list_for_each_entry_safe(mark, lmark, &free_list, i.free_i_list) {
fsnotify_destroy_mark(mark);
struct fsnotify_group *group;

spin_lock(&mark->lock);
fsnotify_get_group(mark->group);
group = mark->group;
spin_unlock(&mark->lock);

fsnotify_destroy_mark(mark, group);
fsnotify_put_mark(mark);
fsnotify_put_group(group);
}
}

Expand Down Expand Up @@ -192,8 +200,8 @@ int fsnotify_add_inode_mark(struct fsnotify_mark *mark,

mark->flags |= FSNOTIFY_MARK_FLAG_INODE;

BUG_ON(!mutex_is_locked(&group->mark_mutex));
assert_spin_locked(&mark->lock);
assert_spin_locked(&group->mark_lock);

spin_lock(&inode->i_lock);

Expand Down
4 changes: 3 additions & 1 deletion fs/notify/inotify/inotify_fsnotify.c
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ static int inotify_handle_event(struct fsnotify_group *group,

fsn_event_priv = &event_priv->fsnotify_event_priv_data;

fsnotify_get_group(group);
fsn_event_priv->group = group;
event_priv->wd = wd;

Expand All @@ -131,7 +132,7 @@ static int inotify_handle_event(struct fsnotify_group *group,
}

if (inode_mark->mask & IN_ONESHOT)
fsnotify_destroy_mark(inode_mark);
fsnotify_destroy_mark(inode_mark, group);

return ret;
}
Expand Down Expand Up @@ -210,6 +211,7 @@ void inotify_free_event_priv(struct fsnotify_event_private_data *fsn_event_priv)
event_priv = container_of(fsn_event_priv, struct inotify_event_private_data,
fsnotify_event_priv_data);

fsnotify_put_group(fsn_event_priv->group);
kmem_cache_free(event_priv_cachep, event_priv);
}

Expand Down
Loading

0 comments on commit 96680d2

Please sign in to comment.