Re: [PATCH bpf-next v4 3/7] bpf: support epoll from bpf struct_ops links.

[Martin KaFai Lau <martin.lau@linux.dev>]

On 5/23/24 11:24 AM, Kui-Feng Lee wrote:
> 
> 
> On 5/23/24 10:23, Martin KaFai Lau wrote:
>> On 5/21/24 3:51 PM, Kui-Feng Lee wrote:
>>> +static __poll_t bpf_link_poll(struct file *file, struct poll_table_struct *pts)
>>> +{
>>> +    struct bpf_link *link = file->private_data;
>>> +
>>> +    if (link->ops->poll)
>>> +        return link->ops->poll(file, pts);
>>> +
>>> +    return 0;
>>
>> The current bpf_link_fops.poll is NULL before this patch. From vfs_poll, it 
>> seems to be DEFAULT_POLLMASK for this case. Please double check.
> 
> 
> Yes, it returns DEFAULT_POLLMASK if file->f_op->epoll is NULL. But,
> before this patch, link can not be added to an epoll. See the
> explanation below.

How about select() and poll() that do not need epoll_ctl() setup?

> 
>>
>>> +}
>>> +
>>>   static const struct file_operations bpf_link_fops = {
>>>   #ifdef CONFIG_PROC_FS
>>>       .show_fdinfo    = bpf_link_show_fdinfo,
>>> @@ -3157,6 +3167,7 @@ static const struct file_operations bpf_link_fops = {
>>>       .release    = bpf_link_release,
>>>       .read        = bpf_dummy_read,
>>>       .write        = bpf_dummy_write,
>>> +    .poll        = bpf_link_poll,
>>
>> Same here. What does the epoll_ctl(EPOLL_CTL_ADD) currently expect for link 
>> (e.g. cgroup) that does not support poll?
>>
> 
> epoll_ctl() always returns -EPERM for files not supporting poll.
> Should I add another instance of struct file_operations to keep the
> consistency for other types of links?

imo, it makes sense to have another instance for link that supports poll such 
that epoll_ctl(EPOLL_CTL_ADD) can fail early for the unsupported links.

> 
>>>   };
>>