Re: [PATCH 1/6] firewire: core: reduce stack usage in bus reset tasklet

From: Stefan Richter
Date: Mon Sep 07 2009 - 17:25:44 EST

Stefan Richter wrote:
pageexec@xxxxxxxxxxx wrote:
On 6 Sep 2009 at 18:48, Stefan Richter wrote:

Index: linux-2.6.31-rc9/drivers/firewire/core-card.c
--- linux-2.6.31-rc9.orig/drivers/firewire/core-card.c
+++ linux-2.6.31-rc9/drivers/firewire/core-card.c
@@ -38,16 +38,21 @@
#include "core.h"
-int fw_compute_block_crc(u32 *block)
+int fw_compute_block_crc(u32 *block, gfp_t flags)
- __be32 be32_block[256];
- int i, length;
+ static __be32 *be32_block;

did you actually mean that to be static? if so, then you might as well allocate the
buffer statically and not worry about a runtime allocation failure.

Uh, that's a cut'n'paste mistake. It shouldn't be static. Thanks, I'll correct that before I put it into linux1394-2.6.git.

+ int i, length = (*block >> 16) & 0xff;
+ be32_block = kmalloc(length * 4, flags);
+ if (WARN_ON(!be32_block))
+ goto out;
- length = (*block >> 16) & 0xff;
for (i = 0; i < length; i++)
be32_block[i] = cpu_to_be32(block[i + 1]);
*block |= crc_itu_t(0, (u8 *) be32_block, length * 4);
+ kfree(be32_block);
+ out:
return length;

This can be optimized further to get rid of the temporary be32_block entirely.

There are two users of fw_compute_block_crc: The functions which generate the local Config ROM CSR and then pass it down to the low-level driver, and the function which which generates the local Topology Map CSR and stores it for core-transaction.c::handle_topology_map() to read on demand.

The low-level driver then needs to convert the Config ROM to __be32[]. Likewise, handle_topology_map needs to convert the Topology Map to __be32[]. They would be both better off if we stored their input as __be32[] in the first place, and of course do so before we compute the CRC. Duh.

I'll rewrite patch 1/6 and 5/6 accordingly.
Stefan Richter
-=====-==--= =--= --===
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at