On Thu, 2019-06-20 at 16:34 -0300, Jason Gunthorpe wrote:
> On Thu, Jun 20, 2019 at 03:09:37PM -0400, Doug Ledford wrote:
> > On Fri, 2019-06-14 at 22:56 +0800, Lijun Ou wrote:
> > > From: Lang Cheng <chenglang@xxxxxxxxxx>
> > >
> > > During removing the driver, we needs to notify the roce engine to
> > > stop working immediately,and symmetrically recycle the hardware
> > > resources requested during initialization.
> > >
> > > The hardware provides a command called function clear that can
> > > package
> > > these operations,so that the driver can only focus on releasing
> > > resources that applied from the operating system.
> > > This patch implements the call of this command.
> > >
> > > Signed-off-by: Lang Cheng <chenglang@xxxxxxxxxx>
> > > Signed-off-by: Lijun Ou <oulijun@xxxxxxxxxx>
> > > V2->V3:
> > > 1. Remove other reset state operations that are not related to
> > > function clear
> >
> >
> > > + msleep(HNS_ROCE_V2_READ_FUNC_CLEAR_FLAG_INTERVAL);
> > > + end = HNS_ROCE_V2_FUNC_CLEAR_TIMEOUT_MSECS;
> > > + while (end) {
> > > + msleep(HNS_ROCE_V2_READ_FUNC_CLEAR_FLAG_FAIL_WAIT);
> > > + end -= HNS_ROCE_V2_READ_FUNC_CLEAR_FLAG_FAIL_WAIT;
> >
> >
> > > +#define HNS_ROCE_V2_FUNC_CLEAR_TIMEOUT_MSECS (249 * 2 * 100)
> > > +#define HNS_ROCE_V2_READ_FUNC_CLEAR_FLAG_INTERVAL 40
> > > +#define HNS_ROCE_V2_READ_FUNC_CLEAR_FLAG_FAIL_WAIT 20
> >
> > I absolutely despise code that does a possible *50* second blocking
> > delay using msleep. However, because I suspect that this is
> > something
> > that should *rarely* ever happen, and instead the common case is
> > that
> > the reset will proceed much faster, and because this is in the
> > teardown/shutdown path of the device where it is a little more
> > acceptable to have a blocking delay, I've taken this to for-next.
>
> I've been telling hns they have to do proper locking and wouldn't
> accept these stupid msleep loops. No kernel code should ever do that,
> it just shows the locking and concurrency model is wrong.
It's an msleep() waiting for a hardware command to complete. Waiting
synchronously for a command that has the purpose of stopping the card's
operation does not sound like an incorrect locking or concurrency model
to me. It sounds sane, albeit annoying.
--
Doug Ledford <dledford@xxxxxxxxxx>
GPG KeyID: B826A3330E572FDD
Fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD
Attachment:
signature.asc
Description: This is a digitally signed message part
