Re: [PATCH v2 01/10] mailbox: Support blocking transfers in atomic context

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Nov 28, 2018 at 3:43 AM Jon Hunter <jonathanh@xxxxxxxxxx> wrote:
>
>
> On 12/11/2018 15:18, Thierry Reding wrote:
> > From: Thierry Reding <treding@xxxxxxxxxx>
> >
> > The mailbox framework supports blocking transfers via completions for
> > clients that can sleep. In order to support blocking transfers in cases
> > where the transmission is not permitted to sleep, add a new ->flush()
> > callback that controller drivers can implement to busy loop until the
> > transmission has been completed. This will automatically be called when
> > available and interrupts are disabled for clients that request blocking
> > transfers.
> >
> > Signed-off-by: Thierry Reding <treding@xxxxxxxxxx>
> > ---
> >  drivers/mailbox/mailbox.c          | 8 ++++++++
> >  include/linux/mailbox_controller.h | 4 ++++
> >  2 files changed, 12 insertions(+)
> >
> > diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
> > index 674b35f402f5..0eaf21259874 100644
> > --- a/drivers/mailbox/mailbox.c
> > +++ b/drivers/mailbox/mailbox.c
> > @@ -267,6 +267,14 @@ int mbox_send_message(struct mbox_chan *chan, void *mssg)
> >               unsigned long wait;
> >               int ret;
> >
> > +             if (irqs_disabled() && chan->mbox->ops->flush) {
> > +                     ret = chan->mbox->ops->flush(chan, chan->cl->tx_tout);
> > +                     if (ret < 0)
> > +                             tx_tick(chan, ret);
> > +
> > +                     return ret;
> > +             }
>
> It seems to me that if mbox_send_message() is called from an atomic
> context AND tx_block is true, then if 'flush' is not populated this
> should be an error condition as we do not wish to call
> wait_for_completion from an atomic context.
>
> I understand that there is some debate about adding such flush support,
> but irrespective of the above change, it seems to me that if the
> mbox_send_message() can be called from an atomic context (which it
> appears to), then it should be detecting if someone is trying to do so
> with 'tx_block' set as this should be an error.
>
Layers within kernel space have to trust each other. A badly written
client can break the consumer in so many ways, we can not catch every
possibility.

> Furthermore, if the underlying mailbox driver can support sending a
> message from an atomic context and busy wait until it is done, surely
> the mailbox framework should provide a means to support this?
>
Being able to put the message on bus in atomic context is a feature -
which we do support. But busy-waiting in a loop is not a feature, and
we don't want to encourage that.

Cheers!



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux