[PATCH v3 2/2] ntdll/tests: Add tests for ProcessQuotaLimits class
Vijay Kiran Kamuju
infyquest at gmail.com
Sat Mar 21 12:21:00 CDT 2020
On Sat, Mar 21, 2020 at 5:18 PM Zebediah Figura <z.figura12 at gmail.com> wrote:
>
> On 3/21/20 11:06 AM, Vijay Kiran Kamuju wrote:
> > On Sat, Mar 21, 2020 at 4:49 PM Zebediah Figura <z.figura12 at gmail.com> wrote:
> >>
> >> Hello Vijay,
> >>
> >> On 3/20/20 4:46 PM, Vijay Kiran Kamuju wrote:
> >>> From: Vijay Kiran Kamuju <infyquest at gmail.com>
> >>> Subject: [PATCH v3 2/2] ntdll/tests: Add tests for ProcessQuotaLimits class
> >>> Message-Id: <CACfa+KKTLOLG6pTUJO-8b7ez43oSVRnbtPKJ9tALc3JqVcFodw at mail.gmail.com>
> >>> Date: Fri, 20 Mar 2020 22:46:50 +0100
> >>>
> >>> Signed-off-by: Vijay Kiran Kamuju <infyquest at gmail.com>
> >>>
> >>> From 6bada725c1bb59bb127729cd5f373c29868d880d Mon Sep 17 00:00:00 2001
> >>> From: Vijay Kiran Kamuju <infyquest at gmail.com>
> >>> Date: Sat, 15 Feb 2020 10:03:51 +0100
> >>> Subject: [PATCH v3 2/2] ntdll/tests: Add tests for ProcessQuotaLimits class
> >>>
> >>> Signed-off-by: Vijay Kiran Kamuju <infyquest at gmail.com>
> >>> ---
> >>> dlls/ntdll/tests/info.c | 72 +++++++++++++++++++++++++++++++++++++++++
> >>> 1 file changed, 72 insertions(+)
> >>>
> >>> diff --git a/dlls/ntdll/tests/info.c b/dlls/ntdll/tests/info.c
> >>> index 8dc8bad645..7d3278b32e 100644
> >>> --- a/dlls/ntdll/tests/info.c
> >>> +++ b/dlls/ntdll/tests/info.c
> >>> @@ -1308,6 +1308,74 @@ static void test_query_process_basic(void)
> >>> ok( pbi.UniqueProcessId > 0, "Expected a ProcessID > 0, got 0\n");
> >>> }
> >>>
> >>> +static void dump_quota_limits(const char *header, const QUOTA_LIMITS *pql)
> >>
> >> Is there any particular reason this needs to be a separate function?
> >> Note that unlike dump_vm_counters(), this is only called once.
> > This is used for tracing the quota values on different test systems
> > with various RAM configurations.
> > If you recommend, I will remove this function.
>
> I'm not necessarily suggesting to remove the traces, I just mean that
> you don't need to put them in a separate function.
Ok, I will do that as well.
>
> >>
> >>> +{
> >>> + trace("%s:\n", header);
> >>> + trace("PagedPoolLimit : %lu\n", pql->PagedPoolLimit);
> >>> + trace("NonPagedPoolLimit : %lu\n", pql->NonPagedPoolLimit);
> >>> + trace("MinimumWorkingSetSize : %lu\n", pql->MinimumWorkingSetSize);
> >>> + trace("MaximumWorkingSetSize : %lu\n", pql->MaximumWorkingSetSize);
> >>> + trace("PagefileLimit : %lu\n", pql->PagefileLimit);
> >>> + trace("TimeLimit : %s\n", wine_dbgstr_longlong(pql->TimeLimit.QuadPart));
> >>
> >> You already test these last four members against fixed values below; I'm
> >> not sure there's a point in printing their values in that case.
> > I am testing these as per previous recommendations, but some of values
> > change by RAM values.
>
> I'm not sure I see the review you're referring to, but my point is that
> the last four members (MinimumWorkingSetSize, MaximumWorkingSetSize,
> PagefileLimit, TimeLimit) are compared against fixed values, so there's
> no point printing their actual values. If the values are different from
> what you expect, you'll already get a failure in the ok() messages
> below. If those values do differ across different machines, then you
> probably shouldn't be using ok() to check them.
Well, I have seen some presentations related to PagedPoolLimit,
NonPagedPoolLimitMinimumWorkingSetSize, MaximumWorkingSetSize
https://techcommunity.microsoft.com/t5/windows-blog-archive/pushing-the-limits-of-windows-paged-and-nonpaged-pool/ba-p/723789
I forgot the link for the ppt presentation.
>
> >>
> >>> +}
> >>> +
> >>> +static void test_query_process_quotalimits(void)
> >>> +{
> >>> + NTSTATUS status;
> >>> + QUOTA_LIMITS quotalimits;
> >>> + ULONG ReturnLength;
> >>> + HANDLE process;
> >>> +
> >>> + status = pNtQueryInformationProcess(NULL, ProcessQuotaLimits, NULL, sizeof(quotalimits), NULL);
> >>> + ok( status == STATUS_ACCESS_VIOLATION || status == STATUS_INVALID_HANDLE,
> >>> + "Expected STATUS_ACCESS_VIOLATION or STATUS_INVALID_HANDLE(W2K3), got %08x\n", status);
> >>> +
> >>> + status = pNtQueryInformationProcess(NULL, ProcessQuotaLimits, "alimits, sizeof(quotalimits), NULL);
> >>> + ok( status == STATUS_INVALID_HANDLE, "Expected STATUS_INVALID_HANDLE, got %08x\n", status);
> >>> +
> >>> + status = pNtQueryInformationProcess( GetCurrentProcess(), ProcessQuotaLimits, "alimits, 2, &ReturnLength);
> >>> + ok( status == STATUS_INFO_LENGTH_MISMATCH, "Expected STATUS_INFO_LENGTH_MISMATCH, got %08x\n", status);
> >>> +
> >>> + process = OpenProcess(PROCESS_QUERY_INFORMATION, FALSE, one_before_last_pid);
> >>> + if (!process)
> >>> + {
> >>> + trace("Could not open process with ID : %d, error : %u. Going to use current one.\n", one_before_last_pid, GetLastError());
> >>> + process = GetCurrentProcess();
> >>> + trace("ProcessQuotaLimits for current process\n");
> >>> + }
> >>> + else
> >>> + trace("ProcessQuotaLimits for process with ID : %d\n", one_before_last_pid);
> >>
> >> I don't think there's a point in doing this for this test. See the
> >> comment regarding one_before_last_pid at the top of the file.
> > I wanted to test on a new process rather than the current process.
>
> Why?
I thought the winetest may take up lot of resources, which might
change the limits for the current process.
I will change it to use current process.
>
> >>
> >>> +
> >>> + memset("alimits, 0, sizeof(quotalimits));
> >>> + status = pNtQueryInformationProcess( process, ProcessQuotaLimits, "alimits, sizeof(quotalimits), &ReturnLength);
> >>> + ok( status == STATUS_SUCCESS, "Expected STATUS_SUCCESS, got %08x\n", status);
> >>> + ok( sizeof(quotalimits) == ReturnLength, "Inconsistent length %d\n", ReturnLength);
> >>> + ok(quotalimits.MinimumWorkingSetSize == 204800,"Got MinimumWorkingSetSize = %lu\n",quotalimits.MinimumWorkingSetSize);
> >>> + ok(quotalimits.MaximumWorkingSetSize == 1413120,"Got MaximumWorkingSetSize = %lu\n",quotalimits.MaximumWorkingSetSize);
> >>> + ok(quotalimits.PagefileLimit == -1,"Got PagefileLimit = %lu\n",quotalimits.PagefileLimit);
> >>
> >> Instead of comparing unsigned values to -1, comparing them to ~0 or
> >> SIZE_MAX might be better.
> > Will do the changes.
> >>
> >>> + ok(quotalimits.TimeLimit.QuadPart == -1,"Got TimeLimit = %s\n",wine_dbgstr_longlong(quotalimits.TimeLimit.QuadPart));
> >>> + CloseHandle(process);
> >>
> >> Style nitpick, the spacing here and elsewhere is rather inconsistent.
> > Will do the changes.
> >>
> >>> + dump_quota_limits("Quota Limits","alimits);
> >>
> >> I think we need less tracing in this test by default, not more. I'd
> >> recommend checking against winetest_debug > 1.
> > Will do the changes.
> >>
> >>> +
> >>> + memset("alimits, 0, sizeof(quotalimits));
> >>> + status = pNtQueryInformationProcess( GetCurrentProcess(), ProcessQuotaLimits, "alimits, sizeof(quotalimits)*2, &ReturnLength);
> >>> + ok( status == STATUS_INFO_LENGTH_MISMATCH,
> >>> + "Expected STATUS_INFO_LENGTH_MISMATCH, got %08x\n", status);
> >> Style nitpick: it seems weird to break this line here when the previous
> >> line is longer and unbroken; I'd break both lines or neither.
> >>
> >> (Using shorter ok() messages might not be a bad idea, either.)
> >>
> > Will do the changes.
> >>> + ok( sizeof(quotalimits) == ReturnLength, "Inconsistent length %d\n", ReturnLength);
> >>> +
> >>> + memset("alimits, 0, sizeof(quotalimits));
> >>> + status = pNtQueryInformationProcess( GetCurrentProcess(), ProcessQuotaLimits, "alimits, sizeof(quotalimits)-1, &ReturnLength);
> >>> + ok( status == STATUS_INFO_LENGTH_MISMATCH,
> >>> + "Expected STATUS_INFO_LENGTH_MISMATCH, got %08x\n", status);
> >>> + ok( sizeof(quotalimits) == ReturnLength, "Inconsistent length %d\n", ReturnLength);
> >>> +
> >>> + memset("alimits, 0, sizeof(quotalimits));
> >>> + status = pNtQueryInformationProcess( GetCurrentProcess(), ProcessQuotaLimits, "alimits, sizeof(quotalimits)+1, &ReturnLength);
> >>> + ok( status == STATUS_INFO_LENGTH_MISMATCH,
> >>> + "Expected STATUS_INFO_LENGTH_MISMATCH, got %08x\n", status);
> >>> + ok( sizeof(quotalimits) == ReturnLength, "Inconsistent length %d\n", ReturnLength);
> >>> +}
> >>> +
> >>> static void dump_vm_counters(const char *header, const VM_COUNTERS *pvi)
> >>> {
> >>> trace("%s:\n", header);
> >>> @@ -2590,6 +2658,10 @@ START_TEST(info)
> >>> trace("Starting test_query_process_basic()\n");
> >>> test_query_process_basic();
> >>>
> >>> + /* 0x1 ProcessQuotaLimits */
> >>> + trace("Starting test_query_process_quotalimits()\n");
> >>> + test_query_process_quotalimits();
> >>> +
> >>> /* 0x2 ProcessIoCounters */
> >>> trace("Starting test_query_process_io()\n");
> >>> test_query_process_io();
> >>>
> >>> --
> >>> 2.25.2
> >>>
> >>
> >
>
>
More information about the wine-devel
mailing list