On 14/04/17 02:35 AM, Christoph Hellwig wrote: >> + >> static inline int is_dma_buf_file(struct file *); >> >> struct dma_buf_list { > > I think the right fix here is to rename the operation to unmap_atomic > and send out a little patch for that ASAP. Ok, I can do that next week. > I'd rather have separate functions for kmap vs kmap_atomic instead of > the flags parameter. And while you're at it just always pass the 0 > offset parameter instead of adding a wrapper.. > > Otherwise this looks good to me. I settled on the flags because I thought the interface could be expanded to do more things like automatically copy iomem to a bounce buffer (with a flag). It'd also be possible to add things like vmap and physical_address to the interface which would cover even more sg_page users. All the implementations would then share the common offset calculations, and switching between them becomes a matter of changing a couple flags. If you're still not convinced by the above arguments then I'll change it but I did have reasons for choosing to do it this way. I am fine with removing the offset versions. I will make that change. Thanks, Logan