linux-kernel - Re: [PATCH] swiotlb: Fix inversed DMA_ATTR_NO_WARN test

Open Source and information security mailing list archives

Message-ID: <CAKMK7uH2+v3_UUqXdCjz_8=H4VRt1zB+dgE99PuuPqd+XM2=-w@mail.gmail.com>
Date:   Wed, 2 May 2018 14:18:56 +0200
From:   Daniel Vetter <daniel@...ll.ch>
To:     Christian König <christian.koenig@....com>,
        Chris Wilson <chris@...is-wilson.co.uk>
Cc:     Michel Dänzer <michel@...nzer.net>,
        Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
        Christoph Hellwig <hch@....de>,
        iommu@...ts.linux-foundation.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        dri-devel <dri-devel@...ts.freedesktop.org>
Subject: Re: [PATCH] swiotlb: Fix inversed DMA_ATTR_NO_WARN test

On Wed, May 2, 2018 at 11:49 AM, Christian König
<ckoenig.leichtzumerken@...il.com> wrote:
> Am 01.05.2018 um 15:24 schrieb Michel Dänzer:
>>
>> From: Michel Dänzer <michel.daenzer@....com>
>>
>> The result was printing the warning only when we were explicitly asked
>> not to.
>>
>> Cc: stable@...r.kernel.org
>> Fixes: 0176adb004065d6815a8e67946752df4cd947c5b "swiotlb: refactor
>>   coherent buffer allocation"
>> Signed-off-by: Michel Dänzer <michel.daenzer@....com>
>
>
> Good catch, looked at the code multiple times and haven't seen that myself
> :)
>
> Reviewed-by: Christian König <christian.koenig@....com>.

Other dma-api backends like cma just shut up when __GFP_NOWARN is
passed. And afaiui Christoph Hellwig has plans to nuke the DMA_ATTR
stuff (or at least clean it up) - should we just remove
DMA_ATTR_NO_WARN and instead only look at __GFP_NOWARN?

For context the CMA patch:

commit ef4650144e76ae361fe4b8c9a0afcd53074cd520
Author: Boris Brezillon <boris.brezillon@...e-electrons.com>
Date:   Fri Oct 13 15:58:01 2017 -0700

    mm/cma.c: take __GFP_NOWARN into account in cma_alloc()

Or maybe we should at least enforce that both or none are set, for
consistency for now?

Cheers, Daniel

>
> Christian.
>
>> ---
>>   lib/swiotlb.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/lib/swiotlb.c b/lib/swiotlb.c
>> index c43ec2271469..e9ac21540628 100644
>> --- a/lib/swiotlb.c
>> +++ b/lib/swiotlb.c
>> @@ -750,7 +750,7 @@ swiotlb_alloc_buffer(struct device *dev, size_t size,
>> dma_addr_t *dma_handle,
>>         swiotlb_tbl_unmap_single(dev, phys_addr, size, DMA_TO_DEVICE,
>>                         DMA_ATTR_SKIP_CPU_SYNC);
>>   out_warn:
>> -       if ((attrs & DMA_ATTR_NO_WARN) && printk_ratelimit()) {
>> +       if (!(attrs & DMA_ATTR_NO_WARN) && printk_ratelimit()) {
>>                 dev_warn(dev,
>>                         "swiotlb: coherent allocation failed, size=%zu\n",
>>                         size);
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@...ts.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives