diff mbox series

[hmm,3/6] mm/hmm: remove unused code and tidy comments

Message ID 20200320164905.21722-4-jgg@ziepe.ca (mailing list archive)
State New, archived
Headers show
Series [hmm,1/6] mm/hmm: remove pgmap checking for devmap pages | expand

Commit Message

Jason Gunthorpe March 20, 2020, 4:49 p.m. UTC 
From: Jason Gunthorpe <jgg@mellanox.com>

Delete several functions that are never called, fix some desync between
comments and structure content, remove an unused ret, and move one
function only used by hmm.c into hmm.c

Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
---
 include/linux/hmm.h | 50 ---------------------------------------------
 mm/hmm.c            | 12 +++++++++++
 2 files changed, 12 insertions(+), 50 deletions(-)

Comments

Ralph Campbell March 20, 2020, 9:46 p.m. UTC | #1 
On 3/20/20 9:49 AM, Jason Gunthorpe wrote:
> From: Jason Gunthorpe <jgg@mellanox.com>
> 
> Delete several functions that are never called, fix some desync between
> comments and structure content, remove an unused ret, and move one
> function only used by hmm.c into hmm.c
> 
> Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>

Reviewed-by: Ralph Campbell <rcampbell@nvidia.com>

> ---
>   include/linux/hmm.h | 50 ---------------------------------------------
>   mm/hmm.c            | 12 +++++++++++
>   2 files changed, 12 insertions(+), 50 deletions(-)
> 
> diff --git a/include/linux/hmm.h b/include/linux/hmm.h
> index bb6be4428633a8..184a8633260f9d 100644
> --- a/include/linux/hmm.h
> +++ b/include/linux/hmm.h
> @@ -120,9 +120,6 @@ enum hmm_pfn_value_e {
>    *
>    * @notifier: a mmu_interval_notifier that includes the start/end
>    * @notifier_seq: result of mmu_interval_read_begin()
> - * @hmm: the core HMM structure this range is active against
> - * @vma: the vm area struct for the range
> - * @list: all range lock are on a list
>    * @start: range virtual start address (inclusive)
>    * @end: range virtual end address (exclusive)
>    * @pfns: array of pfns (big enough for the range)
> @@ -131,7 +128,6 @@ enum hmm_pfn_value_e {
>    * @default_flags: default flags for the range (write, read, ... see hmm doc)
>    * @pfn_flags_mask: allows to mask pfn flags so that only default_flags matter
>    * @pfn_shifts: pfn shift value (should be <= PAGE_SHIFT)

s/pfn_shifts/pfn_shift

> - * @valid: pfns array did not change since it has been fill by an HMM function
>    * @dev_private_owner: owner of device private pages
>    */
>   struct hmm_range {
> @@ -171,52 +167,6 @@ static inline struct page *hmm_device_entry_to_page(const struct hmm_range *rang
>   	return pfn_to_page(entry >> range->pfn_shift);
>   }
>   
> -/*
> - * hmm_device_entry_to_pfn() - return pfn value store in a device entry
> - * @range: range use to decode device entry value
> - * @entry: device entry to extract pfn from
> - * Return: pfn value if device entry is valid, -1UL otherwise
> - */
> -static inline unsigned long
> -hmm_device_entry_to_pfn(const struct hmm_range *range, uint64_t pfn)
> -{
> -	if (pfn == range->values[HMM_PFN_NONE])
> -		return -1UL;
> -	if (pfn == range->values[HMM_PFN_ERROR])
> -		return -1UL;
> -	if (pfn == range->values[HMM_PFN_SPECIAL])
> -		return -1UL;
> -	if (!(pfn & range->flags[HMM_PFN_VALID]))
> -		return -1UL;
> -	return (pfn >> range->pfn_shift);
> -}
> -
> -/*
> - * hmm_device_entry_from_page() - create a valid device entry for a page
> - * @range: range use to encode HMM pfn value
> - * @page: page for which to create the device entry
> - * Return: valid device entry for the page
> - */
> -static inline uint64_t hmm_device_entry_from_page(const struct hmm_range *range,
> -						  struct page *page)
> -{
> -	return (page_to_pfn(page) << range->pfn_shift) |
> -		range->flags[HMM_PFN_VALID];
> -}
> -
> -/*
> - * hmm_device_entry_from_pfn() - create a valid device entry value from pfn
> - * @range: range use to encode HMM pfn value
> - * @pfn: pfn value for which to create the device entry
> - * Return: valid device entry for the pfn
> - */
> -static inline uint64_t hmm_device_entry_from_pfn(const struct hmm_range *range,
> -						 unsigned long pfn)
> -{
> -	return (pfn << range->pfn_shift) |
> -		range->flags[HMM_PFN_VALID];
> -}
> -
>   /* Don't fault in missing PTEs, just snapshot the current state. */
>   #define HMM_FAULT_SNAPSHOT		(1 << 1)
>   
> diff --git a/mm/hmm.c b/mm/hmm.c
> index b4f662eadb7a7c..687d21c675ee60 100644
> --- a/mm/hmm.c
> +++ b/mm/hmm.c
> @@ -37,6 +37,18 @@ enum {
>   	NEED_WRITE_FAULT = 1 << 1,
>   };
>   
> +/*
> + * hmm_device_entry_from_pfn() - create a valid device entry value from pfn
> + * @range: range use to encode HMM pfn value
> + * @pfn: pfn value for which to create the device entry
> + * Return: valid device entry for the pfn
> + */
> +static uint64_t hmm_device_entry_from_pfn(const struct hmm_range *range,
> +					  unsigned long pfn)
> +{
> +	return (pfn << range->pfn_shift) | range->flags[HMM_PFN_VALID];
> +}
> +
>   static int hmm_pfns_fill(unsigned long addr, unsigned long end,
>   		struct hmm_range *range, enum hmm_pfn_value_e value)
>   {
>
Christoph Hellwig March 21, 2020, 8:39 a.m. UTC | #2 
On Fri, Mar 20, 2020 at 01:49:02PM -0300, Jason Gunthorpe wrote:
> From: Jason Gunthorpe <jgg@mellanox.com>
> 
> Delete several functions that are never called, fix some desync between
> comments and structure content, remove an unused ret, and move one
> function only used by hmm.c into hmm.c

This looks good:

Signed-off-by: Christoph Hellwig <hch@lst.de>

Btw, the top of file comment in include/linux/hmm.h really needs some
work as well.  In fact I think it should be mostly removed with any
remaining useful bit moved to Documentation/vm/hmm.rst.
Jason Gunthorpe March 23, 2020, 5:24 p.m. UTC | #3 
On Sat, Mar 21, 2020 at 09:39:02AM +0100, Christoph Hellwig wrote:
> On Fri, Mar 20, 2020 at 01:49:02PM -0300, Jason Gunthorpe wrote:
> > From: Jason Gunthorpe <jgg@mellanox.com>
> > 
> > Delete several functions that are never called, fix some desync between
> > comments and structure content, remove an unused ret, and move one
> > function only used by hmm.c into hmm.c
> 
> This looks good:
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

You mean Reviewed-by?
 
> Btw, the top of file comment in include/linux/hmm.h really needs some
> work as well.  In fact I think it should be mostly removed with any
> remaining useful bit moved to Documentation/vm/hmm.rst.

Okay, in v2 I'll just deleted the top, the only thing in this file now
is hmm_range_fault() and it can be adaquately described by its
kdoc comments.

Thanks,
Jason
Jason Gunthorpe March 23, 2020, 5:24 p.m. UTC | #4 
On Fri, Mar 20, 2020 at 02:46:09PM -0700, Ralph Campbell wrote:
> 
> On 3/20/20 9:49 AM, Jason Gunthorpe wrote:
> > From: Jason Gunthorpe <jgg@mellanox.com>
> > 
> > Delete several functions that are never called, fix some desync between
> > comments and structure content, remove an unused ret, and move one
> > function only used by hmm.c into hmm.c
> > 
> > Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
> 
> Reviewed-by: Ralph Campbell <rcampbell@nvidia.com>
> 
> >   include/linux/hmm.h | 50 ---------------------------------------------
> >   mm/hmm.c            | 12 +++++++++++
> >   2 files changed, 12 insertions(+), 50 deletions(-)
> > 
> > diff --git a/include/linux/hmm.h b/include/linux/hmm.h
> > index bb6be4428633a8..184a8633260f9d 100644
> > +++ b/include/linux/hmm.h
> > @@ -120,9 +120,6 @@ enum hmm_pfn_value_e {
> >    *
> >    * @notifier: a mmu_interval_notifier that includes the start/end
> >    * @notifier_seq: result of mmu_interval_read_begin()
> > - * @hmm: the core HMM structure this range is active against
> > - * @vma: the vm area struct for the range
> > - * @list: all range lock are on a list
> >    * @start: range virtual start address (inclusive)
> >    * @end: range virtual end address (exclusive)
> >    * @pfns: array of pfns (big enough for the range)
> > @@ -131,7 +128,6 @@ enum hmm_pfn_value_e {
> >    * @default_flags: default flags for the range (write, read, ... see hmm doc)
> >    * @pfn_flags_mask: allows to mask pfn flags so that only default_flags matter
> >    * @pfn_shifts: pfn shift value (should be <= PAGE_SHIFT)
> 
> s/pfn_shifts/pfn_shift

Got it in v2, thanks

Jason
Christoph Hellwig March 23, 2020, 5:27 p.m. UTC | #5 
On Mon, Mar 23, 2020 at 02:24:27PM -0300, Jason Gunthorpe wrote:
> > This looks good:
> > 
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> You mean Reviewed-by?

Yes, sorry.
diff mbox series

Patch

diff --git a/include/linux/hmm.h b/include/linux/hmm.h
index bb6be4428633a8..184a8633260f9d 100644
--- a/include/linux/hmm.h
+++ b/include/linux/hmm.h
@@ -120,9 +120,6 @@  enum hmm_pfn_value_e {
  *
  * @notifier: a mmu_interval_notifier that includes the start/end
  * @notifier_seq: result of mmu_interval_read_begin()
- * @hmm: the core HMM structure this range is active against
- * @vma: the vm area struct for the range
- * @list: all range lock are on a list
  * @start: range virtual start address (inclusive)
  * @end: range virtual end address (exclusive)
  * @pfns: array of pfns (big enough for the range)
@@ -131,7 +128,6 @@  enum hmm_pfn_value_e {
  * @default_flags: default flags for the range (write, read, ... see hmm doc)
  * @pfn_flags_mask: allows to mask pfn flags so that only default_flags matter
  * @pfn_shifts: pfn shift value (should be <= PAGE_SHIFT)
- * @valid: pfns array did not change since it has been fill by an HMM function
  * @dev_private_owner: owner of device private pages
  */
 struct hmm_range {
@@ -171,52 +167,6 @@  static inline struct page *hmm_device_entry_to_page(const struct hmm_range *rang
 	return pfn_to_page(entry >> range->pfn_shift);
 }
 
-/*
- * hmm_device_entry_to_pfn() - return pfn value store in a device entry
- * @range: range use to decode device entry value
- * @entry: device entry to extract pfn from
- * Return: pfn value if device entry is valid, -1UL otherwise
- */
-static inline unsigned long
-hmm_device_entry_to_pfn(const struct hmm_range *range, uint64_t pfn)
-{
-	if (pfn == range->values[HMM_PFN_NONE])
-		return -1UL;
-	if (pfn == range->values[HMM_PFN_ERROR])
-		return -1UL;
-	if (pfn == range->values[HMM_PFN_SPECIAL])
-		return -1UL;
-	if (!(pfn & range->flags[HMM_PFN_VALID]))
-		return -1UL;
-	return (pfn >> range->pfn_shift);
-}
-
-/*
- * hmm_device_entry_from_page() - create a valid device entry for a page
- * @range: range use to encode HMM pfn value
- * @page: page for which to create the device entry
- * Return: valid device entry for the page
- */
-static inline uint64_t hmm_device_entry_from_page(const struct hmm_range *range,
-						  struct page *page)
-{
-	return (page_to_pfn(page) << range->pfn_shift) |
-		range->flags[HMM_PFN_VALID];
-}
-
-/*
- * hmm_device_entry_from_pfn() - create a valid device entry value from pfn
- * @range: range use to encode HMM pfn value
- * @pfn: pfn value for which to create the device entry
- * Return: valid device entry for the pfn
- */
-static inline uint64_t hmm_device_entry_from_pfn(const struct hmm_range *range,
-						 unsigned long pfn)
-{
-	return (pfn << range->pfn_shift) |
-		range->flags[HMM_PFN_VALID];
-}
-
 /* Don't fault in missing PTEs, just snapshot the current state. */
 #define HMM_FAULT_SNAPSHOT		(1 << 1)
 
diff --git a/mm/hmm.c b/mm/hmm.c
index b4f662eadb7a7c..687d21c675ee60 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -37,6 +37,18 @@  enum {
 	NEED_WRITE_FAULT = 1 << 1,
 };
 
+/*
+ * hmm_device_entry_from_pfn() - create a valid device entry value from pfn
+ * @range: range use to encode HMM pfn value
+ * @pfn: pfn value for which to create the device entry
+ * Return: valid device entry for the pfn
+ */
+static uint64_t hmm_device_entry_from_pfn(const struct hmm_range *range,
+					  unsigned long pfn)
+{
+	return (pfn << range->pfn_shift) | range->flags[HMM_PFN_VALID];
+}
+
 static int hmm_pfns_fill(unsigned long addr, unsigned long end,
 		struct hmm_range *range, enum hmm_pfn_value_e value)
 {