On Wed, Jun 22, 2016 at 10:47:27AM -0400, Chuck Lever wrote: > > > On Jun 22, 2016, at 10:04 AM, Sagi Grimberg <sagi@xxxxxxxxxxx> wrote: > > > > > >> + /* This is overkill, but hardware requires that the > >> + * PBL array begins at a properly aligned address and > >> + * never occupies the last 8 bytes of a page. > >> + */ > >> + mr->pages = (__be64 *)get_zeroed_page(GFP_KERNEL); > >> + if (!mr->pages) > >> return -ENOMEM; > > > > Again, I'm not convinced that this is a better choice then allocating > > the exact needed size as dma coherent, but given that the dma coherent > > allocations are always page aligned I wander if it's not the same > > effect... > > My concerns with DMA coherent were: > > 1. That pool may be a somewhat limited resource? > > 2. IMO DMA-API.txt suggests DMA coherent will perform less > well in some cases. Macro benchmarks I ran seemed to show > there was a slight performance hit with that approach, though > it was nearly in the noise. > > I agree that the over-allocation in the streaming solution is a > concern. But as you say, there may be little we can do about it. According to [1] dma_alloc_coherent doesn't allocate from pool, but calls to the __get_free_page(). "A DMA pool is an allocation mechanism for small, coherent DMA mappings. Mappings obtained from dma_alloc_coherent may have a minimum size of one page." > > Wrt to Or's comment, the device's maximum page list depth > is advertised to consumers via the device's attributes. However, > it would be defensive if there was a sanity check added in > mlx4_alloc_priv_pages to ensure that the max_pages argument > is a reasonable value (ie, that the calculated array size does > indeed fit into a page). > > > In any event, we can move forward with this for now: > > > > Reviewed-by: Sagi Grimberg <sagi@xxxxxxxxxxx> > > Thanks, I'll add that! Though as before, I'm happy to drop this > patch if there is a different preferred official fix. We submitted your version of patch with minor changes in comments and commit message together with Sagi's ROB tag [2]. [1] http://www.makelinux.net/ldd3/chp-15-sect-4 [2] https://patchwork.kernel.org/patch/9193075/ > -- > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html
Attachment:
signature.asc
Description: Digital signature
