From: joelf@ti•com (Joel Fernandes)
To: linux-arm-kernel@lists•infradead.org
Subject: [patch 6/6] dma: edma: Provide granular accounting
Date: Fri, 18 Apr 2014 11:22:28 -0500 [thread overview]
Message-ID: <53515144.3070905@ti.com> (raw)
In-Reply-To: <alpine.DEB.2.02.1404180844330.22697@ionos.tec.linutronix.de>
Hi Thomas,
On 04/18/2014 02:03 AM, Thomas Gleixner wrote:
[..]
>>> @@ -645,7 +652,17 @@ static void edma_callback(unsigned ch_nu
>>> vchan_cookie_complete(&edesc->vdesc);
>>> edma_execute(echan);
>>> } else {
>>> + int i, n;
>>> +
>>> dev_dbg(dev, "Intermediate transfer complete on channel %d\n", ch_num);
>>> +
>>> + /* Update statistics for tx_status */
>>> + n = edesc->processed;
>>> + for (i = edesc->processed_stat; i < n; i++)
>>> + edesc->residue -= edesc->pset[i].len;
>>> + edesc->processed_stat = n;
>>> + edesc->residue_stat = edesc->residue;
>>> +
>>
>> This is a bit of a NAK since it is O(n) on every intermediate transfer
>> completion. I'm not sure of a better way to do it but I certainly don't
>> want the IRQ slowed down anymore... Is there a way to pre-calculate the
>> size of the intermediate transfer and store it somewhere for accounting?
>
> Yeah, I can do that. Adds an extra field to edma_pset, but that
> shouldnt hurt.
Sure, I guess without adding any more space than you initially did
(added a note on len element in the structure below)
then do something like:
edma_pset[edesc->processed_stat].sum - edema_pset[edesc->proccessed].sum
and subtract that from the residue in O(1).
len element in pset can be got rid off as the same sum can be used to
find it in tx_status by len = pset[i].sum - pset[i-1].sum.
>> Also another thing is I don't think processed_stat is required at all,
>> because I think you introduced it for the possibility that there might
>> be delays in processing interrupts for intermediate transfers. That is
>
> No, I introduced it to not walk the full list of psets in every
> tx_status() call. It's keeping track of the already finished slots.
Ah, ok. cool.
>> actually an impossibility because such a scenario would result in
>> catastrophic failure anyway. Because for intermediate transfer
>> interrupts shouldn't be missed, so in this case you can just do:
>>
>> for (i = 1; i <= MAX_NR_SG; i++) {
>> edesc->residue -= edesc->pset[edesc->processed - i].len;
>> }
>>
>> I think that'll work.
>
> No. The point is:
>
> submit()
>
> tx_status()
>
> We dont want to iterate through all psets when we are polling in a
> loop. So we progress processed_stat when we see a finished slot.
> And thereby we adjust edesc->residue.
Ah ok, that's certainly fast to skip all progressive updates and jump to
the current.
>>> +
>>> + /* Otherwise mark it done and update residue[_stat]. */
>>> + edesc->processed_stat++;
>>> + edesc->residue -= pset->len;
>>> + edesc->residue_stat = edesc->residue;
>>> + }
>>
>>
>> Also, just curious - doesn't calling status too aggressively result in
>> any slowness for you? Since vchan lock spinlock will be held during the
>> duration of this processing, and interrupts would be disabled causing
>> edma_callback interrupt delays possibly. I'm not saying you introduced
>> the lock or anything because status would previously also hold the lock.
>> I just haven't played status enough to know how it will behave when
>> happening concurrently with DMA transfers.
>
> Well, in the case I'm looking at (cyclic transfer) I do not care about
> the interrupt at all. I get a notification for the first incoming
> packet via the peripheral and go from there.
>
> But yes, it might cause slight delays for the interrupt in the SG
> case. Though the irq disabled region is small if you actively poll as
> we keep track of the processed slots and therefor only have one or two
> readouts to process.
>
> But as Russell pointed out, this should be rewritten by updating the
> chain on the fly and not waiting until MAX_SG has been processed.
Ok, sounds good. Thanks.
regards,
-Joel
next prev parent reply other threads:[~2014-04-18 16:22 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-04-17 14:40 [patch 0/6] dma: edma: Provide granular residue accounting Thomas Gleixner
2014-04-17 14:40 ` [patch 1/6] dma: edma: Sanitize residue reporting Thomas Gleixner
2014-04-18 0:02 ` Joel Fernandes
2014-04-17 14:40 ` [patch 2/6] dma: edma: Check the current decriptor first in tx_status() Thomas Gleixner
2014-04-17 14:40 ` [patch 3/6] dma: edma: Create private pset struct Thomas Gleixner
2014-04-17 23:56 ` Joel Fernandes
2014-04-17 14:40 ` [patch 5/6] edma: Make reading the position of active channels work Thomas Gleixner
2014-04-18 0:47 ` Joel Fernandes
2014-04-18 1:02 ` Joel Fernandes
2014-04-18 6:40 ` Thomas Gleixner
2014-04-18 9:24 ` Thomas Gleixner
2014-04-18 16:40 ` Joel Fernandes
2014-04-17 14:40 ` [patch 4/6] dma: edma: Store transfer data in edma_desc and edma_pset Thomas Gleixner
2014-04-17 14:40 ` [patch 6/6] dma: edma: Provide granular accounting Thomas Gleixner
2014-04-18 0:45 ` Joel Fernandes
2014-04-18 7:03 ` Thomas Gleixner
2014-04-18 16:22 ` Joel Fernandes [this message]
2014-04-17 20:07 ` [patch 0/6] dma: edma: Provide granular residue accounting Russell King - ARM Linux
2014-04-17 20:31 ` Thomas Gleixner
2014-04-17 20:38 ` Russell King - ARM Linux
2014-04-17 21:14 ` Thomas Gleixner
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=53515144.3070905@ti.com \
--to=joelf@ti$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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