linux.git/fs/notify/mark.c, branch v4.6

fsnotify: turn fsnotify reaper thread into a workqueue job

2016-02-19T00:23:24+00:00

We don't require a dedicated thread for fsnotify cleanup.  Switch it
over to a workqueue job instead that runs on the system_unbound_wq.

In the interest of not thrashing the queued job too often when there are
a lot of marks being removed, we delay the reaper job slightly when
queueing it, to allow several to gather on the list.

Signed-off-by: Jeff Layton 
Tested-by: Eryu Guan 
Reviewed-by: Jan Kara 
Cc: Eric Paris 
Signed-off-by: Andrew Morton 
Signed-off-by: Linus Torvalds

Revert "fsnotify: destroy marks with call_srcu instead of dedicated thread"

2016-02-19T00:23:24+00:00

This reverts commit c510eff6beba ("fsnotify: destroy marks with
call_srcu instead of dedicated thread").

Eryu reported that he was seeing some OOM kills kick in when running a
testcase that adds and removes inotify marks on a file in a tight loop.

The above commit changed the code to use call_srcu to clean up the
marks.  While that does (in principle) work, the srcu callback job is
limited to cleaning up entries in small batches and only once per jiffy.
It's easily possible to overwhelm that machinery with too many call_srcu
callbacks, and Eryu's reproduer did just that.

There's also another potential problem with using call_srcu here.  While
you can obviously sleep while holding the srcu_read_lock, the callbacks
run under local_bh_disable, so you can't sleep there.

It's possible when putting the last reference to the fsnotify_mark that
we'll end up putting a chain of references including the fsnotify_group,
uid, and associated keys.  While I don't see any obvious ways that that
could occurs, it's probably still best to avoid using call_srcu here
after all.

This patch reverts the above patch.  A later patch will take a different
approach to eliminated the dedicated thread here.

Signed-off-by: Jeff Layton 
Reported-by: Eryu Guan 
Tested-by: Eryu Guan 
Cc: Jan Kara 
Cc: Eric Paris 
Signed-off-by: Andrew Morton 
Signed-off-by: Linus Torvalds

fsnotify: destroy marks with call_srcu instead of dedicated thread

2016-01-15T00:00:49+00:00

At the time that this code was originally written, call_srcu didn't
exist, so this thread was required to ensure that we waited for that
SRCU grace period to settle before finally freeing the object.

It does exist now however and we can much more efficiently use call_srcu
to handle this.  That also allows us to potentially use srcu_barrier to
ensure that they are all of the callbacks have run before proceeding.
In order to conserve space, we union the rcu_head with the g_list.

This will be necessary for nfsd which will allocate marks from a
dedicated slabcache.  We have to be able to ensure that all of the
objects are destroyed before destroying the cache.  That's fairly

Signed-off-by: Jeff Layton 
Cc: Eric Paris 
Reviewed-by: Jan Kara 
Cc: "Paul E. McKenney" 
Signed-off-by: Andrew Morton 
Signed-off-by: Linus Torvalds

fsnotify: get rid of fsnotify_destroy_mark_locked()

2015-09-04T23:54:41+00:00

fsnotify_destroy_mark_locked() is subtle to use because it temporarily
releases group->mark_mutex.  To avoid future problems with this
function, split it into two.

fsnotify_detach_mark() is the part that needs group->mark_mutex and
fsnotify_free_mark() is the part that must be called outside of
group->mark_mutex.  This way it's much clearer what's going on and we
also avoid some pointless acquisitions of group->mark_mutex.

Signed-off-by: Jan Kara 
Signed-off-by: Andrew Morton 
Signed-off-by: Linus Torvalds

fsnotify: remove mark->free_list

2015-09-04T23:54:41+00:00

Free list is used when all marks on given inode / mount should be
destroyed when inode / mount is going away.  However we can free all of
the marks without using a special list with some care.

Signed-off-by: Jan Kara 
Signed-off-by: Andrew Morton 
Signed-off-by: Linus Torvalds

fsnotify: fix oops in fsnotify_clear_marks_by_group_flags()

2015-08-07T01:39:41+00:00

fsnotify_clear_marks_by_group_flags() can race with
fsnotify_destroy_marks() so that when fsnotify_destroy_mark_locked()
drops mark_mutex, a mark from the list iterated by
fsnotify_clear_marks_by_group_flags() can be freed and thus the next
entry pointer we have cached may become stale and we dereference free
memory.

Fix the problem by first moving marks to free to a special private list
and then always free the first entry in the special list.  This method
is safe even when entries from the list can disappear once we drop the
lock.

Signed-off-by: Jan Kara 
Reported-by: Ashish Sangwan 
Reviewed-by: Ashish Sangwan 
Cc: Lino Sanfilippo 
Cc: 
Signed-off-by: Andrew Morton 
Signed-off-by: Linus Torvalds

Revert "fsnotify: fix oops in fsnotify_clear_marks_by_group_flags()"

2015-07-21T23:06:53+00:00

This reverts commit a2673b6e040663bf16a552f8619e6bde9f4b9acf.

Kinglong Mee reports a memory leak with that patch, and Jan Kara confirms:

 "Thanks for report! You are right that my patch introduces a race
  between fsnotify kthread and fsnotify_destroy_group() which can result
  in leaking inotify event on group destruction.

  I haven't yet decided whether the right fix is not to queue events for
  dying notification group (as that is pointless anyway) or whether we
  should just fix the original problem differently...  Whenever I look
  at fsnotify code mark handling I get lost in the maze of locks, lists,
  and subtle differences between how different notification systems
  handle notification marks :( I'll think about it over night"

and after thinking about it, Jan says:

 "OK, I have looked into the code some more and I found another
  relatively simple way of fixing the original oops.  It will be IMHO
  better than trying to fixup this issue which has more potential for
  breakage.  I'll ask Linus to revert the fsnotify fix he already merged
  and send a new fix"

Reported-by: Kinglong Mee 
Requested-by: Jan Kara 
Cc: Andrew Morton 
Signed-off-by: Linus Torvalds

fsnotify: fix oops in fsnotify_clear_marks_by_group_flags()

2015-07-17T23:39:54+00:00

fsnotify_clear_marks_by_group_flags() can race with
fsnotify_destroy_marks() so when fsnotify_destroy_mark_locked() drops
mark_mutex, a mark from the list iterated by
fsnotify_clear_marks_by_group_flags() can be freed and we dereference free
memory in the loop there.

Fix the problem by keeping mark_mutex held in
fsnotify_destroy_mark_locked().  The reason why we drop that mutex is that
we need to call a ->freeing_mark() callback which may acquire mark_mutex
again.  To avoid this and similar lock inversion issues, we move the call
to ->freeing_mark() callback to the kthread destroying the mark.

Signed-off-by: Jan Kara 
Reported-by: Ashish Sangwan 
Suggested-by: Lino Sanfilippo 
Cc: 
Signed-off-by: Andrew Morton 
Signed-off-by: Linus Torvalds

fsnotify: remove destroy_list from fsnotify_mark

2014-12-13T20:42:53+00:00

destroy_list is used to track marks which still need waiting for srcu
period end before they can be freed.  However by the time mark is added to
destroy_list it isn't in group's list of marks anymore and thus we can
reuse fsnotify_mark->g_list for queueing into destroy_list.  This saves
two pointers for each fsnotify_mark.

Signed-off-by: Jan Kara 
Cc: Eric Paris 
Cc: Heinrich Schuchardt 
Signed-off-by: Andrew Morton 
Signed-off-by: Linus Torvalds

fsnotify: unify inode and mount marks handling

2014-12-13T20:42:53+00:00

There's a lot of common code in inode and mount marks handling.  Factor it
out to a common helper function.

Signed-off-by: Jan Kara 
Cc: Eric Paris 
Cc: Heinrich Schuchardt 
Signed-off-by: Andrew Morton 
Signed-off-by: Linus Torvalds