[PATCH 5/5] UBUNTU: SAUCE: nvme: Fix erroneous parameter to dma_map_sg_attrs()

Seth Forshee seth.forshee at canonical.com
Fri Nov 18 14:57:19 UTC 2016


On Fri, Nov 18, 2016 at 07:25:54AM -0700, Tim Gardner wrote:
> On 11/18/2016 07:09 AM, Seth Forshee wrote:
> > On Fri, Nov 18, 2016 at 06:58:08AM -0700, Tim Gardner wrote:
> >> On 11/18/2016 06:36 AM, Seth Forshee wrote:
> >>> On Wed, Nov 16, 2016 at 07:35:56AM -0700, Tim Gardner wrote:
> >>>> BugLink: http://bugs.launchpad.net/bugs/1637565
> >>>>
> >>>> Signed-off-by: Tim Gardner <tim.gardner at canonical.com>
> >>>> ---
> >>>>  drivers/nvme/host/pci.c | 3 ++-
> >>>>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> >>>> index 5256448..3931ceb 100644
> >>>> --- a/drivers/nvme/host/pci.c
> >>>> +++ b/drivers/nvme/host/pci.c
> >>>> @@ -623,6 +623,7 @@ static int nvme_map_data(struct nvme_dev *dev, struct request *req,
> >>>>  	enum dma_data_direction dma_dir = rq_data_dir(req) ?
> >>>>  			DMA_TO_DEVICE : DMA_FROM_DEVICE;
> >>>>  	int ret = BLK_MQ_RQ_QUEUE_ERROR;
> >>>> +	DEFINE_DMA_ATTRS(attrs);
> >>>
> >>> You also need this:
> >>>
> >>>         dma_set_attr(DMA_ATTR_NO_WARN, &attrs);
> >>>
> >>
> >> DMA_ATTR_NO_WARN was not introduced until v4.9-rc1. I think map_sg()
> >> expects something from the enumerated set 'enum dma_attr'.
> >>
> >> I did find an instance of its use in arch/powerpc/kernel/iommu.c where
> >> DMA_ATTR_NO_WARN is being queried. I think that is an erroneous backport
> >> on my part, but harmless.
> > 
> > In that case you should just revert
> > 7c50722ad76b1b90538912fb84e2c3e206fab327 (nvme: use the DMA_ATTR_NO_WARN
> > attribute).
> > 
> 
> But that'll break powerpc. I'm inclined to leave it alone since it
> really is harmless. Or address it in another bug.

All 7c50722ad76b does is change the call to dma_map_sg() to
dma_map_sg_attrs(), for the sole purpose of passing DMA_ATTR_NO_WARN.
That backport is erroneous though because in xenial dma_map_sg_attrs()
needs a pointer to the flags, not an int containing the flags. So it's
broken _now_, because when ppc_iommu_map_sg() calls dma_get_attr() it's
expecting a pointer, and we've given it something else.

I only see two possibilities. Either we want to pass DMA_ATTR_NO_WARN,
and we need to define the attrs, set DMA_ATTR_NO_WARN, and pass a
pointer to the attrs. Or we don't want to pass DMA_ATTR_NO_WARN, so we
can just call dma_map_sg() (which passes NULL for the attrs, and
dma_get_attr() knows to check for that). It seems to me we want the
first option.

But you're right, it probably is material for a separate bug.




More information about the kernel-team mailing list