On 5.11.20 г. 20:17 ч., Steven Rostedt wrote:
On Thu, 5 Nov 2020 16:58:51 +0200 "Yordan Karadzhov (VMware)" <y.karadz@xxxxxxxxx> wrote:I see the problem. This is definitely wrong. What if in addition to "n_streams" I add another counter called "last_stream_added" and initialize this counter to -1? Then I can add streams like this: kshark_ctx->stream[++kshark_ctx->last_stream_added] = stream; ++kshark_ctx->n_streams;You may want to do instead:I'm thinking of doing something like this: struct kshark_ctx { [..] unsigned int free_stream; [..] }; if (kshark_ctx->free_stream >= kshark_ctx->n_streams) { kshark_ctx->stream[++kshark_ctx->n_streams] = stream; kshark_ctx->free_stream = kshark_ctx->n_streams; /* BTW, the stream array should be allocated to the n_streams, and reallocated when it grows. I don't think we want a huge stream array to handle all the bits when not used. */ } else { int new_stream = kshark_ctx->free_stream; kshark_ctx->free_stream = kshark_index(kshark_ctx->stream[new_stream]); kshark_ctx->stream[new_stream] = stream; } For freeing (index i): kshark_ctx->stream[i] = kshark_ptr(kshark_ctx->free_stream); kshark_ctx->free_stream = i; We could define the following (note, I just used these names for the functions, they could be named something else): #define KSHARK_INDEX_MASK ((1 << NR_OF_BITS_FOR_STREAM) - 1) #define KSHARK_INVALID_STREAM (~((1UL << NR_OF_BITS_FOR_STREAM) - 1)) static inline int kshark_index(void *ptr) { unsigned long index = (unsigned long)ptr; return (int)(index & KSHARK_INDEX_MASK); } static inline void *kshark_ptr(unsigned int index) { unsigned long p; p = KSHARK_INVALID_STREAM | index; return (void *)p; } The KSHARK_INVALID_STREAM and KSHARK_INDEX_MASK, would allow us to do something like this if we wanted to loop through all streams: static inline bool kshark_is_valid_stream(void *ptr) { unsigned long p = (unsigned long)ptr; return (p & KSHARK_INVALID_STREAM) == KSHARK_INVALID_STREAM; } The above works because the address of setting all those bits, would put the address into the kernel space (illegal user space address). for (i = 0; i < kshark_ctx->n_streams; i++) { if (!kshark_is_valid_stream(kshark_ctx->stream[i])) continue; /* process valid stream */ }
Hi Steven,I am not sure I understand correctly your pseudo-code, so please correct me if my interpretation is wrong.
In the normal case when a new stream is added the corresponding object will be allocated and added to the array of pointers. Later if a stream is removed, instead of freeing the memory we will just manipulate it pointer so that it point to nowhere and this manipulation can be detected by the kshark_is_valid_stream(). Now if we want to add stream again, we will take the broken pointer, will restore its original value and will reuse the object without a new allocations.
And at the very end we will have to free all pointers (original or manipulated).
Is this what you are suggesting? Thanks! Y.
-- Steve
|