Correct commit mask for page data size

Correct commit mask for page data size

[Jason Behmer]

Hi Steven,
We're wondering what the correct number of bits to take from the
commit field is when determining the size of the page data.  The
format file shows the bottom 56 bits not overlapping with anything:

        field: local_t commit;  offset:8;       size:8; signed:1;
        field: int overwrite;   offset:8;       size:1; signed:1;

We first naively interpreted this as the size, but eventually ran into
cases where this gave back a nonsense result.  But then in our
investigation of what the correct thing to do is, we found conflicting
answers.

In the kernel we see that commit is often updated to write, which is
masked against RB_WRITE_MASK.  So it seems taking the bottom 20 bits
is correct.  However, in trace-cmd, a fairly authoritative parser, we
see that COMMIT_MASK is set to take the bottom 27 bits and set that to
the page data size.

Could you provide some guidance?

Thanks,
Jason

Re: Correct commit mask for page data size

[Steven Rostedt]

On Mon, 1 Apr 2019 06:49:07 -0700
Jason Behmer <jbehmer@google.com> wrote:

Hi Jason,

I just noticed this email. I know it's a late response, but since you
Cc'd LKML, I figured I would respond anyway, and at least have an
answer in the archives ;-)

> Hi Steven,
> We're wondering what the correct number of bits to take from the
> commit field is when determining the size of the page data.  The
> format file shows the bottom 56 bits not overlapping with anything:
> 
>         field: local_t commit;  offset:8;       size:8; signed:1;
>         field: int overwrite;   offset:8;       size:1; signed:1;
> 
> We first naively interpreted this as the size, but eventually ran into
> cases where this gave back a nonsense result.  But then in our
> investigation of what the correct thing to do is, we found conflicting
> answers.

Yeah, I hated that above, but the format didn't have a good way to show
the overwrite without breaking existing tools :-/

> 
> In the kernel we see that commit is often updated to write, which is
> masked against RB_WRITE_MASK.  So it seems taking the bottom 20 bits
> is correct.  However, in trace-cmd, a fairly authoritative parser, we
> see that COMMIT_MASK is set to take the bottom 27 bits and set that to
> the page data size.

The way the kernel uses that number is that the first 20 bits are the
size. Then we have an internal counter (top 12 bits) used for
synchronizing when the trace crosses pages. But these internal numbers
will never be exposed when it is sent off to the reader. Hence, those
bits are meaningless.

Now I probably could make the trace-cmd header just use those 20 bits,
as they never will be used for the size. When I wrote that, I just made
sure that the flags that are added to the page by the reader code was
not set. Which is why there is a discrepancy between the two masks.
> 
> Could you provide some guidance?

Thanks for pointing this out. Again, the reason for the difference is
that they were created from two different perspectives. One was that it
would use the top 12 bytes for internal purposes, the other was just to
allow for up to 5 flags by the reader.

Does that make sense?

-- Steve

Re: Correct commit mask for page data size

[Jason Behmer]

Yup, that makes sense, thanks for the response.


On Thu, May 23, 2019 at 7:54 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Mon, 1 Apr 2019 06:49:07 -0700
> Jason Behmer <jbehmer@google.com> wrote:
>
> Hi Jason,
>
> I just noticed this email. I know it's a late response, but since you
> Cc'd LKML, I figured I would respond anyway, and at least have an
> answer in the archives ;-)
>
> > Hi Steven,
> > We're wondering what the correct number of bits to take from the
> > commit field is when determining the size of the page data.  The
> > format file shows the bottom 56 bits not overlapping with anything:
> >
> >         field: local_t commit;  offset:8;       size:8; signed:1;
> >         field: int overwrite;   offset:8;       size:1; signed:1;
> >
> > We first naively interpreted this as the size, but eventually ran into
> > cases where this gave back a nonsense result.  But then in our
> > investigation of what the correct thing to do is, we found conflicting
> > answers.
>
> Yeah, I hated that above, but the format didn't have a good way to show
> the overwrite without breaking existing tools :-/
>
> >
> > In the kernel we see that commit is often updated to write, which is
> > masked against RB_WRITE_MASK.  So it seems taking the bottom 20 bits
> > is correct.  However, in trace-cmd, a fairly authoritative parser, we
> > see that COMMIT_MASK is set to take the bottom 27 bits and set that to
> > the page data size.
>
> The way the kernel uses that number is that the first 20 bits are the
> size. Then we have an internal counter (top 12 bits) used for
> synchronizing when the trace crosses pages. But these internal numbers
> will never be exposed when it is sent off to the reader. Hence, those
> bits are meaningless.
>
> Now I probably could make the trace-cmd header just use those 20 bits,
> as they never will be used for the size. When I wrote that, I just made
> sure that the flags that are added to the page by the reader code was
> not set. Which is why there is a discrepancy between the two masks.
> >
> > Could you provide some guidance?
>
> Thanks for pointing this out. Again, the reason for the difference is
> that they were created from two different perspectives. One was that it
> would use the top 12 bytes for internal purposes, the other was just to
> allow for up to 5 flags by the reader.
>
> Does that make sense?
>
> -- Steve
>