On 2017-10-05 12:46:34 [-0600], Logan Gunthorpe wrote:
> I still don't want this merged as it makes the code less clear, but I tested
> it and it does not work.
>
> On 05/10/17 04:51 AM, Sebastian Andrzej Siewior wrote:
>
> > @@ -451,7 +452,7 @@ static void mrpc_complete_cmd(struct switchtec_dev *stdev)
> > stuser->read_len);
> > out:
> > - complete_all(&stuser->comp);
> > + wake_up_interruptible(&stuser->cmd_comp);
>
> I think you forgot to set cmd_done here. Once I add that, it did start
> working correctly.
oh, makes sense.
> > list_del_init(&stuser->list);
> > stuser_put(stuser);
> > stdev->mrpc_busy = 0;
> > @@ -721,10 +722,11 @@ static ssize_t switchtec_dev_read(struct file *filp, char __user *data,
> > mutex_unlock(&stdev->mrpc_mutex);
> > if (filp->f_flags & O_NONBLOCK) {
> > - if (!try_wait_for_completion(&stuser->comp))
> > + if (!stuser->cmd_done)
> > return -EAGAIN;
>
> I'd probably also suggest using READ_ONCE when reading cmd_done.
Why so? There is no way the compiler could have cached the value
somehow.
> Logan
Sebastian
