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. > -Alex > > > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx> > > --- > > net/ceph/messenger.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c > > index d3bb656308b4..3f8453773cc8 100644 > > --- a/net/ceph/messenger.c > > +++ b/net/ceph/messenger.c > > @@ -894,10 +894,9 @@ static bool ceph_msg_data_pages_advance(struct ceph_msg_data_cursor *cursor, > > return false; /* no more data */ > > > > /* Move on to the next page; offset is already at 0 */ > > - > > BUG_ON(cursor->page_index >= cursor->page_count); > > cursor->page_index++; > > - cursor->last_piece = cursor->resid <= PAGE_SIZE; > > + cursor->last_piece = cursor->page_offset + cursor->resid <= PAGE_SIZE; > > > > return true; > > } > -- Jeff Layton <jlayton@xxxxxxxxxx>
