Subject: Re: [PATCH v9 16/17] vduse: Introduce VDUSE - vDPA Device in Userspace

From: Greg KH <[email-masked]>
Date: Wed, 14 Jul 2021 08:47:36 +0200

On Wed, Jul 14, 2021 at 02:02:50PM +0800, Jason Wang wrote:
> 
> 在 2021/7/14 下午1:54, Michael S. Tsirkin 写道:
> > On Wed, Jul 14, 2021 at 01:45:39PM +0800, Jason Wang wrote:
> > > > +static int vduse_dev_msg_sync(struct vduse_dev *dev,
> > > > +			      struct vduse_dev_msg *msg)
> > > > +{
> > > > +	int ret;
> > > > +
> > > > +	init_waitqueue_head(&msg->waitq);
> > > > +	spin_lock(&dev->msg_lock);
> > > > +	msg->req.request_id = dev->msg_unique++;
> > > > +	vduse_enqueue_msg(&dev->send_list, msg);
> > > > +	wake_up(&dev->waitq);
> > > > +	spin_unlock(&dev->msg_lock);
> > > > +
> > > > +	wait_event_killable_timeout(msg->waitq, msg->completed,
> > > > +				    VDUSE_REQUEST_TIMEOUT * HZ);
> > > > +	spin_lock(&dev->msg_lock);
> > > > +	if (!msg->completed) {
> > > > +		list_del(&msg->list);
> > > > +		msg->resp.result = VDUSE_REQ_RESULT_FAILED;
> > > > +	}
> > > > +	ret = (msg->resp.result == VDUSE_REQ_RESULT_OK) ? 0 : -EIO;
> > > 
> > > I think we should mark the device as malfunction when there is a timeout and
> > > forbid any userspace operations except for the destroy aftwards for safety.
> > This looks like if one tried to run gdb on the program the behaviour
> > will change completely because kernel wants it to respond within
> > specific time. Looks like a receipe for heisenbugs.
> > 
> > Let's not build interfaces with arbitrary timeouts like that.
> > Interruptible wait exists for this very reason.
> 
> 
> The problem is. Do we want userspace program like modprobe to be stuck for
> indefinite time and expect the administrator to kill that?

Why would modprobe be stuck for forever?

Is this on the module probe path?