Re: fanotify read returns with errno == EOPENSTALE

[Jan Kara <jack@xxxxxxx>]

On Thu 20-04-17 14:33:04, Amir Goldstein wrote:
> 
> Sorry I messed up the previous patch. please try this one:
> 
> diff --git a/fs/notify/fanotify/fanotify_user.c
> b/fs/notify/fanotify/fanotify_user.c
> index 2b37f27..7864354 100644
> --- a/fs/notify/fanotify/fanotify_user.c
> +++ b/fs/notify/fanotify/fanotify_user.c
> @@ -295,6 +295,16 @@ static ssize_t fanotify_read(struct file *file,
> char __user *buf,
>                 }
> 
>                 ret = copy_event_to_user(group, kevent, buf);
> +               if (unlikely(ret == -EOPENSTALE)) {
> +                       /*
> +                        * We cannot report events with stale fd so drop it.
> +                        * Setting ret to 0 will continue the event loop and
> +                        * do the right thing if there are no more events to
> +                        * read (i.e. return bytes read, -EAGAIN or wait).
> +                        */
> +                       ret = 0;
> +               }
> +
>                 /*
>                  * Permission events get queued to wait for response.  Other
>                  * events can be destroyed now.
> @@ -305,7 +315,7 @@ static ssize_t fanotify_read(struct file *file,
> char __user *buf,
>                                 break;
>                 } else {
>  #ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS
> -                       if (ret < 0) {
> +                       if (ret <= 0) {
>                                 FANOTIFY_PE(kevent)->response = FAN_DENY;
>                                 wake_up(&group->fanotify_data.access_waitq);
>                                 break;

I don't think you want to break out of the reading loop when ret == 0 and
the code might be more readable as:

               if (!(kevent->mask & FAN_ALL_PERM_EVENTS)) {
                        fsnotify_destroy_event(group, kevent);
               } else {
#ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS
                        if (ret <= 0) {
                                FANOTIFY_PE(kevent)->response = FAN_DENY;
                                wake_up(&group->fanotify_data.access_waitq);
                        } else {
                                spin_lock(&group->notification_lock);
                                list_add_tail(&kevent->list,
                                              &group->fanotify_data.access_list);
                                spin_unlock(&group->notification_lock);
                        }
#endif
                }
                if (ret < 0)
                        break;

Hmm?

								Honza
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR