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: <CAHp75VcPLV-483ihn_RmHnDOc0iFMSdYj5_SrNevyqrrpeWatQ@mail.gmail.com>
Date:	Wed, 6 Jul 2016 20:46:57 +0300
From:	Andy Shevchenko <andy.shevchenko@...il.com>
To:	"Franklin S Cooper Jr." <fcooper@...com>
Cc:	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 Wed, Jul 6, 2016 at 8:09 PM, Franklin S Cooper Jr. <fcooper@...com> 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)

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.

-- 
With Best Regards,
Andy Shevchenko

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ