linux-kernel - Re: [PATCH 2/4] mtd: sh_flctl: drop unused variable

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]

Message-ID: <3139913.OhcnyJWC3N@avalon>
Date:	Mon, 04 May 2015 10:53:28 +0300
From:	Laurent Pinchart <laurent.pinchart@...asonboard.com>
To:	Nicholas Mc Guire <der.herr@...r.at>
Cc:	Vinod Koul <vinod.koul@...el.com>,
	Nicholas Mc Guire <hofrat@...dl.org>,
	Kuninori Morimoto <kuninori.morimoto.gx@...esas.com>,
	David Woodhouse <dwmw2@...radead.org>,
	Brian Norris <computersforpeace@...il.com>,
	Wolfram Sang <wsa@...-dreams.de>,
	Arnd Bergmann <arnd@...db.de>, linux-mtd@...ts.infradead.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/4] mtd: sh_flctl: drop unused variable

Hi Nicholas,

On Monday 04 May 2015 08:03:46 Nicholas Mc Guire wrote:
> On Mon, 04 May 2015, Vinod Koul wrote:
> > On Sun, May 03, 2015 at 10:33:43PM +0300, Laurent Pinchart wrote:
> > > On Saturday 02 May 2015 09:57:08 Nicholas Mc Guire wrote:
> > > > shdma_tx_submit() called via dmaengine_submit() returns the assigned
> > > > cookie but this is not used here so the variable and assignment can
> > > > be dropped.
> > > > 
> > > > Signed-off-by: Nicholas Mc Guire <hofrat@...dl.org>
> > > 
> > > I would rephrase the commit message to avoid mentioning
> > > shdma_tx_submit() as that's not relevant. Something like
> > > "dmaengine_submit() returns the assigned cookie but this is not used
> > > here so the variable and assignment can be dropped."
> > 
> > And I am bit surrised about taht. Ideally the driver should use the cookie
> > to check the status later on...
> 
> looking at other drivers it seems like the drivers should call
> dma_submit_error(cookie); on the received cookie - which does:
>   return cookie < 0 ? cookie : 0;
> but doing that after dmaengine_submit() which actually already queued the
> this request in shdma_base.cc:shdma_tx_submit()

Don't take shdma into account. There's no guarantee that the DMA engine will 
be an SH DMAC on all platforms where the flctl driver will be used. 
Furthermore, the shdma implementation might change in the future. You should 
consider the DMA engine API only and comply with its requirements.

> might not be that helpful
> and looking at dma_cookie_assign() I do not see how the condition that
> dma_submit_error is checking for ever could occur as it can't go below
> cookie = DMA_MIN_COOKIE which is defined to 1 (include/linux/dmaengine.h)
> 
> As other drivers seem to not be doing more with the returned cookie than
> calling dma_submit_error() on it this seems ok here ...but I'm not that
> deep into this - my starting point was a simple API inconsisteny in the
> use of wait_for_completion_timeout() :)

-- 
Regards,

Laurent Pinchart

--
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/