From: Joao Martins <joao.m.martins@oracle.com>
To: "Verma, Vishal L" <vishal.l.verma@intel.com>,
"Williams, Dan J" <dan.j.williams@intel.com>
Cc: "linux-nvdimm@lists.01.org" <linux-nvdimm@lists.01.org>
Subject: Re: [PATCH ndctl v2 10/10] daxctl/test: Add tests for dynamic dax regions
Date: Wed, 16 Dec 2020 11:28:55 +0000 [thread overview]
Message-ID: <392bf623-115e-1ad6-dd88-c20f0cd26c7f@oracle.com> (raw)
In-Reply-To: <68ae54e05494ca82db67107bb1f44b982146b5bf.camel@intel.com>
On 12/16/20 10:25 AM, Verma, Vishal L wrote:
> On Thu, 2020-12-10 at 15:01 +0000, Joao Martins wrote:
>> On 7/21/20 5:49 PM, Joao Martins wrote:
>>> On 7/13/20 5:08 PM, Joao Martins wrote:
>>>> Add a couple tests which exercise the new sysfs based
>>>> interface for Soft-Reserved regions (by EFI/HMAT, or
>>>> efi_fake_mem).
>>>>
>>>> The tests exercise the daxctl orchestration surrounding
>>>> for creating/disabling/destroying/reconfiguring devices.
>>>> Furthermore it exercises dax region space allocation
>>>> code paths particularly:
>>>>
>>>> 1) zeroing out and reconfiguring a dax device from
>>>> its current size to be max available and back to initial
>>>> size
>>>>
>>>> 2) creates devices from holes in the beginning,
>>>> middle of the region.
>>>>
>>>> 3) reconfigures devices in a interleaving fashion
>>>>
>>>> 4) test adjust of the region towards beginning and end
>>>>
>>>> The tests assume you pass a valid efi_fake_mem parameter
>>>> marked as EFI_MEMORY_SP e.g.
>>>>
>>>> efi_fake_mem=112G@16G:0x40000
>>>>
>>>> Naturally it bails out from the test if hmem device driver
>>>> isn't loaded or builtin. If hmem regions are found, only
>>>> region 0 is used, and the others remain untouched.
>>>>
>>>> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
>>>
>>> Following your suggestion[0], I added a couple more validations
>>> to this test suite, covering the mappings. So on top of this patch
>>> I added the following snip below the scissors mark. Mainly, I check
>>> that the size calculated by mappingNNNN is the same as advertised by
>>> the sysfs size attribute, thus looping through all the mappings.
>>>
>>> Perhaps it would be enough to have just such validation in setup_dev()
>>> to catch the bug in [0]. But I went ahead and also validated the test
>>> cases where a certain amount of mappings are meant to be created.
>>>
>>> My only worry is the last piece in daxctl_test_adjust() where we might
>>> be tying too much on how a kernel version picks space from the region;
>>> should this logic change in an unforeseeable future (e.g. allowing space
>>> at the beginning to be adjusted). Otherwise, if this no concern, let me
>>> know and I can resend a v3 with the adjustment below.
>>>
>>
>> Ping?
>
> Hi Joao,
>
> Thanks for the patience on these, I've gone through the patches in
> preparation for the next release, and they all look mostly fine. I had a
> few minor fixups - to the documentation and the test (fixup module name,
> and shellcheck complaints). I've appended a diff below of all the fixups
> I added.
>
The adjustments are looking good AFAICT.
> I've also included the patch below for the mapping size validation. I
> think the concern for future kernel layout changes is valid, but if/when
> that happens, we can always come back and relax or adjust the test as
> needed. So for now, I think having a pickier test should be better than
> not having one.
>
Yeah, makes sense.
Thanks!
Joao
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
next prev parent reply other threads:[~2020-12-16 11:29 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-13 16:08 [PATCH ndctl v2 00/10] daxctl: Support for sub-dividing soft-reserved regions Joao Martins
2020-07-13 16:08 ` [PATCH ndctl v2 01/10] daxctl: Cleanup whitespace Joao Martins
2020-07-13 16:08 ` [PATCH ndctl v2 02/10] libdaxctl: add daxctl_dev_set_size() Joao Martins
2020-07-13 16:08 ` [PATCH ndctl v2 03/10] daxctl: add resize support in reconfigure-device Joao Martins
2020-07-13 16:08 ` [PATCH ndctl v2 04/10] daxctl: add command to disable devdax device Joao Martins
2020-07-13 16:08 ` [PATCH ndctl v2 05/10] daxctl: add command to enable " Joao Martins
2020-07-13 16:08 ` [PATCH ndctl v2 06/10] libdaxctl: add daxctl_region_create_dev() Joao Martins
2020-07-13 16:08 ` [PATCH ndctl v2 07/10] daxctl: add command to create device Joao Martins
2020-07-13 16:08 ` [PATCH ndctl v2 08/10] libdaxctl: add daxctl_region_destroy_dev() Joao Martins
2020-07-13 16:08 ` [PATCH ndctl v2 09/10] daxctl: add command to destroy device Joao Martins
2020-07-13 16:08 ` [PATCH ndctl v2 10/10] daxctl/test: Add tests for dynamic dax regions Joao Martins
2020-07-21 16:49 ` Joao Martins
2020-12-10 15:01 ` Joao Martins
2020-12-16 10:25 ` Verma, Vishal L
2020-12-16 11:28 ` Joao Martins [this message]
2020-12-16 10:25 ` Verma, Vishal L
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=392bf623-115e-1ad6-dd88-c20f0cd26c7f@oracle.com \
--to=joao.m.martins@oracle.com \
--cc=dan.j.williams@intel.com \
--cc=linux-nvdimm@lists.01.org \
--cc=vishal.l.verma@intel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.