On Thu, Nov 05, 2020 at 01:45:16AM +0100, Andrew Lunn wrote: > > +/* Manage all NAPI instances for the control interface. > > + * > > + * We only have one RX queue and one Tx Conf queue for all > > + * switch ports. Therefore, we only need to enable the NAPI instance once, > > the > > + * first time one of the switch ports runs .dev_open(). > > + */ > > + > > +static void dpaa2_switch_enable_ctrl_if_napi(struct ethsw_core *ethsw) > > +{ > > + int i; > > + > > + /* a new interface is using the NAPI instance */ > > + ethsw->napi_users++; > > + > > + /* if there is already a user of the instance, return */ > > + if (ethsw->napi_users > 1) > > + return; > > Does there need to be any locking here? Or does it rely on RTNL? > Maybe a comment would be nice, or a check that RTNL is actually held. >
It relies on the RTNL. I'll add an assert on the RTNL lock and a comment to go with that. > > + > > + if (!dpaa2_switch_has_ctrl_if(ethsw)) > > + return; > > + > > + for (i = 0; i < DPAA2_SWITCH_RX_NUM_FQS; i++) > > + napi_enable(ðsw->fq[i].napi); > > +} > > > +static void dpaa2_switch_rx(struct dpaa2_switch_fq *fq, > > + const struct dpaa2_fd *fd) > > +{ > > + struct ethsw_core *ethsw = fq->ethsw; > > + struct ethsw_port_priv *port_priv; > > + struct net_device *netdev; > > + struct vlan_ethhdr *hdr; > > + struct sk_buff *skb; > > + u16 vlan_tci, vid; > > + int if_id = -1; > > + int err; > > + > > + /* prefetch the frame descriptor */ > > + prefetch(fd); > > Does this actually do any good, given that the next call: > > > + > > + /* get switch ingress interface ID */ > > + if_id = upper_32_bits(dpaa2_fd_get_flc(fd)) & 0x0000FFFF; > > is accessing the frame descriptor? The idea of prefetch is to let it > bring it into the cache while you are busy doing something else, > hopefully with something which is already cache hot. > I'll check w and w/o the prefetch but, most probably, it doesn't help. Thanks. Ioana