linux-kernel - Re: [PATCH 1/4] scatterlist: Introduce some helper functions

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, 03 Mar 2016 20:15:42 +0100
From:	Robert Jarzmik <robert.jarzmik@...e.fr>
To:	Baolin Wang <baolin.wang@...aro.org>
Cc:	herbert@...dor.apana.org.au, davem@...emloft.net, agk@...hat.com,
	snitzer@...hat.com, axboe@...com, dm-devel@...hat.com,
	akpm@...ux-foundation.org, david.s.gordon@...el.com,
	thomas.lendacky@....com, yamada.masahiro@...ionext.com,
	smueller@...onox.de, tadeusz.struk@...el.com,
	standby24x7@...il.com, shli@...nel.org, broonie@...nel.org,
	linus.walleij@...aro.org, arnd@...db.de,
	linux-kernel@...r.kernel.org, linux-crypto@...r.kernel.org,
	linux-raid@...r.kernel.org
Subject: Re: [PATCH 1/4] scatterlist: Introduce some helper functions

Baolin Wang <baolin.wang@...aro.org> writes:

> @@ -212,6 +212,37 @@ static inline void sg_unmark_end(struct scatterlist *sg)
>  }
>  
>  /**
> + * sg_is_contiguous - Check if the scatterlists are contiguous
> + * @sga: SG entry
> + * @sgb: SG entry
> + *
> + * Description:
> + *   If the sga scatterlist is contiguous with the sgb scatterlist,
> + *   that means they can be merged together.
> + *
> + **/
> +static inline bool sg_is_contiguous(struct scatterlist *sga,
> +				    struct scatterlist *sgb)
> +{
> +	return ((sga->page_link & ~0x3UL) + sga->offset + sga->length ==
> +		(sgb->page_link & ~0x3UL));
> +}
I don't understand that one.
sga->page_link is a pointer to a "struct page *". How can it be added to an
offset within a page ???

> @@ -370,6 +370,65 @@ int sg_alloc_table(struct sg_table *table, unsigned int
> nents, gfp_t gfp_mask)
...
>  /**
> + * sg_add_sg_to_table - Add one scatterlist into sg table
> + * @sgt:	The sg table header to use
> + * @src:	The sg need to be added into sg table
> + *
> + * Description:
> + *   The 'nents' member indicates how many scatterlists added in the sg table.
> + *   Copy the @src@ scatterlist into sg table and increase 'nents' member.
> + *
> + **/
> +int sg_add_sg_to_table(struct sg_table *sgt, struct scatterlist *src)
> +{
> +	unsigned int i = 0, orig_nents = sgt->orig_nents;
> +	struct scatterlist *sgl = sgt->sgl;
> +	struct scatterlist *sg;
> +
> +	/* Check if there are enough space for the new sg to be added */
> +	if (sgt->nents >= sgt->orig_nents)
> +		return -EINVAL;
I must admit I don't understand that one either : how do comparing the number of
"mapped" entries against the number of "allocated" entries determines if there
is enough room ?

> +/**
> + * sg_alloc_empty_table - Allocate one empty sg table
> + * @sgt:	The sg table header to use
> + * @nents:	Number of entries in sg list
> + * @gfp_mask:	GFP allocation mask
> + *
> + *  Description:
> + *    Allocate and initialize an sg table. The 'nents' member of sg_table
> + *    indicates how many scatterlists added in the sg table. It should set
> + *    0 which means there are no scatterlists added in this sg table now.
> + *
> + **/
> +int sg_alloc_empty_table(struct sg_table *sgt, unsigned int nents,
> +			 gfp_t gfp_mask)
As for this one, there has to be a purpose for it I fail to see. From far away
it looks exactly like sg_alloc_table(), excepting it "works around" the nents >
0 protection of __sg_alloc_table().
What is exactly the need for this one, and if it's usefull why not simply
changing the __sg_alloc_table() "nents > 0" test and see what the outcome of the
review will be ?

Cheers.

-- 
Robert