On Wed, Mar 2, 2022 at 5:15 PM Jeff Layton <jlayton@xxxxxxxxxx> wrote:
>
> On Wed, 2022-03-02 at 09:41 -0600, Alex Elder wrote:
> > On 3/2/22 9:37 AM, Jeff Layton wrote:
> > > It's possible we'll have less than a page's worth of residual data, that
> > > is stradding the last two pages in the array. That will make it
> > > incorrectly set the last_piece boolean when it shouldn't.
> > >
> > > Account for a non-zero cursor->page_offset when advancing.
> >
> > It's been quite a while I looked at this code, but isn't
> > cursor->resid supposed to be the number of bytes remaining,
> > irrespective of the offset?
> >
>
> Correct. The "residual" bytes in the cursor, AFAICT.
>
> > Have you found this to cause a failure?
> >
>
> Not with the existing code in libceph, as it only ever seems to advance
> to the end of the page. I'm working on some patches to allow for sparse
> reads though, and with those in place I need to sometimes advance to
> arbitrary positions in the array, and this reliably causes a BUG_ON() to
> trip.
Hi Jeff,
Which BUG_ON? Can you explain what "advance to arbitrary positions in
the array" means?
I think you may be misusing ceph_msg_data_pages_advance() because
cursor->page_offset _has_ to be zero at that point: we have just
determined that we are done with the current page and incremented
cursor->page_index (i.e. moved to the next page). The way we
determine whether we are done with the current page is by testing
cursor->page_offset == 0, so either your change is a no-op or
something else is broken.
Thanks,
Ilya
