[lkml]   [2016]   [Jul]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [RFC] [PATCH v2 1/3] scatterlist: Add support to clone scatterlist

On 07/06/2016 12:46 PM, Andy Shevchenko wrote:
> On Wed, Jul 6, 2016 at 8:09 PM, Franklin S Cooper Jr. <> wrote:
>>>> + last_entry = sg_is_last(sgl);
>>>> + *sgl = *orig_sgl;
>>>> +
>>>> + /*
>>>> + * If page_link is pointing to a chained sgl then set it to
>>>> + * zero since we already compensated for chained sgl. If
>>>> + * page_link is pointing to a page then clear bits 1 and 0.
>>>> + */
>>>> + if (sg_is_chain(orig_sgl))
>>>> + sgl->page_link = 0x0;
>>>> + else
>>>> + sgl->page_link &= ~0x03;
>>>> +
>>>> + if (last_entry) {
>>>> + sg_dma_len(sgl) = len;
>>> Hmm... If it's not DMA mapped and CONFIG_NEED_SG_DMA_LENGTH=y, you
>>> will set dma_length field and len otherwise. Is it on purpose?
>> Thanks for pointing this out. I didn't take into consideration the above
>> scenario.
>> After looking at this more it seems like on occasion dma_length !=
>> length. I see this occurring in various map_sg functions for several
>> architectures.
>> I'm simply trying to reduce the overall sgl length by a certain amount.
>> I'm not sure what the proper approach would be since dma_length and
>> length can be set to different values. Simply setting sgl->dma_length to
>> sgl->length or vice a versa seems like it can be problematic in some
>> scenarios. What confuses me even more is if dma_length != length then
>> how can sg_nents_for_len only use the sgl length property.
>> Any thoughts on what would be the best way to handle this?
> First come to my mind is to refuse to clone if SG table is DMA mapped.
> Imagine the scenario where you have for example last entry which your
> caller asked to split as
> total_entry_len = 37235 (input)
> dma_length = 65536 (64k alignment)

Do you know of an example where dma_length will be set to some kind of
alignment size rather than based on the number of bytes you want
transfered? Or maybe I'm miss understanding this.

> asked_len = 32768 (Split chunks: 32768 + 4467)
> resulting_dma_len = ?
> What do you put here? Initially it perhaps means the same 64k. But how
> to distinguish?
> (I couldn't come with better one, but there are many alike scenarios)
> Only hack we have is to supply struct device of the DMA device to this
> SG table where you can get max segment size (but I dunno about
> alignment).
> P.S. I'm not familiar of the details of all these.

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.

 \ /
  Last update: 2016-07-06 22:21    [W:0.053 / U:2.612 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site