On Wed, May 20, 2009 at 1:09 AM, Andre Noll [off-list ref] wrote:

On Mon, May 18, 2009 at 05:59:51PM -0700, Dan Williams wrote:

quoted

+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.
 */

quoted

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