On Mon, 2016-07-18 at 18:39 +0000, Lars-Peter Clausen wrote:
> On 07/18/2016 06:52 PM, Jiang, Dave wrote:
> > On Sun, 2016-07-17 at 20:37 +0200, Lars-Peter Clausen wrote:
> > > On 07/15/2016 10:17 PM, Dave Jiang wrote:
> > > > The following series implements a way for DMA drivers to report
> > > > the
> > > > result
> > > > of a transaction via callback in order to notify the upper
> > > > layer.
> > > > Thanks
> > > > Lars for providing guidance.
> > > >
> > > > I added usage of the new mechanism in the ioatdma driver and
> > > > also
> > > > updated
> > > > the NTB code that uses this mechanism to receive the error.
> > > > Jon, I
> > > > assume
> > > > you'll let Vinod push the two NTB patches since it's part of
> > > > the
> > > > series.
> > >
> > > This mostly looks good. A few things I've noticed in a couple of
> > > patches.
> > >
> > > The dmaengine_desc_callback fields should not be directly
> > > accessed,
> > > it
> > > should be fully transparent to drives. The whole idea behind the
> > > stuct and
> > > its helper functions is to separate functionality and
> > > implementation,
> > > so
> > > that the implementation can be changed without affecting
> > > functionality.
> > >
> > > E.g. a common pattern where this happens is
> > >
> > > if (cb.callback) {
> > > spin_unlock(...);
> > > dmaengine_desc_callback_invoke(&cb);
> > > spin_lock(...);
> > > }
> > >
> > > This will break if the client uses callback_result instead of
> > > callback. The
> > > easiest way to resolve this is probably to add a helper function
> > > that
> > > checks
> > > if the callback should be called.
> >
> > Ok I'll do that. Mainly I was trying to preserve the original code
> > and
> > do the minimal alteration but I don't think the check is really
> > necessary since invoke checks as well.
>
> Some drivers have to unlock a spinlock before calling the callback
> and then
> relock it once the callback finished and they don't want to go
> through this
> if there is not callback.
>
Yes. I'm adding a helper function for those instances that involve
spinlocks. ��.n��������+%������w��{.n��������)�)�� jg��� �����ݢj����G���� ���j:+v���w�m������w�������h�����٥
