Re: fanotify read returns with errno == EOPENSTALE

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Apr 20, 2017 at 6:06 PM, Amir Goldstein <amir73il@xxxxxxxxx> wrote:
> On Thu, Apr 20, 2017 at 5:20 PM, Jan Kara <jack@xxxxxxx> wrote:
>> 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?
>>
>

On Fri, Apr 21, 2017 at 5:27 PM, Marko Rauhamaa
<marko.rauhamaa@xxxxxxxxxxxx> wrote:
> Amir Goldstein <amir73il@xxxxxxxxx>:
>
>> Did you notice Jan's comments on my patch? I had a bug that broke out
>> of the loop. Without his corrections read will return even if there
>> are more events in the queue.
>
> Yes, I now tried Jan's fix, and it did the trick.
>
> It will now take months or years before distros have a proper fix. In
> the interim, I will absorb EOPENSTALE and schedule a reread in that
> situation.
>
> Thank you both for your attention.
>
>

Marko,

Were you able to verify that both blocking and non-blocking mode work correctly?
May I add your Tested-by and Reported-by tags?

Thanks!
Amir.
--
To unsubscribe from this list: send the line "unsubscribe linux-api" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux