Minki/linux
1
0
Fork 1
mirror of https://github.com/torvalds/linux.git synced 2024-10-30 08:42:47 +00:00
Code Issues Packages Projects Releases Wiki Activity

fsnotify: Lock object list with connector lock

Browse Source 
So far list of marks attached to an object (inode / vfsmount) was
protected by i_lock or mnt_root->d_lock. This dictates that the list
must be empty before the object can be destroyed although the list is
now anchored in the fsnotify_mark_connector structure. Protect the list
by a spinlock in the fsnotify_mark_connector structure to decouple
lifetime of a list of marks from a lifetime of the object. This also
simplifies the code quite a bit since we don't have to differentiate
between inode and vfsmount lists in quite a few places anymore.

Reviewed-by: Miklos Szeredi <mszeredi@redhat.com>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Jan Kara <jack@suse.cz>
This commit is contained in:
Jan Kara 2017-02-01 08:19:43 +01:00
parent 2629718dd2
commit 04662cab59
2 changed files with 34 additions and 59 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

90
fs/notify/mark.c
View File

@ -33,7 +33,7 @@
* *
* group->mark_mutex * group->mark_mutex
* mark->lock * mark->lock
* inode->i_lock * mark->connector->lock
* *
* group->mark_mutex protects the marks_list anchored inside a given group and * group->mark_mutex protects the marks_list anchored inside a given group and
* each mark is hooked via the g_list. It also protects the groups private * each mark is hooked via the g_list. It also protects the groups private
@ -44,10 +44,12 @@
* is assigned to as well as the access to a reference of the inode/vfsmount * is assigned to as well as the access to a reference of the inode/vfsmount
* that is being watched by the mark. * that is being watched by the mark.
* *
* inode->i_lock protects the i_fsnotify_marks list anchored inside a * mark->connector->lock protects the list of marks anchored inside an
* given inode and each mark is hooked via the i_list. (and sorta the * inode / vfsmount and each mark is hooked via the i_list.
* free_i_list)
* *
* A list of notification marks relating to inode / mnt is contained in
* fsnotify_mark_connector. That structure is alive as long as there are any
* marks in the list and is also protected by fsnotify_mark_srcu.
* *
* LIFETIME: * LIFETIME:
* Inode marks survive between when they are added to an inode and when their * Inode marks survive between when they are added to an inode and when their
@ -110,8 +112,10 @@ static void __fsnotify_recalc_mask(struct fsnotify_mark_connector *conn)
u32 new_mask = 0; u32 new_mask = 0;
struct fsnotify_mark *mark; struct fsnotify_mark *mark;
assert_spin_locked(&conn->lock);
hlist_for_each_entry(mark, &conn->list, obj_list) hlist_for_each_entry(mark, &conn->list, obj_list)
new_mask |= mark->mask; new_mask |= mark->mask;
if (conn->flags & FSNOTIFY_OBJ_TYPE_INODE) if (conn->flags & FSNOTIFY_OBJ_TYPE_INODE)
conn->inode->i_fsnotify_mask = new_mask; conn->inode->i_fsnotify_mask = new_mask;
else if (conn->flags & FSNOTIFY_OBJ_TYPE_VFSMOUNT) else if (conn->flags & FSNOTIFY_OBJ_TYPE_VFSMOUNT)
@ -128,31 +132,20 @@ void fsnotify_recalc_mask(struct fsnotify_mark_connector *conn)
if (!conn) if (!conn)
return; return;
if (conn->flags & FSNOTIFY_OBJ_TYPE_INODE) spin_lock(&conn->lock);
spin_lock(&conn->inode->i_lock);
else
spin_lock(&conn->mnt->mnt_root->d_lock);
__fsnotify_recalc_mask(conn); __fsnotify_recalc_mask(conn);
if (conn->flags & FSNOTIFY_OBJ_TYPE_INODE) { spin_unlock(&conn->lock);
spin_unlock(&conn->inode->i_lock); if (conn->flags & FSNOTIFY_OBJ_TYPE_INODE)
__fsnotify_update_child_dentry_flags(conn->inode); __fsnotify_update_child_dentry_flags(conn->inode);
} else {
spin_unlock(&conn->mnt->mnt_root->d_lock);
}
} }
static struct inode *fsnotify_detach_from_object(struct fsnotify_mark *mark) static struct inode *fsnotify_detach_from_object(struct fsnotify_mark *mark)
{ {
struct fsnotify_mark_connector *conn; struct fsnotify_mark_connector *conn;
struct inode *inode = NULL; struct inode *inode = NULL;
spinlock_t *lock;
conn = mark->connector; conn = mark->connector;
if (conn->flags & FSNOTIFY_OBJ_TYPE_INODE) spin_lock(&conn->lock);
lock = &conn->inode->i_lock;
else
lock = &conn->mnt->mnt_root->d_lock;
spin_lock(lock);
hlist_del_init_rcu(&mark->obj_list); hlist_del_init_rcu(&mark->obj_list);
if (hlist_empty(&conn->list)) { if (hlist_empty(&conn->list)) {
if (conn->flags & FSNOTIFY_OBJ_TYPE_INODE) if (conn->flags & FSNOTIFY_OBJ_TYPE_INODE)
@ -160,7 +153,7 @@ static struct inode *fsnotify_detach_from_object(struct fsnotify_mark *mark)
} }
__fsnotify_recalc_mask(conn); __fsnotify_recalc_mask(conn);
mark->connector = NULL; mark->connector = NULL;
spin_unlock(lock); spin_unlock(&conn->lock);
return inode; return inode;
} }
@ -326,7 +319,6 @@ int fsnotify_compare_groups(struct fsnotify_group *a, struct fsnotify_group *b)
static int fsnotify_attach_connector_to_object( static int fsnotify_attach_connector_to_object(
struct fsnotify_mark_connector **connp, struct fsnotify_mark_connector **connp,
spinlock_t *lock,
struct inode *inode, struct inode *inode,
struct vfsmount *mnt) struct vfsmount *mnt)
{ {
@ -335,6 +327,7 @@ static int fsnotify_attach_connector_to_object(
conn = kmem_cache_alloc(fsnotify_mark_connector_cachep, GFP_KERNEL); conn = kmem_cache_alloc(fsnotify_mark_connector_cachep, GFP_KERNEL);
if (!conn) if (!conn)
return -ENOMEM; return -ENOMEM;
spin_lock_init(&conn->lock);
INIT_HLIST_HEAD(&conn->list); INIT_HLIST_HEAD(&conn->list);
if (inode) { if (inode) {
conn->flags = FSNOTIFY_OBJ_TYPE_INODE; conn->flags = FSNOTIFY_OBJ_TYPE_INODE;
@ -344,16 +337,13 @@ static int fsnotify_attach_connector_to_object(
conn->mnt = mnt; conn->mnt = mnt;
} }
/* /*
* Make sure 'conn' initialization is visible. Matches * cmpxchg() provides the barrier so that readers of *connp can see
* lockless_dereference() in fsnotify(). * only initialized structure
*/ */
smp_wmb(); if (cmpxchg(connp, NULL, conn)) {
spin_lock(lock); /* Someone else created list structure for us */
if (!*connp)
*connp = conn;
else
kmem_cache_free(fsnotify_mark_connector_cachep, conn); kmem_cache_free(fsnotify_mark_connector_cachep, conn);
spin_unlock(lock); }
return 0; return 0;
} }
@ -371,35 +361,30 @@ static int fsnotify_add_mark_list(struct fsnotify_mark *mark,
struct fsnotify_mark *lmark, *last = NULL; struct fsnotify_mark *lmark, *last = NULL;
struct fsnotify_mark_connector *conn; struct fsnotify_mark_connector *conn;
struct fsnotify_mark_connector **connp; struct fsnotify_mark_connector **connp;
spinlock_t *lock;
int cmp; int cmp;
int err = 0; int err = 0;
if (WARN_ON(!inode && !mnt)) if (WARN_ON(!inode && !mnt))
return -EINVAL; return -EINVAL;
if (inode) { if (inode)
connp = &inode->i_fsnotify_marks; connp = &inode->i_fsnotify_marks;
lock = &inode->i_lock; else
} else {
connp = &real_mount(mnt)->mnt_fsnotify_marks; connp = &real_mount(mnt)->mnt_fsnotify_marks;
lock = &mnt->mnt_root->d_lock;
}
if (!*connp) { if (!*connp) {
err = fsnotify_attach_connector_to_object(connp, lock, err = fsnotify_attach_connector_to_object(connp, inode, mnt);
inode, mnt);
if (err) if (err)
return err; return err;
} }
spin_lock(&mark->lock); spin_lock(&mark->lock);
spin_lock(lock);
conn = *connp; conn = *connp;
spin_lock(&conn->lock);
/* is mark the first mark? */ /* is mark the first mark? */
if (hlist_empty(&conn->list)) { if (hlist_empty(&conn->list)) {
hlist_add_head_rcu(&mark->obj_list, &conn->list); hlist_add_head_rcu(&mark->obj_list, &conn->list);
if (inode) if (inode)
__iget(inode); igrab(inode);
goto added; goto added;
} }
@ -425,7 +410,7 @@ static int fsnotify_add_mark_list(struct fsnotify_mark *mark,
added: added:
mark->connector = conn; mark->connector = conn;
out_err: out_err:
spin_unlock(lock); spin_unlock(&conn->lock);
spin_unlock(&mark->lock); spin_unlock(&mark->lock);
return err; return err;
} }
@ -449,7 +434,7 @@ int fsnotify_add_mark_locked(struct fsnotify_mark *mark,
* LOCKING ORDER!!!! * LOCKING ORDER!!!!
* group->mark_mutex * group->mark_mutex
* mark->lock * mark->lock
* inode->i_lock * mark->connector->lock
*/ */
spin_lock(&mark->lock); spin_lock(&mark->lock);
mark->flags |= FSNOTIFY_MARK_FLAG_ALIVE | FSNOTIFY_MARK_FLAG_ATTACHED; mark->flags |= FSNOTIFY_MARK_FLAG_ALIVE | FSNOTIFY_MARK_FLAG_ATTACHED;
@ -505,24 +490,19 @@ struct fsnotify_mark *fsnotify_find_mark(struct fsnotify_mark_connector *conn,
struct fsnotify_group *group) struct fsnotify_group *group)
{ {
struct fsnotify_mark *mark; struct fsnotify_mark *mark;
spinlock_t *lock;
if (!conn) if (!conn)
return NULL; return NULL;
if (conn->flags & FSNOTIFY_OBJ_TYPE_INODE) spin_lock(&conn->lock);
lock = &conn->inode->i_lock;
else
lock = &conn->mnt->mnt_root->d_lock;
spin_lock(lock);
hlist_for_each_entry(mark, &conn->list, obj_list) { hlist_for_each_entry(mark, &conn->list, obj_list) {
if (mark->group == group) { if (mark->group == group) {
fsnotify_get_mark(mark); fsnotify_get_mark(mark);
spin_unlock(lock); spin_unlock(&conn->lock);
return mark; return mark;
} }
} }
spin_unlock(lock); spin_unlock(&conn->lock);
return NULL; return NULL;
} }
@ -595,16 +575,10 @@ void fsnotify_detach_group_marks(struct fsnotify_group *group)
void fsnotify_destroy_marks(struct fsnotify_mark_connector *conn) void fsnotify_destroy_marks(struct fsnotify_mark_connector *conn)
{ {
struct fsnotify_mark *mark; struct fsnotify_mark *mark;
spinlock_t *lock;
if (!conn) if (!conn)
return; return;
if (conn->flags & FSNOTIFY_OBJ_TYPE_INODE)
lock = &conn->inode->i_lock;
else
lock = &conn->mnt->mnt_root->d_lock;
while (1) { while (1) {
/* /*
* We have to be careful since we can race with e.g. * We have to be careful since we can race with e.g.
@ -613,15 +587,15 @@ void fsnotify_destroy_marks(struct fsnotify_mark_connector *conn)
* we are holding mark reference so mark cannot be freed and * we are holding mark reference so mark cannot be freed and
* calling fsnotify_destroy_mark() more than once is fine. * calling fsnotify_destroy_mark() more than once is fine.
*/ */
spin_lock(lock); spin_lock(&conn->lock);
if (hlist_empty(&conn->list)) { if (hlist_empty(&conn->list)) {
spin_unlock(lock); spin_unlock(&conn->lock);
break; break;
} }
mark = hlist_entry(conn->list.first, struct fsnotify_mark, mark = hlist_entry(conn->list.first, struct fsnotify_mark,
obj_list); obj_list);
fsnotify_get_mark(mark); fsnotify_get_mark(mark);
spin_unlock(lock); spin_unlock(&conn->lock);
fsnotify_destroy_mark(mark, mark->group); fsnotify_destroy_mark(mark, mark->group);
fsnotify_put_mark(mark); fsnotify_put_mark(mark);
} }

3
include/linux/fsnotify_backend.h
View File

@ -201,6 +201,7 @@ struct fsnotify_group {
* inode / vfsmount gets freed. * inode / vfsmount gets freed.
*/ */
struct fsnotify_mark_connector { struct fsnotify_mark_connector {
spinlock_t lock;
#define FSNOTIFY_OBJ_TYPE_INODE 0x01 #define FSNOTIFY_OBJ_TYPE_INODE 0x01
#define FSNOTIFY_OBJ_TYPE_VFSMOUNT 0x02 #define FSNOTIFY_OBJ_TYPE_VFSMOUNT 0x02
unsigned int flags; /* Type of object [lock] */ unsigned int flags; /* Type of object [lock] */
@ -240,7 +241,7 @@ struct fsnotify_mark {
struct list_head g_list; struct list_head g_list;
/* Protects inode / mnt pointers, flags, masks */ /* Protects inode / mnt pointers, flags, masks */
spinlock_t lock; spinlock_t lock;
/* List of marks for inode / vfsmount [obj_lock] */ /* List of marks for inode / vfsmount [connector->lock] */
struct hlist_node obj_list; struct hlist_node obj_list;
/* Head of list of marks for an object [mark->lock, group->mark_mutex] */ /* Head of list of marks for an object [mark->lock, group->mark_mutex] */
struct fsnotify_mark_connector *connector; struct fsnotify_mark_connector *connector;