Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Wed, 13 Nov 2019 13:49:43 -0500
From:   Jerome Glisse <jglisse@redhat.com>
To:     Christoph Hellwig <hch@lst.de>
Cc:     jgg@ziepe.ca, linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mm/hmm: remove hmm_range_dma_map and hmm_range_dma_unmap
Message-ID: <20191113184943.GA4319@redhat.com>
References: <20191113134528.21187-1-hch@lst.de>
MIME-Version: 1.0
In-Reply-To: <20191113134528.21187-1-hch@lst.de>
User-Agent: Mutt/1.12.1 (2019-06-15)
Content-Type: text/plain; charset=WINDOWS-1252
Content-Transfer-Encoding: quoted-printable
Content-Disposition: inline
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, Nov 13, 2019 at 02:45:28PM +0100, Christoph Hellwig wrote:
> These two functions have never been used since they were added.

The mlx5 convertion (which has been posted few times now) uses them
dunno what Jason plans is on that front.

>=20
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  include/linux/hmm.h |  23 -------
>  mm/hmm.c            | 147 --------------------------------------------
>  2 files changed, 170 deletions(-)
>=20
> diff --git a/include/linux/hmm.h b/include/linux/hmm.h
> index b4af51735232..922c8d015cdb 100644
> --- a/include/linux/hmm.h
> +++ b/include/linux/hmm.h
> @@ -230,34 +230,11 @@ static inline uint64_t hmm_device_entry_from_pfn(co=
nst struct hmm_range *range,
>   * Please see Documentation/vm/hmm.rst for how to use the range API.
>   */
>  long hmm_range_fault(struct hmm_range *range, unsigned int flags);
> -
> -long hmm_range_dma_map(struct hmm_range *range,
> -=09=09       struct device *device,
> -=09=09       dma_addr_t *daddrs,
> -=09=09       unsigned int flags);
> -long hmm_range_dma_unmap(struct hmm_range *range,
> -=09=09=09 struct device *device,
> -=09=09=09 dma_addr_t *daddrs,
> -=09=09=09 bool dirty);
>  #else
>  static inline long hmm_range_fault(struct hmm_range *range, unsigned int=
 flags)
>  {
>  =09return -EOPNOTSUPP;
>  }
> -
> -static inline long hmm_range_dma_map(struct hmm_range *range,
> -=09=09=09=09     struct device *device, dma_addr_t *daddrs,
> -=09=09=09=09     unsigned int flags)
> -{
> -=09return -EOPNOTSUPP;
> -}
> -
> -static inline long hmm_range_dma_unmap(struct hmm_range *range,
> -=09=09=09=09       struct device *device,
> -=09=09=09=09       dma_addr_t *daddrs, bool dirty)
> -{
> -=09return -EOPNOTSUPP;
> -}
>  #endif
> =20
>  /*
> diff --git a/mm/hmm.c b/mm/hmm.c
> index 9e9d3f4ea17c..ab883eefe847 100644
> --- a/mm/hmm.c
> +++ b/mm/hmm.c
> @@ -698,150 +698,3 @@ long hmm_range_fault(struct hmm_range *range, unsig=
ned int flags)
>  =09return (hmm_vma_walk.last - range->start) >> PAGE_SHIFT;
>  }
>  EXPORT_SYMBOL(hmm_range_fault);
> -
> -/**
> - * hmm_range_dma_map - hmm_range_fault() and dma map page all in one.
> - * @range:=09range being faulted
> - * @device:=09device to map page to
> - * @daddrs:=09array of dma addresses for the mapped pages
> - * @flags:=09HMM_FAULT_*
> - *
> - * Return: the number of pages mapped on success (including zero), or an=
y
> - * status return from hmm_range_fault() otherwise.
> - */
> -long hmm_range_dma_map(struct hmm_range *range, struct device *device,
> -=09=09dma_addr_t *daddrs, unsigned int flags)
> -{
> -=09unsigned long i, npages, mapped;
> -=09long ret;
> -
> -=09ret =3D hmm_range_fault(range, flags);
> -=09if (ret <=3D 0)
> -=09=09return ret ? ret : -EBUSY;
> -
> -=09npages =3D (range->end - range->start) >> PAGE_SHIFT;
> -=09for (i =3D 0, mapped =3D 0; i < npages; ++i) {
> -=09=09enum dma_data_direction dir =3D DMA_TO_DEVICE;
> -=09=09struct page *page;
> -
> -=09=09/*
> -=09=09 * FIXME need to update DMA API to provide invalid DMA address
> -=09=09 * value instead of a function to test dma address value. This
> -=09=09 * would remove lot of dumb code duplicated accross many arch.
> -=09=09 *
> -=09=09 * For now setting it to 0 here is good enough as the pfns[]
> -=09=09 * value is what is use to check what is valid and what isn't.
> -=09=09 */
> -=09=09daddrs[i] =3D 0;
> -
> -=09=09page =3D hmm_device_entry_to_page(range, range->pfns[i]);
> -=09=09if (page =3D=3D NULL)
> -=09=09=09continue;
> -
> -=09=09/* Check if range is being invalidated */
> -=09=09if (mmu_range_check_retry(range->notifier,
> -=09=09=09=09=09  range->notifier_seq)) {
> -=09=09=09ret =3D -EBUSY;
> -=09=09=09goto unmap;
> -=09=09}
> -
> -=09=09/* If it is read and write than map bi-directional. */
> -=09=09if (range->pfns[i] & range->flags[HMM_PFN_WRITE])
> -=09=09=09dir =3D DMA_BIDIRECTIONAL;
> -
> -=09=09daddrs[i] =3D dma_map_page(device, page, 0, PAGE_SIZE, dir);
> -=09=09if (dma_mapping_error(device, daddrs[i])) {
> -=09=09=09ret =3D -EFAULT;
> -=09=09=09goto unmap;
> -=09=09}
> -
> -=09=09mapped++;
> -=09}
> -
> -=09return mapped;
> -
> -unmap:
> -=09for (npages =3D i, i =3D 0; (i < npages) && mapped; ++i) {
> -=09=09enum dma_data_direction dir =3D DMA_TO_DEVICE;
> -=09=09struct page *page;
> -
> -=09=09page =3D hmm_device_entry_to_page(range, range->pfns[i]);
> -=09=09if (page =3D=3D NULL)
> -=09=09=09continue;
> -
> -=09=09if (dma_mapping_error(device, daddrs[i]))
> -=09=09=09continue;
> -
> -=09=09/* If it is read and write than map bi-directional. */
> -=09=09if (range->pfns[i] & range->flags[HMM_PFN_WRITE])
> -=09=09=09dir =3D DMA_BIDIRECTIONAL;
> -
> -=09=09dma_unmap_page(device, daddrs[i], PAGE_SIZE, dir);
> -=09=09mapped--;
> -=09}
> -
> -=09return ret;
> -}
> -EXPORT_SYMBOL(hmm_range_dma_map);
> -
> -/**
> - * hmm_range_dma_unmap() - unmap range of that was map with hmm_range_dm=
a_map()
> - * @range: range being unmapped
> - * @device: device against which dma map was done
> - * @daddrs: dma address of mapped pages
> - * @dirty: dirty page if it had the write flag set
> - * Return: number of page unmapped on success, -EINVAL otherwise
> - *
> - * Note that caller MUST abide by mmu notifier or use HMM mirror and abi=
de
> - * to the sync_cpu_device_pagetables() callback so that it is safe here =
to
> - * call set_page_dirty(). Caller must also take appropriate locks to avo=
id
> - * concurrent mmu notifier or sync_cpu_device_pagetables() to make progr=
ess.
> - */
> -long hmm_range_dma_unmap(struct hmm_range *range,
> -=09=09=09 struct device *device,
> -=09=09=09 dma_addr_t *daddrs,
> -=09=09=09 bool dirty)
> -{
> -=09unsigned long i, npages;
> -=09long cpages =3D 0;
> -
> -=09/* Sanity check. */
> -=09if (range->end <=3D range->start)
> -=09=09return -EINVAL;
> -=09if (!daddrs)
> -=09=09return -EINVAL;
> -=09if (!range->pfns)
> -=09=09return -EINVAL;
> -
> -=09npages =3D (range->end - range->start) >> PAGE_SHIFT;
> -=09for (i =3D 0; i < npages; ++i) {
> -=09=09enum dma_data_direction dir =3D DMA_TO_DEVICE;
> -=09=09struct page *page;
> -
> -=09=09page =3D hmm_device_entry_to_page(range, range->pfns[i]);
> -=09=09if (page =3D=3D NULL)
> -=09=09=09continue;
> -
> -=09=09/* If it is read and write than map bi-directional. */
> -=09=09if (range->pfns[i] & range->flags[HMM_PFN_WRITE]) {
> -=09=09=09dir =3D DMA_BIDIRECTIONAL;
> -
> -=09=09=09/*
> -=09=09=09 * See comments in function description on why it is
> -=09=09=09 * safe here to call set_page_dirty()
> -=09=09=09 */
> -=09=09=09if (dirty)
> -=09=09=09=09set_page_dirty(page);
> -=09=09}
> -
> -=09=09/* Unmap and clear pfns/dma address */
> -=09=09dma_unmap_page(device, daddrs[i], PAGE_SIZE, dir);
> -=09=09range->pfns[i] =3D range->values[HMM_PFN_NONE];
> -=09=09/* FIXME see comments in hmm_vma_dma_map() */
> -=09=09daddrs[i] =3D 0;
> -=09=09cpages++;
> -=09}
> -
> -=09return cpages;
> -}
> -EXPORT_SYMBOL(hmm_range_dma_unmap);
> --=20
> 2.20.1
>=20