[Skiboot] [PATCH 1/2] ipmi/opal: Provide ipmi_dequeue_msg() wrapper and invoke it on completion

[Benjamin Herrenschmidt]

On Sat, 2015-06-27 at 10:17 +0530, Neelesh Gupta wrote:
> Hi Alistair,
> 
> On 06/26/2015 12:46 PM, Alistair Popple wrote:
> 
> > Hi,
> > 
> > On Fri, 26 Jun 2015 12:11:38 Neelesh Gupta wrote:
> > 
> > <snip>
> > 
> > > Why should client forget doing it ? If it does so, then of course 
> > > programming error
> > > and should be fixed.
> > Yes, but there are no circumstances that I know of in which the client should 
> > not dequeue the message so why introduce the possibility of these types of 
> > errors?
> 
> May be client wants to do something different with ipmi_cmd_done() ->
> msg->error()
> case, like giving a 'retry'... but my point was more from the
> interface point of view that
> looked nicer to me..

It doesn't make any sense. The queuing is internal to the backend, there
is no driver on earth that leaves the client the job of dequeuing a
request that is completed. There would be no point. Even in the case
of a "retry", that would simply be sent back to the back of the queue.

> > > In line with, when client does alloc(), it has to free() and when it 
> > > does queue_msg(),
> > > it should be doing dequeue_msg() after completion or whenever it thinks 
> > > doing so..
> > > IMO backend should not decide when to dequeue the message.
> > > 
> > > Moreover, backend may want to do more that just list_del() in 
> > > dequeue_msg() ..
> > > like updating the state or number of outstanding requests etc.. that all 
> > > can nicely
> > > be exposed though an interface.
> > Why can't you do that with the current interface? You can just call 
> > ipmi_cmd_done() to notify the client and once that returns the backend can 
> > update whatever flags, etc. it needs...
> > 
> > > > I agree that there is currently no API to dequeue an ipmi message (mainly
> > > > because no one has needed it yet) but it should just be a thin wrapper in
> > > > core/ipmi.c that calls backend->dequeue_msg().
> > > I think that's what I added as part of this patch in core/ipmi.c
> > Yes, that bit is fine and should be submitted as a separate patch adding just 
> > that.
> 
> It contradicts.. in one hand backend dequeing the message its own, on
> the other
> hand providing a client interface to dequeue the message ...
> so we should rather delete the existing dequeue_msg() callback for the
> sake of
> client not having any impression of dequeuing the message and that
> will be taken
> care by all the backend drivers..

There is one use for the generic dequeue which is to cancel a request
that's already in the queue.

Ben.

> Regards,
> Neelesh.
> 
> > Regards,
> > 
> > Alistair
> > 
> > > Thanks,
> > > Neelesh.
> > > 
> > > > - Alistair
> > > > 
> > > > On Wed, 24 Jun 2015 22:40:32 Neelesh Gupta wrote:
> > > > 
> 
> _______________________________________________
> Skiboot mailing list
> Skiboot at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/skiboot