dmaengine.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Vinod Koul <vkoul@kernel.org>
To: Arnd Bergmann <arnd@arndb.de>
Cc: "Linus Walleij" <linus.walleij@linaro.org>,
	"Allen Pais" <apais@linux.microsoft.com>,
	"Vincent Guittot" <vincent.guittot@linaro.org>,
	olivier.dautricourt@orolia.com, "Stefan Roese" <sr@denx.de>,
	"Kees Cook" <keescook@chromium.org>,
	linux-hardening@vger.kernel.org,
	"Ludovic Desroches" <ludovic.desroches@microchip.com>,
	"Tudor Ambarus" <tudor.ambarus@microchip.com>,
	"Florian Fainelli" <f.fainelli@gmail.com>,
	"Ray Jui" <rjui@broadcom.com>,
	"Scott Branden" <sbranden@broadcom.com>,
	bcm-kernel-feedback-list <bcm-kernel-feedback-list@broadcom.com>,
	"Nicolas Saenz Julienne" <nsaenz@kernel.org>,
	"Paul Cercueil" <paul@crapouillou.net>,
	Eugeniy.Paltsev@synopsys.com,
	"Gustavo Pimentel" <gustavo.pimentel@synopsys.com>,
	"Viresh Kumar" <vireshk@kernel.org>,
	"Andy Shevchenko" <andriy.shevchenko@linux.intel.com>,
	"Leo Li" <leoyang.li@nxp.com>,
	zw@zh-kernel.org, "Zhou Wang" <wangzhou1@hisilicon.com>,
	"Shawn Guo" <shawnguo@kernel.org>,
	"Sascha Hauer" <s.hauer@pengutronix.de>,
	"Sean Wang" <sean.wang@mediatek.com>,
	"Matthias Brugger" <matthias.bgg@gmail.com>,
	"Andreas Färber" <afaerber@suse.de>,
	"Manivannan Sadhasivam" <mani@kernel.org>,
	"Logan Gunthorpe" <logang@deltatee.com>,
	"Sanjay R Mehta" <sanju.mehta@amd.com>,
	"Daniel Mack" <daniel@zonque.org>,
	"Haojian Zhuang" <haojian.zhuang@gmail.com>,
	"Robert Jarzmik" <robert.jarzmik@free.fr>,
	"Andy Gross" <agross@kernel.org>,
	"Bjorn Andersson" <bjorn.andersson@linaro.org>,
	"Krzysztof Kozlowski" <krzysztof.kozlowski@linaro.org>,
	green.wan@sifive.com, "Orson Zhai" <orsonzhai@gmail.com>,
	"Baolin Wang" <baolin.wang7@gmail.com>,
	"Lyra Zhang" <zhang.lyra@gmail.com>,
	"Patrice CHOTARD" <patrice.chotard@foss.st.com>,
	"Chen-Yu Tsai" <wens@csie.org>,
	"Jernej Škrabec" <jernej.skrabec@gmail.com>,
	"Samuel Holland" <samuel@sholland.org>,
	dmaengine@vger.kernel.org,
	"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>
Subject: Re: [RFC 1/1] drivers/dma/*: replace tasklets with workqueue
Date: Fri, 27 May 2022 13:36:16 +0530	[thread overview]
Message-ID: <YpCGePbo9B/Z7slV@matsya> (raw)
In-Reply-To: <CAK8P3a0j_rziihsgHnG5bHMxmPbOkAhT6_+CCE4iFZy7HzQrLw@mail.gmail.com>

On 25-05-22, 13:03, Arnd Bergmann wrote:
> On Wed, May 25, 2022 at 11:24 AM Linus Walleij <linus.walleij@linaro.org> wrote:
> > On Tue, Apr 19, 2022 at 11:17 PM Allen Pais <apais@linux.microsoft.com> wrote:
> >
> > > The tasklet is an old API which will be deprecated, workqueue API
> > > cab be used instead of them.
> > >
> > > This patch replaces the tasklet usage in drivers/dma/* with a
> > > simple work.
> > >
> > > Github: https://github.com/KSPP/linux/issues/94
> > >
> > > Signed-off-by: Allen Pais <apais@linux.microsoft.com>
> >
> > Paging Vincent Guittot and Arnd Bergmann on the following question
> > on this patch set:
> >
> > - Will replacing tasklets with workque like this negatively impact the
> >   performance on DMA engine bottom halves?
> 
> I think it will in some cases but not others. The problem I see is that
> the short patch description makes it sound like a trivial conversion of a
> single subsystem, but in reality this interacts with all the drivers using
> DMA engines, including tty/serial, sound, mmc and spi.
> 
> In many cases, the change is an improvement, but I can see a number
> of ways this might go wrong:
> 
> - for audio, waiting to schedule the workqueue task may add enough
>   latency to lead to audible skips
> 
> - for serial, transferring a few characters through DMA is probably
>   more expensive now than using MMIO, which might mean that
>   there may no longer be a point in using DMA in the first place.
> 
> - Some drivers such as dw_mmc schedule another tasklet from the
>   callback. If the tasklet is turned into a workqueue, this becomes
>   a bit pointless unless we change the called drivers first.

Yes and there are assumptions in the peripheral drivers about the
context of callback which right now is tasklet, that needs to be updated
as well..

> What might work better in the case of the dmaengine API would
> be an approach like:
> 
> 1. add helper functions to call the callback functions from a
>     tasklet locally defined in drivers/dma/dmaengine.c to allow
>     deferring it from hardirq context
> 
> 2. Change all  tasklets that are not part of the callback
>     mechanism to work queue functions, I only see
>     xilinx_dpdma_chan_err_task in the patch, but there
>     may be more
> 
> 3. change all drivers to move their custom tasklets back into
>     hardirq context and instead call the new helper for deferring
>     the callback.
> 
> 4. Extend the dmaengine callback API to let slave drivers
>     pick hardirq, tasklet or task context for the callback.
>     task context can mean either a workqueue, or a threaded
>     IRQ here, with the default remaining the tasklet version.

That does sound a good idea, but I dont know who will use the workqueue
or a threaded context here, it might be that most would default to
hardirq or tasklet context for obvious reasons...

> 
> 5. Change slave drivers to pick either hardirq or task context
>     depending on their requirements
> 
> 6. Remove the tasklet version.
> 
> This is of course a lot more complicated than Allen's
> approach, but I think the end result would be much better.
> 
>          Arnd

-- 
~Vinod

  parent reply	other threads:[~2022-05-27  8:07 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-19 21:16 [RFC 0/1] refactor all tasklet users into other APIs Allen Pais
2022-04-19 21:16 ` [RFC 1/1] drivers/dma/*: replace tasklets with workqueue Allen Pais
2022-04-20  2:31   ` Kees Cook
2022-04-22 16:48     ` Allen Pais
2022-04-25 15:06   ` Linus Walleij
2022-04-25 23:59     ` Allen Pais
2022-04-25 15:56   ` Krzysztof Kozlowski
2022-04-25 19:55     ` Gustavo A. R. Silva
2022-04-28  9:29       ` Andy Shevchenko
     [not found]         ` <DA101ED8-F99F-4DCB-9CB7-370A62C44B65@linux.microsoft.com>
2022-05-12 21:54           ` Linus Walleij
2022-05-16 11:40             ` Vinod Koul
2022-04-26  0:04     ` Allen Pais
2022-04-27  2:45   ` Vinod Koul
2022-04-27 15:20     ` Dave Jiang
2022-04-27 15:58       ` Allen Pais
2022-04-27 15:53     ` Allen Pais
2022-05-25  9:24   ` Linus Walleij
2022-05-25  9:52     ` Vincent Guittot
2022-05-25 11:03     ` Arnd Bergmann
2022-05-25 15:14       ` David Laight
     [not found]         ` <6E248F41-6687-4F2B-B847-DB5459BA1344@linux.microsoft.com>
2022-05-25 17:48           ` Arnd Bergmann
2022-05-25 18:04             ` Allen Pais
2022-05-27  8:06       ` Vinod Koul [this message]
2022-05-27 10:59         ` Arnd Bergmann
2022-05-31  6:56           ` Vinod Koul
     [not found]             ` <0A9EDEDC-9E6C-47F8-89C0-48DCDD3F9DE3@linux.microsoft.com>
2022-05-31 18:02               ` Arnd Bergmann
2022-05-31 18:19                 ` Allen Pais
2022-05-31 19:27                   ` Arnd Bergmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YpCGePbo9B/Z7slV@matsya \
    --to=vkoul@kernel.org \
    --cc=Eugeniy.Paltsev@synopsys.com \
    --cc=afaerber@suse.de \
    --cc=agross@kernel.org \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=apais@linux.microsoft.com \
    --cc=arnd@arndb.de \
    --cc=baolin.wang7@gmail.com \
    --cc=bcm-kernel-feedback-list@broadcom.com \
    --cc=bjorn.andersson@linaro.org \
    --cc=daniel@zonque.org \
    --cc=dmaengine@vger.kernel.org \
    --cc=f.fainelli@gmail.com \
    --cc=green.wan@sifive.com \
    --cc=gustavo.pimentel@synopsys.com \
    --cc=haojian.zhuang@gmail.com \
    --cc=jernej.skrabec@gmail.com \
    --cc=keescook@chromium.org \
    --cc=krzysztof.kozlowski@linaro.org \
    --cc=leoyang.li@nxp.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=logang@deltatee.com \
    --cc=ludovic.desroches@microchip.com \
    --cc=mani@kernel.org \
    --cc=matthias.bgg@gmail.com \
    --cc=nsaenz@kernel.org \
    --cc=olivier.dautricourt@orolia.com \
    --cc=orsonzhai@gmail.com \
    --cc=patrice.chotard@foss.st.com \
    --cc=paul@crapouillou.net \
    --cc=rjui@broadcom.com \
    --cc=robert.jarzmik@free.fr \
    --cc=s.hauer@pengutronix.de \
    --cc=samuel@sholland.org \
    --cc=sanju.mehta@amd.com \
    --cc=sbranden@broadcom.com \
    --cc=sean.wang@mediatek.com \
    --cc=shawnguo@kernel.org \
    --cc=sr@denx.de \
    --cc=tudor.ambarus@microchip.com \
    --cc=vincent.guittot@linaro.org \
    --cc=vireshk@kernel.org \
    --cc=wangzhou1@hisilicon.com \
    --cc=wens@csie.org \
    --cc=zhang.lyra@gmail.com \
    --cc=zw@zh-kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).