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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aeaf3067-59cf-5973-be17-3224de566ff8@wanadoo.fr>
Date:   Mon, 23 Aug 2021 19:21:24 +0200
From:   Christophe JAILLET <christophe.jaillet@...adoo.fr>
To:     Jason Gunthorpe <jgg@...dia.com>
Cc:     mike.marciniszyn@...nelisnetworks.com,
        dennis.dalessandro@...nelisnetworks.com, dledford@...hat.com,
        aditr@...are.com, pv-drivers@...are.com,
        linux-rdma@...r.kernel.org, linux-kernel@...r.kernel.org,
        kernel-janitors@...r.kernel.org
Subject: Re: [PATCH] RDMA: switch from 'pci_' to 'dma_' API

Le 23/08/2021 à 18:59, Jason Gunthorpe a écrit :
> On Sun, Aug 22, 2021 at 02:24:44PM +0200, Christophe JAILLET wrote:
>> The wrappers in include/linux/pci-dma-compat.h should go away.
>>
>> The patch has been generated with the coccinelle script below.
>>
>> It has been hand modified to use 'dma_set_mask_and_coherent()' instead of
>> 'pci_set_dma_mask()/pci_set_consistent_dma_mask()' when applicable.
>> This is less verbose.
>>
>> It has been compile tested.
>>
[...]
>>
>> This patch is mostly mechanical and compile tested. I hope it is ok to
>> update the "drivers/infiniband/hw/" directory all at once.
> 
> I think I would have preferred to see the more tricky
> dma_set_mask_and_coherent() conversion as its own patch, but it looks
> OK

Hi, I agree, but as I already answered to another reviewer:

"
When I started this task proposed by Christoph Hellwig ([1]), their were 
150 files using using 'pci_set_dma_mask()' ([2]). Many of them were good 
candidate for using 'dma_set_mask_and_coherent()' but this 
transformation can not easily be done by coccinelle because it depends 
on the way the code has been written.

So, I decided to hand modify and include the transformation in the many 
patches have sent to remove this deprecated API.
Up to now, it has never been an issue.

I *DO* know that it should have been a 2 steps process but this clean-up 
was too big for me (i.e. 150 files) and doing the job twice was 
discouraging.

My first motivation was to remove the deprecated API. Simplifying code 
and using 'dma_set_mask_and_coherent()' when applicable was just a bonus.

[...]

[1]: https://marc.info/?l=kernel-janitors&m=158745678307186&w=4
[2]: https://elixir.bootlin.com/linux/v5.8/A/ident/pci_set_dma_mask
"

> 
>> ---
>>   drivers/infiniband/hw/hfi1/pcie.c             | 11 ++-------
>>   drivers/infiniband/hw/hfi1/user_exp_rcv.c     | 13 +++++------
>>   drivers/infiniband/hw/mthca/mthca_eq.c        | 21 +++++++++--------
>>   drivers/infiniband/hw/mthca/mthca_main.c      | 15 ++----------
>>   drivers/infiniband/hw/mthca/mthca_memfree.c   | 23 +++++++++++--------
>>   drivers/infiniband/hw/qib/qib_file_ops.c      | 12 +++++-----
>>   drivers/infiniband/hw/qib/qib_init.c          |  4 ++--
>>   drivers/infiniband/hw/qib/qib_user_pages.c    | 12 +++++-----
>>   .../infiniband/hw/vmw_pvrdma/pvrdma_main.c    | 14 +++--------
>>   9 files changed, 51 insertions(+), 74 deletions(-)
> 
> Applied to for-next, thanks

So thanks for your understanding and accepting this proposal as-is.

CJ


> 
> Jason
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ