Hi Changwei and Joseph,
EIO sounds more reasonable, thanks a lot for your suggestions, and I will
send patch v2 later.
thanks,
Jun
On 2018/3/29 9:09, Changwei Ge wrote:
> Hi Jun,
>
> On 2018/3/28 17:51, Joseph Qi wrote:
>>
>>
>> On 18/3/28 15:02, piaojun wrote:
>>> Hi Joseph,
>>>
>>> On 2018/3/28 12:58, Joseph Qi wrote:
>>>>
>>>>
>>>> On 18/3/28 11:50, piaojun wrote:
>>>>> We need check len for bio_add_page() to make sure the bio has been set up
>>>>> correctly, otherwise we may submit incorrect data to device.
>>>>>
>>>>> Signed-off-by: Jun Piao <piao...@huawei.com>
>>>>> Reviewed-by: Yiwen Jiang <jiangyi...@huawei.com>
>>>>> ---
>>>>> fs/ocfs2/cluster/heartbeat.c | 11 ++++++++++-
>>>>> 1 file changed, 10 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/fs/ocfs2/cluster/heartbeat.c b/fs/ocfs2/cluster/heartbeat.c
>>>>> index ea8c551..43ad79f 100644
>>>>> --- a/fs/ocfs2/cluster/heartbeat.c
>>>>> +++ b/fs/ocfs2/cluster/heartbeat.c
>>>>> @@ -570,7 +570,16 @@ static struct bio *o2hb_setup_one_bio(struct
>>>>> o2hb_region *reg,
>>>>> current_page, vec_len, vec_start);
>>>>>
>>>>> len = bio_add_page(bio, page, vec_len, vec_start);
>>>>> - if (len != vec_len) break;
>>>>> + if (len != vec_len) {
>>>>> + mlog(ML_ERROR, "Adding page[%d] to bio failed, "
>>>>> + "page %p, len %d, vec_len %u, vec_start %u, "
>>>>> + "bi_sector %llu\n", current_page, page, len,
>>>>> + vec_len, vec_start,
>>>>> + (unsigned long long)bio->bi_iter.bi_sector);
>>>>> + bio_put(bio);
>>>>> + bio = ERR_PTR(-EFAULT);
>>>>
>>>> IMO, EFAULT is not an appropriate error code here.
>>>> If __bio_add_page returns 0, some are caused by bio checking failed.
>>>> Also I've noticed that several other callers just use ENOMEM, so I think
>>>> EINVAL or ENOMEM may be better.
>>>
>>> __bio_add_page has been deleted in patch c66a14d07c13, and I notice that
>>> other callers always use -EFAULT or -EIO. I'm afraid we are not basing on
>>> the same kernel source.
>>>
>>
>> Oops... Yes, I was looking an old kernel...
>> EIO sounds reasonable, but I don't know why EFAULT since it means "Bad
>> address".
>
> I agree with Joseph that EFAULT seems unreasonable for this exception cached.
> But your trick looks good to me.
> After applying a more appropriate error number, please feel free to add my:
> Reviewed-by: Changwei Ge <ge.chang...@h3c.com>
>
> Thanks,
> Changwei
>
>
>>
>> Thanks,
>> Joseph
>>
>> _______________________________________________
>> Ocfs2-devel mailing list
>> Ocfs2-devel@oss.oracle.com
>> https://oss.oracle.com/mailman/listinfo/ocfs2-devel
>>
> .
>
_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel
