diff mbox

[v2,08/20] fsnotify: generalize iteration of marks by object type

Message ID 1522934301-6520-9-git-send-email-amir73il@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Amir Goldstein April 5, 2018, 1:18 p.m. UTC
Make some code that handles marks of object types inode and vfsmount
generic, so it can handle other object types.

Introduce foreach_obj_type macros to iterate marks by object type
with or without consulting a type mask.

This is going to be used for adding mark of another object type
(super block mark).

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/notify/fsnotify.c             | 54 ++++++++++++++++++++++++----------------
 fs/notify/mark.c                 | 23 +++++++++++------
 include/linux/fsnotify_backend.h | 38 +++++++++++++++++++++-------
 3 files changed, 76 insertions(+), 39 deletions(-)

Comments

Jan Kara April 17, 2018, 5:01 p.m. UTC | #1
On Thu 05-04-18 16:18:09, Amir Goldstein wrote:
> Make some code that handles marks of object types inode and vfsmount
> generic, so it can handle other object types.
> 
> Introduce foreach_obj_type macros to iterate marks by object type
> with or without consulting a type mask.
> 
> This is going to be used for adding mark of another object type
> (super block mark).
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  fs/notify/fsnotify.c             | 54 ++++++++++++++++++++++++----------------
>  fs/notify/mark.c                 | 23 +++++++++++------
>  include/linux/fsnotify_backend.h | 38 +++++++++++++++++++++-------
>  3 files changed, 76 insertions(+), 39 deletions(-)
> 
> diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
> index 665f681dd913..1759121db50d 100644
> --- a/fs/notify/fsnotify.c
> +++ b/fs/notify/fsnotify.c
> @@ -263,21 +263,31 @@ static struct fsnotify_mark *fsnotify_next_mark(struct fsnotify_mark *mark)
>  /*
>   * iter_info is a multi head priority queue of marks.
>   * fsnotify_iter_first() picks a subset of marks from queue heads, all with the
> - * same priority and clears the type_mask for other marks.
> + * same group and clears the type_mask for other marks.
>   * Returns the type_mask of the chosen subset.
>   */
>  static unsigned int fsnotify_iter_first(struct fsnotify_iter_info *iter_info)
>  {
> -	struct fsnotify_mark *inode_mark = iter_info->inode_mark;
> -	struct fsnotify_mark *vfsmount_mark = iter_info->vfsmount_mark;
> -
> -	if (inode_mark && vfsmount_mark) {
> -		int cmp = fsnotify_compare_groups(inode_mark->group,
> -						  vfsmount_mark->group);
> -		if (cmp > 0)
> -			iter_info->type_mask &= ~FSNOTIFY_OBJ_TYPE_INODE_FL;
> -		else if (cmp < 0)
> -			iter_info->type_mask &= ~FSNOTIFY_OBJ_TYPE_VFSMOUNT_FL;
> +	struct fsnotify_group *max_prio_group = NULL;
> +	unsigned int type_mask = iter_info->type_mask;
> +	struct fsnotify_mark *mark;
> +	int type;
> +
> +	if (!type_mask || is_power_of_2(type_mask))
> +		return type_mask;
> +
> +	/* Choose max prio group among groups of all queue heads */
> +	fsnotify_iter_foreach_obj_type_mark(iter_info, type, mark) {

Does this macro really bring any benefit over:

	fsnotify_foreach_obj_type(type) {
		mark = iter_info->marks[type];

Frankly the second makes it clearer what's going on and you don't have to
wonder whether unset types are also included or not... What might be worth it
is something like fsnotify_iter_foreach_mark() which would iterate over all
*set* marks. But then there's the question whether you are iterating over
all types set in the mask or all types set in the ->marks array so probably
that will be error prone.

> diff --git a/fs/notify/mark.c b/fs/notify/mark.c
> index ef44808b28ca..3df2d5ecf0b7 100644
> --- a/fs/notify/mark.c
> +++ b/fs/notify/mark.c
> @@ -294,12 +294,12 @@ static void fsnotify_put_mark_wake(struct fsnotify_mark *mark)
>  
>  bool fsnotify_prepare_user_wait(struct fsnotify_iter_info *iter_info)
>  {
> -	/* This can fail if mark is being removed */
> -	if (!fsnotify_get_mark_safe(iter_info->inode_mark))
> -		return false;
> -	if (!fsnotify_get_mark_safe(iter_info->vfsmount_mark)) {
> -		fsnotify_put_mark_wake(iter_info->inode_mark);
> -		return false;
> +	int type;
> +
> +	fsnotify_foreach_obj_type(type) {
> +		/* This can fail if mark is being removed */
> +		if (!fsnotify_get_mark_safe(iter_info->marks[type]))
> +			goto fail;
>  	}
>  
>  	/*
> @@ -310,13 +310,20 @@ bool fsnotify_prepare_user_wait(struct fsnotify_iter_info *iter_info)
>  	srcu_read_unlock(&fsnotify_mark_srcu, iter_info->srcu_idx);
>  
>  	return true;
> +
> +fail:
> +	while (type-- > 0)

Can you please rewrite it as:

	for (type--; type >= 0; type--)

I know I'm lame but your version made me think for a while whether index 0
and the last index will be actually treated correctly. And we had bugs like
this in the past...

> +		fsnotify_put_mark_wake(iter_info->marks[type]);
> +	return false;
>  }
>  

								Honza
Amir Goldstein April 17, 2018, 7:11 p.m. UTC | #2
On Tue, Apr 17, 2018 at 8:01 PM, Jan Kara <jack@suse.cz> wrote:
> On Thu 05-04-18 16:18:09, Amir Goldstein wrote:
>> Make some code that handles marks of object types inode and vfsmount
>> generic, so it can handle other object types.
>>
>> Introduce foreach_obj_type macros to iterate marks by object type
>> with or without consulting a type mask.
>>
>> This is going to be used for adding mark of another object type
>> (super block mark).
>>
>> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
>> ---
>>  fs/notify/fsnotify.c             | 54 ++++++++++++++++++++++++----------------
>>  fs/notify/mark.c                 | 23 +++++++++++------
>>  include/linux/fsnotify_backend.h | 38 +++++++++++++++++++++-------
>>  3 files changed, 76 insertions(+), 39 deletions(-)
>>
>> diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
>> index 665f681dd913..1759121db50d 100644
>> --- a/fs/notify/fsnotify.c
>> +++ b/fs/notify/fsnotify.c
>> @@ -263,21 +263,31 @@ static struct fsnotify_mark *fsnotify_next_mark(struct fsnotify_mark *mark)
>>  /*
>>   * iter_info is a multi head priority queue of marks.
>>   * fsnotify_iter_first() picks a subset of marks from queue heads, all with the
>> - * same priority and clears the type_mask for other marks.
>> + * same group and clears the type_mask for other marks.
>>   * Returns the type_mask of the chosen subset.
>>   */
>>  static unsigned int fsnotify_iter_first(struct fsnotify_iter_info *iter_info)
>>  {
>> -     struct fsnotify_mark *inode_mark = iter_info->inode_mark;
>> -     struct fsnotify_mark *vfsmount_mark = iter_info->vfsmount_mark;
>> -
>> -     if (inode_mark && vfsmount_mark) {
>> -             int cmp = fsnotify_compare_groups(inode_mark->group,
>> -                                               vfsmount_mark->group);
>> -             if (cmp > 0)
>> -                     iter_info->type_mask &= ~FSNOTIFY_OBJ_TYPE_INODE_FL;
>> -             else if (cmp < 0)
>> -                     iter_info->type_mask &= ~FSNOTIFY_OBJ_TYPE_VFSMOUNT_FL;
>> +     struct fsnotify_group *max_prio_group = NULL;
>> +     unsigned int type_mask = iter_info->type_mask;
>> +     struct fsnotify_mark *mark;
>> +     int type;
>> +
>> +     if (!type_mask || is_power_of_2(type_mask))
>> +             return type_mask;
>> +
>> +     /* Choose max prio group among groups of all queue heads */
>> +     fsnotify_iter_foreach_obj_type_mark(iter_info, type, mark) {
>
> Does this macro really bring any benefit over:
>
>         fsnotify_foreach_obj_type(type) {
>                 mark = iter_info->marks[type];

You mean:
       mark = fsnotify_iter_type_mark(info, type, mark);

Which is non NULL only if type was selected.
Needs a better name??
How about fsnotify_iter_test_type_mark() to counter
fsnotify_iter_set_type_mark()?

>
> Frankly the second makes it clearer what's going on and you don't have to
> wonder whether unset types are also included or not... What might be worth it
> is something like fsnotify_iter_foreach_mark() which would iterate over all
> *set* marks. But then there's the question whether you are iterating over
> all types set in the mask or all types set in the ->marks array so probably
> that will be error prone.

I had such a macro in an earlier version, but it was quite ugly so I let it go.
I'll get rid of fsnotify_iter_foreach_obj_type_mark() and see if it looks nicer.

>
>> diff --git a/fs/notify/mark.c b/fs/notify/mark.c
>> index ef44808b28ca..3df2d5ecf0b7 100644
>> --- a/fs/notify/mark.c
>> +++ b/fs/notify/mark.c
>> @@ -294,12 +294,12 @@ static void fsnotify_put_mark_wake(struct fsnotify_mark *mark)
>>
>>  bool fsnotify_prepare_user_wait(struct fsnotify_iter_info *iter_info)
>>  {
>> -     /* This can fail if mark is being removed */
>> -     if (!fsnotify_get_mark_safe(iter_info->inode_mark))
>> -             return false;
>> -     if (!fsnotify_get_mark_safe(iter_info->vfsmount_mark)) {
>> -             fsnotify_put_mark_wake(iter_info->inode_mark);
>> -             return false;
>> +     int type;
>> +
>> +     fsnotify_foreach_obj_type(type) {
>> +             /* This can fail if mark is being removed */
>> +             if (!fsnotify_get_mark_safe(iter_info->marks[type]))
>> +                     goto fail;
>>       }
>>
>>       /*
>> @@ -310,13 +310,20 @@ bool fsnotify_prepare_user_wait(struct fsnotify_iter_info *iter_info)
>>       srcu_read_unlock(&fsnotify_mark_srcu, iter_info->srcu_idx);
>>
>>       return true;
>> +
>> +fail:
>> +     while (type-- > 0)
>
> Can you please rewrite it as:
>
>         for (type--; type >= 0; type--)
>
> I know I'm lame but your version made me think for a while whether index 0
> and the last index will be actually treated correctly. And we had bugs like
> this in the past...
>

Sure.

Thanks,
Amir.
Jan Kara April 18, 2018, 8:34 a.m. UTC | #3
On Tue 17-04-18 22:11:04, Amir Goldstein wrote:
> On Tue, Apr 17, 2018 at 8:01 PM, Jan Kara <jack@suse.cz> wrote:
> > On Thu 05-04-18 16:18:09, Amir Goldstein wrote:
> >> Make some code that handles marks of object types inode and vfsmount
> >> generic, so it can handle other object types.
> >>
> >> Introduce foreach_obj_type macros to iterate marks by object type
> >> with or without consulting a type mask.
> >>
> >> This is going to be used for adding mark of another object type
> >> (super block mark).
> >>
> >> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> >> ---
> >>  fs/notify/fsnotify.c             | 54 ++++++++++++++++++++++++----------------
> >>  fs/notify/mark.c                 | 23 +++++++++++------
> >>  include/linux/fsnotify_backend.h | 38 +++++++++++++++++++++-------
> >>  3 files changed, 76 insertions(+), 39 deletions(-)
> >>
> >> diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
> >> index 665f681dd913..1759121db50d 100644
> >> --- a/fs/notify/fsnotify.c
> >> +++ b/fs/notify/fsnotify.c
> >> @@ -263,21 +263,31 @@ static struct fsnotify_mark *fsnotify_next_mark(struct fsnotify_mark *mark)
> >>  /*
> >>   * iter_info is a multi head priority queue of marks.
> >>   * fsnotify_iter_first() picks a subset of marks from queue heads, all with the
> >> - * same priority and clears the type_mask for other marks.
> >> + * same group and clears the type_mask for other marks.
> >>   * Returns the type_mask of the chosen subset.
> >>   */
> >>  static unsigned int fsnotify_iter_first(struct fsnotify_iter_info *iter_info)
> >>  {
> >> -     struct fsnotify_mark *inode_mark = iter_info->inode_mark;
> >> -     struct fsnotify_mark *vfsmount_mark = iter_info->vfsmount_mark;
> >> -
> >> -     if (inode_mark && vfsmount_mark) {
> >> -             int cmp = fsnotify_compare_groups(inode_mark->group,
> >> -                                               vfsmount_mark->group);
> >> -             if (cmp > 0)
> >> -                     iter_info->type_mask &= ~FSNOTIFY_OBJ_TYPE_INODE_FL;
> >> -             else if (cmp < 0)
> >> -                     iter_info->type_mask &= ~FSNOTIFY_OBJ_TYPE_VFSMOUNT_FL;
> >> +     struct fsnotify_group *max_prio_group = NULL;
> >> +     unsigned int type_mask = iter_info->type_mask;
> >> +     struct fsnotify_mark *mark;
> >> +     int type;
> >> +
> >> +     if (!type_mask || is_power_of_2(type_mask))
> >> +             return type_mask;
> >> +
> >> +     /* Choose max prio group among groups of all queue heads */
> >> +     fsnotify_iter_foreach_obj_type_mark(iter_info, type, mark) {
> >
> > Does this macro really bring any benefit over:
> >
> >         fsnotify_foreach_obj_type(type) {
> >                 mark = iter_info->marks[type];
> 
> You mean:
>        mark = fsnotify_iter_type_mark(info, type, mark);
> 
> Which is non NULL only if type was selected.

Hum, probably but it seems a bit confusing (and thus error prone in the
long term). So how about making fsnotify_iter_next() just advancing mark
pointers based on report_mask (to move over marks that were already
reported) and then clearing report_mask.
fsnotify_iter_select_report_types() would the walk over iter_info->marks
array, consider all non-NULL entries for reporting, select among them
those with the highest priority, and set report_mask appropriately.
That way we won't even need fsnotify_iter_set_{inode|vfsmount}_mark()
macros.

> Needs a better name??
> How about fsnotify_iter_test_type_mark() to counter
> fsnotify_iter_set_type_mark()?

If there are not many places, that need fsnotify_iter_type_mark() (which
currently it seems there are not), then I'd just make it
fsnotify_iter_should_report_type(iter, type) which returns bool - whether
that type should be reported to or not. And then just explicitely use
iter->marks[type] if type should be reported. IMO fsnotify_iter_type_mark()
does not offer much advantage over this as you have to test for mark being
NULL anyway.

If there are many places that need fsnotify_iter_type_mark(), then I guess
we could provide iteration only over types that should be reported. Does
that sound good?

								Honza
Amir Goldstein April 18, 2018, 12:31 p.m. UTC | #4
On Wed, Apr 18, 2018 at 11:34 AM, Jan Kara <jack@suse.cz> wrote:
> On Tue 17-04-18 22:11:04, Amir Goldstein wrote:
>> On Tue, Apr 17, 2018 at 8:01 PM, Jan Kara <jack@suse.cz> wrote:
>> > On Thu 05-04-18 16:18:09, Amir Goldstein wrote:
>> >> Make some code that handles marks of object types inode and vfsmount
>> >> generic, so it can handle other object types.
>> >>
>> >> Introduce foreach_obj_type macros to iterate marks by object type
>> >> with or without consulting a type mask.
>> >>
>> >> This is going to be used for adding mark of another object type
>> >> (super block mark).
>> >>
>> >> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
>> >> ---
>> >>  fs/notify/fsnotify.c             | 54 ++++++++++++++++++++++++----------------
>> >>  fs/notify/mark.c                 | 23 +++++++++++------
>> >>  include/linux/fsnotify_backend.h | 38 +++++++++++++++++++++-------
>> >>  3 files changed, 76 insertions(+), 39 deletions(-)
>> >>
>> >> diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
>> >> index 665f681dd913..1759121db50d 100644
>> >> --- a/fs/notify/fsnotify.c
>> >> +++ b/fs/notify/fsnotify.c
>> >> @@ -263,21 +263,31 @@ static struct fsnotify_mark *fsnotify_next_mark(struct fsnotify_mark *mark)
>> >>  /*
>> >>   * iter_info is a multi head priority queue of marks.
>> >>   * fsnotify_iter_first() picks a subset of marks from queue heads, all with the
>> >> - * same priority and clears the type_mask for other marks.
>> >> + * same group and clears the type_mask for other marks.
>> >>   * Returns the type_mask of the chosen subset.
>> >>   */
>> >>  static unsigned int fsnotify_iter_first(struct fsnotify_iter_info *iter_info)
>> >>  {
>> >> -     struct fsnotify_mark *inode_mark = iter_info->inode_mark;
>> >> -     struct fsnotify_mark *vfsmount_mark = iter_info->vfsmount_mark;
>> >> -
>> >> -     if (inode_mark && vfsmount_mark) {
>> >> -             int cmp = fsnotify_compare_groups(inode_mark->group,
>> >> -                                               vfsmount_mark->group);
>> >> -             if (cmp > 0)
>> >> -                     iter_info->type_mask &= ~FSNOTIFY_OBJ_TYPE_INODE_FL;
>> >> -             else if (cmp < 0)
>> >> -                     iter_info->type_mask &= ~FSNOTIFY_OBJ_TYPE_VFSMOUNT_FL;
>> >> +     struct fsnotify_group *max_prio_group = NULL;
>> >> +     unsigned int type_mask = iter_info->type_mask;
>> >> +     struct fsnotify_mark *mark;
>> >> +     int type;
>> >> +
>> >> +     if (!type_mask || is_power_of_2(type_mask))
>> >> +             return type_mask;
>> >> +
>> >> +     /* Choose max prio group among groups of all queue heads */
>> >> +     fsnotify_iter_foreach_obj_type_mark(iter_info, type, mark) {
>> >
>> > Does this macro really bring any benefit over:
>> >
>> >         fsnotify_foreach_obj_type(type) {
>> >                 mark = iter_info->marks[type];
>>
>> You mean:
>>        mark = fsnotify_iter_type_mark(info, type, mark);
>>
>> Which is non NULL only if type was selected.
>
> Hum, probably but it seems a bit confusing (and thus error prone in the
> long term). So how about making fsnotify_iter_next() just advancing mark
> pointers based on report_mask (to move over marks that were already
> reported) and then clearing report_mask.

OK, but no need to clear report_mask in fsnotify_iter_next(), because
select_report_types will recalculate the report mask.

> fsnotify_iter_select_report_types() would the walk over iter_info->marks
> array, consider all non-NULL entries for reporting, select among them
> those with the highest priority, and set report_mask appropriately.
> That way we won't even need fsnotify_iter_set_{inode|vfsmount}_mark()
> macros.

Right.

>
>> Needs a better name??
>> How about fsnotify_iter_test_type_mark() to counter
>> fsnotify_iter_set_type_mark()?
>
> If there are not many places, that need fsnotify_iter_type_mark() (which
> currently it seems there are not), then I'd just make it
> fsnotify_iter_should_report_type(iter, type) which returns bool - whether
> that type should be reported to or not. And then just explicitely use
> iter->marks[type] if type should be reported. IMO fsnotify_iter_type_mark()
> does not offer much advantage over this as you have to test for mark being
> NULL anyway.
>

I left convenience macros:
fsnotify_iter_should_report_type(iter, type)
fsnotify_iter_set_report_type(iter, type)
fsnotify_iter_set_report_type_mark(iter, type, mark)

> If there are many places that need fsnotify_iter_type_mark(), then I guess
> we could provide iteration only over types that should be reported. Does
> that sound good?
>

Yes, result looks much better IMO.
Rabased on your for_test branch and push to
https://github.com/amir73il/linux/commits/fanotify_sb

Thanks!
Amir.
diff mbox

Patch

diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
index 665f681dd913..1759121db50d 100644
--- a/fs/notify/fsnotify.c
+++ b/fs/notify/fsnotify.c
@@ -263,21 +263,31 @@  static struct fsnotify_mark *fsnotify_next_mark(struct fsnotify_mark *mark)
 /*
  * iter_info is a multi head priority queue of marks.
  * fsnotify_iter_first() picks a subset of marks from queue heads, all with the
- * same priority and clears the type_mask for other marks.
+ * same group and clears the type_mask for other marks.
  * Returns the type_mask of the chosen subset.
  */
 static unsigned int fsnotify_iter_first(struct fsnotify_iter_info *iter_info)
 {
-	struct fsnotify_mark *inode_mark = iter_info->inode_mark;
-	struct fsnotify_mark *vfsmount_mark = iter_info->vfsmount_mark;
-
-	if (inode_mark && vfsmount_mark) {
-		int cmp = fsnotify_compare_groups(inode_mark->group,
-						  vfsmount_mark->group);
-		if (cmp > 0)
-			iter_info->type_mask &= ~FSNOTIFY_OBJ_TYPE_INODE_FL;
-		else if (cmp < 0)
-			iter_info->type_mask &= ~FSNOTIFY_OBJ_TYPE_VFSMOUNT_FL;
+	struct fsnotify_group *max_prio_group = NULL;
+	unsigned int type_mask = iter_info->type_mask;
+	struct fsnotify_mark *mark;
+	int type;
+
+	if (!type_mask || is_power_of_2(type_mask))
+		return type_mask;
+
+	/* Choose max prio group among groups of all queue heads */
+	fsnotify_iter_foreach_obj_type_mark(iter_info, type, mark) {
+		if (mark &&
+		    fsnotify_compare_groups(max_prio_group, mark->group) > 0)
+			max_prio_group = mark->group;
+	}
+
+	/* Clear the type mask for marks with other groups */
+	fsnotify_iter_foreach_obj_type_mark(iter_info, type, mark) {
+		if (mark &&
+		    fsnotify_compare_groups(max_prio_group, mark->group) != 0)
+			iter_info->type_mask &= ~(1U << type);
 	}
 
 	return iter_info->type_mask;
@@ -289,17 +299,17 @@  static unsigned int fsnotify_iter_first(struct fsnotify_iter_info *iter_info)
  */
 static void fsnotify_iter_next(struct fsnotify_iter_info *iter_info)
 {
-	if (iter_info->type_mask & FSNOTIFY_OBJ_TYPE_INODE_FL)
-		fsnotify_iter_set_inode_mark(iter_info,
-			fsnotify_next_mark(iter_info->inode_mark));
-	else if (iter_info->inode_mark)
-		iter_info->type_mask |= FSNOTIFY_OBJ_TYPE_INODE_FL;
-
-	if (iter_info->type_mask & FSNOTIFY_OBJ_TYPE_VFSMOUNT_FL)
-		fsnotify_iter_set_vfsmount_mark(iter_info,
-			fsnotify_next_mark(iter_info->vfsmount_mark));
-	else if (iter_info->vfsmount_mark)
-		iter_info->type_mask |= FSNOTIFY_OBJ_TYPE_VFSMOUNT_FL;
+	int type;
+
+	fsnotify_foreach_obj_type(type) {
+		unsigned int flag = 1U << type;
+
+		if (iter_info->type_mask & flag)
+			fsnotify_iter_set_type_mark(iter_info, type,
+				fsnotify_next_mark(iter_info->marks[type]));
+		else if (iter_info->marks[type])
+			iter_info->type_mask |= flag;
+	}
 }
 
 /*
diff --git a/fs/notify/mark.c b/fs/notify/mark.c
index ef44808b28ca..3df2d5ecf0b7 100644
--- a/fs/notify/mark.c
+++ b/fs/notify/mark.c
@@ -294,12 +294,12 @@  static void fsnotify_put_mark_wake(struct fsnotify_mark *mark)
 
 bool fsnotify_prepare_user_wait(struct fsnotify_iter_info *iter_info)
 {
-	/* This can fail if mark is being removed */
-	if (!fsnotify_get_mark_safe(iter_info->inode_mark))
-		return false;
-	if (!fsnotify_get_mark_safe(iter_info->vfsmount_mark)) {
-		fsnotify_put_mark_wake(iter_info->inode_mark);
-		return false;
+	int type;
+
+	fsnotify_foreach_obj_type(type) {
+		/* This can fail if mark is being removed */
+		if (!fsnotify_get_mark_safe(iter_info->marks[type]))
+			goto fail;
 	}
 
 	/*
@@ -310,13 +310,20 @@  bool fsnotify_prepare_user_wait(struct fsnotify_iter_info *iter_info)
 	srcu_read_unlock(&fsnotify_mark_srcu, iter_info->srcu_idx);
 
 	return true;
+
+fail:
+	while (type-- > 0)
+		fsnotify_put_mark_wake(iter_info->marks[type]);
+	return false;
 }
 
 void fsnotify_finish_user_wait(struct fsnotify_iter_info *iter_info)
 {
+	int type;
+
 	iter_info->srcu_idx = srcu_read_lock(&fsnotify_mark_srcu);
-	fsnotify_put_mark_wake(iter_info->inode_mark);
-	fsnotify_put_mark_wake(iter_info->vfsmount_mark);
+	fsnotify_foreach_obj_type(type)
+		fsnotify_put_mark_wake(iter_info->marks[type]);
 }
 
 /*
diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
index 62e05b8a9bc7..2f8c4255e679 100644
--- a/include/linux/fsnotify_backend.h
+++ b/include/linux/fsnotify_backend.h
@@ -211,34 +211,54 @@  enum fsnotify_obj_type {
 #define FSNOTIFY_OBJ_ALL_TYPES_MASK	((1U << FSNOTIFY_OBJ_TYPE_MAX) - 1)
 
 struct fsnotify_iter_info {
-	struct fsnotify_mark *inode_mark;
-	struct fsnotify_mark *vfsmount_mark;
+	struct fsnotify_mark *marks[FSNOTIFY_OBJ_TYPE_MAX];
 	unsigned int type_mask;
 	int srcu_idx;
 };
 
+static inline struct fsnotify_mark *fsnotify_iter_type_mark(
+		struct fsnotify_iter_info *iter_info, int type)
+{
+	return (iter_info->type_mask & (1U << type)) ?
+		iter_info->marks[type] : NULL;
+}
+
+static inline void fsnotify_iter_set_type_mark(
+		struct fsnotify_iter_info *iter_info,
+		int type, struct fsnotify_mark *mark)
+{
+	iter_info->marks[type] = mark;
+	if (mark)
+		iter_info->type_mask |= (1U << type);
+	else
+		iter_info->type_mask &= ~(1U << type);
+}
+
 #define FSNOTIFY_ITER_FUNCS(name, NAME) \
 static inline struct fsnotify_mark *fsnotify_iter_##name##_mark( \
 		struct fsnotify_iter_info *iter_info) \
 { \
-	return (iter_info->type_mask & FSNOTIFY_OBJ_TYPE_##NAME##_FL) ? \
-		iter_info->name##_mark : NULL; \
+	return fsnotify_iter_type_mark(iter_info, FSNOTIFY_OBJ_TYPE_##NAME); \
 } \
 \
 static inline void fsnotify_iter_set_##name##_mark( \
 		struct fsnotify_iter_info *iter_info, \
 		struct fsnotify_mark *mark) \
 { \
-	iter_info->name##_mark = mark; \
-	if (mark) \
-		iter_info->type_mask |= FSNOTIFY_OBJ_TYPE_##NAME##_FL; \
-	else \
-		iter_info->type_mask &= ~FSNOTIFY_OBJ_TYPE_##NAME##_FL; \
+	fsnotify_iter_set_type_mark(iter_info, FSNOTIFY_OBJ_TYPE_##NAME, mark); \
 }
 
 FSNOTIFY_ITER_FUNCS(inode, INODE)
 FSNOTIFY_ITER_FUNCS(vfsmount, VFSMOUNT)
 
+#define fsnotify_foreach_obj_type(type) \
+	for (type = 0; type < FSNOTIFY_OBJ_TYPE_MAX; type++)
+
+#define fsnotify_iter_foreach_obj_type_mark(iter, type, mark) \
+	for (type = 0, mark = fsnotify_iter_type_mark(iter, type); \
+	     type < FSNOTIFY_OBJ_TYPE_MAX; \
+	     type++, mark = fsnotify_iter_type_mark(iter, type)) \
+
 /*
  * Inode / vfsmount point to this structure which tracks all marks attached to
  * the inode / vfsmount. The reference to inode / vfsmount is held by this