On Wed, May 20, 2009 at 1:09 AM, Andre Noll <maan@xxxxxxxxxxxxxxx> wrote:
> On Mon, May 18, 2009 at 05:59:51PM -0700, Dan Williams wrote:
>
>> +static size_t scribble_len(int num)
>> +{
>> + size_t len;
>> +
>> + /* return enough space for an array of page pointers and dma
>> + * addresses for the ddf raid6 layout
>> + */
>> + len = sizeof(struct page *) * (num+2) + sizeof(addr_conv_t) * (num+2);
>> +
>> + return len;
>> +}
>
> The comment is a bit misleading as the function only returns the
> _amount_ of space needed. It should probably also explain the meaning
> of the "+2".
Ok I updated this to:
/**
* scribble_len - return the required size of the scribble region
* @num - total number of disks in the array
*
* The size must be enough to contain:
* 1/ a struct page pointer for each device in the array +2
* 2/ room to convert each entry in (1) to its corresponding dma
* (dma_map_page()) or page (page_address()) address.
*
* Note: the +2 is for the destination buffers of the ddf/raid6 case where we
* calculate over all devices (not just the data blocks), using zeros in place
* of the P and Q blocks.
*/
>
>> +/* return a pointer to the address conversion region of the scribble buffer */
>> +static addr_conv_t *sh_to_addr_conv(struct stripe_head *sh)
>> +{
>> + return sh->scribble + sizeof(struct page *) * (sh->disks + 2);
>> +}
>
> Maybe it's safer to return NULL if sh->scribble is NULL.
...yes, and a big fat warning, because it should never be NULL.
Thanks for the review,
Dan
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
