diff mbox series

[kvmtool,v1,03/17] Rename parameter in mmap_anon_or_hugetlbfs()

Message ID 20221115111549.2784927-4-tabba@google.com (mailing list archive)
State New, archived
Headers show
Series Use memfd for guest vm memory allocation | expand

Commit Message

Fuad Tabba Nov. 15, 2022, 11:15 a.m. UTC
For consistency with other similar functions in the same file and
for brevity.

No functional change intended.

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 include/kvm/util.h | 2 +-
 util/util.c        | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

Comments

Alexandru Elisei Nov. 23, 2022, 4:40 p.m. UTC | #1
Hi,

On Tue, Nov 15, 2022 at 11:15:35AM +0000, Fuad Tabba wrote:
> For consistency with other similar functions in the same file and
> for brevity.
> 
> No functional change intended.
> 
> Signed-off-by: Fuad Tabba <tabba@google.com>
> ---
>  include/kvm/util.h | 2 +-
>  util/util.c        | 6 +++---
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/include/kvm/util.h b/include/kvm/util.h
> index b0c3684..61a205b 100644
> --- a/include/kvm/util.h
> +++ b/include/kvm/util.h
> @@ -140,6 +140,6 @@ static inline int pow2_size(unsigned long x)
>  }
>  
>  struct kvm;
> -void *mmap_anon_or_hugetlbfs(struct kvm *kvm, const char *hugetlbfs_path, u64 size);
> +void *mmap_anon_or_hugetlbfs(struct kvm *kvm, const char *htlbfs_path, u64 size);
>  
>  #endif /* KVM__UTIL_H */
> diff --git a/util/util.c b/util/util.c
> index 093bd3b..22b64b6 100644
> --- a/util/util.c
> +++ b/util/util.c
> @@ -118,14 +118,14 @@ static void *mmap_hugetlbfs(struct kvm *kvm, const char *htlbfs_path, u64 size)
>  }
>  
>  /* This function wraps the decision between hugetlbfs map (if requested) or normal mmap */
> -void *mmap_anon_or_hugetlbfs(struct kvm *kvm, const char *hugetlbfs_path, u64 size)
> +void *mmap_anon_or_hugetlbfs(struct kvm *kvm, const char *htlbfs_path, u64 size)

All the functions that deal with hugetlbfs have "hugetlbfs" in the name,
and the kvm_config field is called hugetlbfs_path. Wouldn't it make more
sense to rename the htlbfs_path parameter to hugetlbs_path instead of the
other way around?

Thanks,
Alex

>  {
> -	if (hugetlbfs_path)
> +	if (htlbfs_path)
>  		/*
>  		 * We don't /need/ to map guest RAM from hugetlbfs, but we do so
>  		 * if the user specifies a hugetlbfs path.
>  		 */
> -		return mmap_hugetlbfs(kvm, hugetlbfs_path, size);
> +		return mmap_hugetlbfs(kvm, htlbfs_path, size);
>  	else {
>  		kvm->ram_pagesize = getpagesize();
>  		return mmap(NULL, size, PROT_RW, MAP_ANON_NORESERVE, -1, 0);
> -- 
> 2.38.1.431.g37b22c650d-goog
>
Fuad Tabba Nov. 23, 2022, 5:44 p.m. UTC | #2
Hi,

On Wed, Nov 23, 2022 at 4:40 PM Alexandru Elisei
<alexandru.elisei@arm.com> wrote:
>
> Hi,
>
> On Tue, Nov 15, 2022 at 11:15:35AM +0000, Fuad Tabba wrote:
> > For consistency with other similar functions in the same file and
> > for brevity.
> >
> > No functional change intended.
> >
> > Signed-off-by: Fuad Tabba <tabba@google.com>
> > ---
> >  include/kvm/util.h | 2 +-
> >  util/util.c        | 6 +++---
> >  2 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/include/kvm/util.h b/include/kvm/util.h
> > index b0c3684..61a205b 100644
> > --- a/include/kvm/util.h
> > +++ b/include/kvm/util.h
> > @@ -140,6 +140,6 @@ static inline int pow2_size(unsigned long x)
> >  }
> >
> >  struct kvm;
> > -void *mmap_anon_or_hugetlbfs(struct kvm *kvm, const char *hugetlbfs_path, u64 size);
> > +void *mmap_anon_or_hugetlbfs(struct kvm *kvm, const char *htlbfs_path, u64 size);
> >
> >  #endif /* KVM__UTIL_H */
> > diff --git a/util/util.c b/util/util.c
> > index 093bd3b..22b64b6 100644
> > --- a/util/util.c
> > +++ b/util/util.c
> > @@ -118,14 +118,14 @@ static void *mmap_hugetlbfs(struct kvm *kvm, const char *htlbfs_path, u64 size)
> >  }
> >
> >  /* This function wraps the decision between hugetlbfs map (if requested) or normal mmap */
> > -void *mmap_anon_or_hugetlbfs(struct kvm *kvm, const char *hugetlbfs_path, u64 size)
> > +void *mmap_anon_or_hugetlbfs(struct kvm *kvm, const char *htlbfs_path, u64 size)
>
> All the functions that deal with hugetlbfs have "hugetlbfs" in the name,
> and the kvm_config field is called hugetlbfs_path. Wouldn't it make more
> sense to rename the htlbfs_path parameter to hugetlbs_path instead of the
> other way around?

I was going for brevity, but changing it the other way around is more
consistent, as you said.

I'll do that when I respin this.

Cheers,
/fuad

> Thanks,
> Alex
>
> >  {
> > -     if (hugetlbfs_path)
> > +     if (htlbfs_path)
> >               /*
> >                * We don't /need/ to map guest RAM from hugetlbfs, but we do so
> >                * if the user specifies a hugetlbfs path.
> >                */
> > -             return mmap_hugetlbfs(kvm, hugetlbfs_path, size);
> > +             return mmap_hugetlbfs(kvm, htlbfs_path, size);
> >       else {
> >               kvm->ram_pagesize = getpagesize();
> >               return mmap(NULL, size, PROT_RW, MAP_ANON_NORESERVE, -1, 0);
> > --
> > 2.38.1.431.g37b22c650d-goog
> >
diff mbox series

Patch

diff --git a/include/kvm/util.h b/include/kvm/util.h
index b0c3684..61a205b 100644
--- a/include/kvm/util.h
+++ b/include/kvm/util.h
@@ -140,6 +140,6 @@  static inline int pow2_size(unsigned long x)
 }
 
 struct kvm;
-void *mmap_anon_or_hugetlbfs(struct kvm *kvm, const char *hugetlbfs_path, u64 size);
+void *mmap_anon_or_hugetlbfs(struct kvm *kvm, const char *htlbfs_path, u64 size);
 
 #endif /* KVM__UTIL_H */
diff --git a/util/util.c b/util/util.c
index 093bd3b..22b64b6 100644
--- a/util/util.c
+++ b/util/util.c
@@ -118,14 +118,14 @@  static void *mmap_hugetlbfs(struct kvm *kvm, const char *htlbfs_path, u64 size)
 }
 
 /* This function wraps the decision between hugetlbfs map (if requested) or normal mmap */
-void *mmap_anon_or_hugetlbfs(struct kvm *kvm, const char *hugetlbfs_path, u64 size)
+void *mmap_anon_or_hugetlbfs(struct kvm *kvm, const char *htlbfs_path, u64 size)
 {
-	if (hugetlbfs_path)
+	if (htlbfs_path)
 		/*
 		 * We don't /need/ to map guest RAM from hugetlbfs, but we do so
 		 * if the user specifies a hugetlbfs path.
 		 */
-		return mmap_hugetlbfs(kvm, hugetlbfs_path, size);
+		return mmap_hugetlbfs(kvm, htlbfs_path, size);
 	else {
 		kvm->ram_pagesize = getpagesize();
 		return mmap(NULL, size, PROT_RW, MAP_ANON_NORESERVE, -1, 0);