Add full support support for dynamic allocation and freeing of epochs

[• hselasky]

Summary

Make sure to reclaim epoch structures when they are freed to support dynamic allocation and freeing of epoch's.

While at it, move the 64 supported epoch control structures, to the static memory domain.
This overall simplifies the management and debugging of system EPOCH's.

Sponsored by: Mellanox Technologies
MFC after: 1 week

[markj]

sys/kern/subr_epoch.c

330

Can't we just use a global sx to synchronize epoch allocation and free?

[• hselasky]

We could use atomics. I don't know if sx's are ready when we allocate epochs.

[markj]

In D25960#575501, @hselasky wrote:

We could use atomics. I don't know if sx's are ready when we allocate epochs.

SI_SUB_EPOCH is long after SI_SUB_LOCK.

[kib]

sys/kern/subr_epoch.c

370

May be we should bite the bullet and return NULL if there is no space in the array ?

[• hselasky]

See updated summary.

[• hselasky]

No more need for e_idx field.

[• hselasky]

Assert valid name before freeing epoch.

[• hselasky]

adding Drew, in case he sees something which is not performancewise clever

[kib]

sys/kern/subr_epoch.c

343

I do not quite understand how this loop works. You only loop up to the top allocated index, not to MAX_EPOCH, so if there is no holes, you cannot allocate anything.

[• hselasky]

Address comment by @kib. The increment of "epoch_count" was accidentially removed by patch update.

[• hselasky]

sys/kern/subr_epoch.c

343

See the epoch_count++ below.

[kib]

sys/kern/subr_epoch.c

364

I suggest renaming this variable into something like epoch_array_max_slot. It is not epoch_count.

[• hselasky]

Rename epoch_count to epoch_array_max_count, as suggested by kib@

[kib]

sys/kern/subr_epoch.c

337–355

This blank line is not needed.

342

Comments should be a proper sentences.

746–748

Is it safe to iterate to epoch_array_max_count instead of MAX_EPOCHS ? Could we miss an update to max_count there and then skip some epoch ?

[• hselasky]

Address comments from @kib

[• hselasky]

sys/kern/subr_epoch.c

746–748

e_name is protected by global_epoch.

[kib]

sys/kern/subr_epoch.c

746–748

From what I see, global epoch ensures that e_name is not cleared until epoch_call_task() ends it global section.

I asked about different situation, where the update from epoch_alloc (most likely, to epoch_array_max_count) is missed by epoch_call_task and then corresponding epoch is not drained by the task.

But I see that you changed the loop limit to MAX_EPOCHS already.

[markj]

sys/kern/subr_epoch.c

367

Please use atomic(9) instead of volatile.

[• hselasky]

Use atomic(9) as suggested by @markj

[markj]

sys/kern/subr_epoch.c

86

Why is it defined like this? I can't see the purpose of val.

[• hselasky]

sys/kern/subr_epoch.c

86

The atomic functions don't accept "const char *ptr", instead of casting it in the code, I make a cast in the union ...

[markj]

sys/kern/subr_epoch.c

86

I don't object but personally I'd prefer the ugliness of the casts to the union. atomic_(load|store)_ptr were changed to not require casts recently, it's only the MD atomic_(load|store)_(acq|rel)_* macros that still need it for now. Please add a comment explaining why it's a union.

[• hselasky]

Address comments from @markj .