Re: [PATCH v12 03/18] kunit: test: add string_stream a std::stream like string builder

[Stephen Boyd]

Quoting Brendan Higgins (2019-08-13 02:12:54)
> On Tue, Aug 13, 2019 at 2:04 AM Brendan Higgins
>  wrote:
> >
> > On Mon, Aug 12, 2019 at 10:30 PM Stephen Boyd  wrote:
> > >
> > > Quoting Brendan Higgins (2019-08-12 22:02:59)
> > > > However, now that I added the kunit_resource_destroy, I thought it
> > > > might be good to free the string_stream after I use it in each call to
> > > > kunit_assert->format(...) in which case I will be using this logic.
> > > >
> > > > So I think the right thing to do is to expose string_stream_destroy so
> > > > kunit_do_assert can clean up when it's done, and then demote
> > > > string_stream_clear to static. Sound good?
> > >
> > > Ok, sure. I don't really see how clearing it explicitly when the
> > > assertion prints vs. never allocating it to begin with is really any
> > > different. Maybe I've missed something though.
> >
> > It's for the case that we *do* print something out. Once we are doing
> > printing, we don't want the fragments anymore.
> 
> Oops, sorry fat fingered: s/doing/done

Yes, but when we print something out we've run into some sort of problem
and then the test is over. So freeing the memory when it fails vs. when
the test is over seems like a minor difference. Or is it also used to
print other informational messages while the test is running?

I'm not particularly worried here, just trying to see if less code is
possible.

___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [RFC PATCH v2 16/19] RDMA/uverbs: Add back pointer to system file object

[Ira Weiny]

On Tue, Aug 13, 2019 at 08:48:42AM -0300, Jason Gunthorpe wrote:
> On Mon, Aug 12, 2019 at 02:15:37PM -0700, Ira Weiny wrote:
> > On Mon, Aug 12, 2019 at 02:56:15PM -0300, Jason Gunthorpe wrote:
> > > On Mon, Aug 12, 2019 at 10:28:27AM -0700, Ira Weiny wrote:
> > > > On Mon, Aug 12, 2019 at 10:00:40AM -0300, Jason Gunthorpe wrote:
> > > > > On Fri, Aug 09, 2019 at 03:58:30PM -0700, ira.we...@intel.com wrote:
> > > > > > From: Ira Weiny 
> > > > > > 
> > > > > > In order for MRs to be tracked against the open verbs context the 
> > > > > > ufile
> > > > > > needs to have a pointer to hand to the GUP code.
> > > > > > 
> > > > > > No references need to be taken as this should be valid for the 
> > > > > > lifetime
> > > > > > of the context.
> > > > > > 
> > > > > > Signed-off-by: Ira Weiny 
> > > > > >  drivers/infiniband/core/uverbs.h  | 1 +
> > > > > >  drivers/infiniband/core/uverbs_main.c | 1 +
> > > > > >  2 files changed, 2 insertions(+)
> > > > > > 
> > > > > > diff --git a/drivers/infiniband/core/uverbs.h 
> > > > > > b/drivers/infiniband/core/uverbs.h
> > > > > > index 1e5aeb39f774..e802ba8c67d6 100644
> > > > > > +++ b/drivers/infiniband/core/uverbs.h
> > > > > > @@ -163,6 +163,7 @@ struct ib_uverbs_file {
> > > > > > struct page *disassociate_page;
> > > > > >  
> > > > > > struct xarray   idr;
> > > > > > +   struct file *sys_file; /* backpointer to system 
> > > > > > file object */
> > > > > >  };
> > > > > 
> > > > > The 'struct file' has a lifetime strictly shorter than the
> > > > > ib_uverbs_file, which is kref'd on its own lifetime. Having a back
> > > > > pointer like this is confouding as it will be invalid for some of the
> > > > > lifetime of the struct.
> > > > 
> > > > Ah...  ok.  I really thought it was the other way around.
> > > > 
> > > > __fput() should not call ib_uverbs_close() until the last reference on 
> > > > struct
> > > > file is released...  What holds references to struct ib_uverbs_file 
> > > > past that?
> > > 
> > > Child fds hold onto the internal ib_uverbs_file until they are closed
> > 
> > The FDs hold the struct file, don't they?
> 
> Only dups, there are other 'child' FDs we can create
> 
> > > Now this has unlocked updates to that data.. you'd need some lock and
> > > get not zero pattern
> > 
> > You can't call "get" here because I'm 99% sure we only get here when struct
> > file has no references left...
> 
> Nope, like I said the other FDs hold the uverbs_file independent of
> the struct file it is related too. 



We don't allow memory registrations to be created with those other FDs...

And I was pretty sure uverbs_destroy_ufile_hw() would take care of (or ensure
that some other thread is) destroying all the MR's we have associated with this
FD.

I'll have to think on this more since uverbs_destroy_ufile_hw() does not
block...  Which means there is a window here within the GUP code...  :-/

> 
> This is why having a back pointer like this is so ugly, it creates a
> reference counting cycle

Yep...  I worked through this...  and it was giving me fits...

Anyway, the struct file is the only object in the core which was reasonable to
store this information in since that is what is passed around to other
processes...

Another idea I explored was to create a callback into the driver from the core
which put the responsibility of printing the pin information on the driver.

But that started to be (and is likely going to be) a pretty complicated "dance"
between the core and the drivers so I went this way...

I also thought about holding some other reference on struct file which would
allow release to be called while keeping struct file around.  But that seemed
crazy...

Ira

___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH v12 12/18] kunit: test: add tests for KUnit managed resources

[Stephen Boyd]

Quoting Brendan Higgins (2019-08-13 00:57:33)
> On Mon, Aug 12, 2019 at 9:31 PM Stephen Boyd  wrote:
> >
> > BTW, maybe kunit allocation APIs should
> > fail the test if they fail to allocate in general. Unless we're unit
> > testing failure to allocate problems.
> 
> Yeah, I thought about that. I wasn't sure how people would feel about
> it, and I thought it would be a pain to tease out all the issues
> arising from aborting in different contexts when someone might not
> expect it.
> 
> I am thinking later we can have kunit_kmalloc_or_abort variants? And
> then we can punt this issue to a later time?
> 

Sure. Sounds good.

___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH v12 09/18] kunit: test: add support for test abort

[Stephen Boyd]

Quoting Brendan Higgins (2019-08-13 00:52:03)
> On Mon, Aug 12, 2019 at 10:56 PM Stephen Boyd  wrote:
> >
> > Quoting Brendan Higgins (2019-08-12 21:57:55)
> > > On Mon, Aug 12, 2019 at 9:22 PM Stephen Boyd  wrote:
> > > >
> > > > Quoting Brendan Higgins (2019-08-12 11:24:12)
> > > > > diff --git a/include/kunit/test.h b/include/kunit/test.h
> > > > > index 2625bcfeb19ac..93381f841e09f 100644
> > > > > --- a/include/kunit/test.h
> > > > > +++ b/include/kunit/test.h
> > > > > @@ -184,6 +191,13 @@ struct kunit {
> > > > > struct list_head resources; /* Protected by lock. */
> > > > >  };
> > > > >
> > > > > +static inline void kunit_set_death_test(struct kunit *test, bool 
> > > > > death_test)
> > > > > +{
> > > > > +   spin_lock(>lock);
> > > > > +   test->death_test = death_test;
> > > > > +   spin_unlock(>lock);
> > > > > +}
> > > >
> > > > These getters and setters are using spinlocks again. It doesn't make any
> > > > sense. It probably needs a rework like was done for the other bool
> > > > member, success.
> > >
> > > No, this is intentional. death_test can transition from false to true
> > > and then back to false within the same test. Maybe that deserves a
> > > comment?
> >
> > Yes. How does it transition from true to false again?
> 
> The purpose is to tell try_catch that it was expected for the test to
> bail out. Given the default implementation there is no way for this to
> happen aside from abort() being called, but in some implementations it
> is possible to implement a fault catcher which allows a test suite to
> recover from an unexpected failure.
> 
> Maybe it would be best to drop this until I add one of those
> alternative implementations.

Ok.

> 
> > Either way, having a spinlock around a read/write API doesn't make sense
> > because it just makes sure that two writes don't overlap, but otherwise
> > does nothing to keep things synchronized. For example a set to true
> > after a set to false when the two calls to set true or false aren't
> > synchronized means they can happen in any order. So I don't see how it
> > needs a spinlock. The lock needs to be one level higher.
> 
> There shouldn't be any cases where one thread is trying to set it
> while another is trying to unset it. The thing I am worried about here
> is making sure all the cores see the write, and making sure no reads
> or writes get reordered before it. So I guess I just want a fence. So
> I take it I should probably have is a WRITE_ONCE here and READ_ONCE in
> the getter?
> 

Are the gets and sets in program order? If so, WRITE_ONCE and READ_ONCE
aren't required. Otherwise, if it's possible for one thread to write it
and another to read it but the threads are ordered with some other
barrier like a completion or lock, then again the macros aren't needed.
It would be good to read memory-barriers.txt to understand when to use
the read/write macros.

___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [RFC PATCH v2 15/19] mm/gup: Introduce vaddr_pin_pages()

[Ira Weiny]

On Tue, Aug 13, 2019 at 08:47:06AM -0300, Jason Gunthorpe wrote:
> On Mon, Aug 12, 2019 at 02:48:55PM -0700, Ira Weiny wrote:
> > On Mon, Aug 12, 2019 at 09:28:14AM -0300, Jason Gunthorpe wrote:
> > > On Fri, Aug 09, 2019 at 03:58:29PM -0700, ira.we...@intel.com wrote:
> > > > From: Ira Weiny 
> > > > 
> > > > The addition of FOLL_LONGTERM has taken on additional meaning for CMA
> > > > pages.
> > > > 
> > > > In addition subsystems such as RDMA require new information to be passed
> > > > to the GUP interface to track file owning information.  As such a simple
> > > > FOLL_LONGTERM flag is no longer sufficient for these users to pin pages.
> > > > 
> > > > Introduce a new GUP like call which takes the newly introduced vaddr_pin
> > > > information.  Failure to pass the vaddr_pin object back to a vaddr_put*
> > > > call will result in a failure if pins were created on files during the
> > > > pin operation.
> > > 
> > > Is this a 'vaddr' in the traditional sense, ie does it work with
> > > something returned by valloc?
> > 
> > ...or malloc in user space, yes.  I think the idea is that it is a user 
> > virtual
> > address.
> 
> valloc is a kernel call

Oh...  I thought you meant this: https://linux.die.net/man/3/valloc

> 
> > So I'm open to suggestions.  Jan gave me this one, so I figured it was 
> > safer to
> > suggest it...
> 
> Should have the word user in it, imho

Fair enough...

user_addr_pin_pages(void __user * addr, ...) ?

uaddr_pin_pages(void __user * addr, ...) ?

I think I like uaddr...

> 
> > > I also wish GUP like functions took in a 'void __user *' instead of
> > > the unsigned long to make this clear :\
> > 
> > Not a bad idea.  But I only see a couple of call sites who actually use a 
> > 'void
> > __user *' to pass into GUP...  :-/
> > 
> > For RDMA the address is _never_ a 'void __user *' AFAICS.
> 
> That is actually a bug, converting from u64 to a 'user VA' needs to go
> through u64_to_user_ptr().

Fair enough.

But there are a lot of call sites throughout the kernel who have the same
bug...  I'm ok with forcing u64_to_user_ptr() to use this new call if others
are.

Ira

___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

剖析嵌入式设计师易犯的错误点

[安獗]

Message-ID: 0106639437589
From: =?xdasrm9??= 
To: 




详 情 请 阅 读 附 件
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [RFC PATCH v2 16/19] RDMA/uverbs: Add back pointer to system file object

[Ira Weiny]

On Tue, Aug 13, 2019 at 03:00:22PM -0300, Jason Gunthorpe wrote:
> On Tue, Aug 13, 2019 at 10:41:42AM -0700, Ira Weiny wrote:
> 
> > And I was pretty sure uverbs_destroy_ufile_hw() would take care of (or 
> > ensure
> > that some other thread is) destroying all the MR's we have associated with 
> > this
> > FD.
> 
> fd's can't be revoked, so destroy_ufile_hw() can't touch them. It
> deletes any underlying HW resources, but the FD persists.

I misspoke.  I should have said associated with this "context".  And of course
uverbs_destroy_ufile_hw() does not touch the FD.  What I mean is that the
struct file which had file_pins hanging off of it would be getting its file
pins destroyed by uverbs_destroy_ufile_hw().  Therefore we don't need the FD
after uverbs_destroy_ufile_hw() is done.

But since it does not block it may be that the struct file is gone before the
MR is actually destroyed.  Which means I think the GUP code would blow up in
that case...  :-(

I was thinking it was the other way around.  And in fact most of the time I
think it is.  But we can't depend on that...

>  
> > > This is why having a back pointer like this is so ugly, it creates a
> > > reference counting cycle
> > 
> > Yep...  I worked through this...  and it was giving me fits...
> > 
> > Anyway, the struct file is the only object in the core which was reasonable 
> > to
> > store this information in since that is what is passed around to other
> > processes...
> 
> It could be passed down in the uattr_bundle, once you are in file operations

What is "It"?  The struct file?  Or the file pin information?

> handle the file is guarenteed to exist, and we've now arranged things

I don't understand what you mean by "... once you are in file operations 
handle... "?

> so the uattr_bundle flows into the umem, as umems can only be
> established under a system call.

"uattr_bundle" == uverbs_attr_bundle right?

The problem is that I don't think the core should be handling
uverbs_attr_bundles directly.  So, I think you are driving at the same idea I
had WRT callbacks into the driver.

The drivers could provide some generic object (in RDMA this could be the
uverbs_attr_bundle) which represents their "context".

The GUP code calls back into the driver with file pin information as it
encounters and pins pages.  The driver, RDMA in this case, associates this
information with the "context".

But for the procfs interface, that context then needs to be associated with any
file which points to it...  For RDMA, or any other "FD based pin mechanism", it
would be up to the driver to "install" a procfs handler into any struct file
which _may_ point to this context.  (before _or_ after memory pins).

Then the procfs code can walk the FD array and if this handler is installed it
knows there is file pin information associated with that struct file and it can
be printed...

This is not impossible.  But I think is a lot harder for drivers to make
right...

Ira

___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH v3] ndctl, check: Ensure mmap of BTT sections work with 64K page-sizes

[Jeff Moyer]

Vaibhav Jain  writes:

> Thanks for reviewing this patch Jeff.
>
> Jeff Moyer  writes:
>
>> Vaibhav Jain  writes:
>>
>>> On PPC64 which uses a 64K page-size, ndtl-check command fails on a BTT
>>> namespace with following error:
>>>
>>> $ sudo ndctl check-namespace namespace0.0 -vv
>>>   namespace0.0: namespace_check: checking namespace0.0
>>>   namespace0.0: btt_discover_arenas: found 1 BTT arena
>>>   namespace0.0: btt_create_mappings: mmap arena[0].info [sz = 0x1000, off = 
>>> 0x1000] failed: Invalid argument
>>>   error checking namespaces: Invalid argument
>>>   checked 0 namespaces
>>>
>>> An error happens when btt_create_mappings() tries to mmap the sections
>>> of the BTT device which are usually 4K offset aligned. However the
>>> mmap() syscall expects the 'offset' argument to be in multiples of
>>> page-size, hence it returns EINVAL causing the btt_create_mappings()
>>> to error out.
>>>
>>> As a fix for the issue this patch proposes addition of two new
>>> functions to 'check.c' namely btt_mmap/btt_unmap that can be used to
>>> map/unmap parts of BTT device to ndctl process address-space. The
>>> functions tweaks the requested 'offset' argument to mmap() ensuring
>>> that its page-size aligned and then fix-ups the returned pointer such
>>> that it points to the requested offset within mmapped region.
>>>
>>> With these newly introduced functions the patch updates the call-sites
>>> in 'check.c' to use these functions instead of mmap/unmap syscalls.
>>> Also since btt_mmap returns NULL if mmap operation fails, the
>>> error check call-sites can be made against NULL instead of MAP_FAILED.
>>>
>>> Reported-by: Harish Sriram 
>>> Signed-off-by: Vaibhav Jain 
>>> ---
>>> Changelog:
>>> v3:
>>> * Fixed a log string that was splitted across multiple lines [Vishal]
>>>
>>> v2:
>>> * Updated patch description to include canonical email address of bug
>>>   reporter. [Vishal]
>>> * Improved the comment describing function btt_mmap() in 'check.c'
>>>   [Vishal]
>>> * Simplified the argument list for btt_mmap() to just accept bttc,
>>>   length and offset. Other arguments for mmap() are derived from these
>>>   args. [Vishal]
>>> * Renamed 'shift' variable in btt_mmap()/unmap() to 'page_offset'
>>>   [Vishal]
>>> * Improved the dbg() messages logged inside
>>>   btt_mmap()/unmap(). [Vishal]
>>> * Return NULL from btt_mmap() in case of an error. [Vishal]
>>> * Changed the all sites to btt_mmap() to perform error check against
>>>   NULL return value instead of MAP_FAILED. [Vishal]
>>> ---
>>>  ndctl/check.c | 93 +--
>>>  1 file changed, 67 insertions(+), 26 deletions(-)
>>>
>>> diff --git a/ndctl/check.c b/ndctl/check.c
>>> index 8a7125053865..5969012eba84 100644
>>> --- a/ndctl/check.c
>>> +++ b/ndctl/check.c
>>> @@ -907,59 +907,100 @@ static int btt_discover_arenas(struct btt_chk *bttc)
>>> return ret;
>>>  }
>>>  
>>> -static int btt_create_mappings(struct btt_chk *bttc)
>>> +/*
>>> + * Wrap call to mmap(2) to work with btt device offsets that are not 
>>> aligned
>>> + * to system page boundary. It works by rounding down the requested offset
>>> + * to sys_page_size when calling mmap(2) and then returning a fixed-up 
>>> pointer
>>> + * to the correct offset in the mmaped region.
>>> + */
>>> +static void *btt_mmap(struct btt_chk *bttc, size_t length, off_t offset)
>>>  {
>>> -   struct arena_info *a;
>>> -   int mmap_flags;
>>> -   int i;
>>> +   off_t page_offset;
>>> +   int prot_flags;
>>> +   uint8_t *addr;
>>>  
>>> if (!bttc->opts->repair)
>>> -   mmap_flags = PROT_READ;
>>> +   prot_flags = PROT_READ;
>>> else
>>> -   mmap_flags = PROT_READ|PROT_WRITE;
>>> +   prot_flags = PROT_READ|PROT_WRITE;
>>> +
>>> +   /* Calculate the page_offset from the system page boundary */
>>> +   page_offset = offset - rounddown(offset, bttc->sys_page_size);
>>> +
>>> +   /* Update the offset and length with the page_offset calculated above */
>>> +   offset -= page_offset;
>>> +   length += page_offset;
>>
>> Don't you have to ensure that the length is also a multiple of the
>> system page size?
>>
>> -Jeff
>
> No, as the BTT info header is 4K in size which isnt in multiple of page
> size on PPC64 where PAGE_SIZE == 64K.
>
> Also I see 'do_mmap()' in kernel always rounding up the 'length' to
> PAGE_SIZE. So any unaligned value for 'length' arg will be handled by the
> kernel.
>
> Finally mmap(2) doesn't put any constraint on the 'length' argument to
> mmap except it should > 0. The 'offset' arg on other hand has a
> constraint which needs to be in multiple of PAGE_SIZE.

Right, this is the part I forgot.  I'd probaby write the map and unmap
routines a bit differently, but what you wrote works.

Reviewed-by: Jeff Moyer 


>>
>>> +
>>> +   addr = mmap(NULL, length, prot_flags, MAP_SHARED, bttc->fd, offset);
>>> +
>>> +   /* If needed fixup the return pointer to correct offset requested */
>>> +   if (addr != MAP_FAILED)
>>> + 

Re: [PATCH v5 1/4] nvdimm: Consider probe return -EOPNOTSUPP as success

[Dan Williams]

Hi Aneesh, logic looks correct but there are some cleanups I'd like to
see and a lead-in patch that I attached.

I've started prefixing nvdimm patches with:

libnvdimm/$component:

...since this patch mostly impacts the pmem driver lets prefix it
"libnvdimm/pmem: "

On Fri, Aug 9, 2019 at 12:45 AM Aneesh Kumar K.V
 wrote:
>
> This patch add -EOPNOTSUPP as return from probe callback to

s/This patch add/Add/

No need to say "this patch" it's obviously a patch.

> indicate we were not able to initialize a namespace due to pfn superblock
> feature/version mismatch. We want to consider this a probe success so that
> we can create new namesapce seed and there by avoid marking the failed
> namespace as the seed namespace.

Please replace usage of "we" with the exact agent involved as which
"we" is being referred to gets confusing for the reader.

i.e. "indicate that the pmem driver was not..." "The nvdimm core wants
to consider this...".

>
> Signed-off-by: Aneesh Kumar K.V 
> ---
>  drivers/nvdimm/bus.c  |  2 +-
>  drivers/nvdimm/pmem.c | 26 ++
>  2 files changed, 23 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/nvdimm/bus.c b/drivers/nvdimm/bus.c
> index 798c5c4aea9c..16c35e6446a7 100644
> --- a/drivers/nvdimm/bus.c
> +++ b/drivers/nvdimm/bus.c
> @@ -95,7 +95,7 @@ static int nvdimm_bus_probe(struct device *dev)
> rc = nd_drv->probe(dev);
> debug_nvdimm_unlock(dev);
>
> -   if (rc == 0)
> +   if (rc == 0 || rc == -EOPNOTSUPP)
> nd_region_probe_success(nvdimm_bus, dev);

This now makes the nd_region_probe_success() helper obviously misnamed
since it now wants to take actions on non-probe success. I attached a
lead-in cleanup that you can pull into your series that renames that
routine to nd_region_advance_seeds().

When you rebase this needs a comment about why EOPNOTSUPP has special handling.

> else
> nd_region_disable(nvdimm_bus, dev);
> diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
> index 4c121dd03dd9..3f498881dd28 100644
> --- a/drivers/nvdimm/pmem.c
> +++ b/drivers/nvdimm/pmem.c
> @@ -490,6 +490,7 @@ static int pmem_attach_disk(struct device *dev,
>
>  static int nd_pmem_probe(struct device *dev)
>  {
> +   int ret;
> struct nd_namespace_common *ndns;
>
> ndns = nvdimm_namespace_common_probe(dev);
> @@ -505,12 +506,29 @@ static int nd_pmem_probe(struct device *dev)
> if (is_nd_pfn(dev))
> return pmem_attach_disk(dev, ndns);
>
> -   /* if we find a valid info-block we'll come back as that personality 
> */
> -   if (nd_btt_probe(dev, ndns) == 0 || nd_pfn_probe(dev, ndns) == 0
> -   || nd_dax_probe(dev, ndns) == 0)

Similar need for an updated comment here to explain the special
translation of error codes.

> +   ret = nd_btt_probe(dev, ndns);
> +   if (ret == 0)
> return -ENXIO;
> +   else if (ret == -EOPNOTSUPP)

Are there cases where the btt driver needs to return EOPNOTSUPP? I'd
otherwise like to keep this special casing constrained to the pfn /
dax info block cases.
From 9ec13a8672e87e0b1c5b9427ab926168e53d55bc Mon Sep 17 00:00:00 2001
From: Dan Williams 
Date: Tue, 13 Aug 2019 13:09:27 -0700
Subject: [PATCH] libnvdimm/region: Rewrite _probe_success() to
 _advance_seeds()

The nd_region_probe_success() helper collides seed management with
nvdimm->busy tracking. Given the 'busy' increment is handled internal to the
nd_region driver 'probe' path move the decrement to the 'remove' path.
With that cleanup the routine can be renamed to the more descriptive
nd_region_advance_seeds().

The change is prompted by an incoming need to optionally advance the
seeds on other events besides 'probe' success.

Cc: "Aneesh Kumar K.V" 
Signed-off-by: Dan Williams 
---
 drivers/nvdimm/bus.c|  7 +---
 drivers/nvdimm/namespace_devs.c | 34 ++---
 drivers/nvdimm/nd-core.h|  3 +-
 drivers/nvdimm/region_devs.c| 68 +
 4 files changed, 41 insertions(+), 71 deletions(-)

diff --git a/drivers/nvdimm/bus.c b/drivers/nvdimm/bus.c
index 29479d3b01b0..ee6de34ae525 100644
--- a/drivers/nvdimm/bus.c
+++ b/drivers/nvdimm/bus.c
@@ -95,10 +95,8 @@ static int nvdimm_bus_probe(struct device *dev)
 	rc = nd_drv->probe(dev);
 	debug_nvdimm_unlock(dev);
 
-	if (rc == 0)
-		nd_region_probe_success(nvdimm_bus, dev);
-	else
-		nd_region_disable(nvdimm_bus, dev);
+	if (rc == 0 && dev->parent && is_nd_region(dev->parent))
+		nd_region_advance_seeds(to_nd_region(dev->parent), dev);
 	nvdimm_bus_probe_end(nvdimm_bus);
 
 	dev_dbg(_bus->dev, "END: %s.probe(%s) = %d\n", dev->driver->name,
@@ -121,7 +119,6 @@ static int nvdimm_bus_remove(struct device *dev)
 		rc = nd_drv->remove(dev);
 		debug_nvdimm_unlock(dev);
 	}
-	nd_region_disable(nvdimm_bus, dev);
 
 	dev_dbg(_bus->dev, "%s.remove(%s) = %d\n", dev->driver->name,
 			dev_name(dev), rc);
diff --git 

Re: [RFC PATCH v2 15/19] mm/gup: Introduce vaddr_pin_pages()

[John Hubbard]

On 8/13/19 10:46 AM, Ira Weiny wrote:
> On Tue, Aug 13, 2019 at 08:47:06AM -0300, Jason Gunthorpe wrote:
>> On Mon, Aug 12, 2019 at 02:48:55PM -0700, Ira Weiny wrote:
>>> On Mon, Aug 12, 2019 at 09:28:14AM -0300, Jason Gunthorpe wrote:
 On Fri, Aug 09, 2019 at 03:58:29PM -0700, ira.we...@intel.com wrote:
> From: Ira Weiny 
...
>>> So I'm open to suggestions.  Jan gave me this one, so I figured it was 
>>> safer to
>>> suggest it...
>>
>> Should have the word user in it, imho
> 
> Fair enough...
> 
> user_addr_pin_pages(void __user * addr, ...) ?
> 
> uaddr_pin_pages(void __user * addr, ...) ?
> 
> I think I like uaddr...
> 

Better to spell out "user". "u" prefixes are used for "unsigned" and it
is just too ambiguous here. Maybe:

vaddr_pin_user_pages()

...which also sounds close enough to get_user_pages() that a bit of
history and continuity is preserved, too.



thanks,
-- 
John Hubbard
NVIDIA
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

[PATCH v13 14/18] kunit: defconfig: add defconfigs for building KUnit tests

[Brendan Higgins]

Add defconfig for UML and a fragment that can be used to configure other
architectures for building KUnit tests. Add option to kunit_tool to use
a defconfig to create the kunitconfig.

Signed-off-by: Brendan Higgins 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Logan Gunthorpe 
---
 arch/um/configs/kunit_defconfig  |  3 +++
 tools/testing/kunit/configs/all_tests.config |  3 +++
 tools/testing/kunit/kunit.py | 28 +---
 tools/testing/kunit/kunit_kernel.py  |  3 ++-
 4 files changed, 32 insertions(+), 5 deletions(-)
 create mode 100644 arch/um/configs/kunit_defconfig
 create mode 100644 tools/testing/kunit/configs/all_tests.config

diff --git a/arch/um/configs/kunit_defconfig b/arch/um/configs/kunit_defconfig
new file mode 100644
index 0..9235b7d42d389
--- /dev/null
+++ b/arch/um/configs/kunit_defconfig
@@ -0,0 +1,3 @@
+CONFIG_KUNIT=y
+CONFIG_KUNIT_TEST=y
+CONFIG_KUNIT_EXAMPLE_TEST=y
diff --git a/tools/testing/kunit/configs/all_tests.config 
b/tools/testing/kunit/configs/all_tests.config
new file mode 100644
index 0..9235b7d42d389
--- /dev/null
+++ b/tools/testing/kunit/configs/all_tests.config
@@ -0,0 +1,3 @@
+CONFIG_KUNIT=y
+CONFIG_KUNIT_TEST=y
+CONFIG_KUNIT_EXAMPLE_TEST=y
diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py
index da11bd62a4b82..0944dea5c3211 100755
--- a/tools/testing/kunit/kunit.py
+++ b/tools/testing/kunit/kunit.py
@@ -11,6 +11,7 @@ import argparse
 import sys
 import os
 import time
+import shutil
 
 from collections import namedtuple
 from enum import Enum, auto
@@ -21,7 +22,7 @@ import kunit_parser
 
 KunitResult = namedtuple('KunitResult', ['status','result'])
 
-KunitRequest = namedtuple('KunitRequest', ['raw_output','timeout', 'jobs', 
'build_dir'])
+KunitRequest = namedtuple('KunitRequest', ['raw_output','timeout', 'jobs', 
'build_dir', 'defconfig'])
 
 class KunitStatus(Enum):
SUCCESS = auto()
@@ -29,8 +30,16 @@ class KunitStatus(Enum):
BUILD_FAILURE = auto()
TEST_FAILURE = auto()
 
+def create_default_kunitconfig():
+   if not os.path.exists(kunit_kernel.KUNITCONFIG_PATH):
+   shutil.copyfile('arch/um/configs/kunit_defconfig',
+   kunit_kernel.KUNITCONFIG_PATH)
+
 def run_tests(linux: kunit_kernel.LinuxSourceTree,
  request: KunitRequest) -> KunitResult:
+   if request.defconfig:
+   create_default_kunitconfig()
+
config_start = time.time()
success = linux.build_reconfig(request.build_dir)
config_end = time.time()
@@ -72,7 +81,7 @@ def run_tests(linux: kunit_kernel.LinuxSourceTree,
else:
return KunitResult(KunitStatus.SUCCESS, test_result)
 
-def main(argv, linux):
+def main(argv, linux=None):
parser = argparse.ArgumentParser(
description='Helps writing and running KUnit tests.')
subparser = parser.add_subparsers(dest='subcommand')
@@ -99,13 +108,24 @@ def main(argv, linux):
'directory.',
type=str, default=None, metavar='build_dir')
 
+   run_parser.add_argument('--defconfig',
+   help='Uses a default kunitconfig.',
+   action='store_true')
+
cli_args = parser.parse_args(argv)
 
if cli_args.subcommand == 'run':
+   if cli_args.defconfig:
+   create_default_kunitconfig()
+
+   if not linux:
+   linux = kunit_kernel.LinuxSourceTree()
+
request = KunitRequest(cli_args.raw_output,
   cli_args.timeout,
   cli_args.jobs,
-  cli_args.build_dir)
+  cli_args.build_dir,
+  cli_args.defconfig)
result = run_tests(linux, request)
if result.status != KunitStatus.SUCCESS:
sys.exit(1)
@@ -113,4 +133,4 @@ def main(argv, linux):
parser.print_help()
 
 if __name__ == '__main__':
-   main(sys.argv[1:], kunit_kernel.LinuxSourceTree())
+   main(sys.argv[1:])
diff --git a/tools/testing/kunit/kunit_kernel.py 
b/tools/testing/kunit/kunit_kernel.py
index 07c0abf2f47df..bf38768353313 100644
--- a/tools/testing/kunit/kunit_kernel.py
+++ b/tools/testing/kunit/kunit_kernel.py
@@ -14,6 +14,7 @@ import os
 import kunit_config
 
 KCONFIG_PATH = '.config'
+KUNITCONFIG_PATH = 'kunitconfig'
 
 class ConfigError(Exception):
"""Represents an error trying to configure the Linux kernel."""
@@ -81,7 +82,7 @@ class LinuxSourceTree(object):
 
def __init__(self):
self._kconfig = kunit_config.Kconfig()
-   self._kconfig.read_from_file('kunitconfig')
+   self._kconfig.read_from_file(KUNITCONFIG_PATH)

[PATCH v13 13/18] kunit: tool: add Python wrappers for running KUnit tests

[Brendan Higgins]

From: Felix Guo 

The ultimate goal is to create minimal isolated test binaries; in the
meantime we are using UML to provide the infrastructure to run tests, so
define an abstract way to configure and run tests that allow us to
change the context in which tests are built without affecting the user.
This also makes pretty and dynamic error reporting, and a lot of other
nice features easier.

kunit_config.py:
  - parse .config and Kconfig files.

kunit_kernel.py: provides helper functions to:
  - configure the kernel using kunitconfig.
  - build the kernel with the appropriate configuration.
  - provide function to invoke the kernel and stream the output back.

kunit_parser.py: parses raw logs returned out by kunit_kernel and
displays them in a user friendly way.

test_data/*: samples of test data for testing kunit.py, kunit_config.py,
etc.

Signed-off-by: Felix Guo 
Signed-off-by: Brendan Higgins 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Logan Gunthorpe 
Reviewed-by: Stephen Boyd 
---
 tools/testing/kunit/.gitignore|   3 +
 tools/testing/kunit/kunit.py  | 116 +++
 tools/testing/kunit/kunit_config.py   |  66 
 tools/testing/kunit/kunit_kernel.py   | 148 +
 tools/testing/kunit/kunit_parser.py   | 310 ++
 tools/testing/kunit/kunit_tool_test.py| 206 
 .../test_is_test_passed-all_passed.log|  32 ++
 .../test_data/test_is_test_passed-crash.log   |  69 
 .../test_data/test_is_test_passed-failure.log |  36 ++
 .../test_is_test_passed-no_tests_run.log  |  75 +
 .../test_output_isolated_correctly.log| 106 ++
 .../test_data/test_read_from_file.kconfig |  17 +
 12 files changed, 1184 insertions(+)
 create mode 100644 tools/testing/kunit/.gitignore
 create mode 100755 tools/testing/kunit/kunit.py
 create mode 100644 tools/testing/kunit/kunit_config.py
 create mode 100644 tools/testing/kunit/kunit_kernel.py
 create mode 100644 tools/testing/kunit/kunit_parser.py
 create mode 100755 tools/testing/kunit/kunit_tool_test.py
 create mode 100644 
tools/testing/kunit/test_data/test_is_test_passed-all_passed.log
 create mode 100644 tools/testing/kunit/test_data/test_is_test_passed-crash.log
 create mode 100644 
tools/testing/kunit/test_data/test_is_test_passed-failure.log
 create mode 100644 
tools/testing/kunit/test_data/test_is_test_passed-no_tests_run.log
 create mode 100644 
tools/testing/kunit/test_data/test_output_isolated_correctly.log
 create mode 100644 tools/testing/kunit/test_data/test_read_from_file.kconfig

diff --git a/tools/testing/kunit/.gitignore b/tools/testing/kunit/.gitignore
new file mode 100644
index 0..c791ff59a37a9
--- /dev/null
+++ b/tools/testing/kunit/.gitignore
@@ -0,0 +1,3 @@
+# Byte-compiled / optimized / DLL files
+__pycache__/
+*.py[cod]
\ No newline at end of file
diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py
new file mode 100755
index 0..da11bd62a4b82
--- /dev/null
+++ b/tools/testing/kunit/kunit.py
@@ -0,0 +1,116 @@
+#!/usr/bin/python3
+# SPDX-License-Identifier: GPL-2.0
+#
+# A thin wrapper on top of the KUnit Kernel
+#
+# Copyright (C) 2019, Google LLC.
+# Author: Felix Guo 
+# Author: Brendan Higgins 
+
+import argparse
+import sys
+import os
+import time
+
+from collections import namedtuple
+from enum import Enum, auto
+
+import kunit_config
+import kunit_kernel
+import kunit_parser
+
+KunitResult = namedtuple('KunitResult', ['status','result'])
+
+KunitRequest = namedtuple('KunitRequest', ['raw_output','timeout', 'jobs', 
'build_dir'])
+
+class KunitStatus(Enum):
+   SUCCESS = auto()
+   CONFIG_FAILURE = auto()
+   BUILD_FAILURE = auto()
+   TEST_FAILURE = auto()
+
+def run_tests(linux: kunit_kernel.LinuxSourceTree,
+ request: KunitRequest) -> KunitResult:
+   config_start = time.time()
+   success = linux.build_reconfig(request.build_dir)
+   config_end = time.time()
+   if not success:
+   return KunitResult(KunitStatus.CONFIG_FAILURE, 'could not 
configure kernel')
+
+   kunit_parser.print_with_timestamp('Building KUnit Kernel ...')
+
+   build_start = time.time()
+   success = linux.build_um_kernel(request.jobs, request.build_dir)
+   build_end = time.time()
+   if not success:
+   return KunitResult(KunitStatus.BUILD_FAILURE, 'could not build 
kernel')
+
+   kunit_parser.print_with_timestamp('Starting KUnit Kernel ...')
+   test_start = time.time()
+
+   test_result = kunit_parser.TestResult(kunit_parser.TestStatus.SUCCESS,
+ [],
+ 'Tests not Parsed.')
+   if request.raw_output:
+   kunit_parser.raw_output(
+   linux.run_kernel(timeout=request.timeout))
+   else:
+   kunit_output = linux.run_kernel(timeout=request.timeout)
+   test_result 

[PATCH v13 18/18] MAINTAINERS: add proc sysctl KUnit test to PROC SYSCTL section

[Brendan Higgins]

Add entry for the new proc sysctl KUnit test to the PROC SYSCTL section,
and add Iurii as a maintainer.

Signed-off-by: Brendan Higgins 
Cc: Iurii Zaikin 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Logan Gunthorpe 
Acked-by: Luis Chamberlain 
---
 MAINTAINERS | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index f0bd77e8a8a2f..0cac78807137b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12965,12 +12965,14 @@ F:Documentation/filesystems/proc.txt
 PROC SYSCTL
 M: Luis Chamberlain 
 M: Kees Cook 
+M: Iurii Zaikin 
 L: linux-ker...@vger.kernel.org
 L: linux-fsde...@vger.kernel.org
 S: Maintained
 F: fs/proc/proc_sysctl.c
 F: include/linux/sysctl.h
 F: kernel/sysctl.c
+F: kernel/sysctl-test.c
 F: tools/testing/selftests/sysctl/
 
 PS3 NETWORK SUPPORT
-- 
2.23.0.rc1.153.gdeed80330f-goog

___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

[PATCH v13 12/18] kunit: test: add tests for KUnit managed resources

[Brendan Higgins]

From: Avinash Kondareddy 

Add unit tests for KUnit managed resources. KUnit managed resources
(struct kunit_resource) are resources that are automatically cleaned up
at the end of a KUnit test, similar to the concept of devm_* managed
resources.

Signed-off-by: Avinash Kondareddy 
Signed-off-by: Brendan Higgins 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Logan Gunthorpe 
---
 kunit/test-test.c | 228 ++
 1 file changed, 228 insertions(+)

diff --git a/kunit/test-test.c b/kunit/test-test.c
index e0ab4bd546eac..5ebe059d16e25 100644
--- a/kunit/test-test.c
+++ b/kunit/test-test.c
@@ -101,3 +101,231 @@ static struct kunit_suite kunit_try_catch_test_suite = {
.test_cases = kunit_try_catch_test_cases,
 };
 kunit_test_suite(kunit_try_catch_test_suite);
+
+/*
+ * Context for testing test managed resources
+ * is_resource_initialized is used to test arbitrary resources
+ */
+struct kunit_test_resource_context {
+   struct kunit test;
+   bool is_resource_initialized;
+   int allocate_order[2];
+   int free_order[2];
+};
+
+static int fake_resource_init(struct kunit_resource *res, void *context)
+{
+   struct kunit_test_resource_context *ctx = context;
+
+   res->allocation = >is_resource_initialized;
+   ctx->is_resource_initialized = true;
+   return 0;
+}
+
+static void fake_resource_free(struct kunit_resource *res)
+{
+   bool *is_resource_initialized = res->allocation;
+
+   *is_resource_initialized = false;
+}
+
+static void kunit_resource_test_init_resources(struct kunit *test)
+{
+   struct kunit_test_resource_context *ctx = test->priv;
+
+   kunit_init_test(>test, "testing_test_init_test");
+
+   KUNIT_EXPECT_TRUE(test, list_empty(>test.resources));
+}
+
+static void kunit_resource_test_alloc_resource(struct kunit *test)
+{
+   struct kunit_test_resource_context *ctx = test->priv;
+   struct kunit_resource *res;
+   kunit_resource_free_t free = fake_resource_free;
+
+   res = kunit_alloc_and_get_resource(>test,
+  fake_resource_init,
+  fake_resource_free,
+  GFP_KERNEL,
+  ctx);
+
+   KUNIT_ASSERT_NOT_ERR_OR_NULL(test, res);
+   KUNIT_EXPECT_PTR_EQ(test,
+   >is_resource_initialized,
+   (bool *) res->allocation);
+   KUNIT_EXPECT_TRUE(test, list_is_last(>node, >test.resources));
+   KUNIT_EXPECT_PTR_EQ(test, free, res->free);
+}
+
+static void kunit_resource_test_destroy_resource(struct kunit *test)
+{
+   struct kunit_test_resource_context *ctx = test->priv;
+   struct kunit_resource *res = kunit_alloc_and_get_resource(
+   >test,
+   fake_resource_init,
+   fake_resource_free,
+   GFP_KERNEL,
+   ctx);
+
+   KUNIT_ASSERT_FALSE(test,
+  kunit_resource_destroy(>test,
+ kunit_resource_instance_match,
+ res->free,
+ res->allocation));
+
+   KUNIT_EXPECT_FALSE(test, ctx->is_resource_initialized);
+   KUNIT_EXPECT_TRUE(test, list_empty(>test.resources));
+}
+
+static void kunit_resource_test_cleanup_resources(struct kunit *test)
+{
+   int i;
+   struct kunit_test_resource_context *ctx = test->priv;
+   struct kunit_resource *resources[5];
+
+   for (i = 0; i < ARRAY_SIZE(resources); i++) {
+   resources[i] = kunit_alloc_and_get_resource(>test,
+   fake_resource_init,
+   fake_resource_free,
+   GFP_KERNEL,
+   ctx);
+   }
+
+   kunit_cleanup(>test);
+
+   KUNIT_EXPECT_TRUE(test, list_empty(>test.resources));
+}
+
+static void kunit_resource_test_mark_order(int order_array[],
+  size_t order_size,
+  int key)
+{
+   int i;
+
+   for (i = 0; i < order_size && order_array[i]; i++)
+   ;
+
+   order_array[i] = key;
+}
+
+#define KUNIT_RESOURCE_TEST_MARK_ORDER(ctx, order_field, key) \
+   kunit_resource_test_mark_order(ctx->order_field,   \
+  ARRAY_SIZE(ctx->order_field),   \
+  key)
+
+static int fake_resource_2_init(struct kunit_resource *res, void *context)
+{
+   struct kunit_test_resource_context *ctx = context;
+
+   KUNIT_RESOURCE_TEST_MARK_ORDER(ctx, allocate_order, 2);
+
+   

[PATCH v13 10/18] kunit: test: add tests for kunit test abort

[Brendan Higgins]

Add KUnit tests for the KUnit test abort mechanism (see preceding
commit). Add tests both for general try catch mechanism as well as
non-architecture specific mechanism.

Signed-off-by: Brendan Higgins 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Logan Gunthorpe 
Reviewed-by: Stephen Boyd 
---
 kunit/Makefile|   3 +-
 kunit/test-test.c | 106 ++
 2 files changed, 108 insertions(+), 1 deletion(-)
 create mode 100644 kunit/test-test.c

diff --git a/kunit/Makefile b/kunit/Makefile
index c9176c9c578c6..769d9402b5d3a 100644
--- a/kunit/Makefile
+++ b/kunit/Makefile
@@ -3,6 +3,7 @@ obj-$(CONFIG_KUNIT) +=  test.o \
assert.o \
try-catch.o
 
-obj-$(CONFIG_KUNIT_TEST) +=string-stream-test.o
+obj-$(CONFIG_KUNIT_TEST) +=test-test.o \
+   string-stream-test.o
 
 obj-$(CONFIG_KUNIT_EXAMPLE_TEST) +=example-test.o
diff --git a/kunit/test-test.c b/kunit/test-test.c
new file mode 100644
index 0..06d34d36b1038
--- /dev/null
+++ b/kunit/test-test.c
@@ -0,0 +1,106 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit test for core test infrastructure.
+ *
+ * Copyright (C) 2019, Google LLC.
+ * Author: Brendan Higgins 
+ */
+#include 
+
+struct kunit_try_catch_test_context {
+   struct kunit_try_catch *try_catch;
+   bool function_called;
+};
+
+static void kunit_test_successful_try(void *data)
+{
+   struct kunit *test = data;
+   struct kunit_try_catch_test_context *ctx = test->priv;
+
+   ctx->function_called = true;
+}
+
+static void kunit_test_no_catch(void *data)
+{
+   struct kunit *test = data;
+
+   KUNIT_FAIL(test, "Catch should not be called\n");
+}
+
+static void kunit_test_try_catch_successful_try_no_catch(struct kunit *test)
+{
+   struct kunit_try_catch_test_context *ctx = test->priv;
+   struct kunit_try_catch *try_catch = ctx->try_catch;
+
+   kunit_try_catch_init(try_catch,
+test,
+kunit_test_successful_try,
+kunit_test_no_catch);
+   kunit_try_catch_run(try_catch, test);
+
+   KUNIT_EXPECT_TRUE(test, ctx->function_called);
+}
+
+static void kunit_test_unsuccessful_try(void *data)
+{
+   struct kunit *test = data;
+   struct kunit_try_catch_test_context *ctx = test->priv;
+   struct kunit_try_catch *try_catch = ctx->try_catch;
+
+   kunit_try_catch_throw(try_catch);
+   KUNIT_FAIL(test, "This line should never be reached\n");
+}
+
+static void kunit_test_catch(void *data)
+{
+   struct kunit *test = data;
+   struct kunit_try_catch_test_context *ctx = test->priv;
+
+   ctx->function_called = true;
+}
+
+static void kunit_test_try_catch_unsuccessful_try_does_catch(struct kunit 
*test)
+{
+   struct kunit_try_catch_test_context *ctx = test->priv;
+   struct kunit_try_catch *try_catch = ctx->try_catch;
+
+   kunit_try_catch_init(try_catch,
+test,
+kunit_test_unsuccessful_try,
+kunit_test_catch);
+   kunit_try_catch_run(try_catch, test);
+
+   KUNIT_EXPECT_TRUE(test, ctx->function_called);
+}
+
+static int kunit_try_catch_test_init(struct kunit *test)
+{
+   struct kunit_try_catch_test_context *ctx;
+
+   ctx = kunit_kzalloc(test, sizeof(*ctx), GFP_KERNEL);
+   if (!ctx)
+   return -ENOMEM;
+
+   test->priv = ctx;
+
+   ctx->try_catch = kunit_kmalloc(test,
+  sizeof(*ctx->try_catch),
+  GFP_KERNEL);
+   if (!ctx->try_catch)
+   return -ENOMEM;
+
+   return 0;
+}
+
+static struct kunit_case kunit_try_catch_test_cases[] = {
+   KUNIT_CASE(kunit_test_try_catch_successful_try_no_catch),
+   KUNIT_CASE(kunit_test_try_catch_unsuccessful_try_does_catch),
+   {}
+};
+
+static struct kunit_suite kunit_try_catch_test_suite = {
+   .name = "kunit-try-catch-test",
+   .init = kunit_try_catch_test_init,
+   .test_cases = kunit_try_catch_test_cases,
+};
+kunit_test_suite(kunit_try_catch_test_suite);
-- 
2.23.0.rc1.153.gdeed80330f-goog

___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

[PATCH v13 15/18] Documentation: kunit: add documentation for KUnit

[Brendan Higgins]

Add documentation for KUnit, the Linux kernel unit testing framework.
- Add intro and usage guide for KUnit
- Add API reference

Signed-off-by: Felix Guo 
Signed-off-by: Brendan Higgins 
Cc: Jonathan Corbet 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Logan Gunthorpe 
Reviewed-by: Stephen Boyd 
---
 Documentation/dev-tools/index.rst   |   1 +
 Documentation/dev-tools/kunit/api/index.rst |  16 +
 Documentation/dev-tools/kunit/api/test.rst  |  11 +
 Documentation/dev-tools/kunit/faq.rst   |  62 +++
 Documentation/dev-tools/kunit/index.rst |  79 +++
 Documentation/dev-tools/kunit/start.rst | 180 ++
 Documentation/dev-tools/kunit/usage.rst | 576 
 7 files changed, 925 insertions(+)
 create mode 100644 Documentation/dev-tools/kunit/api/index.rst
 create mode 100644 Documentation/dev-tools/kunit/api/test.rst
 create mode 100644 Documentation/dev-tools/kunit/faq.rst
 create mode 100644 Documentation/dev-tools/kunit/index.rst
 create mode 100644 Documentation/dev-tools/kunit/start.rst
 create mode 100644 Documentation/dev-tools/kunit/usage.rst

diff --git a/Documentation/dev-tools/index.rst 
b/Documentation/dev-tools/index.rst
index b0522a4dd1073..09dee10d25928 100644
--- a/Documentation/dev-tools/index.rst
+++ b/Documentation/dev-tools/index.rst
@@ -24,6 +24,7 @@ whole; patches welcome!
gdb-kernel-debugging
kgdb
kselftest
+   kunit/index
 
 
 .. only::  subproject and html
diff --git a/Documentation/dev-tools/kunit/api/index.rst 
b/Documentation/dev-tools/kunit/api/index.rst
new file mode 100644
index 0..9b9bffe5d41a0
--- /dev/null
+++ b/Documentation/dev-tools/kunit/api/index.rst
@@ -0,0 +1,16 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=
+API Reference
+=
+.. toctree::
+
+   test
+
+This section documents the KUnit kernel testing API. It is divided into the
+following sections:
+
+= 
==
+:doc:`test`   documents all of the standard testing API
+  excluding mocking or mocking related 
features.
+= 
==
diff --git a/Documentation/dev-tools/kunit/api/test.rst 
b/Documentation/dev-tools/kunit/api/test.rst
new file mode 100644
index 0..aaa97f17e5b32
--- /dev/null
+++ b/Documentation/dev-tools/kunit/api/test.rst
@@ -0,0 +1,11 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+
+Test API
+
+
+This file documents all of the standard testing API excluding mocking or 
mocking
+related features.
+
+.. kernel-doc:: include/kunit/test.h
+   :internal:
diff --git a/Documentation/dev-tools/kunit/faq.rst 
b/Documentation/dev-tools/kunit/faq.rst
new file mode 100644
index 0..bf2095112d899
--- /dev/null
+++ b/Documentation/dev-tools/kunit/faq.rst
@@ -0,0 +1,62 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+==
+Frequently Asked Questions
+==
+
+How is this different from Autotest, kselftest, etc?
+
+KUnit is a unit testing framework. Autotest, kselftest (and some others) are
+not.
+
+A `unit test `_ is supposed to
+test a single unit of code in isolation, hence the name. A unit test should be
+the finest granularity of testing and as such should allow all possible code
+paths to be tested in the code under test; this is only possible if the code
+under test is very small and does not have any external dependencies outside of
+the test's control like hardware.
+
+There are no testing frameworks currently available for the kernel that do not
+require installing the kernel on a test machine or in a VM and all require
+tests to be written in userspace and run on the kernel under test; this is true
+for Autotest, kselftest, and some others, disqualifying any of them from being
+considered unit testing frameworks.
+
+Does KUnit support running on architectures other than UML?
+===
+
+Yes, well, mostly.
+
+For the most part, the KUnit core framework (what you use to write the tests)
+can compile to any architecture; it compiles like just another part of the
+kernel and runs when the kernel boots. However, there is some infrastructure,
+like the KUnit Wrapper (``tools/testing/kunit/kunit.py``) that does not support
+other architectures.
+
+In short, this means that, yes, you can run KUnit on other architectures, but
+it might require more work than using KUnit on UML.
+
+For more information, see :ref:`kunit-on-non-uml`.
+
+What is the difference between a unit test and these other kinds of tests?
+==
+Most existing tests for the Linux kernel would be categorized as an integration
+test, or an end-to-end test.
+
+- A unit test is supposed to 

[PATCH v13 17/18] kernel/sysctl-test: Add null pointer test for sysctl.c:proc_dointvec()

[Brendan Higgins]

From: Iurii Zaikin 

KUnit tests for initialized data behavior of proc_dointvec that is
explicitly checked in the code. Includes basic parsing tests including
int min/max overflow.

Signed-off-by: Iurii Zaikin 
Signed-off-by: Brendan Higgins 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Logan Gunthorpe 
Acked-by: Luis Chamberlain 
Reviewed-by: Stephen Boyd 
---
 kernel/Makefile  |   2 +
 kernel/sysctl-test.c | 392 +++
 lib/Kconfig.debug|  11 ++
 3 files changed, 405 insertions(+)
 create mode 100644 kernel/sysctl-test.c

diff --git a/kernel/Makefile b/kernel/Makefile
index ef0d95a190b41..63e9ea6122c2c 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -113,6 +113,8 @@ obj-$(CONFIG_TORTURE_TEST) += torture.o
 obj-$(CONFIG_HAS_IOMEM) += iomem.o
 obj-$(CONFIG_RSEQ) += rseq.o
 
+obj-$(CONFIG_SYSCTL_KUNIT_TEST) += sysctl-test.o
+
 obj-$(CONFIG_GCC_PLUGIN_STACKLEAK) += stackleak.o
 KASAN_SANITIZE_stackleak.o := n
 KCOV_INSTRUMENT_stackleak.o := n
diff --git a/kernel/sysctl-test.c b/kernel/sysctl-test.c
new file mode 100644
index 0..2a63241a8453b
--- /dev/null
+++ b/kernel/sysctl-test.c
@@ -0,0 +1,392 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit test of proc sysctl.
+ */
+
+#include 
+#include 
+
+#define KUNIT_PROC_READ 0
+#define KUNIT_PROC_WRITE 1
+
+static int i_zero;
+static int i_one_hundred = 100;
+
+/*
+ * Test that proc_dointvec will not try to use a NULL .data field even when the
+ * length is non-zero.
+ */
+static void sysctl_test_api_dointvec_null_tbl_data(struct kunit *test)
+{
+   struct ctl_table null_data_table = {
+   .procname = "foo",
+   /*
+* Here we are testing that proc_dointvec behaves correctly when
+* we give it a NULL .data field. Normally this would point to a
+* piece of memory where the value would be stored.
+*/
+   .data   = NULL,
+   .maxlen = sizeof(int),
+   .mode   = 0644,
+   .proc_handler   = proc_dointvec,
+   .extra1 = _zero,
+   .extra2 = _one_hundred,
+   };
+   /*
+* proc_dointvec expects a buffer in user space, so we allocate one. We
+* also need to cast it to __user so sparse doesn't get mad.
+*/
+   void __user *buffer = (void __user *)kunit_kzalloc(test, sizeof(int),
+  GFP_USER);
+   size_t len;
+   loff_t pos;
+
+   /*
+* We don't care what the starting length is since proc_dointvec should
+* not try to read because .data is NULL.
+*/
+   len = 1234;
+   KUNIT_EXPECT_EQ(test, 0, proc_dointvec(_data_table,
+  KUNIT_PROC_READ, buffer, ,
+  ));
+   KUNIT_EXPECT_EQ(test, (size_t)0, len);
+
+   /*
+* See above.
+*/
+   len = 1234;
+   KUNIT_EXPECT_EQ(test, 0, proc_dointvec(_data_table,
+  KUNIT_PROC_WRITE, buffer, ,
+  ));
+   KUNIT_EXPECT_EQ(test, (size_t)0, len);
+}
+
+/*
+ * Similar to the previous test, we create a struct ctrl_table that has a .data
+ * field that proc_dointvec cannot do anything with; however, this time it is
+ * because we tell proc_dointvec that the size is 0.
+ */
+static void sysctl_test_api_dointvec_table_maxlen_unset(struct kunit *test)
+{
+   int data = 0;
+   struct ctl_table data_maxlen_unset_table = {
+   .procname = "foo",
+   .data   = ,
+   /*
+* So .data is no longer NULL, but we tell proc_dointvec its
+* length is 0, so it still shouldn't try to use it.
+*/
+   .maxlen = 0,
+   .mode   = 0644,
+   .proc_handler   = proc_dointvec,
+   .extra1 = _zero,
+   .extra2 = _one_hundred,
+   };
+   void __user *buffer = (void __user *)kunit_kzalloc(test, sizeof(int),
+  GFP_USER);
+   size_t len;
+   loff_t pos;
+
+   /*
+* As before, we don't care what buffer length is because proc_dointvec
+* cannot do anything because its internal .data buffer has zero length.
+*/
+   len = 1234;
+   KUNIT_EXPECT_EQ(test, 0, proc_dointvec(_maxlen_unset_table,
+  KUNIT_PROC_READ, buffer, ,
+  ));
+   KUNIT_EXPECT_EQ(test, (size_t)0, len);
+
+   /*
+* See previous comment.
+*/
+   len = 1234;
+   KUNIT_EXPECT_EQ(test, 0, proc_dointvec(_maxlen_unset_table,
+  KUNIT_PROC_WRITE, buffer, ,
+   

[PATCH v13 16/18] MAINTAINERS: add entry for KUnit the unit testing framework

[Brendan Higgins]

Add myself as maintainer of KUnit, the Linux kernel's unit testing
framework.

Signed-off-by: Brendan Higgins 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Logan Gunthorpe 
Reviewed-by: Stephen Boyd 
---
 MAINTAINERS | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index a2c343ee3b2ca..f0bd77e8a8a2f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8799,6 +8799,17 @@ S:   Maintained
 F: tools/testing/selftests/
 F: Documentation/dev-tools/kselftest*
 
+KERNEL UNIT TESTING FRAMEWORK (KUnit)
+M: Brendan Higgins 
+L: linux-kselft...@vger.kernel.org
+L: kunit-...@googlegroups.com
+W: https://google.github.io/kunit-docs/third_party/kernel/docs/
+S: Maintained
+F: Documentation/dev-tools/kunit/
+F: include/kunit/
+F: kunit/
+F: tools/testing/kunit/
+
 KERNEL USERMODE HELPER
 M: Luis Chamberlain 
 L: linux-ker...@vger.kernel.org
-- 
2.23.0.rc1.153.gdeed80330f-goog

___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

[PATCH v13 05/18] kunit: test: add the concept of expectations

[Brendan Higgins]

Add support for expectations, which allow properties to be specified and
then verified in tests.

Signed-off-by: Brendan Higgins 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Logan Gunthorpe 
Reviewed-by: Stephen Boyd 
---
 include/kunit/test.h | 834 ++-
 kunit/test.c |  62 
 2 files changed, 893 insertions(+), 3 deletions(-)

diff --git a/include/kunit/test.h b/include/kunit/test.h
index f264ffe58f008..4c41e6bde1588 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -9,6 +9,8 @@
 #ifndef _KUNIT_TEST_H
 #define _KUNIT_TEST_H
 
+#include 
+#include 
 #include 
 #include 
 
@@ -341,7 +343,7 @@ void __printf(3, 4) kunit_printk(const char *level,
  * a variable number of format parameters just like printk().
  */
 #define kunit_info(test, fmt, ...) \
-   kunit_printk(KERN_INFO, test, fmt, ##__VA_ARGS__)
+   kunit_printk(KERN_INFO, test, fmt, ##__VA_ARGS__)
 
 /**
  * kunit_warn() - Prints a WARN level message associated with the current test.
@@ -351,7 +353,7 @@ void __printf(3, 4) kunit_printk(const char *level,
  * Prints a warning level message.
  */
 #define kunit_warn(test, fmt, ...) \
-   kunit_printk(KERN_WARNING, test, fmt, ##__VA_ARGS__)
+   kunit_printk(KERN_WARNING, test, fmt, ##__VA_ARGS__)
 
 /**
  * kunit_err() - Prints an ERROR level message associated with the current 
test.
@@ -361,6 +363,832 @@ void __printf(3, 4) kunit_printk(const char *level,
  * Prints an error level message.
  */
 #define kunit_err(test, fmt, ...) \
-   kunit_printk(KERN_ERR, test, fmt, ##__VA_ARGS__)
+   kunit_printk(KERN_ERR, test, fmt, ##__VA_ARGS__)
+
+/**
+ * KUNIT_SUCCEED() - A no-op expectation. Only exists for code clarity.
+ * @test: The test context object.
+ *
+ * The opposite of KUNIT_FAIL(), it is an expectation that cannot fail. In 
other
+ * words, it does nothing and only exists for code clarity. See
+ * KUNIT_EXPECT_TRUE() for more information.
+ */
+#define KUNIT_SUCCEED(test) do {} while (0)
+
+void kunit_do_assertion(struct kunit *test,
+   struct kunit_assert *assert,
+   bool pass,
+   const char *fmt, ...);
+
+#define KUNIT_ASSERTION(test, pass, assert_class, INITIALIZER, fmt, ...) do {  
\
+   struct assert_class __assertion = INITIALIZER; \
+   kunit_do_assertion(test,   \
+  &__assertion.assert,\
+  pass,   \
+  fmt,\
+  ##__VA_ARGS__); \
+} while (0)
+
+
+#define KUNIT_FAIL_ASSERTION(test, assert_type, fmt, ...) \
+   KUNIT_ASSERTION(test,  \
+   false, \
+   kunit_fail_assert, \
+   KUNIT_INIT_FAIL_ASSERT_STRUCT(test, assert_type),  \
+   fmt,   \
+   ##__VA_ARGS__)
+
+/**
+ * KUNIT_FAIL() - Always causes a test to fail when evaluated.
+ * @test: The test context object.
+ * @fmt: an informational message to be printed when the assertion is made.
+ * @...: string format arguments.
+ *
+ * The opposite of KUNIT_SUCCEED(), it is an expectation that always fails. In
+ * other words, it always results in a failed expectation, and consequently
+ * always causes the test case to fail when evaluated. See KUNIT_EXPECT_TRUE()
+ * for more information.
+ */
+#define KUNIT_FAIL(test, fmt, ...)\
+   KUNIT_FAIL_ASSERTION(test, \
+KUNIT_EXPECTATION,\
+fmt,  \
+##__VA_ARGS__)
+
+#define KUNIT_UNARY_ASSERTION(test,   \
+ assert_type, \
+ condition,   \
+ expected_true,   \
+ fmt, \
+ ...) \
+   KUNIT_ASSERTION(test,  \
+   !!(condition) == !!expected_true,  \
+   kunit_unary_assert,\
+   KUNIT_INIT_UNARY_ASSERT_STRUCT(test,   

[PATCH v13 03/18] kunit: test: add string_stream a std::stream like string builder

[Brendan Higgins]

A number of test features need to do pretty complicated string printing
where it may not be possible to rely on a single preallocated string
with parameters.

So provide a library for constructing the string as you go similar to
C++'s std::string. string_stream is really just a string builder,
nothing more.

Signed-off-by: Brendan Higgins 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Logan Gunthorpe 
---
 include/kunit/string-stream.h |  51 
 kunit/Makefile|   3 +-
 kunit/string-stream.c | 217 ++
 3 files changed, 270 insertions(+), 1 deletion(-)
 create mode 100644 include/kunit/string-stream.h
 create mode 100644 kunit/string-stream.c

diff --git a/include/kunit/string-stream.h b/include/kunit/string-stream.h
new file mode 100644
index 0..fe98a00b75a9c
--- /dev/null
+++ b/include/kunit/string-stream.h
@@ -0,0 +1,51 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * C++ stream style string builder used in KUnit for building messages.
+ *
+ * Copyright (C) 2019, Google LLC.
+ * Author: Brendan Higgins 
+ */
+
+#ifndef _KUNIT_STRING_STREAM_H
+#define _KUNIT_STRING_STREAM_H
+
+#include 
+#include 
+#include 
+
+struct string_stream_fragment {
+   struct kunit *test;
+   struct list_head node;
+   char *fragment;
+};
+
+struct string_stream {
+   size_t length;
+   struct list_head fragments;
+   /* length and fragments are protected by this lock */
+   spinlock_t lock;
+   struct kunit *test;
+   gfp_t gfp;
+};
+
+struct kunit;
+
+struct string_stream *alloc_string_stream(struct kunit *test, gfp_t gfp);
+
+int __printf(2, 3) string_stream_add(struct string_stream *stream,
+const char *fmt, ...);
+
+int string_stream_vadd(struct string_stream *stream,
+  const char *fmt,
+  va_list args);
+
+char *string_stream_get_string(struct string_stream *stream);
+
+int string_stream_append(struct string_stream *stream,
+struct string_stream *other);
+
+bool string_stream_is_empty(struct string_stream *stream);
+
+int string_stream_destroy(struct string_stream *stream);
+
+#endif /* _KUNIT_STRING_STREAM_H */
diff --git a/kunit/Makefile b/kunit/Makefile
index 5efdc4dea2c08..275b565a0e81f 100644
--- a/kunit/Makefile
+++ b/kunit/Makefile
@@ -1 +1,2 @@
-obj-$(CONFIG_KUNIT) += test.o
+obj-$(CONFIG_KUNIT) += test.o \
+   string-stream.o
diff --git a/kunit/string-stream.c b/kunit/string-stream.c
new file mode 100644
index 0..e6d17aacca30d
--- /dev/null
+++ b/kunit/string-stream.c
@@ -0,0 +1,217 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * C++ stream style string builder used in KUnit for building messages.
+ *
+ * Copyright (C) 2019, Google LLC.
+ * Author: Brendan Higgins 
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+struct string_stream_fragment_alloc_context {
+   struct kunit *test;
+   int len;
+   gfp_t gfp;
+};
+
+static int string_stream_fragment_init(struct kunit_resource *res,
+  void *context)
+{
+   struct string_stream_fragment_alloc_context *ctx = context;
+   struct string_stream_fragment *frag;
+
+   frag = kunit_kzalloc(ctx->test, sizeof(*frag), ctx->gfp);
+   if (!frag)
+   return -ENOMEM;
+
+   frag->test = ctx->test;
+   frag->fragment = kunit_kmalloc(ctx->test, ctx->len, ctx->gfp);
+   if (!frag->fragment)
+   return -ENOMEM;
+
+   res->allocation = frag;
+
+   return 0;
+}
+
+static void string_stream_fragment_free(struct kunit_resource *res)
+{
+   struct string_stream_fragment *frag = res->allocation;
+
+   list_del(>node);
+   kunit_kfree(frag->test, frag->fragment);
+   kunit_kfree(frag->test, frag);
+}
+
+static struct string_stream_fragment *alloc_string_stream_fragment(
+   struct kunit *test, int len, gfp_t gfp)
+{
+   struct string_stream_fragment_alloc_context context = {
+   .test = test,
+   .len = len,
+   .gfp = gfp
+   };
+
+   return kunit_alloc_resource(test,
+   string_stream_fragment_init,
+   string_stream_fragment_free,
+   gfp,
+   );
+}
+
+static int string_stream_fragment_destroy(struct string_stream_fragment *frag)
+{
+   return kunit_resource_destroy(frag->test,
+ kunit_resource_instance_match,
+ string_stream_fragment_free,
+ frag);
+}
+
+int string_stream_vadd(struct string_stream *stream,
+  const char *fmt,
+  va_list args)
+{
+   struct string_stream_fragment *frag_container;
+   int len;
+   va_list 

[PATCH v13 07/18] kunit: test: add initial tests

[Brendan Higgins]

Add a test for string stream along with a simpler example.

Signed-off-by: Brendan Higgins 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Logan Gunthorpe 
Reviewed-by: Stephen Boyd 
---
 kunit/Kconfig  | 21 +
 kunit/Makefile |  4 ++
 kunit/example-test.c   | 88 ++
 kunit/string-stream-test.c | 52 ++
 4 files changed, 165 insertions(+)
 create mode 100644 kunit/example-test.c
 create mode 100644 kunit/string-stream-test.c

diff --git a/kunit/Kconfig b/kunit/Kconfig
index 330ae83527c23..8541ef95b65ad 100644
--- a/kunit/Kconfig
+++ b/kunit/Kconfig
@@ -14,4 +14,25 @@ config KUNIT
  architectures. For more information, please see
  Documentation/dev-tools/kunit/.
 
+config KUNIT_TEST
+   bool "KUnit test for KUnit"
+   depends on KUNIT
+   help
+ Enables the unit tests for the KUnit test framework. These tests test
+ the KUnit test framework itself; the tests are both written using
+ KUnit and test KUnit. This option should only be enabled for testing
+ purposes by developers interested in testing that KUnit works as
+ expected.
+
+config KUNIT_EXAMPLE_TEST
+   bool "Example test for KUnit"
+   depends on KUNIT
+   help
+ Enables an example unit test that illustrates some of the basic
+ features of KUnit. This test only exists to help new users understand
+ what KUnit is and how it is used. Please refer to the example test
+ itself, kunit/example-test.c, for more information. This option is
+ intended for curious hackers who would like to understand how to use
+ KUnit for kernel development.
+
 endmenu
diff --git a/kunit/Makefile b/kunit/Makefile
index 6dcbe309036b8..4e46450bcb3a8 100644
--- a/kunit/Makefile
+++ b/kunit/Makefile
@@ -1,3 +1,7 @@
 obj-$(CONFIG_KUNIT) += test.o \
string-stream.o \
assert.o
+
+obj-$(CONFIG_KUNIT_TEST) +=string-stream-test.o
+
+obj-$(CONFIG_KUNIT_EXAMPLE_TEST) +=example-test.o
diff --git a/kunit/example-test.c b/kunit/example-test.c
new file mode 100644
index 0..f64a829aa441f
--- /dev/null
+++ b/kunit/example-test.c
@@ -0,0 +1,88 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Example KUnit test to show how to use KUnit.
+ *
+ * Copyright (C) 2019, Google LLC.
+ * Author: Brendan Higgins 
+ */
+
+#include 
+
+/*
+ * This is the most fundamental element of KUnit, the test case. A test case
+ * makes a set EXPECTATIONs and ASSERTIONs about the behavior of some code; if
+ * any expectations or assertions are not met, the test fails; otherwise, the
+ * test passes.
+ *
+ * In KUnit, a test case is just a function with the signature
+ * `void (*)(struct kunit *)`. `struct kunit` is a context object that stores
+ * information about the current test.
+ */
+static void example_simple_test(struct kunit *test)
+{
+   /*
+* This is an EXPECTATION; it is how KUnit tests things. When you want
+* to test a piece of code, you set some expectations about what the
+* code should do. KUnit then runs the test and verifies that the code's
+* behavior matched what was expected.
+*/
+   KUNIT_EXPECT_EQ(test, 1 + 1, 2);
+}
+
+/*
+ * This is run once before each test case, see the comment on
+ * example_test_suite for more information.
+ */
+static int example_test_init(struct kunit *test)
+{
+   kunit_info(test, "initializing\n");
+
+   return 0;
+}
+
+/*
+ * Here we make a list of all the test cases we want to add to the test suite
+ * below.
+ */
+static struct kunit_case example_test_cases[] = {
+   /*
+* This is a helper to create a test case object from a test case
+* function; its exact function is not important to understand how to
+* use KUnit, just know that this is how you associate test cases with a
+* test suite.
+*/
+   KUNIT_CASE(example_simple_test),
+   {}
+};
+
+/*
+ * This defines a suite or grouping of tests.
+ *
+ * Test cases are defined as belonging to the suite by adding them to
+ * `kunit_cases`.
+ *
+ * Often it is desirable to run some function which will set up things which
+ * will be used by every test; this is accomplished with an `init` function
+ * which runs before each test case is invoked. Similarly, an `exit` function
+ * may be specified which runs after every test case and can be used to for
+ * cleanup. For clarity, running tests in a test suite would behave as follows:
+ *
+ * suite.init(test);
+ * suite.test_case[0](test);
+ * suite.exit(test);
+ * suite.init(test);
+ * suite.test_case[1](test);
+ * suite.exit(test);
+ * ...;
+ */
+static struct kunit_suite example_test_suite = {
+   .name = "example",
+   .init = example_test_init,
+   .test_cases = example_test_cases,
+};
+
+/*
+ * This registers the above 

[PATCH v13 09/18] kunit: test: add support for test abort

[Brendan Higgins]

Add support for aborting/bailing out of test cases, which is needed for
implementing assertions.

An assertion is like an expectation, but bails out of the test case
early if the assertion is not met. The idea with assertions is that you
use them to state all the preconditions for your test. Logically
speaking, these are the premises of the test case, so if a premise isn't
true, there is no point in continuing the test case because there are no
conclusions that can be drawn without the premises. Whereas, the
expectation is the thing you are trying to prove.

Signed-off-by: Brendan Higgins 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Logan Gunthorpe 
---
 include/kunit/test.h  |   2 +
 include/kunit/try-catch.h |  75 +
 kunit/Makefile|   3 +-
 kunit/test.c  | 137 +-
 kunit/try-catch.c | 118 
 5 files changed, 319 insertions(+), 16 deletions(-)
 create mode 100644 include/kunit/try-catch.h
 create mode 100644 kunit/try-catch.c

diff --git a/include/kunit/test.h b/include/kunit/test.h
index 4c41e6bde1588..f52c7c65ef651 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -10,6 +10,7 @@
 #define _KUNIT_TEST_H
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -167,6 +168,7 @@ struct kunit {
 
/* private: internal use only. */
const char *name; /* Read only after initialization! */
+   struct kunit_try_catch try_catch;
/*
 * success starts as true, and may only be set to false during a test
 * case; thus, it is safe to update this across multiple threads using
diff --git a/include/kunit/try-catch.h b/include/kunit/try-catch.h
new file mode 100644
index 0..404f336cbdc85
--- /dev/null
+++ b/include/kunit/try-catch.h
@@ -0,0 +1,75 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * An API to allow a function, that may fail, to be executed, and recover in a
+ * controlled manner.
+ *
+ * Copyright (C) 2019, Google LLC.
+ * Author: Brendan Higgins 
+ */
+
+#ifndef _KUNIT_TRY_CATCH_H
+#define _KUNIT_TRY_CATCH_H
+
+#include 
+
+typedef void (*kunit_try_catch_func_t)(void *);
+
+struct completion;
+struct kunit;
+
+/**
+ * struct kunit_try_catch - provides a generic way to run code which might 
fail.
+ * @test: The test case that is currently being executed.
+ * @try_completion: Completion that the control thread waits on while test 
runs.
+ * @try_result: Contains any errno obtained while running test case.
+ * @try: The function, the test case, to attempt to run.
+ * @catch: The function called if @try bails out.
+ * @context: used to pass user data to the try and catch functions.
+ *
+ * kunit_try_catch provides a generic, architecture independent way to execute
+ * an arbitrary function of type kunit_try_catch_func_t which may bail out by
+ * calling kunit_try_catch_throw(). If kunit_try_catch_throw() is called, @try
+ * is stopped at the site of invocation and @catch is called.
+ *
+ * struct kunit_try_catch provides a generic interface for the functionality
+ * needed to implement kunit->abort() which in turn is needed for implementing
+ * assertions. Assertions allow stating a precondition for a test simplifying
+ * how test cases are written and presented.
+ *
+ * Assertions are like expectations, except they abort (call
+ * kunit_try_catch_throw()) when the specified condition is not met. This is
+ * useful when you look at a test case as a logical statement about some piece
+ * of code, where assertions are the premises for the test case, and the
+ * conclusion is a set of predicates, rather expectations, that must all be
+ * true. If your premises are violated, it does not makes sense to continue.
+ */
+struct kunit_try_catch {
+   /* private: internal use only. */
+   struct kunit *test;
+   struct completion *try_completion;
+   int try_result;
+   kunit_try_catch_func_t try;
+   kunit_try_catch_func_t catch;
+   void *context;
+};
+
+void kunit_try_catch_init(struct kunit_try_catch *try_catch,
+ struct kunit *test,
+ kunit_try_catch_func_t try,
+ kunit_try_catch_func_t catch);
+
+void kunit_try_catch_run(struct kunit_try_catch *try_catch, void *context);
+
+void __noreturn kunit_try_catch_throw(struct kunit_try_catch *try_catch);
+
+static inline int kunit_try_catch_get_result(struct kunit_try_catch *try_catch)
+{
+   return try_catch->try_result;
+}
+
+/*
+ * Exposed for testing only.
+ */
+void kunit_generic_try_catch_init(struct kunit_try_catch *try_catch);
+
+#endif /* _KUNIT_TRY_CATCH_H */
diff --git a/kunit/Makefile b/kunit/Makefile
index 4e46450bcb3a8..c9176c9c578c6 100644
--- a/kunit/Makefile
+++ b/kunit/Makefile
@@ -1,6 +1,7 @@
 obj-$(CONFIG_KUNIT) += test.o \
string-stream.o \
-   assert.o
+   

[PATCH v13 01/18] kunit: test: add KUnit test runner core

[Brendan Higgins]

Add core facilities for defining unit tests; this provides a common way
to define test cases, functions that execute code which is under test
and determine whether the code under test behaves as expected; this also
provides a way to group together related test cases in test suites (here
we call them test_modules).

Just define test cases and how to execute them for now; setting
expectations on code will be defined later.

Signed-off-by: Brendan Higgins 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Logan Gunthorpe 
Reviewed-by: Luis Chamberlain 
Reviewed-by: Stephen Boyd 
---
 include/kunit/test.h | 179 
 kunit/Kconfig|  17 
 kunit/Makefile   |   1 +
 kunit/test.c | 191 +++
 4 files changed, 388 insertions(+)
 create mode 100644 include/kunit/test.h
 create mode 100644 kunit/Kconfig
 create mode 100644 kunit/Makefile
 create mode 100644 kunit/test.c

diff --git a/include/kunit/test.h b/include/kunit/test.h
new file mode 100644
index 0..e0b34acb9ee4e
--- /dev/null
+++ b/include/kunit/test.h
@@ -0,0 +1,179 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Base unit test (KUnit) API.
+ *
+ * Copyright (C) 2019, Google LLC.
+ * Author: Brendan Higgins 
+ */
+
+#ifndef _KUNIT_TEST_H
+#define _KUNIT_TEST_H
+
+#include 
+
+struct kunit;
+
+/**
+ * struct kunit_case - represents an individual test case.
+ * @run_case: the function representing the actual test case.
+ * @name: the name of the test case.
+ *
+ * A test case is a function with the signature, ``void (*)(struct kunit *)``
+ * that makes expectations (see KUNIT_EXPECT_TRUE()) about code under test. 
Each
+ * test case is associated with a  kunit_suite and will be run after the
+ * suite's init function and followed by the suite's exit function.
+ *
+ * A test case should be static and should only be created with the 
KUNIT_CASE()
+ * macro; additionally, every array of test cases should be terminated with an
+ * empty test case.
+ *
+ * Example:
+ *
+ * .. code-block:: c
+ *
+ * void add_test_basic(struct kunit *test)
+ * {
+ * KUNIT_EXPECT_EQ(test, 1, add(1, 0));
+ * KUNIT_EXPECT_EQ(test, 2, add(1, 1));
+ * KUNIT_EXPECT_EQ(test, 0, add(-1, 1));
+ * KUNIT_EXPECT_EQ(test, INT_MAX, add(0, INT_MAX));
+ * KUNIT_EXPECT_EQ(test, -1, add(INT_MAX, INT_MIN));
+ * }
+ *
+ * static struct kunit_case example_test_cases[] = {
+ * KUNIT_CASE(add_test_basic),
+ * {}
+ * };
+ *
+ */
+struct kunit_case {
+   void (*run_case)(struct kunit *test);
+   const char *name;
+
+   /* private: internal use only. */
+   bool success;
+};
+
+/**
+ * KUNIT_CASE - A helper for creating a  kunit_case
+ * @test_name: a reference to a test case function.
+ *
+ * Takes a symbol for a function representing a test case and creates a
+ *  kunit_case object from it. See the documentation for
+ *  kunit_case for an example on how to use it.
+ */
+#define KUNIT_CASE(test_name) { .run_case = test_name, .name = #test_name }
+
+/**
+ * struct kunit_suite - describes a related collection of  kunit_case s.
+ * @name: the name of the test. Purely informational.
+ * @init: called before every test case.
+ * @exit: called after every test case.
+ * @test_cases: a null terminated array of test cases.
+ *
+ * A kunit_suite is a collection of related  kunit_case s, such that
+ * @init is called before every test case and @exit is called after every test
+ * case, similar to the notion of a *test fixture* or a *test class* in other
+ * unit testing frameworks like JUnit or Googletest.
+ *
+ * Every  kunit_case must be associated with a kunit_suite for KUnit to
+ * run it.
+ */
+struct kunit_suite {
+   const char name[256];
+   int (*init)(struct kunit *test);
+   void (*exit)(struct kunit *test);
+   struct kunit_case *test_cases;
+};
+
+/**
+ * struct kunit - represents a running instance of a test.
+ * @priv: for user to store arbitrary data. Commonly used to pass data created
+ * in the init function (see  kunit_suite).
+ *
+ * Used to store information about the current context under which the test is
+ * running. Most of this data is private and should only be accessed indirectly
+ * via public functions; the one exception is @priv which can be used by the
+ * test writer to store arbitrary data.
+ */
+struct kunit {
+   void *priv;
+
+   /* private: internal use only. */
+   const char *name; /* Read only after initialization! */
+   /*
+* success starts as true, and may only be set to false during a test
+* case; thus, it is safe to update this across multiple threads using
+* WRITE_ONCE; however, as a consequence, it may only be read after the
+* test case finishes once all threads associated with the test case
+* have terminated.
+*/
+   bool success; /* Read only after 

[PATCH v13 08/18] objtool: add kunit_try_catch_throw to the noreturn list

[Brendan Higgins]

Fix the following warning seen on GCC 7.3:
  kunit/test-test.o: warning: objtool: kunit_test_unsuccessful_try() falls 
through to next function kunit_test_catch()

kunit_try_catch_throw is a function added in the following patch in this
series; it allows KUnit, a unit testing framework for the kernel, to
bail out of a broken test. As a consequence, it is a new __noreturn
function that objtool thinks is broken (as seen above). So fix this
warning by adding kunit_try_catch_throw to objtool's noreturn list.

Reported-by: kbuild test robot 
Signed-off-by: Brendan Higgins 
Acked-by: Josh Poimboeuf 
Link: https://www.spinics.net/lists/linux-kbuild/msg21708.html
Cc: Peter Zijlstra 
---
 tools/objtool/check.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 176f2f0840609..0c8e17f946cda 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -145,6 +145,7 @@ static bool __dead_end_function(struct objtool_file *file, 
struct symbol *func,
"usercopy_abort",
"machine_real_restart",
"rewind_stack_do_exit",
+   "kunit_try_catch_throw",
};
 
if (!func)
-- 
2.23.0.rc1.153.gdeed80330f-goog

___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

[PATCH v13 00/18] kunit: introduce KUnit, the Linux kernel unit testing framework

[Brendan Higgins]

## TL;DR

This revision addresses comments from Stephen and Bjorn Helgaas. Most
changes are pretty minor stuff that doesn't affect the API in anyway.
One significant change, however, is that I added support for freeing
kunit_resource managed resources before the test case is finished via
kunit_resource_destroy(). Additionally, Bjorn pointed out that I broke
KUnit on certain configurations (like the default one for x86, whoops).

Based on Stephen's feedback on the previous change, I think we are
pretty close. I am not expecting any significant changes from here on
out.

## Background

This patch set proposes KUnit, a lightweight unit testing and mocking
framework for the Linux kernel.

Unlike Autotest and kselftest, KUnit is a true unit testing framework;
it does not require installing the kernel on a test machine or in a VM
(however, KUnit still allows you to run tests on test machines or in VMs
if you want[1]) and does not require tests to be written in userspace
running on a host kernel. Additionally, KUnit is fast: From invocation
to completion KUnit can run several dozen tests in about a second.
Currently, the entire KUnit test suite for KUnit runs in under a second
from the initial invocation (build time excluded).

KUnit is heavily inspired by JUnit, Python's unittest.mock, and
Googletest/Googlemock for C++. KUnit provides facilities for defining
unit test cases, grouping related test cases into test suites, providing
common infrastructure for running tests, mocking, spying, and much more.

### What's so special about unit testing?

A unit test is supposed to test a single unit of code in isolation,
hence the name. There should be no dependencies outside the control of
the test; this means no external dependencies, which makes tests orders
of magnitudes faster. Likewise, since there are no external dependencies,
there are no hoops to jump through to run the tests. Additionally, this
makes unit tests deterministic: a failing unit test always indicates a
problem. Finally, because unit tests necessarily have finer granularity,
they are able to test all code paths easily solving the classic problem
of difficulty in exercising error handling code.

### Is KUnit trying to replace other testing frameworks for the kernel?

No. Most existing tests for the Linux kernel are end-to-end tests, which
have their place. A well tested system has lots of unit tests, a
reasonable number of integration tests, and some end-to-end tests. KUnit
is just trying to address the unit test space which is currently not
being addressed.

### More information on KUnit

There is a bunch of documentation near the end of this patch set that
describes how to use KUnit and best practices for writing unit tests.
For convenience I am hosting the compiled docs here[2].

Additionally for convenience, I have applied these patches to a
branch[3]. The repo may be cloned with:
git clone https://kunit.googlesource.com/linux
This patchset is on the kunit/rfc/v5.3/v13 branch.

## Changes Since Last Version

- Added support for freeing kunit_resources (KUnit managed resources)
  via kunit_resource_destroy() as suggested by Stephen.
- Promoted WARN() after __noreturn function to BUG() in
  "[PATCH v13 09/18] kunit: test: add support for test abort" as
  suggested by Stephen.
- Dropped concept of death test since I am not actually using it yet as
  pointed out by Stephen.
- Replaced usage of warn_slowpath_fmt with WARN in kunit_do_assertion
  since warn_slowpath_fmt is not available on some build configurations,
  as pointed out by Bjorn.
- Lots of other minor changes suggested by Stephen.

[1] 
https://google.github.io/kunit-docs/third_party/kernel/docs/usage.html#kunit-on-non-uml-architectures
[2] https://google.github.io/kunit-docs/third_party/kernel/docs/
[3] https://kunit.googlesource.com/linux/+/kunit/rfc/v5.3/v13

-- 
2.23.0.rc1.153.gdeed80330f-goog

___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

[PATCH v13 04/18] kunit: test: add assertion printing library

[Brendan Higgins]

Add `struct kunit_assert` and friends which provide a structured way to
capture data from an expectation or an assertion (introduced later in
the series) so that it may be printed out in the event of a failure.

Signed-off-by: Brendan Higgins 
Reviewed-by: Stephen Boyd 
---
 include/kunit/assert.h | 356 +
 kunit/Makefile |   3 +-
 kunit/assert.c | 141 
 3 files changed, 499 insertions(+), 1 deletion(-)
 create mode 100644 include/kunit/assert.h
 create mode 100644 kunit/assert.c

diff --git a/include/kunit/assert.h b/include/kunit/assert.h
new file mode 100644
index 0..db6a0fca09b49
--- /dev/null
+++ b/include/kunit/assert.h
@@ -0,0 +1,356 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Assertion and expectation serialization API.
+ *
+ * Copyright (C) 2019, Google LLC.
+ * Author: Brendan Higgins 
+ */
+
+#ifndef _KUNIT_ASSERT_H
+#define _KUNIT_ASSERT_H
+
+#include 
+#include 
+
+struct kunit;
+
+/**
+ * enum kunit_assert_type - Type of expectation/assertion.
+ * @KUNIT_ASSERTION: Used to denote that a kunit_assert represents an 
assertion.
+ * @KUNIT_EXPECTATION: Denotes that a kunit_assert represents an expectation.
+ *
+ * Used in conjunction with a  kunit_assert to denote whether it
+ * represents an expectation or an assertion.
+ */
+enum kunit_assert_type {
+   KUNIT_ASSERTION,
+   KUNIT_EXPECTATION,
+};
+
+/**
+ * struct kunit_assert - Data for printing a failed assertion or expectation.
+ * @test: the test case this expectation/assertion is associated with.
+ * @type: the type (either an expectation or an assertion) of this 
kunit_assert.
+ * @line: the source code line number that the expectation/assertion is at.
+ * @file: the file path of the source file that the expectation/assertion is 
in.
+ * @message: an optional message to provide additional context.
+ * @format: a function which formats the data in this kunit_assert to a string.
+ *
+ * Represents a failed expectation/assertion. Contains all the data necessary 
to
+ * format a string to a user reporting the failure.
+ */
+struct kunit_assert {
+   struct kunit *test;
+   enum kunit_assert_type type;
+   int line;
+   const char *file;
+   struct va_format message;
+   void (*format)(const struct kunit_assert *assert,
+  struct string_stream *stream);
+};
+
+/**
+ * KUNIT_INIT_VA_FMT_NULL - Default initializer for struct va_format.
+ *
+ * Used inside a struct initialization block to initialize struct va_format to
+ * default values where fmt and va are null.
+ */
+#define KUNIT_INIT_VA_FMT_NULL { .fmt = NULL, .va = NULL }
+
+/**
+ * KUNIT_INIT_ASSERT_STRUCT() - Initializer for a  kunit_assert.
+ * @kunit: The test case that this expectation/assertion is associated with.
+ * @assert_type: The type (assertion or expectation) of this kunit_assert.
+ * @fmt: The formatting function which builds a string out of this 
kunit_assert.
+ *
+ * The base initializer for a  kunit_assert.
+ */
+#define KUNIT_INIT_ASSERT_STRUCT(kunit, assert_type, fmt) {   \
+   .test = kunit, \
+   .type = assert_type,   \
+   .file = __FILE__,  \
+   .line = __LINE__,  \
+   .message = KUNIT_INIT_VA_FMT_NULL, \
+   .format = fmt  \
+}
+
+void kunit_base_assert_format(const struct kunit_assert *assert,
+ struct string_stream *stream);
+
+void kunit_assert_print_msg(const struct kunit_assert *assert,
+   struct string_stream *stream);
+
+/**
+ * struct kunit_fail_assert - Represents a plain fail expectation/assertion.
+ * @assert: The parent of this type.
+ *
+ * Represents a simple KUNIT_FAIL/KUNIT_ASSERT_FAILURE that always fails.
+ */
+struct kunit_fail_assert {
+   struct kunit_assert assert;
+};
+
+void kunit_fail_assert_format(const struct kunit_assert *assert,
+ struct string_stream *stream);
+
+/**
+ * KUNIT_INIT_FAIL_ASSERT_STRUCT() - Initializer for  kunit_fail_assert.
+ * @test: The test case that this expectation/assertion is associated with.
+ * @type: The type (assertion or expectation) of this kunit_assert.
+ *
+ * Initializes a  kunit_fail_assert. Intended to be used in
+ * KUNIT_EXPECT_* and KUNIT_ASSERT_* macros.
+ */
+#define KUNIT_INIT_FAIL_ASSERT_STRUCT(test, type) {   \
+   .assert = KUNIT_INIT_ASSERT_STRUCT(test,   \
+  type,   \
+  kunit_fail_assert_format)   \
+}
+
+/**
+ * struct kunit_unary_assert - Represents a 

[PATCH v13 06/18] kbuild: enable building KUnit

[Brendan Higgins]

KUnit is a new unit testing framework for the kernel and when used is
built into the kernel as a part of it. Add KUnit to the root Kconfig and
Makefile to allow it to be actually built.

Signed-off-by: Brendan Higgins 
Acked-by: Masahiro Yamada 
Cc: Michal Marek 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Logan Gunthorpe 
Reviewed-by: Stephen Boyd 
---
 Kconfig  | 2 ++
 Makefile | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/Kconfig b/Kconfig
index e10b3ee084d4d..47886dbd6c2a6 100644
--- a/Kconfig
+++ b/Kconfig
@@ -32,3 +32,5 @@ source "lib/Kconfig"
 source "lib/Kconfig.debug"
 
 source "Documentation/Kconfig"
+
+source "kunit/Kconfig"
diff --git a/Makefile b/Makefile
index 23cdf1f413646..3795d0a5d0376 100644
--- a/Makefile
+++ b/Makefile
@@ -1005,6 +1005,8 @@ PHONY += prepare0
 ifeq ($(KBUILD_EXTMOD),)
 core-y += kernel/ certs/ mm/ fs/ ipc/ security/ crypto/ block/
 
+core-$(CONFIG_KUNIT) += kunit/
+
 vmlinux-dirs   := $(patsubst %/,%,$(filter %/, $(init-y) $(init-m) \
 $(core-y) $(core-m) $(drivers-y) $(drivers-m) \
 $(net-y) $(net-m) $(libs-y) $(libs-m) $(virt-y)))
-- 
2.23.0.rc1.153.gdeed80330f-goog

___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

[PATCH v13 02/18] kunit: test: add test resource management API

[Brendan Higgins]

Create a common API for test managed resources like memory and test
objects. A lot of times a test will want to set up infrastructure to be
used in test cases; this could be anything from just wanting to allocate
some memory to setting up a driver stack; this defines facilities for
creating "test resources" which are managed by the test infrastructure
and are automatically cleaned up at the conclusion of the test.

Signed-off-by: Brendan Higgins 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Logan Gunthorpe 
Reviewed-by: Stephen Boyd 
---
 include/kunit/test.h | 187 +++
 kunit/test.c | 163 +
 2 files changed, 350 insertions(+)

diff --git a/include/kunit/test.h b/include/kunit/test.h
index e0b34acb9ee4e..f264ffe58f008 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -9,8 +9,72 @@
 #ifndef _KUNIT_TEST_H
 #define _KUNIT_TEST_H
 
+#include 
 #include 
 
+struct kunit_resource;
+
+typedef int (*kunit_resource_init_t)(struct kunit_resource *, void *);
+typedef void (*kunit_resource_free_t)(struct kunit_resource *);
+
+/**
+ * struct kunit_resource - represents a *test managed resource*
+ * @allocation: for the user to store arbitrary data.
+ * @free: a user supplied function to free the resource. Populated by
+ * kunit_alloc_resource().
+ *
+ * Represents a *test managed resource*, a resource which will automatically be
+ * cleaned up at the end of a test case.
+ *
+ * Example:
+ *
+ * .. code-block:: c
+ *
+ * struct kunit_kmalloc_params {
+ * size_t size;
+ * gfp_t gfp;
+ * };
+ *
+ * static int kunit_kmalloc_init(struct kunit_resource *res, void *context)
+ * {
+ * struct kunit_kmalloc_params *params = context;
+ * res->allocation = kmalloc(params->size, params->gfp);
+ *
+ * if (!res->allocation)
+ * return -ENOMEM;
+ *
+ * return 0;
+ * }
+ *
+ * static void kunit_kmalloc_free(struct kunit_resource *res)
+ * {
+ * kfree(res->allocation);
+ * }
+ *
+ * void *kunit_kmalloc(struct kunit *test, size_t size, gfp_t gfp)
+ * {
+ * struct kunit_kmalloc_params params;
+ * struct kunit_resource *res;
+ *
+ * params.size = size;
+ * params.gfp = gfp;
+ *
+ * res = kunit_alloc_resource(test, kunit_kmalloc_init,
+ * kunit_kmalloc_free, );
+ * if (res)
+ * return res->allocation;
+ *
+ * return NULL;
+ * }
+ */
+struct kunit_resource {
+   void *allocation;
+   kunit_resource_free_t free;
+
+   /* private: internal use only. */
+   struct list_head node;
+};
+
 struct kunit;
 
 /**
@@ -109,6 +173,13 @@ struct kunit {
 * have terminated.
 */
bool success; /* Read only after test_case finishes! */
+   spinlock_t lock; /* Guards all mutable test state. */
+   /*
+* Because resources is a list that may be updated multiple times (with
+* new resources) from any thread associated with a test case, we must
+* protect it with some type of lock.
+*/
+   struct list_head resources; /* Protected by lock. */
 };
 
 void kunit_init_test(struct kunit *test, const char *name);
@@ -141,6 +212,122 @@ int kunit_run_tests(struct kunit_suite *suite);
}  \
late_initcall(kunit_suite_init##suite)
 
+/*
+ * Like kunit_alloc_resource() below, but returns the struct kunit_resource
+ * object that contains the allocation. This is mostly for testing purposes.
+ */
+struct kunit_resource *kunit_alloc_and_get_resource(struct kunit *test,
+   kunit_resource_init_t init,
+   kunit_resource_free_t free,
+   gfp_t internal_gfp,
+   void *context);
+
+/**
+ * kunit_alloc_resource() - Allocates a *test managed resource*.
+ * @test: The test context object.
+ * @init: a user supplied function to initialize the resource.
+ * @free: a user supplied function to free the resource.
+ * @internal_gfp: gfp to use for internal allocations, if unsure, use 
GFP_KERNEL
+ * @context: for the user to pass in arbitrary data to the init function.
+ *
+ * Allocates a *test managed resource*, a resource which will automatically be
+ * cleaned up at the end of a test case. See  kunit_resource for an
+ * example.
+ *
+ * NOTE: KUnit needs to allocate memory for each kunit_resource object. You 
must
+ * specify an @internal_gfp that is compatible with the use context of your
+ * resource.
+ */
+static inline void *kunit_alloc_resource(struct kunit *test,
+kunit_resource_init_t init,
+

[PATCH v13 11/18] kunit: test: add the concept of assertions

[Brendan Higgins]

Add support for assertions which are like expectations except the test
terminates if the assertion is not satisfied.

The idea with assertions is that you use them to state all the
preconditions for your test. Logically speaking, these are the premises
of the test case, so if a premise isn't true, there is no point in
continuing the test case because there are no conclusions that can be
drawn without the premises. Whereas, the expectation is the thing you
are trying to prove. It is not used universally in x-unit style test
frameworks, but I really like it as a convention.  You could still
express the idea of a premise using the above idiom, but I think
KUNIT_ASSERT_* states the intended idea perfectly.

Signed-off-by: Brendan Higgins 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Logan Gunthorpe 
Reviewed-by: Stephen Boyd 
---
 include/kunit/test.h   | 284 -
 kunit/string-stream-test.c |   2 +-
 kunit/test-test.c  |   7 +-
 3 files changed, 284 insertions(+), 9 deletions(-)

diff --git a/include/kunit/test.h b/include/kunit/test.h
index f52c7c65ef651..d521d7e5be6ee 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -86,9 +86,10 @@ struct kunit;
  * @name: the name of the test case.
  *
  * A test case is a function with the signature, ``void (*)(struct kunit *)``
- * that makes expectations (see KUNIT_EXPECT_TRUE()) about code under test. 
Each
- * test case is associated with a  kunit_suite and will be run after the
- * suite's init function and followed by the suite's exit function.
+ * that makes expectations and assertions (see KUNIT_EXPECT_TRUE() and
+ * KUNIT_ASSERT_TRUE()) about code under test. Each test case is associated 
with
+ * a  kunit_suite and will be run after the suite's init function and
+ * followed by the suite's exit function.
  *
  * A test case should be static and should only be created with the 
KUNIT_CASE()
  * macro; additionally, every array of test cases should be terminated with an
@@ -1193,4 +1194,281 @@ do {
   \
fmt,   \
##__VA_ARGS__)
 
+#define KUNIT_ASSERT_FAILURE(test, fmt, ...) \
+   KUNIT_FAIL_ASSERTION(test, KUNIT_ASSERTION, fmt, ##__VA_ARGS__)
+
+/**
+ * KUNIT_ASSERT_TRUE() - Sets an assertion that @condition is true.
+ * @test: The test context object.
+ * @condition: an arbitrary boolean expression. The test fails and aborts when
+ * this does not evaluate to true.
+ *
+ * This and assertions of the form `KUNIT_ASSERT_*` will cause the test case to
+ * fail *and immediately abort* when the specified condition is not met. Unlike
+ * an expectation failure, it will prevent the test case from continuing to 
run;
+ * this is otherwise known as an *assertion failure*.
+ */
+#define KUNIT_ASSERT_TRUE(test, condition) \
+   KUNIT_TRUE_ASSERTION(test, KUNIT_ASSERTION, condition)
+
+#define KUNIT_ASSERT_TRUE_MSG(test, condition, fmt, ...)  \
+   KUNIT_TRUE_MSG_ASSERTION(test, \
+KUNIT_ASSERTION,  \
+condition,\
+fmt,  \
+##__VA_ARGS__)
+
+/**
+ * KUNIT_ASSERT_FALSE() - Sets an assertion that @condition is false.
+ * @test: The test context object.
+ * @condition: an arbitrary boolean expression.
+ *
+ * Sets an assertion that the value that @condition evaluates to is false. This
+ * is the same as KUNIT_EXPECT_FALSE(), except it causes an assertion failure
+ * (see KUNIT_ASSERT_TRUE()) when the assertion is not met.
+ */
+#define KUNIT_ASSERT_FALSE(test, condition) \
+   KUNIT_FALSE_ASSERTION(test, KUNIT_ASSERTION, condition)
+
+#define KUNIT_ASSERT_FALSE_MSG(test, condition, fmt, ...) \
+   KUNIT_FALSE_MSG_ASSERTION(test,\
+ KUNIT_ASSERTION, \
+ condition,   \
+ fmt, \
+ ##__VA_ARGS__)
+
+/**
+ * KUNIT_ASSERT_EQ() - Sets an assertion that @left and @right are equal.
+ * @test: The test context object.
+ * @left: an arbitrary expression that evaluates to a primitive C type.
+ * @right: an arbitrary expression that evaluates to a primitive C type.
+ *
+ * Sets an assertion that the values that @left and @right evaluate to are
+ * equal. This is the same as KUNIT_EXPECT_EQ(), except it causes an assertion
+ * failure (see KUNIT_ASSERT_TRUE()) when the assertion is not met.
+ */
+#define KUNIT_ASSERT_EQ(test, left, right) \
+   

Re: [PATCH v12 13/18] kunit: tool: add Python wrappers for running KUnit tests

[Stephen Boyd]

Quoting Brendan Higgins (2019-08-12 11:24:16)
> From: Felix Guo 
> 
> The ultimate goal is to create minimal isolated test binaries; in the
> meantime we are using UML to provide the infrastructure to run tests, so
> define an abstract way to configure and run tests that allow us to
> change the context in which tests are built without affecting the user.
> This also makes pretty and dynamic error reporting, and a lot of other
> nice features easier.
> 
> kunit_config.py:
>   - parse .config and Kconfig files.
> 
> kunit_kernel.py: provides helper functions to:
>   - configure the kernel using kunitconfig.
>   - build the kernel with the appropriate configuration.
>   - provide function to invoke the kernel and stream the output back.
> 
> kunit_parser.py: parses raw logs returned out by kunit_kernel and
> displays them in a user friendly way.
> 
> test_data/*: samples of test data for testing kunit.py, kunit_config.py,
> etc.
> 
> Signed-off-by: Felix Guo 
> Signed-off-by: Brendan Higgins 
> Reviewed-by: Greg Kroah-Hartman 
> Reviewed-by: Logan Gunthorpe 
> ---

Reviewed-by: Stephen Boyd 

I look forward to the single binary solution, but until then, we have
this.

___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH v12 10/18] kunit: test: add tests for kunit test abort

[Brendan Higgins]

On Mon, Aug 12, 2019 at 10:57 PM Stephen Boyd  wrote:
>
> Quoting Brendan Higgins (2019-08-12 22:06:04)
> > On Mon, Aug 12, 2019 at 9:24 PM Stephen Boyd  wrote:
> > >
> > > Quoting Brendan Higgins (2019-08-12 11:24:13)
> > > > +
> > > > +static int kunit_try_catch_test_init(struct kunit *test)
> > > > +{
> > > > +   struct kunit_try_catch_test_context *ctx;
> > > > +
> > > > +   ctx = kunit_kzalloc(test, sizeof(*ctx), GFP_KERNEL);
> > >
> > > Can this fail? Should return -ENOMEM in that case?
> >
> > Yes, I should do that.
>
> Looks like it's asserted to not be an error. If it's pushed into the API
> then there's nothing to do here, and you can have my reviewed-by on this
> patch.
>
> Reviewed-by: Stephen Boyd 

Cool, thanks!
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH v12 03/18] kunit: test: add string_stream a std::stream like string builder

[Brendan Higgins]

On Tue, Aug 13, 2019 at 2:04 AM Brendan Higgins
 wrote:
>
> On Mon, Aug 12, 2019 at 10:30 PM Stephen Boyd  wrote:
> >
> > Quoting Brendan Higgins (2019-08-12 22:02:59)
> > > On Mon, Aug 12, 2019 at 9:56 PM Stephen Boyd  wrote:
> > > >
> > > > Quoting Brendan Higgins (2019-08-12 17:41:05)
> > > > > On Mon, Aug 12, 2019 at 4:59 PM Stephen Boyd  wrote:
> > > > > >
> > > > > > > kunit_resource_destroy (respective equivalents to devm_kfree, and
> > > > > > > devres_destroy) and use kunit_kfree here?
> > > > > > >
> > > > > >
> > > > > > Yes, or drop the API entirely? Does anything need this 
> > > > > > functionality?
> > > > >
> > > > > Drop the kunit_resource API? I would strongly prefer not to.
> > > >
> > > > No. I mean this API, string_stream_clear(). Does anything use it?
> > >
> > > Oh, right. No.
> > >
> > > However, now that I added the kunit_resource_destroy, I thought it
> > > might be good to free the string_stream after I use it in each call to
> > > kunit_assert->format(...) in which case I will be using this logic.
> > >
> > > So I think the right thing to do is to expose string_stream_destroy so
> > > kunit_do_assert can clean up when it's done, and then demote
> > > string_stream_clear to static. Sound good?
> >
> > Ok, sure. I don't really see how clearing it explicitly when the
> > assertion prints vs. never allocating it to begin with is really any
> > different. Maybe I've missed something though.
>
> It's for the case that we *do* print something out. Once we are doing
> printing, we don't want the fragments anymore.

Oops, sorry fat fingered: s/doing/done
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH v12 12/18] kunit: test: add tests for KUnit managed resources

[Brendan Higgins]

On Mon, Aug 12, 2019 at 9:31 PM Stephen Boyd  wrote:
>
> Quoting Brendan Higgins (2019-08-12 11:24:15)
> > +
> > +static int kunit_resource_test_init(struct kunit *test)
> > +{
> > +   struct kunit_test_resource_context *ctx =
> > +   kzalloc(sizeof(*ctx), GFP_KERNEL);
> > +
> > +   if (!ctx)
> > +   return -ENOMEM;
>
> Should this use the test assertion logic to make sure that it's not
> failing allocations during init?

Yep. Will fix.

> BTW, maybe kunit allocation APIs should
> fail the test if they fail to allocate in general. Unless we're unit
> testing failure to allocate problems.

Yeah, I thought about that. I wasn't sure how people would feel about
it, and I thought it would be a pain to tease out all the issues
arising from aborting in different contexts when someone might not
expect it.

I am thinking later we can have kunit_kmalloc_or_abort variants? And
then we can punt this issue to a later time?

> > +
> > +   test->priv = ctx;
> > +
> > +   kunit_init_test(>test, "test_test_context");
> > +
> > +   return 0;
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH v12 09/18] kunit: test: add support for test abort

[Brendan Higgins]

On Mon, Aug 12, 2019 at 10:56 PM Stephen Boyd  wrote:
>
> Quoting Brendan Higgins (2019-08-12 21:57:55)
> > On Mon, Aug 12, 2019 at 9:22 PM Stephen Boyd  wrote:
> > >
> > > Quoting Brendan Higgins (2019-08-12 11:24:12)
> > > > diff --git a/include/kunit/test.h b/include/kunit/test.h
> > > > index 2625bcfeb19ac..93381f841e09f 100644
> > > > --- a/include/kunit/test.h
> > > > +++ b/include/kunit/test.h
> > > > @@ -176,6 +178,11 @@ struct kunit {
> > > >  */
> > > > bool success; /* Read only after test_case finishes! */
> > > > spinlock_t lock; /* Gaurds all mutable test state. */
> > > > +   /*
> > > > +* death_test may be both set and unset from multiple threads 
> > > > in a test
> > > > +* case.
> > > > +*/
> > > > +   bool death_test; /* Protected by lock. */
> > > > /*
> > > >  * Because resources is a list that may be updated multiple 
> > > > times (with
> > > >  * new resources) from any thread associated with a test case, 
> > > > we must
> > > > @@ -184,6 +191,13 @@ struct kunit {
> > > > struct list_head resources; /* Protected by lock. */
> > > >  };
> > > >
> > > > +static inline void kunit_set_death_test(struct kunit *test, bool 
> > > > death_test)
> > > > +{
> > > > +   spin_lock(>lock);
> > > > +   test->death_test = death_test;
> > > > +   spin_unlock(>lock);
> > > > +}
> > >
> > > These getters and setters are using spinlocks again. It doesn't make any
> > > sense. It probably needs a rework like was done for the other bool
> > > member, success.
> >
> > No, this is intentional. death_test can transition from false to true
> > and then back to false within the same test. Maybe that deserves a
> > comment?
>
> Yes. How does it transition from true to false again?

The purpose is to tell try_catch that it was expected for the test to
bail out. Given the default implementation there is no way for this to
happen aside from abort() being called, but in some implementations it
is possible to implement a fault catcher which allows a test suite to
recover from an unexpected failure.

Maybe it would be best to drop this until I add one of those
alternative implementations.

> Either way, having a spinlock around a read/write API doesn't make sense
> because it just makes sure that two writes don't overlap, but otherwise
> does nothing to keep things synchronized. For example a set to true
> after a set to false when the two calls to set true or false aren't
> synchronized means they can happen in any order. So I don't see how it
> needs a spinlock. The lock needs to be one level higher.

There shouldn't be any cases where one thread is trying to set it
while another is trying to unset it. The thing I am worried about here
is making sure all the cores see the write, and making sure no reads
or writes get reordered before it. So I guess I just want a fence. So
I take it I should probably have is a WRITE_ONCE here and READ_ONCE in
the getter?

> >
> > > > +
> > > >  void kunit_init_test(struct kunit *test, const char *name);
> > > >
> > > >  int kunit_run_tests(struct kunit_suite *suite);
> > > > diff --git a/include/kunit/try-catch.h b/include/kunit/try-catch.h
> > > > new file mode 100644
> > > > index 0..8a414a9af0b64
> > > > --- /dev/null
> > > > +++ b/include/kunit/try-catch.h
> [...]
> > > > +
> > > > +/*
> > > > + * struct kunit_try_catch - provides a generic way to run code which 
> > > > might fail.
> > > > + * @context: used to pass user data to the try and catch functions.
> > > > + *
> > > > + * kunit_try_catch provides a generic, architecture independent way to 
> > > > execute
> > > > + * an arbitrary function of type kunit_try_catch_func_t which may bail 
> > > > out by
> > > > + * calling kunit_try_catch_throw(). If kunit_try_catch_throw() is 
> > > > called, @try
> > > > + * is stopped at the site of invocation and @catch is catch is called.
> > > > + *
> > > > + * struct kunit_try_catch provides a generic interface for the 
> > > > functionality
> > > > + * needed to implement kunit->abort() which in turn is needed for 
> > > > implementing
> > > > + * assertions. Assertions allow stating a precondition for a test 
> > > > simplifying
> > > > + * how test cases are written and presented.
> > > > + *
> > > > + * Assertions are like expectations, except they abort (call
> > > > + * kunit_try_catch_throw()) when the specified condition is not met. 
> > > > This is
> > > > + * useful when you look at a test case as a logical statement about 
> > > > some piece
> > > > + * of code, where assertions are the premises for the test case, and 
> > > > the
> > > > + * conclusion is a set of predicates, rather expectations, that must 
> > > > all be
> > > > + * true. If your premises are violated, it does not makes sense to 
> > > > continue.
> > > > + */
> > > > +struct kunit_try_catch {
> > > > +   /* private: internal use only. */
> > > > +   struct kunit *test;
> > > > +  

Re: [PATCH v3] ndctl, check: Ensure mmap of BTT sections work with 64K page-sizes

[Vaibhav Jain]

Thanks for reviewing this patch Jeff.

Jeff Moyer  writes:

> Vaibhav Jain  writes:
>
>> On PPC64 which uses a 64K page-size, ndtl-check command fails on a BTT
>> namespace with following error:
>>
>> $ sudo ndctl check-namespace namespace0.0 -vv
>>   namespace0.0: namespace_check: checking namespace0.0
>>   namespace0.0: btt_discover_arenas: found 1 BTT arena
>>   namespace0.0: btt_create_mappings: mmap arena[0].info [sz = 0x1000, off = 
>> 0x1000] failed: Invalid argument
>>   error checking namespaces: Invalid argument
>>   checked 0 namespaces
>>
>> An error happens when btt_create_mappings() tries to mmap the sections
>> of the BTT device which are usually 4K offset aligned. However the
>> mmap() syscall expects the 'offset' argument to be in multiples of
>> page-size, hence it returns EINVAL causing the btt_create_mappings()
>> to error out.
>>
>> As a fix for the issue this patch proposes addition of two new
>> functions to 'check.c' namely btt_mmap/btt_unmap that can be used to
>> map/unmap parts of BTT device to ndctl process address-space. The
>> functions tweaks the requested 'offset' argument to mmap() ensuring
>> that its page-size aligned and then fix-ups the returned pointer such
>> that it points to the requested offset within mmapped region.
>>
>> With these newly introduced functions the patch updates the call-sites
>> in 'check.c' to use these functions instead of mmap/unmap syscalls.
>> Also since btt_mmap returns NULL if mmap operation fails, the
>> error check call-sites can be made against NULL instead of MAP_FAILED.
>>
>> Reported-by: Harish Sriram 
>> Signed-off-by: Vaibhav Jain 
>> ---
>> Changelog:
>> v3:
>> * Fixed a log string that was splitted across multiple lines [Vishal]
>>
>> v2:
>> * Updated patch description to include canonical email address of bug
>>   reporter. [Vishal]
>> * Improved the comment describing function btt_mmap() in 'check.c'
>>   [Vishal]
>> * Simplified the argument list for btt_mmap() to just accept bttc,
>>   length and offset. Other arguments for mmap() are derived from these
>>   args. [Vishal]
>> * Renamed 'shift' variable in btt_mmap()/unmap() to 'page_offset'
>>   [Vishal]
>> * Improved the dbg() messages logged inside
>>   btt_mmap()/unmap(). [Vishal]
>> * Return NULL from btt_mmap() in case of an error. [Vishal]
>> * Changed the all sites to btt_mmap() to perform error check against
>>   NULL return value instead of MAP_FAILED. [Vishal]
>> ---
>>  ndctl/check.c | 93 +--
>>  1 file changed, 67 insertions(+), 26 deletions(-)
>>
>> diff --git a/ndctl/check.c b/ndctl/check.c
>> index 8a7125053865..5969012eba84 100644
>> --- a/ndctl/check.c
>> +++ b/ndctl/check.c
>> @@ -907,59 +907,100 @@ static int btt_discover_arenas(struct btt_chk *bttc)
>>  return ret;
>>  }
>>  
>> -static int btt_create_mappings(struct btt_chk *bttc)
>> +/*
>> + * Wrap call to mmap(2) to work with btt device offsets that are not aligned
>> + * to system page boundary. It works by rounding down the requested offset
>> + * to sys_page_size when calling mmap(2) and then returning a fixed-up 
>> pointer
>> + * to the correct offset in the mmaped region.
>> + */
>> +static void *btt_mmap(struct btt_chk *bttc, size_t length, off_t offset)
>>  {
>> -struct arena_info *a;
>> -int mmap_flags;
>> -int i;
>> +off_t page_offset;
>> +int prot_flags;
>> +uint8_t *addr;
>>  
>>  if (!bttc->opts->repair)
>> -mmap_flags = PROT_READ;
>> +prot_flags = PROT_READ;
>>  else
>> -mmap_flags = PROT_READ|PROT_WRITE;
>> +prot_flags = PROT_READ|PROT_WRITE;
>> +
>> +/* Calculate the page_offset from the system page boundary */
>> +page_offset = offset - rounddown(offset, bttc->sys_page_size);
>> +
>> +/* Update the offset and length with the page_offset calculated above */
>> +offset -= page_offset;
>> +length += page_offset;
>
> Don't you have to ensure that the length is also a multiple of the
> system page size?
>
> -Jeff

No, as the BTT info header is 4K in size which isnt in multiple of page
size on PPC64 where PAGE_SIZE == 64K.

Also I see 'do_mmap()' in kernel always rounding up the 'length' to
PAGE_SIZE. So any unaligned value for 'length' arg will be handled by the
kernel.

Finally mmap(2) doesn't put any constraint on the 'length' argument to
mmap except it should > 0. The 'offset' arg on other hand has a
constraint which needs to be in multiple of PAGE_SIZE.

>
>> +
>> +addr = mmap(NULL, length, prot_flags, MAP_SHARED, bttc->fd, offset);
>> +
>> +/* If needed fixup the return pointer to correct offset requested */
>> +if (addr != MAP_FAILED)
>> +addr += page_offset;
>> +
>> +dbg(bttc, "addr = %p, length = %#lx, offset = %#lx, page_offset = 
>> %#lx\n",
>> +(void *) addr, length, offset, page_offset);
>> +
>> +return addr == MAP_FAILED ? NULL : addr;
>> +}
>> +
>> +static void btt_unmap(struct btt_chk 

Re: [PATCH v12 14/18] kunit: defconfig: add defconfigs for building KUnit tests

[Brendan Higgins]

On Mon, Aug 12, 2019 at 9:39 PM Stephen Boyd  wrote:
>
> Quoting Brendan Higgins (2019-08-12 11:24:17)
> > diff --git a/arch/um/configs/kunit_defconfig 
> > b/arch/um/configs/kunit_defconfig
> > new file mode 100644
> > index 0..bfe49689038f1
> > --- /dev/null
> > +++ b/arch/um/configs/kunit_defconfig
> > @@ -0,0 +1,8 @@
> > +CONFIG_OF=y
> > +CONFIG_OF_UNITTEST=y
> > +CONFIG_OF_OVERLAY=y
> > +CONFIG_I2C=y
> > +CONFIG_I2C_MUX=y
> > +CONFIG_KUNIT=y
> > +CONFIG_KUNIT_TEST=y
> > +CONFIG_KUNIT_EXAMPLE_TEST=y
> > diff --git a/tools/testing/kunit/configs/all_tests.config 
> > b/tools/testing/kunit/configs/all_tests.config
> > new file mode 100644
> > index 0..bfe49689038f1
> > --- /dev/null
> > +++ b/tools/testing/kunit/configs/all_tests.config
> > @@ -0,0 +1,8 @@
> > +CONFIG_OF=y
> > +CONFIG_OF_UNITTEST=y
> > +CONFIG_OF_OVERLAY=y
> > +CONFIG_I2C=y
> > +CONFIG_I2C_MUX=y
>
> Are these above config options necessary? I don't think they're part of
> the patch series anymore so it looks odd to enable the OF unittests and
> i2c configs.

Oh whoops, I forgot that we dropped the OF_UNITTEST from this
patchset. Will fix.

> > +CONFIG_KUNIT=y
> > +CONFIG_KUNIT_TEST=y
> > +CONFIG_KUNIT_EXAMPLE_TEST=y
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH v12 03/18] kunit: test: add string_stream a std::stream like string builder

[Brendan Higgins]

On Mon, Aug 12, 2019 at 10:30 PM Stephen Boyd  wrote:
>
> Quoting Brendan Higgins (2019-08-12 22:02:59)
> > On Mon, Aug 12, 2019 at 9:56 PM Stephen Boyd  wrote:
> > >
> > > Quoting Brendan Higgins (2019-08-12 17:41:05)
> > > > On Mon, Aug 12, 2019 at 4:59 PM Stephen Boyd  wrote:
> > > > >
> > > > > > kunit_resource_destroy (respective equivalents to devm_kfree, and
> > > > > > devres_destroy) and use kunit_kfree here?
> > > > > >
> > > > >
> > > > > Yes, or drop the API entirely? Does anything need this functionality?
> > > >
> > > > Drop the kunit_resource API? I would strongly prefer not to.
> > >
> > > No. I mean this API, string_stream_clear(). Does anything use it?
> >
> > Oh, right. No.
> >
> > However, now that I added the kunit_resource_destroy, I thought it
> > might be good to free the string_stream after I use it in each call to
> > kunit_assert->format(...) in which case I will be using this logic.
> >
> > So I think the right thing to do is to expose string_stream_destroy so
> > kunit_do_assert can clean up when it's done, and then demote
> > string_stream_clear to static. Sound good?
>
> Ok, sure. I don't really see how clearing it explicitly when the
> assertion prints vs. never allocating it to begin with is really any
> different. Maybe I've missed something though.

It's for the case that we *do* print something out. Once we are doing
printing, we don't want the fragments anymore.
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm