On Wed, Aug 20, 2025 at 10:37:01PM +0200, Andrew Lunn wrote:
> > +static int mucse_mbx_fw_post_req(struct mucse_hw *hw,
> > + struct mbx_fw_cmd_req *req,
> > + struct mbx_req_cookie *cookie)
> > +{
> > + int len = le16_to_cpu(req->datalen);
> > + int err;
> > +
> > + cookie->errcode = 0;
> > + cookie->done = 0;
> > + init_waitqueue_head(&cookie->wait);
> > + err = mutex_lock_interruptible(&hw->mbx.lock);
> > + if (err)
> > + return err;
> > + err = mucse_write_mbx(hw, (u32 *)req, len);
> > + if (err)
> > + goto out;
> > + err = wait_event_timeout(cookie->wait,
> > + cookie->done == 1,
> > + cookie->timeout_jiffies);
> > +
> > + if (!err)
> > + err = -ETIMEDOUT;
> > + else
> > + err = 0;
> > + if (!err && cookie->errcode)
> > + err = cookie->errcode;
> > +out:
> > + mutex_unlock(&hw->mbx.lock);
> > + return err;
>
> What is your design with respect to mutex_lock_interruptible() and
> then calling wait_event_timeout() which will ignore signals?
>
> Is your intention that you can always ^C the driver, and it will clean
> up whatever it was doing and return -EINTR? Such unwinding can be
> tricky and needs careful review. Before i do that, i just want to make
> sure this is your intention, and you yourself have carefully reviewed
> the code.
>
> Andrew
>
>
'mucse_mbx_fw_post_req' is designed can be called by 'cat /sys/xxx', So I used
xx_interruptible() before.
The design sequence is:
write_mbx with cookie ------> fw ----> dirver_irq_handler(call wake_up)
| |
V V
wait_event_xxxx -------------------> free(cookie)
But if ^C just after 'wait_event_interruptible_timeout', cookie will
be free before fw really response, a crash will happen.
cookie pointer is in mbx.req, and fw response with no change.
write_mbx with cookie ------> fw ---------> dirver_irq_handler(call wake_up)
| |
V V
wait_event_xxxx ---> free(cookie) crash with freed cookie
|
v
^C
So I use goto retry if -ERESTARTSYS with wait_event_interruptible_timeout.
And it is the same with wait_event_timeout.
If ^C in mutex_lock_interruptible, it is safe return since no write to
fw and no response from fw.
