linux-kernel - Re: [RFC] [PATCH v2 1/3] scatterlist: Add support to clone scatterlist

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:	Thu, 7 Jul 2016 10:58:02 -0500
From:	"Franklin S Cooper Jr." <fcooper@...com>
To:	Robert Jarzmik <robert.jarzmik@...e.fr>
CC:	Andy Shevchenko <andy.shevchenko@...il.com>,
	<david.s.gordon@...el.com>, Jens Axboe <axboe@...com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Ming Lin <ming.l@....samsung.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Mark Brown <broonie@...nel.org>,
	linux-spi <linux-spi@...r.kernel.org>,
	Sekhar Nori <nsekhar@...com>,
	Peter Ujfalusi <peter.ujfalusi@...com>
Subject: Re: [RFC] [PATCH v2 1/3] scatterlist: Add support to clone
 scatterlist



On 07/06/2016 05:04 PM, Robert Jarzmik wrote:
> "Franklin S Cooper Jr." <fcooper@...com> writes:
> 
>> Unfortunately, for the original purpose of this series the scatterlist I
>> want to clone is indeed DMA mapped.
>>
>> The times I've seen dma_length != length is when the dma_map_sg is
>> trying to combine contiguous sgl (ex ppc_iommu_map_sg and dma_map_cont).
>> So maybe it is better to subtract a value from length and dma_length
>> rather than explicitly setting it to a value. This way it wouldn't
>> matter that dma_length and length aren't equal.
>>
>> This looks like a similar approach used by sg_split_mapped. Although
>> sg_split skips first x number of bytes while I want to skip the last x
>> number of bytes.
>>
>> Maybe Robert can add his thoughts since he created sg_split.
> 
> Hi,
> 
> I've not read it all, but the above chapter is right : dma_length might be
> different from length when a dma mapping operation coallesced 2 sg entries into
> a "DMA contiguous one". This coallescing might happen for at least for 2 reasons
> as far as I know :
>  - 2 sg entries are physically contiguous, ie :
>    sg_phys(sga) + sga.length == sg_phys(sgb)
>  - 2 sg entries are DMA contiguous when an IOMMU maps them contiguously, ie :
>    sg_dma_address(sga) + sga.length == sg_dma_address(sgb)
> 
> Moreover, this 2 coallescing cases imply a "shift" between (page_link, length)
> and (dma_address and dma_length). For example a mapped sglist might look like :
>  -sg0: page_link=>page@0k, length=4096, dma_address=0, dma_length=8192
>  -sg1: page_link=>page@4k, length=4096, dma_address=8192, dma_length=16
>                                                           \=> see the shift here
>                                                               coming from sg2
>  -sg2: page_link=>page@8k, length=16, dma_address=0, dma_length=0
> 
> For these "tricky" cases, at the time I created sg_split I had done a tester as
> well. It's very basic, doesn't cover all the corner cases, is a bit dumb, but
> you might have a look, and the brain cost you'll pay to adapt it to test what
> you want will hopefully pay off by the knowledge gained on scatterlist. It is
> appended at the end of the mail.


Thanks Robert for your input and sharing your test program. After
looking at sg_split and test program I realized that I can use it
instead of creating my own clone function. Seems like you and your patch
reviewers already considered and dealt with the above complex scenario.

I updated my patchset to use sg_split rather than my custom function
since it seems to solve my original problem. I plan on sending out a v3
that incorporates this change.
> 
> Cheers.
>