On 2021-04-27 1:31 p.m., Jason Gunthorpe wrote:
> On Thu, Apr 08, 2021 at 11:01:12AM -0600, Logan Gunthorpe wrote:
>> +/*
>> + * dma_maps_sg_attrs returns 0 on error and > 0 on success.
>> + * It should never return a value < 0.
>> + */
>
> Also it is weird a function that can't return 0 is returning an int type
Yes, Christoph mentioned in the last series that this should probably
change to an unsigned but I wasn't really sure if that change should be
a part of the P2PDMA series.
>> +int dma_map_sg_attrs(struct device *dev, struct scatterlist *sg, int nents,
>> + enum dma_data_direction dir, unsigned long attrs)
>> +{
>> + int ents;
>> +
>> + ents = __dma_map_sg_attrs(dev, sg, nents, dir, attrs);
>> BUG_ON(ents < 0);
>
> if (WARN_ON(ents < 0))
> return 0;
>
> instead of bug on?
It was BUG_ON in the original code. So I felt I should leave it.
> Also, I see only 8 users of this function. How about just fix them all
> to support negative returns and use this as the p2p API instead of
> adding new API?
Well there might be 8 users of dma_map_sg_attrs() but there are a very
large number of dma_map_sg(). Seems odd to me to single out the first as
requiring these changes, but leave the latter.
> Add the opposite logic flag, 'DMA_ATTRS_NO_ERROR' and pass it through
> the other api entry callers that can't handle it?
I'm not that opposed to this. But it will make this series a fair bit
longer to change the 8 map_sg_attrs() usages.
Logan
