[rds-devel] [PATCH rds linux-next v2 2/2] net/rds: remove user triggered WARN_ON in rds_sendmsg

[santosh.shilimkar at oracle.com]

On 12/11/18 1:41 PM, Gerd Rausch wrote:
> 
> 
> On 11/12/2018 13.32, santosh.shilimkar at oracle.com wrote:
> 
>>>> --- a/net/rds/message.c
>>>> +++ b/net/rds/message.c
>>>> @@ -313,11 +313,14 @@ struct scatterlist *rds_message_alloc_sgs(struct rds_message *rm, int nents)
>>>>        struct scatterlist *sg_first = (struct scatterlist *) &rm[1];
>>>>        struct scatterlist *sg_ret;
>>>>    -    WARN_ON(rm->m_used_sgs + nents > rm->m_total_sgs);
>>>> -    WARN_ON(!nents);
>>>> -
>>>> -    if (rm->m_used_sgs + nents > rm->m_total_sgs)
>>>> +    if (rm->m_used_sgs + nents > rm->m_total_sgs) {
>>>> +        pr_warn("rds: alloc sgs failed! total %d used %d nents %d\n",
>>>> +            rm->m_total_sgs, rm->m_used_sgs, nents);
>>>>            return NULL;
>>>> +    }
>>>> +
>>>> +    if (!nents)
>>>> +        pr_warn("rds: alloc sgs failed! nents 0\n");
>> I believe your are taking about above pr_warn, for nents==0 case.
>> Its ok to drop the pr_warn but if the behavior leads to
>> corruption, then lets fail the nents==0 case to instead of
>> silently proceeding.
>>
> 
> Correct.
> 
> IMHO a BUG_ON(!nents) beats memory corruption any day.
> 
BUG_ON() isn't desirable since it takes the entire server down for
one modules misbahvior. So lets just return error back to the
user and abort the send message path. Thanks for expanding your point.

Regards,
Santosh