Re: [PATCH 1/2] kernel-shark: Fix a bug in ksmodel_set_next_bin_edge() part II

[Yordan Karadzhov <ykaradzhov@vmware.com>]

Hi Steven,

My idea was to make clear that those two patches are strongly connected 
to the patch another patch called:
kernel-shark: Fix a bug in ksmodel_set_next_bin_edge()

which was send long time ago.

Should I resend with another subject?
Thanks!
Yordan


On 12.06.19 г. 17:28 ч., Steven Rostedt wrote:
> 
> Hi Yordan,
> 
> Please use a more descriptive subject. Seeing a "part II" and a "part
> III" in a git log --oneline isn't very useful.
> 
> Something like:
> 
>   kernel-shark: Only increment the upper edge bin without touching lower edge.
> 
> Or something that describes what is being done in a bit more detail.
> 
> Thanks!
> 
> -- Steve
> 
> On Wed, 12 Jun 2019 17:20:52 +0300
> Yordan Karadzhov <ykaradzhov@vmware.com> wrote:
> 
>> On a first glance this patch may looks like reverting commit
>> 9336dd6bcd38 (kernel-shark: Fix a bug in ksmodel_set_next_bin_edge())
>>
>> The point is that for the last bin we want to increment its upper edge
>> used  when checking if the bin is empty, but we do not want to touch
>> the lower edge time used by kshark_find_entry_by_time().
>>
>> Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
>> ---
>>   kernel-shark/src/libkshark-model.c | 22 ++++++++++++++++------
>>   1 file changed, 16 insertions(+), 6 deletions(-)
>>
>> diff --git a/kernel-shark/src/libkshark-model.c b/kernel-shark/src/libkshark-model.c
>> index 978cd70..0cac924 100644
>> --- a/kernel-shark/src/libkshark-model.c
>> +++ b/kernel-shark/src/libkshark-model.c
>> @@ -260,20 +260,30 @@ static size_t ksmodel_set_upper_edge(struct kshark_trace_histo *histo)
>>   static void ksmodel_set_next_bin_edge(struct kshark_trace_histo *histo,
>>   				      size_t bin, size_t last_row)
>>   {
>> -	size_t time, next_bin = bin + 1;
>> +	size_t time_min, time_max, next_bin = bin + 1;
>>   	ssize_t row;
>>   
>> -	/* Calculate the beginning of the next bin. */
>> -	time = histo->min + next_bin * histo->bin_size;
>> +	/* Calculate the beginning and the end of the next bin. */
>> +	time_min = histo->min + next_bin * histo->bin_size;
>> +	time_max = time_min + histo->bin_size;
>> +	/*
>> +	 * The timestamp of the very last entry of the dataset can be exactly
>> +	 * equal to the value of the upper edge of the range. This is very
>> +	 * likely to happen when we use ksmodel_set_in_range_bining(). In this
>> +	 * case we have to increase the size of the very last bin in order to
>> +	 * make sure that the last entry of the dataset will fall into it.
>> +	 */
>> +	if (next_bin == histo->n_bins - 1)
>> +		++time_max;
>>   
>>   	/*
>>   	 * Find the index of the first entry inside
>> -	 * the next bin (timestamp > time).
>> +	 * the next bin (timestamp > time_min).
>>   	 */
>> -	row = kshark_find_entry_by_time(time, histo->data, last_row,
>> +	row = kshark_find_entry_by_time(time_min, histo->data, last_row,
>>   					histo->data_size - 1);
>>   
>> -	if (row < 0 || histo->data[row]->ts >= time + histo->bin_size) {
>> +	if (row < 0 || histo->data[row]->ts >= time_max) {
>>   		/* The bin is empty. */
>>   		histo->map[next_bin] = KS_EMPTY_BIN;
>>   		return;
>