Re: [PATCH] dma/idxd: fix burst capacity calculation

[Bruce Richardson <bruce.richardson@intel.com>]

On Mon, Jan 10, 2022 at 01:44:06PM +0000, Pai G, Sunil wrote:
> Hi Bruce,
> 
> > what values for the write_idx and ids_returned vars give this error, and how
> > does masking help? I'd expect masking to increase the number of times the
> > function returns zero, rather than decreasing it.
> 
> 
> Here are the values from the idxd dump:
> dev_capa: 0x500000051 - mem2mem sva handles_errors copy fill
> max_vchans_supported: 1
> nb_vchans_configured: 1
> silent_mode: off
>  IDXD Private Data ==
> Portal: 0x7ffff7ffb000
> Config: { ring_size: 4096 }
> Batch ring (sz = 129, max_batches = 128):
> 62370  62402  62434  62466  62498  62530  62562  62594  62626  62658  62690  62722  62754  62786  62818  62850  62882  62914  62946  62978  63010  63042  63074  6
> 3106  63138  63170  63202  63234  63266  63298  63330  63362  63394  63426  63458  63490  63522  63554  63586  63618  63650  63682  63714  63746  63778  63810  63842  63874  63906  63938  63970  64002  64034  64066  64098  6413
> 0  64162  64194  64226  64258  64290  64322  64354  64386  64418  64450  64482  64514  64546  64578  64610  64642  64674  64706  64738  64770  64802  64834  64866  64898  64930  64962  64994  65026  65058  65090  65122  65154
> 65186  65218  65250  65282  65314  65346  65378  65410  65442  65474  65506 [rd ptr]  2 [wr ptr]  61442  61474  61506  61538  61570  61602  61634  61666  61698  61730  61762  61794  61826  61858  61890  61922  61954  61986  620
> 18  62050  62082  62114  62146  62178  62210  62242  62274  62306  62338
>   Curr batch: start = 2, size = 0
>   IDS: avail = 65506, returned: 65506
> max packets per batch from hw: 1024
> batches left: 127, ring space left: 4064
> idxd->desc_ring_mask: 4095, used_space: 4128, used_space: 4128, idxd->max_batch_size: 1024, idxd->batch_size: 0
> write_idx: 4098, idxd->batch_idx_read: 98, idxd->batch_idx_write: 99, idxd->desc_ring_mask - used_space: 65503
> 
> relevant data from above:
> write_idx: 4098 , IDS returned: 65506, idxd->desc_ring_mask: 4095
> 
> without the fix :
> used_space = write_idx - idxd->ids_returned; (4098 - 65506)   = -61408
> 
> with fix:
> used_space = (write_idx - idxd->ids_returned)& idxd->desc_ring_mask; (4098 - 65506)&4095   = 32
> which seems to match the rd ptr and wr ptr.
> 
Thanks, Sunil, that's clear now.
Rather than clamping at the end, I think it may be more logical to clamp
the ids_returned value at the start instead. How about the following diff -
does that also fix it for you?

/Bruce

--- a/drivers/dma/idxd/idxd_common.c
+++ b/drivers/dma/idxd/idxd_common.c
@@ -472,6 +472,7 @@ uint16_t
 idxd_burst_capacity(const void *dev_private, uint16_t vchan __rte_unused)
 {
        const struct idxd_dmadev *idxd = dev_private;
+       const uint16_t read_idx = idxd->ids_returned & idxd->desc_ring_mask;
        uint16_t write_idx = idxd->batch_start + idxd->batch_size;
        uint16_t used_space;
 
@@ -481,9 +482,9 @@ idxd_burst_capacity(const void *dev_private, uint16_t vchan __rte_unused)
                return 0;
 
        /* For descriptors, check for wrap-around on write but not read */
-       if (idxd->ids_returned > write_idx)
+       if (read_idx > write_idx)
                write_idx += idxd->desc_ring_mask + 1;
-       used_space = write_idx - idxd->ids_returned;
+       used_space = write_idx - read_idx;
 
        return RTE_MIN((idxd->desc_ring_mask - used_space), idxd->max_batch_size);
 }