| Message ID | 20231101005033.21995-3-xiubli@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | libceph: sparse-read misc fixes | expand |
On Wed, Nov 1, 2023 at 1:52 AM <xiubli@redhat.com> wrote: > > From: Xiubo Li <xiubli@redhat.com> > > For sparse reading the real length of the data should equal to the > total length from the extent array. > > URL: https://tracker.ceph.com/issues/62081 > Signed-off-by: Xiubo Li <xiubli@redhat.com> > Reviewed-by: Jeff Layton <jlayton@kernel.org> > --- > net/ceph/osd_client.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c > index 800a2acec069..7af35106acaf 100644 > --- a/net/ceph/osd_client.c > +++ b/net/ceph/osd_client.c > @@ -5921,6 +5921,13 @@ static int osd_sparse_read(struct ceph_connection *con, > fallthrough; > case CEPH_SPARSE_READ_DATA: > if (sr->sr_index >= count) { > + if (sr->sr_datalen && count) { > + pr_warn_ratelimited("sr_datalen %d sr_index %d count %d\n", > + sr->sr_datalen, sr->sr_index, > + count); > + WARN_ON_ONCE(sr->sr_datalen); Hi Xiubo, I don't think we need both pr_warn_ratelimited and WARN_ON_ONCE? This is a state machine, so the stack trace that WARN_ON_ONCE would dump is unlikely to be of any help. Thanks, Ilya
On 11/3/23 20:49, Ilya Dryomov wrote: > On Wed, Nov 1, 2023 at 1:52 AM <xiubli@redhat.com> wrote: >> From: Xiubo Li <xiubli@redhat.com> >> >> For sparse reading the real length of the data should equal to the >> total length from the extent array. >> >> URL: https://tracker.ceph.com/issues/62081 >> Signed-off-by: Xiubo Li <xiubli@redhat.com> >> Reviewed-by: Jeff Layton <jlayton@kernel.org> >> --- >> net/ceph/osd_client.c | 9 +++++++++ >> 1 file changed, 9 insertions(+) >> >> diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c >> index 800a2acec069..7af35106acaf 100644 >> --- a/net/ceph/osd_client.c >> +++ b/net/ceph/osd_client.c >> @@ -5921,6 +5921,13 @@ static int osd_sparse_read(struct ceph_connection *con, >> fallthrough; >> case CEPH_SPARSE_READ_DATA: >> if (sr->sr_index >= count) { >> + if (sr->sr_datalen && count) { >> + pr_warn_ratelimited("sr_datalen %d sr_index %d count %d\n", >> + sr->sr_datalen, sr->sr_index, >> + count); >> + WARN_ON_ONCE(sr->sr_datalen); > Hi Xiubo, > > I don't think we need both pr_warn_ratelimited and WARN_ON_ONCE? This > is a state machine, so the stack trace that WARN_ON_ONCE would dump is > unlikely to be of any help. Okay, makes sense. Let me remove the WARN_ON_ONCE. Thanks - Xiubo > Thanks, > > Ilya >
diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c index 800a2acec069..7af35106acaf 100644 --- a/net/ceph/osd_client.c +++ b/net/ceph/osd_client.c @@ -5921,6 +5921,13 @@ static int osd_sparse_read(struct ceph_connection *con, fallthrough; case CEPH_SPARSE_READ_DATA: if (sr->sr_index >= count) { + if (sr->sr_datalen && count) { + pr_warn_ratelimited("sr_datalen %d sr_index %d count %d\n", + sr->sr_datalen, sr->sr_index, + count); + WARN_ON_ONCE(sr->sr_datalen); + } + sr->sr_state = CEPH_SPARSE_READ_HDR; goto next_op; } @@ -5928,6 +5935,8 @@ static int osd_sparse_read(struct ceph_connection *con, eoff = sr->sr_extent[sr->sr_index].off; elen = sr->sr_extent[sr->sr_index].len; + sr->sr_datalen -= elen; + dout("[%d] ext %d off 0x%llx len 0x%llx\n", o->o_osd, sr->sr_index, eoff, elen);