mailbox: forward the hrtimer if not queued and under a lock

This reverts commit c7dacf5b0f32957b24ef29df1207dc2cd8307743, "mailbox: avoid timer start from callback" The previous commit was reverted since it lead to a race that caused the hrtimer to not be started at all. The check for hrtimer_active() in msg_submit() will return true if the callback function txdone_hrtimer() is currently running. This function could return HRTIMER_NORESTART and then the timer will not be restarted, and also msg_submit() will not start the timer. This will lead to a message actually being submitted but no timer will start to check for its compleation. The original fix that added checking hrtimer_active() was added to avoid a warning with hrtimer_forward. Looking in the kernel another solution to avoid this warning is to check hrtimer_is_queued() before calling hrtimer_forward_now() instead. This however requires a lock so the timer is not started by msg_submit() inbetween this check and the hrtimer_forward() call.

Björn Ardö committed 4 years ago

Commit bca1a101 parent c25f7782 files · +14 −6Browse files

Unified · Split

2 files

drivers/mailbox/mailbox.c +13 −6
include/linux/mailbox_controller.h +1 −0

drivers/mailbox/mailbox.c+13−6

@@ -82,11 +82,11 @@ static void msg_submit(struct mbox_chan *chan) · 81 unmodified lines
82	82	exit:
83	83	spin_unlock_irqrestore(&chan->lock, flags);
84	84
85		− /* kick start the timer immediately to avoid delays */
86	85	if (!err && (chan->txdone_method & TXDONE_BY_POLL)) {
87		− /* but only if not already active */
88		− if (!hrtimer_active(&chan->mbox->poll_hrt))
89		− hrtimer_start(&chan->mbox->poll_hrt, 0, HRTIMER_MODE_REL);
	86	+ /* kick start the timer immediately to avoid delays */
	87	+ spin_lock_irqsave(&chan->mbox->poll_hrt_lock, flags);
	88	+ hrtimer_start(&chan->mbox->poll_hrt, 0, HRTIMER_MODE_REL);
	89	+ spin_unlock_irqrestore(&chan->mbox->poll_hrt_lock, flags);
90	90	}
91	91	}
92	92
@@ -120,20 +120,26 @@ static enum hrtimer_restart txdone_hrtimer(struct hrtimer *hrtimer) · 27 unmodified lines
120	120	container_of(hrtimer, struct mbox_controller, poll_hrt);
121	121	bool txdone, resched = false;
122	122	int i;
	123	+ unsigned long flags;
123	124
124	125	for (i = 0; i < mbox->num_chans; i++) {
125	126	struct mbox_chan *chan = &mbox->chans[i];
126	127
127	128	if (chan->active_req && chan->cl) {
128		− resched = true;
129	129	txdone = chan->mbox->ops->last_tx_done(chan);
130	130	if (txdone)
131	131	tx_tick(chan, 0);
	132	+ else
	133	+ resched = true;
132	134	}
133	135	}
134	136
135	137	if (resched) {
136		− hrtimer_forward_now(hrtimer, ms_to_ktime(mbox->txpoll_period));
	138	+ spin_lock_irqsave(&mbox->poll_hrt_lock, flags);
	139	+ if (!hrtimer_is_queued(hrtimer))
	140	+ hrtimer_forward_now(hrtimer, ms_to_ktime(mbox->txpoll_period));
	141	+ spin_unlock_irqrestore(&mbox->poll_hrt_lock, flags);
	142	+
137	143	return HRTIMER_RESTART;
138	144	}
139	145	return HRTIMER_NORESTART;
@@ -500,6 +506,7 @@ int mbox_controller_register(struct mbox_controller *mbox) · 360 unmodified lines
500	506	hrtimer_init(&mbox->poll_hrt, CLOCK_MONOTONIC,
501	507	HRTIMER_MODE_REL);
502	508	mbox->poll_hrt.function = txdone_hrtimer;
	509	+ spin_lock_init(&mbox->poll_hrt_lock);
503	510	}
504	511
505	512	for (i = 0; i < mbox->num_chans; i++) {

include/linux/mailbox_controller.h+1−0

@@ -83,6 +83,7 @@ struct mbox_controller { · 82 unmodified lines
83	83	const struct of_phandle_args *sp);
84	84	/* Internal to API */
85	85	struct hrtimer poll_hrt;
	86	+ spinlock_t poll_hrt_lock;
86	87	struct list_head node;
87	88	};
88	89