diff mbox series

[v2,2/2] libceph: check the data length when finishes

Message ID 20231101005033.21995-3-xiubli@redhat.com (mailing list archive)
State New, archived
Headers show
Series libceph: sparse-read misc fixes | expand

Commit Message

Xiubo Li Nov. 1, 2023, 12:50 a.m. UTC
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(+)

Comments

Ilya Dryomov Nov. 3, 2023, 12:49 p.m. UTC | #1
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
Xiubo Li Nov. 6, 2023, 12:07 a.m. UTC | #2
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 mbox series

Patch

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);