Re: [PATCH 03/31] fs: introduce new ->get_poll_head and ->poll_mask methods

[Christoph Hellwig]

 On Wed, Jan 10, 2018 at 04:31:35PM +0000, Al Viro wrote:
> *snort*
> 
> Seeing that random drivers are, by far, the majority of instances...
> What I wonder is how many of them conform to that pattern and how
> many can be massaged to that form.
> 
> How painful would it be, to pick an instance with more than one wait
> queue involved, to convert drivers/char/random.c to that form?

Attached.  Not very painful at all.  Would be even nicer if we had a
wait_event version that can deal with keys, but I can look into that.

> static unsigned int vtpm_proxy_fops_poll(struct file *filp, poll_table *wait)
> {
>         struct proxy_dev *proxy_dev = filp->private_data;
>         unsigned ret;
> 
>         poll_wait(filp, &proxy_dev->wq, wait);
> 
>         ret = POLLOUT;
> 
>         mutex_lock(&proxy_dev->buf_lock);
> 
>         if (proxy_dev->req_len)
>                 ret |= POLLIN | POLLRDNORM;
> 
>         if (!(proxy_dev->state & STATE_OPENED_FLAG))
>                 ret |= POLLHUP;
> 
>         mutex_unlock(&proxy_dev->buf_lock);
> 
>         return ret;
> } 
> (mainline drivers/char/tpm/tpm_vtpm_proxy.c)

Yeah.  And what exactly is the lock protecting given that each
of them checks a single smaller than register sized variable?

> Is that mutex_lock() in there a bug?  Another fun case is dma_buf_poll()...

Right now it is not a bug, but it is not very helpful behavior.  We
actually have quite a few of those, and not allowing ->poll_mask to
is a good thing to catch these.