| Message ID | 20200717105139.25293-5-alex.bennee@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | candidate fixes for 5.1-rc1 (shippable, semihosting, OOM tcg) | expand |
On Fri, 17 Jul 2020, Alex Bennée wrote: > This will be used in a future patch. For POSIX systems _SC_PHYS_PAGES > isn't standardised but at least appears in the man pages for > Open/FreeBSD. The result is advisory so any users of it shouldn't just > fail if we can't work it out. > > The win32 stub currently returns 0 until someone with a Windows system > can develop and test a patch. > > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > Cc: BALATON Zoltan <balaton@eik.bme.hu> > Cc: Christian Ehrhardt <christian.ehrhardt@canonical.com> > --- > include/qemu/osdep.h | 10 ++++++++++ > util/oslib-posix.c | 11 +++++++++++ > util/oslib-win32.c | 6 ++++++ > 3 files changed, 27 insertions(+) > > diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h > index 4841b5c6b5f..7ff209983e2 100644 > --- a/include/qemu/osdep.h > +++ b/include/qemu/osdep.h > @@ -665,4 +665,14 @@ static inline void qemu_reset_optind(void) > */ > char *qemu_get_host_name(Error **errp); > > +/** > + * qemu_get_host_physmem: > + * > + * Operating system agnostiv way of querying host memory. Typo: agnostiv -> agnostic > + * > + * Returns amount of physical memory on the system. This is purely > + * advisery and may return 0 if we can't work it out. > + */ > +size_t qemu_get_host_physmem(void); > + > #endif > diff --git a/util/oslib-posix.c b/util/oslib-posix.c > index 36bf8593f8c..d9da782b896 100644 > --- a/util/oslib-posix.c > +++ b/util/oslib-posix.c > @@ -839,3 +839,14 @@ char *qemu_get_host_name(Error **errp) > > return g_steal_pointer(&hostname); > } > + > +size_t qemu_get_host_physmem(void) > +{ > +#ifdef _SC_PHYS_PAGES > + long pages = sysconf(_SC_PHYS_PAGES); > + if (pages > 0) { > + return pages * qemu_real_host_page_size; The Linux man page warns that this product may overflow so maybe you could return pages here. > + } > +#endif > + return 0; > +} > diff --git a/util/oslib-win32.c b/util/oslib-win32.c > index 7eedbe5859a..31030463cc9 100644 > --- a/util/oslib-win32.c > +++ b/util/oslib-win32.c > @@ -828,3 +828,9 @@ char *qemu_get_host_name(Error **errp) > > return g_utf16_to_utf8(tmp, size, NULL, NULL, NULL); > } > + > +size_t qemu_get_host_physmem(void) > +{ > + /* currently unimplemented */ > + return 0; > +} For Windows this may help: https://stackoverflow.com/questions/5553665/get-ram-system-size not sure about other OSes. Regards, BALATON Zoltan
On Fri, Jul 17, 2020 at 3:32 PM BALATON Zoltan <balaton@eik.bme.hu> wrote: > On Fri, 17 Jul 2020, Alex Bennée wrote: > > This will be used in a future patch. For POSIX systems _SC_PHYS_PAGES > > isn't standardised but at least appears in the man pages for > > Open/FreeBSD. The result is advisory so any users of it shouldn't just > > fail if we can't work it out. > > > > The win32 stub currently returns 0 until someone with a Windows system > > can develop and test a patch. > > > > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > > Cc: BALATON Zoltan <balaton@eik.bme.hu> > > Cc: Christian Ehrhardt <christian.ehrhardt@canonical.com> > > --- > > include/qemu/osdep.h | 10 ++++++++++ > > util/oslib-posix.c | 11 +++++++++++ > > util/oslib-win32.c | 6 ++++++ > > 3 files changed, 27 insertions(+) > > > > diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h > > index 4841b5c6b5f..7ff209983e2 100644 > > --- a/include/qemu/osdep.h > > +++ b/include/qemu/osdep.h > > @@ -665,4 +665,14 @@ static inline void qemu_reset_optind(void) > > */ > > char *qemu_get_host_name(Error **errp); > > > > +/** > > + * qemu_get_host_physmem: > > + * > > + * Operating system agnostiv way of querying host memory. > > Typo: agnostiv -> agnostic > > > + * > > + * Returns amount of physical memory on the system. This is purely > > + * advisery and may return 0 if we can't work it out. > > + */ > > +size_t qemu_get_host_physmem(void); > > + > > #endif > > diff --git a/util/oslib-posix.c b/util/oslib-posix.c > > index 36bf8593f8c..d9da782b896 100644 > > --- a/util/oslib-posix.c > > +++ b/util/oslib-posix.c > > @@ -839,3 +839,14 @@ char *qemu_get_host_name(Error **errp) > > > > return g_steal_pointer(&hostname); > > } > > + > > +size_t qemu_get_host_physmem(void) > > +{ > > +#ifdef _SC_PHYS_PAGES > > + long pages = sysconf(_SC_PHYS_PAGES); > > + if (pages > 0) { > > + return pages * qemu_real_host_page_size; > > The Linux man page warns that this product may overflow so maybe you could > return pages here. > The caller might be even less aware of that than this function - so maybe better handle it here. How about handling overflows and cutting it to MiB before returning? > > + } > > +#endif > > + return 0; > > +} > > diff --git a/util/oslib-win32.c b/util/oslib-win32.c > > index 7eedbe5859a..31030463cc9 100644 > > --- a/util/oslib-win32.c > > +++ b/util/oslib-win32.c > > @@ -828,3 +828,9 @@ char *qemu_get_host_name(Error **errp) > > > > return g_utf16_to_utf8(tmp, size, NULL, NULL, NULL); > > } > > + > > +size_t qemu_get_host_physmem(void) > > +{ > > + /* currently unimplemented */ > > + return 0; > > +} > > For Windows this may help: > > https://stackoverflow.com/questions/5553665/get-ram-system-size > > not sure about other OSes. > > Regards, > BALATON Zoltan -- Christian Ehrhardt Staff Engineer, Ubuntu Server Canonical Ltd <div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Jul 17, 2020 at 3:32 PM BALATON Zoltan <<a href="mailto:balaton@eik.bme.hu">balaton@eik.bme.hu</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Fri, 17 Jul 2020, Alex Bennée wrote:<br> > This will be used in a future patch. For POSIX systems _SC_PHYS_PAGES<br> > isn't standardised but at least appears in the man pages for<br> > Open/FreeBSD. The result is advisory so any users of it shouldn't just<br> > fail if we can't work it out.<br> ><br> > The win32 stub currently returns 0 until someone with a Windows system<br> > can develop and test a patch.<br> ><br> > Signed-off-by: Alex Bennée <<a href="mailto:alex.bennee@linaro.org" target="_blank">alex.bennee@linaro.org</a>><br> > Cc: BALATON Zoltan <<a href="mailto:balaton@eik.bme.hu" target="_blank">balaton@eik.bme.hu</a>><br> > Cc: Christian Ehrhardt <<a href="mailto:christian.ehrhardt@canonical.com" target="_blank">christian.ehrhardt@canonical.com</a>><br> > ---<br> > include/qemu/osdep.h | 10 ++++++++++<br> > util/oslib-posix.c | 11 +++++++++++<br> > util/oslib-win32.c | 6 ++++++<br> > 3 files changed, 27 insertions(+)<br> ><br> > diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h<br> > index 4841b5c6b5f..7ff209983e2 100644<br> > --- a/include/qemu/osdep.h<br> > +++ b/include/qemu/osdep.h<br> > @@ -665,4 +665,14 @@ static inline void qemu_reset_optind(void)<br> > */<br> > char *qemu_get_host_name(Error **errp);<br> ><br> > +/**<br> > + * qemu_get_host_physmem:<br> > + *<br> > + * Operating system agnostiv way of querying host memory.<br> <br> Typo: agnostiv -> agnostic<br> <br> > + *<br> > + * Returns amount of physical memory on the system. This is purely<br> > + * advisery and may return 0 if we can't work it out.<br> > + */<br> > +size_t qemu_get_host_physmem(void);<br> > +<br> > #endif<br> > diff --git a/util/oslib-posix.c b/util/oslib-posix.c<br> > index 36bf8593f8c..d9da782b896 100644<br> > --- a/util/oslib-posix.c<br> > +++ b/util/oslib-posix.c<br> > @@ -839,3 +839,14 @@ char *qemu_get_host_name(Error **errp)<br> ><br> > return g_steal_pointer(&hostname);<br> > }<br> > +<br> > +size_t qemu_get_host_physmem(void)<br> > +{<br> > +#ifdef _SC_PHYS_PAGES<br> > + long pages = sysconf(_SC_PHYS_PAGES);<br> > + if (pages > 0) {<br> > + return pages * qemu_real_host_page_size;<br> <br> The Linux man page warns that this product may overflow so maybe you could <br> return pages here.<br></blockquote><div><br></div><div>The caller might be even less aware of that than this function - so maybe better handle it here.</div><div>How about handling overflows and cutting it to MiB before returning?</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> > + }<br> > +#endif<br> > + return 0;<br> > +}<br> > diff --git a/util/oslib-win32.c b/util/oslib-win32.c<br> > index 7eedbe5859a..31030463cc9 100644<br> > --- a/util/oslib-win32.c<br> > +++ b/util/oslib-win32.c<br> > @@ -828,3 +828,9 @@ char *qemu_get_host_name(Error **errp)<br> ><br> > return g_utf16_to_utf8(tmp, size, NULL, NULL, NULL);<br> > }<br> > +<br> > +size_t qemu_get_host_physmem(void)<br> > +{<br> > + /* currently unimplemented */<br> > + return 0;<br> > +}<br> <br> For Windows this may help:<br> <br> <a href="https://stackoverflow.com/questions/5553665/get-ram-system-size" rel="noreferrer" target="_blank">https://stackoverflow.com/questions/5553665/get-ram-system-size</a><br> <br> not sure about other OSes.<br> <br> Regards,<br> BALATON Zoltan</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail_signature">Christian Ehrhardt<br>Staff Engineer, Ubuntu Server<br>Canonical Ltd</div></div>
On 7/17/20 7:24 AM, Christian Ehrhardt wrote: > > +size_t qemu_get_host_physmem(void) > > +{ > > +#ifdef _SC_PHYS_PAGES > > + long pages = sysconf(_SC_PHYS_PAGES); > > + if (pages > 0) { > > + return pages * qemu_real_host_page_size; > > The Linux man page warns that this product may overflow so maybe you could > return pages here. > > > The caller might be even less aware of that than this function - so maybe > better handle it here. > How about handling overflows and cutting it to MiB before returning? Indeed, the caller may be less aware, so we should handle it here. But I don't think truncating to MiB helps at all, because again, the caller has to handle overflow. Better, I think, to saturate the result to ~(size_t)0 and leave it at that. r~
On 7/17/20 3:51 AM, Alex Bennée wrote: > +size_t qemu_get_host_physmem(void) > +{ > +#ifdef _SC_PHYS_PAGES > + long pages = sysconf(_SC_PHYS_PAGES); > + if (pages > 0) { > + return pages * qemu_real_host_page_size; > + } > +#endif > + return 0; > +} Is it worth examining our own RLIMIT_{AS,DATA} as well? I suppose that's not usually what is tweaked in the example of a ram-limited container... r~
Richard Henderson <richard.henderson@linaro.org> writes: > On 7/17/20 7:24 AM, Christian Ehrhardt wrote: >> > +size_t qemu_get_host_physmem(void) >> > +{ >> > +#ifdef _SC_PHYS_PAGES >> > + long pages = sysconf(_SC_PHYS_PAGES); >> > + if (pages > 0) { >> > + return pages * qemu_real_host_page_size; >> >> The Linux man page warns that this product may overflow so maybe you could >> return pages here. >> >> >> The caller might be even less aware of that than this function - so maybe >> better handle it here. >> How about handling overflows and cutting it to MiB before returning? > > Indeed, the caller may be less aware, so we should handle it here. But I don't > think truncating to MiB helps at all, because again, the caller has to handle > overflow. > > Better, I think, to saturate the result to ~(size_t)0 and leave it at > that. So I went for: size_t qemu_get_host_physmem(void) { #ifdef _SC_PHYS_PAGES long pages = sysconf(_SC_PHYS_PAGES); if (pages > 0) { if (pages > SIZE_MAX / qemu_real_host_page_size) { return SIZE_MAX; } else { return pages * qemu_real_host_page_size; } } #endif return 0; } apparently the first case of saturating integer arithmetic outside of the instruction emulation in QEMU :-/ -- Alex Bennée
Richard Henderson <richard.henderson@linaro.org> writes: > On 7/17/20 3:51 AM, Alex Bennée wrote: >> +size_t qemu_get_host_physmem(void) >> +{ >> +#ifdef _SC_PHYS_PAGES >> + long pages = sysconf(_SC_PHYS_PAGES); >> + if (pages > 0) { >> + return pages * qemu_real_host_page_size; >> + } >> +#endif >> + return 0; >> +} > > Is it worth examining our own RLIMIT_{AS,DATA} as well? We don't anywhere else in the code so for now I'd say not. > > I suppose that's not usually what is tweaked in the example of a ram-limited > container... > > > r~ -- Alex Bennée
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h index 4841b5c6b5f..7ff209983e2 100644 --- a/include/qemu/osdep.h +++ b/include/qemu/osdep.h @@ -665,4 +665,14 @@ static inline void qemu_reset_optind(void) */ char *qemu_get_host_name(Error **errp); +/** + * qemu_get_host_physmem: + * + * Operating system agnostiv way of querying host memory. + * + * Returns amount of physical memory on the system. This is purely + * advisery and may return 0 if we can't work it out. + */ +size_t qemu_get_host_physmem(void); + #endif diff --git a/util/oslib-posix.c b/util/oslib-posix.c index 36bf8593f8c..d9da782b896 100644 --- a/util/oslib-posix.c +++ b/util/oslib-posix.c @@ -839,3 +839,14 @@ char *qemu_get_host_name(Error **errp) return g_steal_pointer(&hostname); } + +size_t qemu_get_host_physmem(void) +{ +#ifdef _SC_PHYS_PAGES + long pages = sysconf(_SC_PHYS_PAGES); + if (pages > 0) { + return pages * qemu_real_host_page_size; + } +#endif + return 0; +} diff --git a/util/oslib-win32.c b/util/oslib-win32.c index 7eedbe5859a..31030463cc9 100644 --- a/util/oslib-win32.c +++ b/util/oslib-win32.c @@ -828,3 +828,9 @@ char *qemu_get_host_name(Error **errp) return g_utf16_to_utf8(tmp, size, NULL, NULL, NULL); } + +size_t qemu_get_host_physmem(void) +{ + /* currently unimplemented */ + return 0; +}
This will be used in a future patch. For POSIX systems _SC_PHYS_PAGES isn't standardised but at least appears in the man pages for Open/FreeBSD. The result is advisory so any users of it shouldn't just fail if we can't work it out. The win32 stub currently returns 0 until someone with a Windows system can develop and test a patch. Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Cc: BALATON Zoltan <balaton@eik.bme.hu> Cc: Christian Ehrhardt <christian.ehrhardt@canonical.com> --- include/qemu/osdep.h | 10 ++++++++++ util/oslib-posix.c | 11 +++++++++++ util/oslib-win32.c | 6 ++++++ 3 files changed, 27 insertions(+) -- 2.20.1