Re: [PATCH v2] powerpc/powernv: Platform dump interface

[Vasant Hegde <hegdevasant@linux.vnet.ibm.com>]

On 03/25/2014 11:22 AM, Anton Blanchard wrote:
> Hi Vasant,
>
>> On 02/09/2014 02:50 AM, Anton Blanchard wrote:
>>>
>>> Hi Vasant,
>>>
>>>> +static void free_dump_sg_list(struct opal_sg_list *list)
>>>> +{
>>>> +	struct opal_sg_list *sg1;
>>>> +	while (list) {
>>>> +		sg1 = list->next;
>>>> +		kfree(list);
>>>> +		list = sg1;
>>>> +	}
>>>> +	list = NULL;
>>>> +}
>>>> +
>>>> +/*
>>>> + * Build dump buffer scatter gather list
>>>> + */
>>>> +static struct opal_sg_list *dump_data_to_sglist(void)
>>>> +{
>>>> +	struct opal_sg_list *sg1, *list = NULL;
>>>> +	void *addr;
>>>> +	int64_t size;
>>>> +
>>>> +	addr = dump_record.buffer;
>>>> +	size = dump_record.size;
>>>> +
>>>> +	sg1 = kzalloc(PAGE_SIZE, GFP_KERNEL);
>>>> +	if (!sg1)
>>>> +		goto nomem;
>>>> +
>>>> +	list = sg1;
>>>> +	sg1->num_entries = 0;
>>>> +	while (size > 0) {
>>>> +		/* Translate virtual address to physical address
>>>> */
>>>> +		sg1->entry[sg1->num_entries].data =
>>>> +			(void *)(vmalloc_to_pfn(addr) <<
>>>> PAGE_SHIFT); +
>>>> +		if (size > PAGE_SIZE)
>>>> +			sg1->entry[sg1->num_entries].length =
>>>> PAGE_SIZE;
>>>> +		else
>>>> +			sg1->entry[sg1->num_entries].length =
>>>> size; +
>>>> +		sg1->num_entries++;
>>>> +		if (sg1->num_entries >= SG_ENTRIES_PER_NODE) {
>>>> +			sg1->next = kzalloc(PAGE_SIZE,
>>>> GFP_KERNEL);
>>>> +			if (!sg1->next)
>>>> +				goto nomem;
>>>> +
>>>> +			sg1 = sg1->next;
>>>> +			sg1->num_entries = 0;
>>>> +		}
>>>> +		addr += PAGE_SIZE;
>>>> +		size -= PAGE_SIZE;
>>>> +	}
>>>> +	return list;
>>>> +
>>>> +nomem:
>>>> +	pr_err("%s : Failed to allocate memory\n", __func__);
>>>> +	free_dump_sg_list(list);
>>>> +	return NULL;
>>>> +}
>>>> +
>>>> +/*
>>>> + * Translate sg list address to absolute
>>>> + */
>>>> +static void sglist_to_phy_addr(struct opal_sg_list *list)
>>>> +{
>>>> +	struct opal_sg_list *sg, *next;
>>>> +
>>>> +	for (sg = list; sg; sg = next) {
>>>> +		next = sg->next;
>>>> +		/* Don't translate NULL pointer for last entry */
>>>> +		if (sg->next)
>>>> +			sg->next = (struct opal_sg_list
>>>> *)__pa(sg->next);
>>>> +		else
>>>> +			sg->next = NULL;
>>>> +
>>>> +		/* Convert num_entries to length */
>>>> +		sg->num_entries =
>>>> +			sg->num_entries * sizeof(struct
>>>> opal_sg_entry) + 16;
>>>> +	}
>>>> +}
>>>> +
>>>> +static void free_dump_data_buf(void)
>>>> +{
>>>> +	vfree(dump_record.buffer);
>>>> +	dump_record.size = 0;
>>>> +}
>>>
>>
>> Anton,
>>
>>> This looks identical to the code in opal-flash.c. Considering how
>>> complicated it is, can we put it somewhere common?
>>
>> Thanks for the review.. Will look into it next week.
>

Anton,

> This doesn't appear to have been fixed in the version that went into
> next.
>

Stewart rewrote dump interface.. That set of patch went into next, not my 
original patch..
Yes.. SG list separation is not yet done.. Never got a chance to look into this 
one.. Will try to
do it soon.

-Vasant

> Anton
>