Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

firewire: octlet AT payloads can be stack-allocated

We do not need slab allocations anymore in order to satisfy
streaming DMA mapping constraints, thanks to commit da28947e7e36
"firewire: ohci: avoid separate DMA mapping for small AT payloads".

(Besides, the slab-allocated buffers that firewire-core, firewire-sbp2,
and firedtv used to provide for 8-byte write and lock requests were
still not fully portable since they crossed cacheline boundaries or
shared a cacheline with unrelated CPU-accessed data. snd-firewire-lib
got this aspect right by using an extra kmalloc/ kfree just for the
8-byte transaction buffer.)

This change replaces kmalloc'ed lock transaction scratch buffers in
firewire-core, firedtv, and snd-firewire-lib by local stack allocations.
Perhaps the most notable result of the change is simpler locking because
there is no need to serialize usages of preallocated per-device buffers
anymore. Also, allocations and deallocations are simpler.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Acked-by: Clemens Ladisch <clemens@ladisch.de>

+30 -52
+8 -8
drivers/firewire/core-card.c
··· 258 258 259 259 if (!card->broadcast_channel_allocated) { 260 260 fw_iso_resource_manage(card, generation, 1ULL << 31, 261 - &channel, &bandwidth, true, 262 - card->bm_transaction_data); 261 + &channel, &bandwidth, true); 263 262 if (channel != 31) { 264 263 fw_notify("failed to allocate broadcast channel\n"); 265 264 return; ··· 293 294 bool root_device_is_cmc; 294 295 bool irm_is_1394_1995_only; 295 296 bool keep_this_irm; 297 + __be32 transaction_data[2]; 296 298 297 299 spin_lock_irq(&card->lock); 298 300 ··· 355 355 goto pick_me; 356 356 } 357 357 358 - card->bm_transaction_data[0] = cpu_to_be32(0x3f); 359 - card->bm_transaction_data[1] = cpu_to_be32(local_id); 358 + transaction_data[0] = cpu_to_be32(0x3f); 359 + transaction_data[1] = cpu_to_be32(local_id); 360 360 361 361 spin_unlock_irq(&card->lock); 362 362 363 363 rcode = fw_run_transaction(card, TCODE_LOCK_COMPARE_SWAP, 364 364 irm_id, generation, SCODE_100, 365 365 CSR_REGISTER_BASE + CSR_BUS_MANAGER_ID, 366 - card->bm_transaction_data, 8); 366 + transaction_data, 8); 367 367 368 368 if (rcode == RCODE_GENERATION) 369 369 /* Another bus reset, BM work has been rescheduled. */ 370 370 goto out; 371 371 372 - bm_id = be32_to_cpu(card->bm_transaction_data[0]); 372 + bm_id = be32_to_cpu(transaction_data[0]); 373 373 374 374 spin_lock_irq(&card->lock); 375 375 if (rcode == RCODE_COMPLETE && generation == card->generation) ··· 490 490 /* 491 491 * Make sure that the cycle master sends cycle start packets. 492 492 */ 493 - card->bm_transaction_data[0] = cpu_to_be32(CSR_STATE_BIT_CMSTR); 493 + transaction_data[0] = cpu_to_be32(CSR_STATE_BIT_CMSTR); 494 494 rcode = fw_run_transaction(card, TCODE_WRITE_QUADLET_REQUEST, 495 495 root_id, generation, SCODE_100, 496 496 CSR_REGISTER_BASE + CSR_STATE_SET, 497 - card->bm_transaction_data, 4); 497 + transaction_data, 4); 498 498 if (rcode == RCODE_GENERATION) 499 499 goto out; 500 500 }
+1 -3
drivers/firewire/core-cdev.c
··· 141 141 int generation; 142 142 u64 channels; 143 143 s32 bandwidth; 144 - __be32 transaction_data[2]; 145 144 struct iso_resource_event *e_alloc, *e_dealloc; 146 145 }; 147 146 ··· 1228 1229 r->channels, &channel, &bandwidth, 1229 1230 todo == ISO_RES_ALLOC || 1230 1231 todo == ISO_RES_REALLOC || 1231 - todo == ISO_RES_ALLOC_ONCE, 1232 - r->transaction_data); 1232 + todo == ISO_RES_ALLOC_ONCE); 1233 1233 /* 1234 1234 * Is this generation outdated already? As long as this resource sticks 1235 1235 * in the idr, it will be scheduled again for a newer generation or at
+11 -10
drivers/firewire/core-iso.c
··· 196 196 */ 197 197 198 198 static int manage_bandwidth(struct fw_card *card, int irm_id, int generation, 199 - int bandwidth, bool allocate, __be32 data[2]) 199 + int bandwidth, bool allocate) 200 200 { 201 201 int try, new, old = allocate ? BANDWIDTH_AVAILABLE_INITIAL : 0; 202 + __be32 data[2]; 202 203 203 204 /* 204 205 * On a 1394a IRM with low contention, try < 1 is enough. ··· 234 233 } 235 234 236 235 static int manage_channel(struct fw_card *card, int irm_id, int generation, 237 - u32 channels_mask, u64 offset, bool allocate, __be32 data[2]) 236 + u32 channels_mask, u64 offset, bool allocate) 238 237 { 239 238 __be32 bit, all, old; 239 + __be32 data[2]; 240 240 int channel, ret = -EIO, retry = 5; 241 241 242 242 old = all = allocate ? cpu_to_be32(~0) : 0; ··· 286 284 } 287 285 288 286 static void deallocate_channel(struct fw_card *card, int irm_id, 289 - int generation, int channel, __be32 buffer[2]) 287 + int generation, int channel) 290 288 { 291 289 u32 mask; 292 290 u64 offset; ··· 295 293 offset = channel < 32 ? CSR_REGISTER_BASE + CSR_CHANNELS_AVAILABLE_HI : 296 294 CSR_REGISTER_BASE + CSR_CHANNELS_AVAILABLE_LO; 297 295 298 - manage_channel(card, irm_id, generation, mask, offset, false, buffer); 296 + manage_channel(card, irm_id, generation, mask, offset, false); 299 297 } 300 298 301 299 /** ··· 324 322 */ 325 323 void fw_iso_resource_manage(struct fw_card *card, int generation, 326 324 u64 channels_mask, int *channel, int *bandwidth, 327 - bool allocate, __be32 buffer[2]) 325 + bool allocate) 328 326 { 329 327 u32 channels_hi = channels_mask; /* channels 31...0 */ 330 328 u32 channels_lo = channels_mask >> 32; /* channels 63...32 */ ··· 337 335 if (channels_hi) 338 336 c = manage_channel(card, irm_id, generation, channels_hi, 339 337 CSR_REGISTER_BASE + CSR_CHANNELS_AVAILABLE_HI, 340 - allocate, buffer); 338 + allocate); 341 339 if (channels_lo && c < 0) { 342 340 c = manage_channel(card, irm_id, generation, channels_lo, 343 341 CSR_REGISTER_BASE + CSR_CHANNELS_AVAILABLE_LO, 344 - allocate, buffer); 342 + allocate); 345 343 if (c >= 0) 346 344 c += 32; 347 345 } ··· 353 351 if (*bandwidth == 0) 354 352 return; 355 353 356 - ret = manage_bandwidth(card, irm_id, generation, *bandwidth, 357 - allocate, buffer); 354 + ret = manage_bandwidth(card, irm_id, generation, *bandwidth, allocate); 358 355 if (ret < 0) 359 356 *bandwidth = 0; 360 357 361 358 if (allocate && ret < 0) { 362 359 if (c >= 0) 363 - deallocate_channel(card, irm_id, generation, c, buffer); 360 + deallocate_channel(card, irm_id, generation, c); 364 361 *channel = ret; 365 362 } 366 363 }
+4 -3
drivers/firewire/core-transaction.c
··· 326 326 * It will contain tag, channel, and sy data instead of a node ID then. 327 327 * 328 328 * The payload buffer at @data is going to be DMA-mapped except in case of 329 - * quadlet-sized payload or of local (loopback) requests. Hence make sure that 330 - * the buffer complies with the restrictions for DMA-mapped memory. The 329 + * @length <= 8 or of local (loopback) requests. Hence make sure that the 330 + * buffer complies with the restrictions of the streaming DMA mapping API. 331 331 * @payload must not be freed before the @callback is called. 332 332 * 333 333 * In case of request types without payload, @data is NULL and @length is 0. ··· 411 411 * 412 412 * Returns the RCODE. See fw_send_request() for parameter documentation. 413 413 * Unlike fw_send_request(), @data points to the payload of the request or/and 414 - * to the payload of the response. 414 + * to the payload of the response. DMA mapping restrictions apply to outbound 415 + * request payloads of >= 8 bytes but not to inbound response payloads. 415 416 */ 416 417 int fw_run_transaction(struct fw_card *card, int tcode, int destination_id, 417 418 int generation, int speed, unsigned long long offset,
+1 -14
drivers/media/dvb/firewire/firedtv-avc.c
··· 1320 1320 { 1321 1321 int ret; 1322 1322 1323 - mutex_lock(&fdtv->avc_mutex); 1324 - 1325 1323 ret = fdtv_read(fdtv, addr, data); 1326 1324 if (ret < 0) 1327 1325 dev_err(fdtv->device, "CMP: read I/O error\n"); 1328 - 1329 - mutex_unlock(&fdtv->avc_mutex); 1330 1326 1331 1327 return ret; 1332 1328 } ··· 1331 1335 { 1332 1336 int ret; 1333 1337 1334 - mutex_lock(&fdtv->avc_mutex); 1335 - 1336 - /* data[] is stack-allocated and should not be DMA-mapped. */ 1337 - memcpy(fdtv->avc_data, data, 8); 1338 - 1339 - ret = fdtv_lock(fdtv, addr, fdtv->avc_data); 1338 + ret = fdtv_lock(fdtv, addr, data); 1340 1339 if (ret < 0) 1341 1340 dev_err(fdtv->device, "CMP: lock I/O error\n"); 1342 - else 1343 - memcpy(data, fdtv->avc_data, 8); 1344 - 1345 - mutex_unlock(&fdtv->avc_mutex); 1346 1341 1347 1342 return ret; 1348 1343 }
+1 -2
include/linux/firewire.h
··· 125 125 struct delayed_work bm_work; /* bus manager job */ 126 126 int bm_retries; 127 127 int bm_generation; 128 - __be32 bm_transaction_data[2]; 129 128 int bm_node_id; 130 129 bool bm_abdicate; 131 130 ··· 446 447 void fw_iso_context_destroy(struct fw_iso_context *ctx); 447 448 void fw_iso_resource_manage(struct fw_card *card, int generation, 448 449 u64 channels_mask, int *channel, int *bandwidth, 449 - bool allocate, __be32 buffer[2]); 450 + bool allocate); 450 451 451 452 #endif /* _LINUX_FIREWIRE_H */
+1 -2
sound/firewire/cmp.c
··· 49 49 enum bus_reset_handling bus_reset_handling) 50 50 { 51 51 struct fw_device *device = fw_parent_device(c->resources.unit); 52 - __be32 *buffer = c->resources.buffer; 53 52 int generation = c->resources.generation; 54 53 int rcode, errors = 0; 55 - __be32 old_arg; 54 + __be32 old_arg, buffer[2]; 56 55 int err; 57 56 58 57 buffer[0] = c->last_pcr_value;
+3 -9
sound/firewire/iso-resources.c
··· 11 11 #include <linux/jiffies.h> 12 12 #include <linux/mutex.h> 13 13 #include <linux/sched.h> 14 - #include <linux/slab.h> 15 14 #include <linux/spinlock.h> 16 15 #include "iso-resources.h" 17 16 ··· 24 25 */ 25 26 int fw_iso_resources_init(struct fw_iso_resources *r, struct fw_unit *unit) 26 27 { 27 - r->buffer = kmalloc(2 * 4, GFP_KERNEL); 28 - if (!r->buffer) 29 - return -ENOMEM; 30 - 31 28 r->channels_mask = ~0uLL; 32 29 r->unit = fw_unit_get(unit); 33 30 mutex_init(&r->mutex); ··· 39 44 void fw_iso_resources_destroy(struct fw_iso_resources *r) 40 45 { 41 46 WARN_ON(r->allocated); 42 - kfree(r->buffer); 43 47 mutex_destroy(&r->mutex); 44 48 fw_unit_put(r->unit); 45 49 } ··· 125 131 126 132 bandwidth = r->bandwidth + r->bandwidth_overhead; 127 133 fw_iso_resource_manage(card, r->generation, r->channels_mask, 128 - &channel, &bandwidth, true, r->buffer); 134 + &channel, &bandwidth, true); 129 135 if (channel == -EAGAIN) { 130 136 mutex_unlock(&r->mutex); 131 137 goto retry_after_bus_reset; ··· 178 184 bandwidth = r->bandwidth + r->bandwidth_overhead; 179 185 180 186 fw_iso_resource_manage(card, r->generation, 1uLL << r->channel, 181 - &channel, &bandwidth, true, r->buffer); 187 + &channel, &bandwidth, true); 182 188 /* 183 189 * When another bus reset happens, pretend that the allocation 184 190 * succeeded; we will try again for the new generation later. ··· 214 220 if (r->allocated) { 215 221 bandwidth = r->bandwidth + r->bandwidth_overhead; 216 222 fw_iso_resource_manage(card, r->generation, 1uLL << r->channel, 217 - &channel, &bandwidth, false, r->buffer); 223 + &channel, &bandwidth, false); 218 224 if (channel < 0) 219 225 dev_err(&r->unit->device, 220 226 "isochronous resource deallocation failed\n");
-1
sound/firewire/iso-resources.h
··· 24 24 unsigned int bandwidth_overhead; 25 25 int generation; /* in which allocation is valid */ 26 26 bool allocated; 27 - __be32 *buffer; 28 27 }; 29 28 30 29 int fw_iso_resources_init(struct fw_iso_resources *r,