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

[Subhash Jadavani <subhashj@codeaurora.org>]

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@codeaurora.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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html