lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 12 Jan 2022 13:58:17 -0800 From: "Martinez, Ricardo" <ricardo.martinez@...ux.intel.com> To: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com> Cc: netdev@...r.kernel.org, linux-wireless@...r.kernel.org, kuba@...nel.org, davem@...emloft.net, johannes@...solutions.net, ryazanov.s.a@...il.com, loic.poulain@...aro.org, m.chetan.kumar@...el.com, chandrashekar.devegowda@...el.com, linuxwwan@...el.com, chiranjeevi.rapolu@...ux.intel.com, haijun.liu@...iatek.com, amir.hanania@...el.com, andriy.shevchenko@...ux.intel.com, dinesh.sharma@...el.com, eliot.lee@...el.com, mika.westerberg@...ux.intel.com, moises.veleta@...el.com, pierre-louis.bossart@...el.com, muralidharan.sethuraman@...el.com, Soumya.Prakash.Mishra@...el.com, sreehari.kancharla@...el.com Subject: Re: [PATCH net-next v3 02/12] net: wwan: t7xx: Add core components On 12/16/2021 3:55 AM, Ilpo Järvinen wrote: ... > >> + switch (reason) { >> + case EXCEPTION_HS_TIMEOUT: >> + dev_err(dev, "BOOT_HS_FAIL\n"); >> + break; >> + >> + case EXCEPTION_EVENT: >> + t7xx_fsm_broadcast_state(ctl, MD_STATE_EXCEPTION); >> + t7xx_md_exception_handshake(ctl->md); >> + >> + cnt = 0; >> + while (cnt < FSM_MD_EX_REC_OK_TIMEOUT_MS / FSM_EVENT_POLL_INTERVAL_MS) { >> + if (kthread_should_stop()) >> + return; >> + >> + spin_lock_irqsave(&ctl->event_lock, flags); >> + event = list_first_entry_or_null(&ctl->event_queue, >> + struct t7xx_fsm_event, entry); >> + if (event) { >> + if (event->event_id == FSM_EVENT_MD_EX) { >> + fsm_del_kf_event(event); >> + } else if (event->event_id == FSM_EVENT_MD_EX_REC_OK) { >> + rec_ok = true; >> + fsm_del_kf_event(event); >> + } >> + } >> + >> + spin_unlock_irqrestore(&ctl->event_lock, flags); >> + if (rec_ok) >> + break; >> + >> + cnt++; >> + /* Try again after 20ms */ >> + msleep(FSM_EVENT_POLL_INTERVAL_MS); >> + } >> + >> + cnt = 0; >> + while (cnt < FSM_MD_EX_PASS_TIMEOUT_MS / FSM_EVENT_POLL_INTERVAL_MS) { >> + if (kthread_should_stop()) >> + return; >> + >> + spin_lock_irqsave(&ctl->event_lock, flags); >> + event = list_first_entry_or_null(&ctl->event_queue, >> + struct t7xx_fsm_event, entry); >> + if (event && event->event_id == FSM_EVENT_MD_EX_PASS) { >> + pass = true; >> + fsm_del_kf_event(event); >> + } >> + >> + spin_unlock_irqrestore(&ctl->event_lock, flags); >> + >> + if (pass) >> + break; >> + cnt++; >> + /* Try again after 20ms */ >> + msleep(FSM_EVENT_POLL_INTERVAL_MS); >> + } > It hurts me a bit to see so much code duplication with only that one > extra if branch + if condition right-hand sides being different. It would > seem like something that could be solved with a helper taking those two > things as parameters. > > I hope the ordering of FSM_EVENT_MD_EX, FSM_EVENT_MD_EX_REC_OK, and > FSM_EVENT_MD_EX_PASS is guaranteed by something. Otherwise, the event > being waited for might not become the first entry in the event_queue and > the loop just keeps waiting until timeout? > Ordering is guaranteed by the modem. Removing code duplication in the next iteration. >> +void t7xx_fsm_clr_event(struct t7xx_fsm_ctl *ctl, enum t7xx_fsm_event_state event_id) >> +{ >> + struct device *dev = &ctl->md->t7xx_dev->pdev->dev; >> + struct t7xx_fsm_event *event, *evt_next; >> + unsigned long flags; >> + >> + spin_lock_irqsave(&ctl->event_lock, flags); >> + list_for_each_entry_safe(event, evt_next, &ctl->event_queue, entry) { >> + dev_err(dev, "Unhandled event %d\n", event->event_id); >> + >> + if (event->event_id == event_id) >> + fsm_del_kf_event(event); >> + } > It seems that only events matching to event_id are removed from the > event_queue. Can that dev_err print the same event over and over again > (I'm assuming here multiple calls to t7xx_fsm_clr_event can occur) because > the other events still remaining in event_queue? > The purpose of this function is just to remove an event if present, it is not relevant if there were other events in the list, so I'll remove the dev_err.
Powered by blists - more mailing lists