linux-kernel - Re: [PATCH v3.5] dw_dmac: return proper residue value

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 25 Jan 2013 09:43:58 +0530
From:	Viresh Kumar <viresh.kumar@...aro.org>
To:	Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc:	Vinod Koul <vinod.koul@...el.com>, linux-kernel@...r.kernel.org,
	spear-devel <spear-devel@...t.st.com>
Subject: Re: [PATCH v3.5] dw_dmac: return proper residue value

On Thu, Jan 24, 2013 at 6:37 PM, Andy Shevchenko
<andriy.shevchenko@...ux.intel.com> wrote:
> Currently the driver returns full length of the active descriptor which is
> wrong. We have to go throught the active descriptor and substract the length of
> each sent children in the chain from the total length along with the actual
> data in the DMA channel registers.
>
> The cyclic case is not handled by this patch due to len field in the descriptor
> structure is left untouched by the original code.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>

Firstly the issue i raised earlier about not updating residue from
tasklet or interrupt
handler was wrong. As i had an older version of code in my mind. This got solved
with following patch:

commit 77bcc497c60ec62dbb84abc809a6e218d53409e9
Author: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Date:   Fri Jan 18 14:14:15 2013 +0200

    dw_dmac: move soft LLP code from tasklet to dwc_scan_descriptors


> ---
> diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
> +static inline u32 dwc_get_residue(struct dw_dma_chan *dwc)
> +{
> +       unsigned long flags;
> +       u32 residue;
> +
> +       spin_lock_irqsave(&dwc->lock, flags);
> +
> +       residue = dwc->residue;

you need to release the lock just here.

> +       if (test_bit(DW_DMA_IS_SOFT_LLP, &dwc->flags) && residue)
> +               residue -= dwc_get_sent(dwc);

why do you need lock while reading the control registers? It looks you didn't
get what i wanted to say earlier. We are using locks to protect some part for
shared data or critical sections. these are playing with dwc transfer
lists, etc..

Probably we don't need a lock to read the control register as nobody can
guarantee that another access is not happening currently. As hardware is
changing this register continuously for the transfer.

Let me know if i am missing something.

> +       spin_unlock_irqrestore(&dwc->lock, flags);
> +       return residue;
> +}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/