linux-kernel - Re: [PATCH v2 1/1] block: blk-merge: don't merge the pages with non-contiguous descriptors

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] [day] [month] [year] [list]

Date:	Thu, 17 Jan 2013 20:28:41 +0530
From:	Subhash Jadavani <subhashj@...eaurora.org>
To:	Russell King - ARM Linux <linux@....linux.org.uk>
CC:	James Bottomley <James.Bottomley@...senPartnership.com>,
	martin.petersen@...cle.com, linux-scsi@...r.kernel.org,
	linux-arm-msm@...r.kernel.org, linux-mmc@...r.kernel.org,
	linux-kernel@...r.kernel.org, Tejun Heo <tj@...nel.org>,
	asias@...hat.com, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v2 1/1] block: blk-merge: don't merge the pages with non-contiguous
 descriptors

On 1/17/2013 4:17 PM, Russell King - ARM Linux wrote:
> On Thu, Jan 17, 2013 at 10:37:42AM +0000, Russell King - ARM Linux wrote:
>> On Thu, Jan 17, 2013 at 09:11:20AM +0000, James Bottomley wrote:
>>> I'd actually prefer page = pfn_to_page(page_to_pfn(page) + 1); because
>>> it makes the code look like the hack it is.  The preferred form for all
>>> iterators like this should be to iterate over the pfn instead of a
>>> pointer into the page arrays, because that will always work correctly no
>>> matter how many weird and wonderful memory schemes we come up with.
>> So, why don't we update the code to do that then?
> Also, couldn't the addition of the scatterlist offset to the page also
> be buggy too?
>
> So, what about this patch which addresses both additions by keeping our
> iterator as a pfn as you suggest.  It also simplifies some of the code
> in the loop too.
>
> Can the original folk with the problem test this patch?

Yes, this patch also fixes the issue.  You may add: Tested-by: Subhash 
Jadavani <subhashj@...eaurora.org> .

Regards,
Subhash

>
>   arch/arm/mm/dma-mapping.c |   18 ++++++++++--------
>   1 files changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> index 6b2fb87..076c26d 100644
> --- a/arch/arm/mm/dma-mapping.c
> +++ b/arch/arm/mm/dma-mapping.c
> @@ -774,25 +774,27 @@ static void dma_cache_maint_page(struct page *page, unsigned long offset,
>   	size_t size, enum dma_data_direction dir,
>   	void (*op)(const void *, size_t, int))
>   {
> +	unsigned long pfn;
> +	size_t left = size;
> +
> +	pfn = page_to_pfn(page) + offset / PAGE_SIZE;
> +	offset %= PAGE_SIZE;
> +
>   	/*
>   	 * A single sg entry may refer to multiple physically contiguous
>   	 * pages.  But we still need to process highmem pages individually.
>   	 * If highmem is not configured then the bulk of this loop gets
>   	 * optimized out.
>   	 */
> -	size_t left = size;
>   	do {
>   		size_t len = left;
>   		void *vaddr;
>   
> +		page = pfn_to_page(pfn);
> +
>   		if (PageHighMem(page)) {
> -			if (len + offset > PAGE_SIZE) {
> -				if (offset >= PAGE_SIZE) {
> -					page += offset / PAGE_SIZE;
> -					offset %= PAGE_SIZE;
> -				}
> +			if (len + offset > PAGE_SIZE)
>   				len = PAGE_SIZE - offset;
> -			}
>   			vaddr = kmap_high_get(page);
>   			if (vaddr) {
>   				vaddr += offset;
> @@ -809,7 +811,7 @@ static void dma_cache_maint_page(struct page *page, unsigned long offset,
>   			op(vaddr, len, dir);
>   		}
>   		offset = 0;
> -		page++;
> +		pfn++;
>   		left -= len;
>   	} while (left);
>   }
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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