On Thu, May 28, 2015 at 1:51 AM, Christoph Hellwig <hch(a)lst.de> wrote:
On Wed, May 27, 2015 at 06:24:42PM -0400, Dan Williams wrote:
> Jens, please pull from...
I don't really think it's quite ready. I don't have time to sort through
the giant steaming pile for now, but there are a few things here:
If it weren't for the fact that I almost always like the new state of
the code after an hch review I'd take offense at that steaming pile
- stong NAK for the linker wrapping abuse in the test module
This capability has been our single largest generator of bug fixes and
regression prevention. Please tell me you have a non-bikeshed
argument why this test approach must die? We need more tests in tree,
not less. That said, it's at the end of the series ready to be lopped
off like a spent booster rocket if it's really a blocker.
- you must run the ioctls past the linux-api list at least
- please make sure the legacy e820 changes are properly tested. I
take of it, but not until Wednesday next week as I'm travelling right
We regularly test the pmem driver, but I'll put an e820 config through
some extra paces. In fact it's already better than the in-tree
version which fails to autoload the driver module (missing module
# grep -o memmap.* /proc/cmdline
memmap=1G!4G libnd.dyndbg rw
# cat /proc/partitions | grep pmem
259 0 1048576 pmem0
# readlink -f /sys/block/pmem0/device
Also a little nitpick on naming: why is this still nd instead of
widely used nvdimm?
It makes the identifier prefixes shorter is the bulk of the reasoning
and a hardware memory resource need not always be a "dimm". If it's
just the top-level directory I'm fine with 'nvdimm' or are you looking
for a rename throughout?
Also as very little of it is related to block drivers
and a lot is management it should be under drivers/ not drivers/block/
Fine with me.