Re: [PATCH v4 1/3] libceph: fail the sparse-read if there still has data in socket

From: Xiubo Li <xiubli@redhat.com>
To: Jeff Layton <jlayton@kernel.org>, ceph-devel@vger.kernel.org
Cc: idryomov@gmail.com, vshankar@redhat.com, mchangir@redhat.com
Subject: Re: [PATCH v4 1/3] libceph: fail the sparse-read if there still has data in socket
Date: Fri, 19 Jan 2024 12:07:42 +0800	[thread overview]
Message-ID: <bd81112b-bdbd-466a-943e-90742c100c47@redhat.com> (raw)
In-Reply-To: <cca21aec6fa2a1728cb85099e1ba750bdf2fd696.camel@kernel.org>

On 1/18/24 22:03, Jeff Layton wrote:
> On Thu, 2024-01-18 at 18:50 +0800, xiubli@redhat.com wrote:
>> From: Xiubo Li <xiubli@redhat.com>
>>
>> Once this happens that means there have bugs.
>>
>> URL: https://tracker.ceph.com/issues/63586
>> Signed-off-by: Xiubo Li <xiubli@redhat.com>
>> ---
>>   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 9be80d01c1dc..f8029b30a3fb 100644
>> --- a/net/ceph/osd_client.c
>> +++ b/net/ceph/osd_client.c
>> @@ -5912,6 +5912,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) {
>> +				pr_warn_ratelimited("sr_datalen %u sr_index %d count %u\n",
>> +						    sr->sr_datalen, sr->sr_index,
>> +						    count);
>> +				return -EREMOTEIO;
>> +			}
>> +
> Ok, so the server has (presumably) sent us a longer value for the
> sr_datalen than was in the extent map?
>
> Why should the sparse read engine care about that? It was (presumably)
> able to do its job of handling the read. Why not just advance past the
> extra junk and try to do another sparse read? Do we really need to fail
> the op for this?

Hi Jeff,

I saw the problem just when I first debugging the sparse-read bug, and 
the length will be very large, more detail and the logs please see the 
tracker https://tracker.ceph.com/issues/63586:

      11741055 <4>[180940.606488] libceph: sr_datalen 251723776 sr_index 
0 count 0

In this case the request could cause the same request being retrying 
infinitely. While just in other case the when the ceph send a incorrect 
data length, how should we do ? Should we retry it ? How could we skip 
it if the length is so large ?

Thanks

- Xiubo

>
>>   			sr->sr_state = CEPH_SPARSE_READ_HDR;
>>   			goto next_op;
>>   		}
>> @@ -5919,6 +5926,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);
>>   