Re: [PATCH v2 10/20] kernel-shark: Start using data streams

[Steven Rostedt <rostedt@xxxxxxxxxxx>]

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 */
	}

-- Steve