On Sat, Jan 12, 2019 at 03:55:31PM +0800, Wei Hu (Xavier) wrote:
>
>
> On 2019/1/12 5:34, Jason Gunthorpe wrote:
> > On Thu, Jan 10, 2019 at 09:57:41PM +0800, Wei Hu (Xavier) wrote:
> >> + /* Check the status of the current software reset process, if in
> >> + * software reset process, wait until software reset process finished,
> >> + * in order to ensure that reset process and this function will not call
> >> + * __hns_roce_hw_v2_uninit_instance at the same time.
> >> + * If a timeout occurs, it indicates that the network subsystem has
> >> + * encountered a serious error and cannot be recovered from the reset
> >> + * processing.
> >> + */
> >> + if (ops->ae_dev_resetting(handle)) {
> >> + dev_warn(dev, "Device is busy in resetting state. waiting.\n");
> >> + end = msecs_to_jiffies(HNS_ROCE_V2_RST_PRC_MAX_TIME) + jiffies;
> >> + while (ops->ae_dev_resetting(handle) &&
> >> + time_before(jiffies, end))
> >> + msleep(20);
> > Really? Does this have to be so ugly? Why isn't there just a simple
> > lock someplace that is held during reset?
> >
> > I'm skeptical that all this strange looking stuff is properly locked
> > and concurrency safe.
> Hi, Jason
>
> The hns3 NIC driver notifies the hns RoCE driver to perform
> reset related processing by calling the .reset_notify() interface
> registered by the RoCE driver.
>
> There is a constraint on the hip08 chip, the NIC driver needs to
> stop the flow before hardware startup reset, otherwise the chip
> may hang up.
>
> We've also thought about using locks, but found using locks can
> lead to more serious problems because of that restriction of the
> chip.
> If using locks here, reset processing may wait for uninstallation
> to complete, this may lead that NIC driver fails to stop the flow
> in time in the reset process, thus causing the chip to hang up.
If you are sleeping then I'm sure a lock can be used instead, how
would it be any different?
Jason
