From: "Juraj Linkeš" <juraj.linkes@pantheon.tech>
To: Nicholas Pratte <npratte@iol.unh.edu>
Cc: jspewock@iol.unh.edu, wathsala.vithanage@arm.com,
Honnappa.Nagarahalli@arm.com, probb@iol.unh.edu,
thomas@monjalon.net, mb@smartsharesystems.com,
bruce.richardson@intel.com, yoan.picchi@foss.arm.com,
paul.szczepanek@arm.com, dev@dpdk.org
Subject: Re: [PATCH v4] dts: Change hugepage runtime config to 2MB Exclusively
Date: Tue, 30 Apr 2024 10:42:23 +0200 [thread overview]
Message-ID: <CAOb5WZZJNtbzc=yCmoiX+6JqKe2CS=ZA2d4pds-oXZSiRjvZ_g@mail.gmail.com> (raw)
In-Reply-To: <CAKXZ7egp4gj9No_T=NO8kQ_SgAz68wr+zihEJ7HQC7bCd16L=Q@mail.gmail.com>
Please leave the context you're addressing. Reading this was a bit
confusing and also hard to understand.
On Mon, Apr 29, 2024 at 7:26 PM Nicholas Pratte <npratte@iol.unh.edu> wrote:
>
> I fixed the docstring under setup_hugepages in os_session, and I also
> made a quick fix to the dts.rst documentation. For the dts.rst
> documentation, I think the following changes make more sense, based on
> the concerns outlined:
>
> (here is a snip of the documentation with the change I made)
> "as doing so prevents accidental/over
> allocation of with hugepage sizes not recommended during runtime due to
> contiguous memory space requirements."
>
Looks like there's a typo: allocation of with hugepage sizes
> With regard to the wording used for the total number of hugepages, I
> could change the wording to either "quantity" or "number;" I think
> quantity makes more sense and is less ambiguous, but I'm curious what
> you think. With reference to your comments about putting this in a
> different patch set, I think a good argument could be made to put this
> kind of a change in out currently existing patch, but I understand the
> argument at both ends. Personally, I am in favor of adding this fix to
> the current patch since we're renaming key/value pairs in the schema
> and yaml already.
>
It looks like quantity is used with both countable and uncountable
nouns, whereas number is only used with countable nouns, so quantity
is also fine.
Let's put it into this patch series.
> As far as the property is concerned, when Jeremy and I discussed how
> to best implement this fix, he suggested that a property might make
> more sense here because of the potential changes that we might make to
> the default size in the future (whether that be by OS, arch or
> otherwise). Ultimately, we settled on inserting a property that
> returns 2048 for now with the understanding that, in the future,
> developers can add logic to the property as needed. Initially, I had
> the hugepage size configured in the manner you described, so the
> property implementation is not something I'm adamant on. I can make
> the suggested change you gave above, or alternatively, if my provided
> reasoning makes sense, I can insert a comment exclaiming the existence
> of the property.
The current expectation, based on the previous discussion, is we won't
be needing any logic, so I'd just make it a class variable (defined in
OSSession, as it's the same for all sessions). We can change it in the
future if we uncover a case where we might need it.
prev parent reply other threads:[~2024-04-30 8:42 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-04 15:31 [PATCH] " Nicholas Pratte
2024-04-05 14:27 ` Patrick Robb
2024-04-06 8:47 ` Morten Brørup
2024-04-06 19:20 ` Patrick Robb
2024-04-06 22:04 ` Morten Brørup
2024-04-08 8:10 ` Bruce Richardson
2024-04-08 9:35 ` Morten Brørup
2024-04-09 16:57 ` Nicholas Pratte
2024-04-09 17:28 ` [PATCH v2] " Nicholas Pratte
2024-04-10 7:23 ` Morten Brørup
2024-04-10 13:50 ` Patrick Robb
2024-04-10 10:43 ` Juraj Linkeš
2024-04-16 18:18 ` [PATCH v3] " Nicholas Pratte
2024-04-16 18:25 ` Morten Brørup
2024-04-16 18:26 ` Nicholas Pratte
2024-04-17 13:46 ` Juraj Linkeš
2024-04-18 16:10 ` [PATCH v4] " Nicholas Pratte
2024-04-25 8:00 ` Juraj Linkeš
2024-04-29 17:26 ` Nicholas Pratte
2024-04-30 8:42 ` Juraj Linkeš [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAOb5WZZJNtbzc=yCmoiX+6JqKe2CS=ZA2d4pds-oXZSiRjvZ_g@mail.gmail.com' \
--to=juraj.linkes@pantheon.tech \
--cc=Honnappa.Nagarahalli@arm.com \
--cc=bruce.richardson@intel.com \
--cc=dev@dpdk.org \
--cc=jspewock@iol.unh.edu \
--cc=mb@smartsharesystems.com \
--cc=npratte@iol.unh.edu \
--cc=paul.szczepanek@arm.com \
--cc=probb@iol.unh.edu \
--cc=thomas@monjalon.net \
--cc=wathsala.vithanage@arm.com \
--cc=yoan.picchi@foss.arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).