Allow persistent data on DAX device being used as KMEM
Subsystems (5)
cover Allow persistent data on DAX device being used as KMEM
Linux supports adding a DAX driver managed memory region as system memory using the KMEM driver (from version 5.1). We would like to use a persistent addressable memory segment as system memory and simultaneously for storing some persistent data.
Motivation: It is already possible to partition an NVDIMM device for RAM and storage by creating separate regions on the device and using one of them with KMEM and another as fsdax. This patch set is a start to trying to get zero copy snapshots of processes which are using the DAX device as RAM. That requires dynamically sharing pages between process RAM and the storage within a single NVDIMM region.
To do this, we add a layer for handling the persistent data which does the following:
1. When a DAX device is added as KMEM, mark all the memory as allocated and pass it up to a module which is aware of the storage layout.
2. This module scans the memory, identifies the unused parts, and frees those memory pages.
3. Further memory from this device is allocated using the kernel memory allocation API. The memory allocation API currently allows the allocation to be limited only based on NUMA node. So this feature works only when the DAX device used as KMEM is the only memory from its NUMA node.
4. Discarding of blocks previously used for persistent data results in those blocks being freed to system memory.
As an example, we implement a simple persistence module using the above framework to provide a block device. A block device assumes all blocks are always available, but in this case we have to get the blocks through the memory allocation API, at an offset not under our control. To provide block device semantics, we maintain an array which maps the logical block number to the real physical page, if one exists. Block device Trim/Discard support is used to mark blocks as unused.
While we have the block device here as an example, a memory filesystem might be a more useful implementation. I am not sure if any of the existing in-memory filesystem structures are suited for persistence. Any suggestions for this are appreciated.
Srinivas Aji (4):
mm/memory_hotplug: Add MHP_ALLOCATE flag which treats hotplugged
memory as allocated
device-dax: Add framework for keeping persistent data in DAX KMEM
device-dax: Add a NONE type for DAX KMEM persistence
device-dax: Add a block device persistent type, BLK, for DAX KMEM
drivers/dax/Kconfig | 13 + drivers/dax/Makefile | 1 + drivers/dax/kmem.c | 312 +++++++++++++++++- drivers/dax/kmem_blk.c | 573 +++++++++++++++++++++++++++++++++ drivers/dax/kmem_persist.h | 47 +++ include/linux/memory_hotplug.h | 4 + mm/internal.h | 3 +- mm/memory_hotplug.c | 15 +- mm/page_alloc.c | 19 +- 9 files changed, 975 insertions(+), 12 deletions(-) create mode 100644 drivers/dax/kmem_blk.c create mode 100644 drivers/dax/kmem_persist.h
Signature · 1 line
2.30.2
RFC PATCH 1/4 mm/memory_hotplug: Add MHP_ALLOCATE flag which treats hotplugged memory as allocated
When memory is added by hotplug, it is treated as free by default. This patch adds a flag MHP_ALLOCATE, which can be used with the add_memory_...() memory hotplug functions to treat the new memory as allocated. This allows the memory contents to be inspected before being freed for system use. This feature is useful when the hotplugged memory is persistent, since it can be scanned for any persistent data which should be retained, rather than freed up for system use and overwritten.
--- include/linux/memory_hotplug.h | 4 ++++ mm/internal.h | 3 ++- mm/memory_hotplug.c | 15 +++++++++++++-- mm/page_alloc.c | 19 ++++++++++++------- 4 files changed, 31 insertions(+), 10 deletions(-)
include/linux/memory_hotplug.h +4 -0
@@ -113,6 +113,10 @@ typedef int __bitwise mhp_t;* implies the node id (nid). */ #define MHP_NID_IS_MGID ((__force mhp_t)BIT(2)) +/* +* Online this memory as when added, and also treat these pages as allocated. +*/ +#define MHP_ALLOCATE ((__force mhp_t)BIT(3))/* * Extended parameters for memory hotplug:
mm/internal.h +2 -1
@@ -356,7 +356,8 @@ extern void __putback_isolated_page(struct page *page, unsigned int order, int mt);extern void memblock_free_pages (struct page *page,unsigned long pfn,unsigned int order); -extern void __free_pages_core (struct page *page,unsigned int order); +extern void __free_pages_core (struct page *page,unsigned int order, +int allocate);extern void prep_compound_page (struct page *page,unsigned int order);extern void post_alloc_hook (struct page *page,unsigned int order,gfp_t gfp_flags);
mm/memory_hotplug.c +13 -2
@@ -177,6 +177,11 @@ static int __init setup_memhp_default_state(char *str) }__setup ("memhp_default_state=" , setup_memhp_default_state); +/* +* If this is set, then this memory is allocated when onlining. +*/ +static bool mhp_allocate; +void mem_hotplug_begin (void ) {cpus_read_lock ();
@@ -595,7 +600,7 @@ void generic_online_page(struct page *page, unsigned int order)* case in page freeing fast path. */ debug_pagealloc_map_pages (page,1 << order); -__free_pages_core (page, order); +__free_pages_core (page, order, mhp_allocate);totalram_pages_add (1UL << order); }EXPORT_SYMBOL_GPL (generic_online_page);
@@ -1417,10 +1422,16 @@ int __ref add_memory_resource(int nid, struct resource *res, mhp_t mhp_flags)if (mhp_flags &MHP_MERGE_RESOURCE )merge_system_ram_resource (res); +if (mhp_flags &MHP_ALLOCATE ) + mhp_allocate = true; +/* online pages if requested */ -if (mhp_default_online_type !=MMOP_OFFLINE ) +if (mhp_default_online_type !=MMOP_OFFLINE || + (mhp_flags &MHP_ALLOCATE ))walk_memory_blocks (start, size,NULL , online_memory_block); + mhp_allocate = false; +return ret; error:if (IS_ENABLED (CONFIG_ARCH_KEEP_MEMBLOCK ))
mm/page_alloc.c +12 -7
@@ -1643,12 +1643,12 @@ static void __free_pages_ok(struct page *page, unsigned int order,__count_vm_events (PGFREE ,1 << order); } -void __free_pages_core (struct page *page,unsigned int order) +void __free_pages_core (struct page *page,unsigned int order,int allocate) {unsigned int nr_pages =1 << order;struct page *p = page;unsigned int loop; - +int count = allocate ?1 :0 ;/* * When initializing the memmap, __init_single_page() sets the refcount * of all pages to 1 ("allocated"/"not free"). We have to set the
@@ -1658,13 +1658,18 @@ void __free_pages_core(struct page *page, unsigned int order)for (loop =0 ; loop < (nr_pages -1 ); loop++, p++) {prefetchw (p +1 );__ClearPageReserved (p); -set_page_count (p,0 ); +set_page_count (p, count); }__ClearPageReserved (p); -set_page_count (p,0 ); +set_page_count (p, count);atomic_long_add (nr_pages, &page_zone (page)->managed_pages); +/* +* Don't free the pages if we want them to appear allocated. +*/ +if (allocate) +return ;/* * Bypass PCP and place fresh pages right to the tail, primarily * relevant for memory onlining.
@@ -1729,7 +1734,7 @@ void __init memblock_free_pages(struct page *page, unsigned long pfn, {if (early_page_uninitialised (pfn))return ; -__free_pages_core (page, order); +__free_pages_core (page, order, false); }/*
@@ -1818,14 +1823,14 @@ static void __init deferred_free_range(unsigned long pfn,if (nr_pages == pageblock_nr_pages && (pfn & (pageblock_nr_pages -1 )) ==0 ) {set_pageblock_migratetype (page,MIGRATE_MOVABLE ); -__free_pages_core (page, pageblock_order); +__free_pages_core (page, pageblock_order, false);return ; }for (i =0 ; i < nr_pages; i++, page++, pfn++) {if ((pfn & (pageblock_nr_pages -1 )) ==0 )set_pageblock_migratetype (page,MIGRATE_MOVABLE ); -__free_pages_core (page,0 ); +__free_pages_core (page,0 , false); } }
Signature · 1 line
2.30.2
Quoted 40 lines · Srinivas Aji
Linux supports adding a DAX driver managed memory region as system memory using the KMEM driver (from version 5.1). We would like to use a persistent addressable memory segment as system memory and simultaneously for storing some persistent data.
Motivation: It is already possible to partition an NVDIMM device for RAM and storage by creating separate regions on the device and using one of them with KMEM and another as fsdax. This patch set is a start to trying to get zero copy snapshots of processes which are using the DAX device as RAM. That requires dynamically sharing pages between process RAM and the storage within a single NVDIMM region.
To do this, we add a layer for handling the persistent data which does the following:
1. When a DAX device is added as KMEM, mark all the memory as allocated and pass it up to a module which is aware of the storage layout.
2. This module scans the memory, identifies the unused parts, and frees those memory pages.
3. Further memory from this device is allocated using the kernel memory allocation API. The memory allocation API currently allows the allocation to be limited only based on NUMA node. So this feature works only when the DAX device used as KMEM is the only memory from its NUMA node.
4. Discarding of blocks previously used for persistent data results in those blocks being freed to system memory.
As an example, we implement a simple persistence module using the above framework to provide a block device. A block device assumes all blocks are always available, but in this case we have to get the blocks through the memory allocation API, at an offset not under our control. To provide block device semantics, we maintain an array which maps the logical block number to the real physical page, if one exists. Block device Trim/Discard support is used to mark blocks as unused.
Quoted 8 lines
While we have the block device here as an example, a memory filesystem might be a more useful implementation. I am not sure if any of the existing in-memory filesystem structures are suited for persistence. Any suggestions for this are appreciated.
Srinivas Aji (4):
mm/memory_hotplug: AddMHP_ALLOCATEflag which treats hotplugged
memory as allocated
Without seeing the actual patches, I am very skeptical that this is the right approach, especially regarding memory onlining/offlining.
virtio-mem achieves something similar (yet different) by hooking into generic_online_page(). From there, you can control what should actually happen with memory that is getting onlined (e.g., free them to the buddy or treat them differently).
Did you evaluate going that path?
Signature · 3 lines
Thanks, David / dhildenb
Quoted 7 lines · David Hildenbrand
Quoted 3 lines
Srinivas Aji (4):
mm/memory_hotplug: AddMHP_ALLOCATEflag which treats hotplugged
memory as allocatedWithout seeing the actual patches, I am very skeptical that this is the right approach, especially regarding memory onlining/offlining.
Quoted 6 lines
virtio-mem achieves something similar (yet different) by hooking into
generic_online_page(). From there, you can control what should actually happen with memory that is getting onlined (e.g., free them to the buddy or treat them differently).Did you evaluate going that path?
Thanks for the feedback. I had not tried this. Let me see if what I am trying here can be done using the online_page_callback mechanism.
Thanks,
Srinivas
RFC PATCH 2/4 device-dax: Add framework for keeping persistent data in DAX KMEM
In reply to Srinivas AjiDAX memory treated as system RAM will be checked for persistent data and passed to the appropriate plugin. The plugin is provided with functions for accessing pages by device offset, and also for allocating and freeing pages from the DAX device provided memory.
Add a config option CONFIG_DEV_DAX_KMEM_PERSIST which controls this feature.
The plugin framework allows multiple formats for the persistent data. The contents of the initial block determine which plugin is used.
A module parameter, persist_format_type, to the kmem module sets the plugin type which is used to format any hotplugged DAX which does not have a recognized initial block.
Note: With just this change but without futher patches which implement plugins for persistence, adding a DAX device as KMEM will only add it as fully allocated.
Limitation: Adding a DAX device as KMEM will succeed only if this memory is the only memory in its NUMA node.
--- drivers/dax/Kconfig | 13 ++ drivers/dax/kmem.c | 266 ++++++++++++++++++++++++++++++++++++- drivers/dax/kmem_persist.h | 43 ++++++ 3 files changed, 320 insertions(+), 2 deletions(-) create mode 100644 drivers/dax/kmem_persist.h
drivers/dax/Kconfig +13 -0
@@ -66,4 +66,17 @@ config DEV_DAX_KMEM Say N if unsure. +config DEV_DAX_KMEM_PERSIST + tristate "KMEM PERSIST: persistent storage together with kmem" + default DEV_DAX_KMEM + depends on DEV_DAX_KMEM + help + Support using a DAX device as system memory while also allowing + persistent data on it. This is done by treating all the + persistent data as pre-allocated when we add the DAX device + as system memory and further acquiring and releasing persistent + data through memory allocate and free. + + Say N if unsure. + endif
drivers/dax/kmem.c +264 -2
323 lines not on this page · raw message
drivers/dax/kmem_persist.h new file +43 -0
43 lines not on this page · raw message
Signature · 1 line
2.30.2
RFC PATCH 3/4 device-dax: Add a NONE type for DAX KMEM persistence
In reply to Srinivas AjiThe NONE type allows us to format DAX KMEM so that all the memory is usable as system memory. This does not allow us to store any persistent data.
--- drivers/dax/kmem.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+)
drivers/dax/kmem.c +44 -0
@@ -474,6 +474,46 @@ int kmem_persist_cleanup(struct dev_dax *dev_dax, return 0; } +/* +* A NONE type for DAX KMEM persistence which does not expose any persistent +* device or filesystem on the memory. +*/ + +static int kmem_persist_none_format (struct dev_dax *dev_dax) +{ +struct kmem_persist_superblock *super = +kmap_local_page (dax_kmem_index_to_page (0 , dev_dax)); + super->magic = kmem_persist_magic; + super->type =KMEM_PERSIST_NONE ; +kunmap_local (super); +return 0 ; +} + +static int kmem_persist_none_probe (struct dev_dax *dev_dax,void **data) +{ +unsigned long num_pages =dax_kmem_num_pages (dev_dax); +unsigned long i; + +for (i =1 ; i < num_pages; i++) +__free_page (dax_kmem_index_to_page (i, dev_dax)); + + *data =NULL ; +return 0 ; +} + +static int kmem_persist_none_cleanup (struct dev_dax *dev_dax,void *data) +{ +__free_page (dax_kmem_index_to_page (0 , dev_dax)); +return 0 ; +} + +static struct kmem_persist_ops kmem_persist_none_ops = { + .type =KMEM_PERSIST_NONE , + .format = kmem_persist_none_format, + .probe = kmem_persist_none_probe, + .cleanup = kmem_persist_none_cleanup +}; +#endif /* CONFIG_DEV_DAX_KMEM_PERSIST */ static struct dax_device_driver device_dax_kmem_driver = {
@@ -493,6 +533,10 @@ static int __init dax_kmem_init(void) rc =dax_driver_register (&device_dax_kmem_driver);if (rc)kfree_const (kmem_name); +#ifdef CONFIG_DEV_DAX_KMEM_PERSIST +if (rc ==0 ) +kmem_persist_type_register (&kmem_persist_none_ops); +#endif return rc; }
Signature · 1 line
2.30.2
RFC PATCH 4/4 device-dax: Add a block device persistent type, BLK, for DAX KMEM
In reply to Srinivas AjiWhen a DAX KMEM device is formatted as of type BLK, adding the DAX memory exposes a block device /dev/kmem<numa_node> . A filesystem can be created on this block device. Blocks which contain data are unavailable for use as system memory, but blocks which are freed up using DISCARD become free for system use.
The implementation uses an array which maps the logical block number to the real block offset in the DAX device. This allows us to keep block device semantics even though allocations can return any page.
--- drivers/dax/Makefile | 1 + drivers/dax/kmem.c | 4 +- drivers/dax/kmem_blk.c | 573 +++++++++++++++++++++++++++++++++++++ drivers/dax/kmem_persist.h | 4 + 4 files changed, 581 insertions(+), 1 deletion(-) create mode 100644 drivers/dax/kmem_blk.c
drivers/dax/Makefile +1 -0
@@ -3,6 +3,7 @@ obj-$(CONFIG_DAX) += dax.o obj-$(CONFIG_DEV_DAX) += device_dax.o obj-$(CONFIG_DEV_DAX_KMEM) += kmem.o obj-$(CONFIG_DEV_DAX_PMEM) += dax_pmem.o +obj-$(CONFIG_DEV_DAX_KMEM_PERSIST) += kmem_blk.o dax-y := super.o dax-y += bus.o
drivers/dax/kmem.c +3 -1
@@ -534,8 +534,10 @@ static int __init dax_kmem_init(void)if (rc)kfree_const (kmem_name);#ifdef CONFIG_DEV_DAX_KMEM_PERSIST -if (rc ==0 ) +if (rc ==0 ) {kmem_persist_type_register (&kmem_persist_none_ops); +kmem_persist_type_register (&kmem_persist_blk_ops); + }#endif return rc; }
drivers/dax/kmem_blk.c new file +573 -0
573 lines not on this page · raw message
drivers/dax/kmem_persist.h +4 -0
@@ -10,6 +10,7 @@ struct dev_dax;enum kmem_persist_type {KMEM_PERSIST_NONE =0 , +KMEM_PERSIST_BLK , };
@@ -40,4 +41,7 @@ unsigned long dax_kmem_num_pages(struct dev_dax *dev_dax);struct page *dax_kmem_alloc_page (struct dev_dax *dev_dax,unsigned long *page_index); +/* Defined in kmem_blk.c */ +extern struct kmem_persist_ops kmem_persist_blk_ops; +#endif
Signature · 1 line
2.30.2
kernel test robot
build botRe: [RFC PATCH 4/4] device-dax: Add a block device persistent type, BLK, for DAX KMEM
[If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in…
Hi Srinivas,
[FYI, it's a private test report for your RFC patch.]
[auto build test ERROR on linus/master]
[also build test ERROR on v5.19 next-20220802]
[If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Srinivas-Aji/Allow-persistent-data-on-DAX-device-being-used-as-KMEM/20220803-021320 base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 0805c6fb39f66e01cb0adccfae8d9e0615c70fd7 config: x86_64-randconfig-a003 (https://download.01.org/0day-ci/archive/20220804/[email protected]/config) compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 495519e5f8232d144ed26e9c18dbcbac6a5f25eb)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/1d821cfc1cd5b2a7034ca77e84b59cf808b09a4f
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Srinivas-Aji/Allow-persistent-data-on-DAX-device-being-used-as-KMEM/20220803-021320 git checkout 1d821cfc1cd5b2a7034ca77e84b59cf808b09a4f
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/dax/
If you fix the issue, kindly add following tag where applicable
All errors (new ones prefixed by >>):
>> drivers/dax/kmem_blk.c:338:21: error: use of undeclared identifier 'QUEUE_FLAG_DISCARD'
blk_queue_flag_set(QUEUE_FLAG_DISCARD, disk->queue);
^
>> drivers/dax/kmem_blk.c:479:11: error: incompatible pointer types passing 'char[47]' to parameter of type 'const struct device *' [-Werror,-Wincompatible-pointer-types]
dev_err("KMEM not formatted for blk, magic %lx type %d\n",
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:144:44: note: expanded from macro 'dev_err'
dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__)
^~~
include/linux/dev_printk.h:110:11: note: expanded from macro 'dev_printk_index_wrap'
_p_func(dev, fmt, ##__VA_ARGS__); \
^~~
include/linux/dev_printk.h:50:36: note: passing argument to parameter 'dev' here
void _dev_err(const struct device *dev, const char *fmt, ...);
^
>> drivers/dax/kmem_blk.c:480:4: error: incompatible integer to pointer conversion passing 'unsigned long' to parameter of type 'const char *' [-Wint-conversion]
super->header.magic, super->header.type);
^~~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:144:57: note: expanded from macro 'dev_err'
dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__)
^~~
include/linux/dev_printk.h:19:22: note: expanded from macro 'dev_fmt'
#define dev_fmt(fmt) fmt
^~~
include/linux/dev_printk.h:110:16: note: expanded from macro 'dev_printk_index_wrap'
_p_func(dev, fmt, ##__VA_ARGS__); \
^~~
include/linux/dev_printk.h:50:53: note: passing argument to parameter 'fmt' here
void _dev_err(const struct device *dev, const char *fmt, ...);
^3 errors generated.
vim +/QUEUE_FLAG_DISCARD +338 drivers/dax/kmem_blk.c
305
306
307
308
309
310 static int kmem_blk_disk_init(struct kmem_blk_data *data)
311 {
312 struct gendisk *disk;
313 int err;
314
315 disk = blk_alloc_disk(data->dev_dax->target_node);
316 data->disk = disk;
317
318 disk->flags = GENHD_FL_NO_PART;
319 disk->fops = &kmem_blk_fops;
320 disk->private_data = data;
321 snprintf(disk->disk_name, DISK_NAME_LEN, "kmem%d",
322 data->dev_dax->target_node);
323
324 set_capacity(disk,
325 data->super->num_index_entries << PAGE_SECTORS_SHIFT);
326
327 // TODO: Handle cases where PAGE_SIZE is too big.
328 /* Set physical and logical block size to PAGE_SIZE */
329 blk_queue_physical_block_size(disk->queue, PAGE_SIZE);
330 blk_queue_logical_block_size(disk->queue, PAGE_SIZE);
331
332 /* Tell the block layer that this is not a rotational device */
333 blk_queue_flag_set(QUEUE_FLAG_NONROT, disk->queue);
334 /* Don't use this for randomness */
335 blk_queue_flag_clear(QUEUE_FLAG_ADD_RANDOM, disk->queue);
336
337 /* Support discard */
> 338 blk_queue_flag_set(QUEUE_FLAG_DISCARD, disk->queue);
339 disk->queue->limits.discard_granularity = PAGE_SIZE;
340 blk_queue_max_discard_sectors(disk->queue, UINT_MAX);
341 /* We can handle WRITE_ZEROES as DISCARD, at units of page size */
342 blk_queue_max_write_zeroes_sectors(disk->queue, UINT_MAX);
343
344 err = add_disk(disk);
345 if (err)
346 goto out_cleanup_disk;
347
348 return 0;
349 out_cleanup_disk:
350 blk_cleanup_disk(data->disk);
351 data->disk = NULL;
352 return err;
353 }
354
355
356 static void kmem_blk_disk_cleanup(struct kmem_blk_data *data)
357 {
358 if (data->disk == NULL)
359 return;
360 del_gendisk(data->disk);
361 blk_cleanup_disk(data->disk);
362 data->disk = NULL;
363 }
364
365 /* Format device with full allocation */
366 static int kmem_blk_format(struct dev_dax *dev_dax)
367 {
368 struct kmem_blk_super *super =
369 kmap_local_page(dax_kmem_index_to_page(0, dev_dax));
370
371 unsigned long num_pages = dax_kmem_num_pages(dev_dax);
372 u64 i;
373 /*
374 * c = a / b => c is largest c s.t. c * b <= a.
375 * c = (a + b - 1) / b is smallest c s.t. c * b >= a
376 * num_index_pages is the largest number such that
377 * 1 + num_index_pages + num_index_pages * index_entries_per_page >= num_pages
378 * num_index_pages *(1 + index_entries_per_page) >= num_pages - 1
379 * num_index_pages =
380 * ((num_pages - 1) + (1 + index_entries_per_page) - 1 ) /
381 * (1 + index_entries_per_page)
382 */
383 u64 num_index_pages =
384 (num_pages + index_entries_per_page - 1) /
385 (1 + index_entries_per_page);
386 super->header.magic = kmem_persist_magic;
387 super->header.type = KMEM_PERSIST_BLK;
388 super->num_index_pages = num_index_pages;
389 super->num_index_entries = num_pages - 1 - num_index_pages;
390
391 for (i = 0; i < num_index_pages; i++) {
392 u64 *index_array =
393 kmap_local_page(dax_kmem_index_to_page(1 + i, dev_dax));
394 #if !defined(KMEM_PERSIST_BLK_FORMAT_FULL)
395 memset(index_array, 0, PAGE_SIZE);
396 #else /* KMEM_PERSIST_BLK_FORMAT_FULL */
397 u64 j;
398
399 for (j = 0; j < index_entries_per_page; j++) {
400 u64 idx =
401 1 + num_index_pages +
402 i * index_entries_per_page + j;
403
404 if (idx >= num_pages)
405 idx = 0;
406 index_array[j] = idx;
407 }
408 #endif
409 kunmap_local(index_array);
410 }
411 kunmap_local(super);
412 return 0;
413 }
414
415 /* Free unused blocks in the dax memory to system */
416 static int kmem_blk_free_unused(struct kmem_blk_data *data)
417 {
418 struct kmem_blk_super *super = data->super;
419 unsigned long num_pages = dax_kmem_num_pages(data->dev_dax);
420 u64 *alloc_bitmap;
421 unsigned long i;
422
423 /* Bitmap for tracking allocated pages. Temporary */
424 alloc_bitmap =
425 kvzalloc(sizeof(u64) * BITS_TO_U64(num_pages), GFP_KERNEL);
426 if (alloc_bitmap == NULL) {
427 dev_err(&data->dev_dax->dev,
428 "Unable to allocate bit array. Not freeing unused space.\n");
429 return -ENOMEM;
430 }
431
432 /* Free up pages unused by block storage to memory */
433 for (i = 0; i < super->num_index_entries; i++) {
434 u64 page_num = data->index_page
435 [i / index_entries_per_page]
436 [i % index_entries_per_page];
437
438 if (page_num != 0) {
439 BUG_ON(page_num < 1 + super->num_index_pages ||
440 page_num >= num_pages);
441 /* Set bit */
442 alloc_bitmap[page_num / 64] |= 1 << (page_num % 64);
443 }
444 }
445
446 for (i = 1 + super->num_index_pages; i < num_pages; i++) {
447 struct page *page;
448
449 if (!(alloc_bitmap[i / 64] & (1 << (i % 64)))) {
450 /* Bit clear. Page not used */
451 page = dax_kmem_index_to_page(i, data->dev_dax);
452 __free_page(page);
453 }
454 }
455
456 kvfree(alloc_bitmap);
457 return 0;
458 }
459
460 static int kmem_blk_probe(struct dev_dax *dev_dax, void **persist_data)
461 {
462 struct device *dev = &dev_dax->dev;
463 struct kmem_blk_super *super;
464 unsigned long i;
465 struct kmem_blk_data *data;
466 unsigned long num_pages = dax_kmem_num_pages(dev_dax);
467
468 if (num_pages == 0) {
469 dev_err(dev, "Dax device for KMEM has no pages\n");
470 *persist_data = NULL;
471 return -1;
472 }
473
474 super = kmap(dax_kmem_index_to_page(0, dev_dax));
475
476 /* Validate superblock magic and type */
477 if (super->header.magic != kmem_persist_magic ||
478 super->header.type != KMEM_PERSIST_BLK) {
> 479 dev_err("KMEM not formatted for blk, magic %lx type %d\n",
> 480 super->header.magic, super->header.type); 481 kunmap(dax_kmem_index_to_page(0, dev_dax));
482 *persist_data = NULL;
483 return -EINVAL;
484 }
485
486 /* Validate superblock index page counts */
487 if (super->num_index_entries <=
488 super->num_index_pages * index_entries_per_page &&
489 1 + super->num_index_pages + super->num_index_entries
490 == num_pages) {
491 dev_info(dev,
492 "Found kmem_blk superblock num_index_entries %llu num_index_pages %llu num_pages %lu\n",
493 super->num_index_entries,
494 super->num_index_pages, num_pages);
495 } else {
496 dev_warn(dev,
497 "Invalid kmem_blk superblock num_index_entries %llu num_index_pages %llu num_pages %lu\n",
498 super->num_index_entries,
499 super->num_index_pages, num_pages);
500 kunmap(dax_kmem_index_to_page(0, dev_dax));
501 *persist_data = NULL;
502 return -EINVAL;
503 }
504
505 data = kzalloc(struct_size(data, index_page, super->num_index_pages),
506 GFP_KERNEL);
507 if (!data) {
508 kunmap(dax_kmem_index_to_page(0, dev_dax));
509 *persist_data = NULL;
510 return -ENOMEM;
511 }
512
513 *persist_data = data;
514 data->dev_dax = dev_dax;
515 data->super = super;
516 spin_lock_init(&data->index_lock);
517
518 for (i = 0; i < super->num_index_pages; i++)
519 data->index_page[i] =
520 kmap(dax_kmem_index_to_page(i + 1, dev_dax));
521
522 kmem_blk_free_unused(data);
523
524 kmem_blk_disk_init(data);
525
526 return 0;
527 }
528
Signature · 2 lines
0-DAY CI Kernel Test Service https://01.org/lkp
Re: [RFC PATCH 4/4] device-dax: Add a block device persistent type, BLK, for DAX KMEM
In reply to Srinivas AjiQuoted 3 lines · On martedì 2 agosto 2022 20:12:07 CEST Srinivas Aji wrote:
When a DAX KMEM device is formatted as of type BLK, adding the DAX memory exposes a block device /dev/kmem<numa_node> . A filesystem can be created on this block device. Blocks which contain data are unavailable for use
as
Quoted 1 line
system memory, but blocks which are freed up using DISCARD become free
for
Quoted 6 lines
system use.
The implementation uses an array which maps the logical block number to the real block offset in the DAX device. This allows us to keep block device semantics even though allocations can return any page.
Quoted 8 lines
Signed-off-by Srinivas Aji <[email protected]>--- drivers/dax/Makefile | 1 + drivers/dax/kmem.c | 4 +- drivers/dax/kmem_blk.c | 573 +++++++++++++++++++++++++++++++++++++ drivers/dax/kmem_persist.h | 4 + 4 files changed, 581 insertions(+), 1 deletion(-) create mode 100644 drivers/dax/kmem_blk.c
From a quick look at this code I see a mix of kmap(), kmap_atomic() and kmap_local_page(). Actually it's not obvious to me why you are still using those kmap() and kmap_atomic() functions since they are being deprecated and shouldn't be used in new code.
Furthermore, there's an ongoing effort towards replacing the latter two functions with kmap_local_page(). This implies that this code, sooner or later, will be refactored to get rid of those two functions.
Please take a look at highmem.rst which was update in mainline a couple of months ago, while a second round of changes are still in Andrew Morton's "mm-unstable" branch.
There are two main problems with kmap(): (1) It comes with an overhead as mapping space is restricted and protected by a global lock for synchronization and (2) it also requires global TLB invalidation when the kmap’s pool wraps and it might block when the mapping space is fully utilized until a slot becomes available.
With kmap_local_page() the mappings are per thread, CPU local, can take page faults, and can be called from any context (including interrupts). It is faster than kmap() in kernels with HIGHMEM enabled. Furthermore, the tasks can be preempted and, when they are scheduled to run again, the kernel virtual addresses are restored and are still valid.
As said, since kmap_local_page() can be also called from atomic context, and since the code shouldn't ever rely on an implicit preempt_disable(), this function can also safely replace kmap_atomic().
I haven't looked closely at your code because I haven't the necessary domain knowledge and experience to comment on design and implementation details.
However, for what regards the mappings, it looks it can be converted to the only use of kmap_local_page and its related helpers even if it is immediately clear that there are some parts that need refactoring to not break the rules of local mapping/unmapping, especially when nesting several kmap_local_page() in loops.
Quoted 38 lines
drivers/dax/Makefile +1
-0@@ -3,6 +3,7 @@ obj-$(CONFIG_DAX) += dax.o obj-$(CONFIG_DEV_DAX) += device_dax.o obj-$(CONFIG_DEV_DAX_KMEM) += kmem.o obj-$(CONFIG_DEV_DAX_PMEM) += dax_pmem.o +obj-$(CONFIG_DEV_DAX_KMEM_PERSIST) += kmem_blk.o dax-y := super.o dax-y += bus.odrivers/dax/kmem.c +3
-1@@ -534,8 +534,10 @@ static int __init dax_kmem_init(void)if (rc)kfree_const (kmem_name);#ifdef CONFIG_DEV_DAX_KMEM_PERSIST -if (rc ==0 ) +if (rc ==0 ) {kmem_persist_type_register (&kmem_persist_none_ops); +kmem_persist_type_register (&kmem_persist_blk_ops); + }#endif return rc; }drivers/dax/kmem_blk.c new file +4
-0@@ -0,0 +1,573 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright(c) 2022 MemVerge. All rights reserved. */ +#include <linux/module.h> +#include <linux/major.h>
drivers/dax/kmem_blk.c line 14
Quoted 10 lines
drivers/dax/kmem_blk.c new file +10
-0+#include <linux/blkdev.h> +#include <linux/bio.h> +#include <linux/highmem.h> +#include <linux/pagemap.h> +#include <linux/bitops.h> +#include <linux/slab.h> +#include "dax-private.h" +#include "kmem_persist.h" + +static const unsigned int index_entries_per_page = (PAGE_SIZE /
sizeof(u64));
Quoted 29 lines
drivers/dax/kmem_blk.c new file +29
-0+ +struct kmem_blk_super { +struct kmem_persist_superblock header; +u64 num_index_pages; +u64 num_index_entries; +} __packed; + +struct kmem_blk_data { +struct dev_dax *dev_dax; +struct gendisk *disk; +spinlock_t index_lock; +struct kmem_blk_super *super; +unsigned long num_index_pages; +u64 *index_page[]; +}; + +// TODO: Make sure locking is sound for concurrent multiple IOs, +// i.e. writes and discards. + +static struct page *kmem_blk_get_page (struct kmem_blk_data *data, +sector_t sector) +{ +pgoff_t i = sector >>PAGE_SECTORS_SHIFT ; +u64 page_num; + +spin_lock (&data->index_lock); + page_num = data->index_page + [i / index_entries_per_page] + [i % index_entries_per_page];
drivers/dax/kmem_blk.c
Quoted 10 lines
drivers/dax/kmem_blk.c new file +10
-0+spin_unlock (&data->index_lock); + +if (page_num) +return dax_kmem_index_to_page (page_num, data->dev_dax); +else +return NULL ; +} + +/* +* copy_to_kmem_blk_setup must be called before copy_to_kmem_blk. It may
sleep.
drivers/dax/kmem_blk.c line 55
Quoted 2 lines
drivers/dax/kmem_blk.c new file +2
-0+*/ +static int kmem_blk_insert_page(struct kmem_blk_data *data, sector_t
sector)
drivers/dax/kmem_blk.c
Quoted 8 lines
drivers/dax/kmem_blk.c new file +8
-0+{ +pgoff_t i = sector >>PAGE_SECTORS_SHIFT ; +struct page *page; +unsigned long page_index =0 ; +u64 page_num;// TODO fixup u64 / unsigned long to use one type? +u64 *index_ptr = + &data->index_page + [i / index_entries_per_page][i %
index_entries_per_page];
Quoted 49 lines
drivers/dax/kmem_blk.c new file +48
-0+ +/* Check if block exists */ +spin_lock (&data->index_lock); + page_num = *index_ptr; +spin_unlock (&data->index_lock); +if (page_num) +return 0 ; + + page =dax_kmem_alloc_page (data->dev_dax, &page_index); +if (!page) { +dev_err (&data->dev_dax->dev,"Cannot allocate page\n" ); +return -1 ; + } + +spin_lock (&data->index_lock); +if (*index_ptr !=0 ) +__free_page (page); +else + *index_ptr = page_index; +spin_unlock (&data->index_lock); + +return 0 ; +} + +static int kmem_blk_discard (struct kmem_blk_data *data, +sector_t sector,size_t n) +{ +pgoff_t i = sector >>PAGE_SECTORS_SHIFT ; +struct page *page; +u64 page_num;// TODO fixup u64 / unsigned long to use one type? +u64 *index_ptr; + +BUG_ON (sector & ((1 <<PAGE_SECTORS_SHIFT ) -1 )); +BUG_ON (n & (PAGE_SIZE -1 )); + +while (n >0 ) { +BUG_ON (i > data->super->num_index_entries); + index_ptr = + &data->index_page + [i / index_entries_per_page] + [i % index_entries_per_page]; +spin_lock (&data->index_lock); + page_num = *index_ptr; +if (page_num) + *index_ptr =0 ; +spin_unlock (&data->index_lock); +if (page_num) { + page = dax_kmem_index_to_page(page_num, data-
dev_dax);
drivers/dax/kmem_blk.c
Quoted 10 lines
drivers/dax/kmem_blk.c new file +10
-0+__free_page (page); + } + i++; + n -=PAGE_SIZE ; + } +return 0 ; +} + +/* +* copy_to_kmem_blk_setup must be called before copy_to_kmem_blk. It may
sleep.
Quoted 2 lines
+ */
+static int copy_to_kmem_blk_setup(structkmem_blk_data*data,sector_t
sector, size_t n)
drivers/dax/kmem_blk.c
Quoted 2 lines
drivers/dax/kmem_blk.c new file +2
-0+{ + unsigned int offset = (sector & (PAGE_SECTORS-1)) <<
SECTOR_SHIFT;
Quoted 5 lines
drivers/dax/kmem_blk.c new file +5
-0+size_t copy; + + copy =min_t (size_t, n,PAGE_SIZE - offset); +if (kmem_blk_insert_page (data, sector)) +return -ENOSPC ;
drivers/dax/kmem_blk.c
Quoted 10 lines
drivers/dax/kmem_blk.c new file +10
-0+if (copy < n) { + sector += copy >>SECTOR_SHIFT ; +if (kmem_blk_insert_page (data, sector)) +return -ENOSPC ; + } +return 0 ; +} + +/* +* Copy n bytes from src to the block device starting at sector. Does
not sleep.
Quoted 2 lines
+ */
+static void copy_to_kmem_blk(structkmem_blk_data*data, const void
*src,
drivers/dax/kmem_blk.c
Quoted 5 lines
drivers/dax/kmem_blk.c new file +5
-0+ sector_t sector, size_t n) +{ +struct page *page; +void *dst; + unsigned int offset = (sector & (PAGE_SECTORS-1)) <<
SECTOR_SHIFT;
drivers/dax/kmem_blk.c line 156
Quoted 9 lines
drivers/dax/kmem_blk.c new file +9
-0+size_t copy; + + copy =min_t (size_t, n,PAGE_SIZE - offset); + page =kmem_blk_get_page (data, sector); +BUG_ON (!page); + + dst =kmap_atomic (page); +memcpy (dst + offset, src, copy); +kunmap_atomic (dst);
Can you please replace the above three lines with memcpy_to_page()?
drivers/dax/kmem_blk.c line 167
Quoted 11 lines
drivers/dax/kmem_blk.c new file +11
-0+ +if (copy < n) { + src += copy; + sector += copy >>SECTOR_SHIFT ; + copy = n - copy; + page =kmem_blk_get_page (data, sector); +BUG_ON (!page); + + dst =kmap_atomic (page); +memcpy (dst, src, copy); +kunmap_atomic (dst);
Same here, please.
drivers/dax/kmem_blk.c
Quoted 5 lines
drivers/dax/kmem_blk.c new file +5
-0+ } +} + +/* +* Copy n bytes to dst from the block device starting at sector. Does
not sleep.
drivers/dax/kmem_blk.c
Quoted 7 lines
+ */
drivers/dax/kmem_blk.c new file +6
-0+static void copy_from_kmem_blk (void *dst,struct kmem_blk_data *data, +sector_t sector,size_t n) +{ +struct page *page; +void *src; + unsigned int offset = (sector & (PAGE_SECTORS-1)) <<
SECTOR_SHIFT;
drivers/dax/kmem_blk.c line 187
Quoted 8 lines
drivers/dax/kmem_blk.c new file +8
-0+size_t copy; + + copy =min_t (size_t, n,PAGE_SIZE - offset); + page =kmem_blk_get_page (data, sector); +if (page) { + src =kmap_atomic (page); +memcpy (dst, src + offset, copy); +kunmap_atomic (src);
Again.
drivers/dax/kmem_blk.c line 199
Quoted 12 lines
drivers/dax/kmem_blk.c new file +12
-0+ }else +memset (dst,0 , copy); + +if (copy < n) { + dst += copy; + sector += copy >>SECTOR_SHIFT ; + copy = n - copy; + page =kmem_blk_get_page (data, sector); +if (page) { + src =kmap_atomic (page); +memcpy (dst, src, copy); +kunmap_atomic (src);
And again :-)
drivers/dax/kmem_blk.c
Quoted 9 lines
drivers/dax/kmem_blk.c new file +9
-0+ }else +memset (dst,0 , copy); + } +} + +/* +* Process a single bvec of a bio. +*/ +static int kmem_blk_do_bvec(struct kmem_blk_data *data, struct page
*page,
drivers/dax/kmem_blk.c
Quoted 1 line
drivers/dax/kmem_blk.c new file +1
-0+ unsigned int len, unsigned int off, unsigned
int op,
drivers/dax/kmem_blk.c line 222
Quoted 13 lines
drivers/dax/kmem_blk.c new file +13
-0+ sector_t sector) +{ +void *mem =NULL ; +int err =0 ; + +if (op ==REQ_OP_WRITE ) { + err =copy_to_kmem_blk_setup (data, sector, len); +if (err) + goto out; + } + +if (page) + mem =kmap_atomic (page);
Are you implicitly relying to preempt_disable()? I'm not following closely. However, if so, you shouldn't rely on it. Please use kmap_local_page() and, if you need this switch and the calls under atomic context use the suited API in explicit ways.
Quoted 43 lines
drivers/dax/kmem_blk.c new file +43
-0+switch (op) { +case REQ_OP_READ : +copy_from_kmem_blk (mem + off, data, sector, len); +flush_dcache_page (page); +break ; +case REQ_OP_WRITE : +flush_dcache_page (page); +copy_to_kmem_blk (data, mem + off, sector, len); +break ; +case REQ_OP_DISCARD : +BUG_ON (page); +kmem_blk_discard (data, sector, len); +break ; +default : +BUG (); +break ; + } +if (mem) +kunmap_atomic (mem); + +out: +return err; +} + +static void kmem_blk_submit_bio (struct bio *bio) +{ +struct kmem_blk_data *data = bio->bi_bdev->bd_disk->private_data; +sector_t sector = bio->bi_iter.bi_sector; +struct bio_vec bvec; +struct bvec_iter iter; + +/* +* DISCARD and WRITE_ZEROES come separately and don't work with +* bio_for_each_segment +*/ +switch (bio_op (bio)) { +case REQ_OP_DISCARD : +case REQ_OP_WRITE_ZEROES : +kmem_blk_discard (data, sector, bio->bi_iter.bi_size); +bio_endio (bio); +return ; +default : +break ;
drivers/dax/kmem_blk.c
Quoted 10 lines
drivers/dax/kmem_blk.c new file +10
-0+ } + +bio_for_each_segment (bvec, bio, iter) { +unsigned int len = bvec.bv_len; +int err; + +/* Don't support un-aligned buffer */ +WARN_ON_ONCE ((bvec.bv_offset & (SECTOR_SIZE -1 )) || + (len & (SECTOR_SIZE -1 ))); + err = kmem_blk_do_bvec(data, bvec.bv_page, len,
bvec.bv_offset,
Quoted 56 lines
drivers/dax/kmem_blk.c new file +56
-0+ bio_op(bio), sector); +if (err) { +bio_io_error (bio); +return ; + } + sector += len >>SECTOR_SHIFT ; + } + +bio_endio (bio); +} + +static int kmem_blk_rw_page (struct block_device *bdev,sector_t sector, +struct page *page,unsigned int op) +{ +struct kmem_blk_data *data = bdev->bd_disk->private_data; +int err; + +if (PageTransHuge (page)) +return -EOPNOTSUPP ; + err =kmem_blk_do_bvec (data, page,PAGE_SIZE ,0 , op, sector); +page_endio (page,op_is_write (op), err); +return err; +} + +static const struct block_device_operations kmem_blk_fops = { + .owner =THIS_MODULE , + .submit_bio = kmem_blk_submit_bio, + .rw_page = kmem_blk_rw_page, +}; + + + + + +static int kmem_blk_disk_init (struct kmem_blk_data *data) +{ +struct gendisk *disk; +int err; + + disk =blk_alloc_disk (data->dev_dax->target_node); + data->disk = disk; + + disk->flags =GENHD_FL_NO_PART ; + disk->fops = &kmem_blk_fops; + disk->private_data = data; +snprintf (disk->disk_name,DISK_NAME_LEN ,"kmem%d" , + data->dev_dax->target_node); + +set_capacity (disk, + data->super->num_index_entries <<PAGE_SECTORS_SHIFT ); + +// TODO: Handle cases where PAGE_SIZE is too big. +/* Set physical and logical block size to PAGE_SIZE */ +blk_queue_physical_block_size (disk->queue,PAGE_SIZE ); +blk_queue_logical_block_size (disk->queue,PAGE_SIZE ); +
drivers/dax/kmem_blk.c
Quoted 10 lines
drivers/dax/kmem_blk.c new file +10
-0+/* Tell the block layer that this is not a rotational device */ +blk_queue_flag_set (QUEUE_FLAG_NONROT , disk->queue); +/* Don't use this for randomness */ +blk_queue_flag_clear (QUEUE_FLAG_ADD_RANDOM , disk->queue); + +/* Support discard */ +blk_queue_flag_set (QUEUE_FLAG_DISCARD , disk->queue); + disk->queue->limits.discard_granularity =PAGE_SIZE ; +blk_queue_max_discard_sectors (disk->queue,UINT_MAX ); +/* We can handle WRITE_ZEROES as DISCARD, at units of page size
*/
Quoted 43 lines
drivers/dax/kmem_blk.c new file +36
-0+blk_queue_max_write_zeroes_sectors (disk->queue,UINT_MAX ); + + err =add_disk (disk); +if (err) + goto out_cleanup_disk; + +return 0 ; +out_cleanup_disk: +blk_cleanup_disk (data->disk); + data->disk =NULL ; +return err; +} + + +static void kmem_blk_disk_cleanup (struct kmem_blk_data *data) +{ +if (data->disk ==NULL ) +return ; +del_gendisk (data->disk); +blk_cleanup_disk (data->disk); + data->disk =NULL ; +} + +/* Format device with full allocation */ +static int kmem_blk_format (struct dev_dax *dev_dax) +{ +struct kmem_blk_super *super = +kmap_local_page (dax_kmem_index_to_page (0 , dev_dax)); + +unsigned long num_pages =dax_kmem_num_pages (dev_dax); +u64 i; +/* +* c = a / b => c is largest c s.t. c * b <= a. +* c = (a + b - 1) / b is smallest c s.t. c * b >= a +* num_index_pages is the largest number such that +* 1 + num_index_pages + num_index_pages * index_entries_per_page =
num_pagesdrivers/dax/kmem_blk.c new file +6
-0+* num_index_pages *(1 + index_entries_per_page) >= num_pages - 1 +* num_index_pages = +* ((num_pages - 1) + (1 + index_entries_per_page) - 1 ) / +* (1 + index_entries_per_page) +*/ + u64 num_index_pages =
drivers/dax/kmem_blk.c
Quoted 10 lines
drivers/dax/kmem_blk.c new file +10
-0+ (num_pages + index_entries_per_page -1 ) / + (1 + index_entries_per_page); + super->header.magic = kmem_persist_magic; + super->header.type =KMEM_PERSIST_BLK ; + super->num_index_pages = num_index_pages; + super->num_index_entries = num_pages -1 - num_index_pages; + +for (i =0 ; i < num_index_pages; i++) { +u64 *index_array = + kmap_local_page(dax_kmem_index_to_page(1 + i,
dev_dax));
Quoted 22 lines
drivers/dax/kmem_blk.c new file +22
-0+#if !defined(KMEM_PERSIST_BLK_FORMAT_FULL ) +memset (index_array,0 ,PAGE_SIZE ); +#else /* KMEM_PERSIST_BLK_FORMAT_FULL */ +u64 j; + +for (j =0 ; j < index_entries_per_page; j++) { +u64 idx = +1 + num_index_pages + + i * index_entries_per_page + j; + +if (idx >= num_pages) + idx =0 ; + index_array[j] = idx; + } +#endif +kunmap_local (index_array); + } +kunmap_local (super); +return 0 ; +} + +/* Free unused blocks in the dax memory to system */
drivers/dax/kmem_blk.c
Quoted 10 lines
drivers/dax/kmem_blk.c new file +10
-0+static int kmem_blk_free_unused (struct kmem_blk_data *data) +{ +struct kmem_blk_super *super = data->super; +unsigned long num_pages =dax_kmem_num_pages (data->dev_dax); +u64 *alloc_bitmap; +unsigned long i; + +/* Bitmap for tracking allocated pages. Temporary */ + alloc_bitmap = + kvzalloc(sizeof(u64) * BITS_TO_U64(num_pages),
GFP_KERNEL);
drivers/dax/kmem_blk.c
Quoted 3 lines
drivers/dax/kmem_blk.c new file +3
-0+if (alloc_bitmap ==NULL ) { + dev_err(&data->dev_dax->dev, + "Unable to allocate bit array. Not freeing
unused space.\n");
drivers/dax/kmem_blk.c
Quoted 11 lines
drivers/dax/kmem_blk.c new file +11
-0+return -ENOMEM ; + } + +/* Free up pages unused by block storage to memory */ +for (i =0 ; i < super->num_index_entries; i++) { +u64 page_num = data->index_page + [i / index_entries_per_page] + [i % index_entries_per_page]; + +if (page_num !=0 ) { + BUG_ON(page_num < 1 + super->num_index_pages
||
drivers/dax/kmem_blk.c
Quoted 3 lines
drivers/dax/kmem_blk.c new file +3
-0+ page_num >= num_pages); +/* Set bit */ + alloc_bitmap[page_num / 64] |= 1 << (page_num
% 64);
Quoted 23 lines
drivers/dax/kmem_blk.c new file +9
-0+ } + } + +for (i =1 + super->num_index_pages; i < num_pages; i++) { +struct page *page; + +if (!(alloc_bitmap[i /64 ] & (1 << (i %64 )))) { +/* Bit clear. Page not used */ + page = dax_kmem_index_to_page(i, data-
dev_dax);drivers/dax/kmem_blk.c new file +13
-0+__free_page (page); + } + } + +kvfree (alloc_bitmap); +return 0 ; +} + +static int kmem_blk_probe (struct dev_dax *dev_dax,void **persist_data) +{ +struct device *dev = &dev_dax->dev; +struct kmem_blk_super *super; +unsigned long i;
drivers/dax/kmem_blk.c line 474
Quoted 10 lines
drivers/dax/kmem_blk.c new file +10
-0+struct kmem_blk_data *data; +unsigned long num_pages =dax_kmem_num_pages (dev_dax); + +if (num_pages ==0 ) { +dev_err (dev,"Dax device for KMEM has no pages\n" ); + *persist_data =NULL ; +return -1 ; + } + + super =kmap (dax_kmem_index_to_page (0 , dev_dax));
This looks better suited for kmap_local_page().
drivers/dax/kmem_blk.c
Quoted 5 lines
drivers/dax/kmem_blk.c new file +5
-0+ +/* Validate superblock magic and type */ +if (super->header.magic != kmem_persist_magic || + super->header.type !=KMEM_PERSIST_BLK ) { + dev_err("KMEM not formatted for blk, magic %lx type
%d\n",
drivers/dax/kmem_blk.c
Quoted 13 lines
drivers/dax/kmem_blk.c new file +13
-0+ super->header.magic, super->header.type); +kunmap (dax_kmem_index_to_page (0 , dev_dax)); + *persist_data =NULL ; +return -EINVAL ; + } + +/* Validate superblock index page counts */ +if (super->num_index_entries <= + super->num_index_pages * index_entries_per_page && +1 + super->num_index_pages + super->num_index_entries + == num_pages) { + dev_info(dev, + "Found kmem_blk superblock num_index_entries
%llu num_index_pages %llu num_pages %lu\n",
drivers/dax/kmem_blk.c
Quoted 5 lines
drivers/dax/kmem_blk.c new file +5
-0+ super->num_index_entries, + super->num_index_pages, num_pages); + }else { + dev_warn(dev, + "Invalid kmem_blk superblock
num_index_entries %llu num_index_pages %llu num_pages %lu\n",
Quoted 14 lines
drivers/dax/kmem_blk.c new file +8
-0+ super->num_index_entries, + super->num_index_pages, num_pages); +kunmap (dax_kmem_index_to_page (0 , dev_dax)); + *persist_data =NULL ; +return -EINVAL ; + } + + data = kzalloc(struct_size(data, index_page, super-
num_index_pages),drivers/dax/kmem_blk.c new file +5
-0+ GFP_KERNEL); +if (!data) { +kunmap (dax_kmem_index_to_page (0 , dev_dax)); + *persist_data =NULL ; +return -ENOMEM ;
drivers/dax/kmem_blk.c line 520
Quoted 10 lines
drivers/dax/kmem_blk.c new file +10
-0+ } + + *persist_data = data; + data->dev_dax = dev_dax; + data->super = super; +spin_lock_init (&data->index_lock); + +for (i =0 ; i < super->num_index_pages; i++) + data->index_page[i] = +kmap (dax_kmem_index_to_page (i +1 , dev_dax));
Nesting of kmap_local_page() is performed on a stack based fashion (LIFO), therefore the kunmap_local() calls must be invoked in reverse order.
Any special problems with this?
Quoted 12 lines
drivers/dax/kmem_blk.c new file +12
-0+ +kmem_blk_free_unused (data); + +kmem_blk_disk_init (data); + +return 0 ; +} + +static int kmem_blk_cleanup (struct dev_dax *dev_dax,void *persist_data) +{ +struct kmem_blk_data *data = persist_data; +unsigned long num_pages =dax_kmem_num_pages (dev_dax);
drivers/dax/kmem_blk.c
Quoted 10 lines
drivers/dax/kmem_blk.c new file +10
-0+unsigned long i; + +if (data ==NULL ) +return -1 ; + +kmem_blk_disk_cleanup (data); + +if (data->super ==0 ) { +for (i =0 ; i < num_pages; i++) + __free_page(dax_kmem_index_to_page(i,
dev_dax));
drivers/dax/kmem_blk.c line 567
Quoted 7 lines
drivers/dax/kmem_blk.c new file +7
-0+ }else { +for (i =0 ; i < data->super->num_index_entries; i++) { +u64 page_num = data->index_page + [i / index_entries_per_page] + [i % index_entries_per_page]; +if (page_num !=0 ) { +
__free_page(dax_kmem_index_to_page(page_num,
drivers/dax/kmem_blk.c
Quoted 1 line
drivers/dax/kmem_blk.c new file +1
-0+
dev_dax));
drivers/dax/kmem_blk.c
Quoted 5 lines
drivers/dax/kmem_blk.c new file +5
-0+ } + } +for (i =0 ; i < data->super->num_index_pages; i++) { +struct page *page = + dax_kmem_index_to_page(1 + i,
dev_dax);
drivers/dax/kmem_persist.h unsigned long dax_kmem_num_pages(struct dev_dax
Quoted 31 lines
drivers/dax/kmem_blk.c new file +18
-0+ data->index_page[i] =NULL ; +kunmap (page); +__free_page (page); + } + data->super =NULL ; +kunmap (dax_kmem_index_to_page (0 , dev_dax)); +__free_page (dax_kmem_index_to_page (0 , dev_dax)); + } +kfree (data); +return 0 ; +} + +struct kmem_persist_ops kmem_persist_blk_ops = { + .type =KMEM_PERSIST_BLK , + .format = kmem_blk_format, + .probe = kmem_blk_probe, + .cleanup = kmem_blk_cleanup +};drivers/dax/kmem_persist.h +1
-0@@ -10,6 +10,7 @@ struct dev_dax;enum kmem_persist_type {KMEM_PERSIST_NONE =0 , +KMEM_PERSIST_BLK , };@@ -40,4 +41,7 @@ unsigned long dax_kmem_num_pages(struct dev_dax
*dev_dax);
Quoted 10 lines
drivers/dax/kmem_persist.h +3
-0struct page *dax_kmem_alloc_page (struct dev_dax *dev_dax,unsigned long *page_index); +/* Defined in kmem_blk.c */ +extern struct kmem_persist_ops kmem_persist_blk_ops; +#endif Signature · 1 line
2.30.2
Thanks,
Fabio
P.S.: I'm Cc'ing Ira Weiny.
kernel test robot
build botRe: [RFC PATCH 4/4] device-dax: Add a block device persistent type, BLK, for DAX KMEM
In reply to Srinivas Aji[If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in…
Hi Srinivas,
[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on linus/master]
[also build test WARNING on v5.19 next-20220803]
[If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Srinivas-Aji/Allow-persistent-data-on-DAX-device-being-used-as-KMEM/20220803-021320 base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 0805c6fb39f66e01cb0adccfae8d9e0615c70fd7 config: s390-buildonly-randconfig-r005-20220804 (https://download.01.org/0day-ci/archive/20220804/[email protected]/config) compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 26dd42705c2af0b8f6e5d6cdb32c9bd5ed9524eb)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# install s390 cross compiling tool for clang build
# apt-get install binutils-s390x-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/1d821cfc1cd5b2a7034ca77e84b59cf808b09a4f
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Srinivas-Aji/Allow-persistent-data-on-DAX-device-being-used-as-KMEM/20220803-021320 git checkout 1d821cfc1cd5b2a7034ca77e84b59cf808b09a4f
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash drivers/dax/
If you fix the issue, kindly add following tag where applicable
All warnings (new ones prefixed by >>):
drivers/dax/kmem_blk.c:338:21: error: use of undeclared identifier 'QUEUE_FLAG_DISCARD'
blk_queue_flag_set(QUEUE_FLAG_DISCARD, disk->queue);
^
>> drivers/dax/kmem_blk.c:479:3: warning: pointer/integer type mismatch in conditional expression ('unsigned long' and 'void *') [-Wconditional-type-mismatch]
dev_err("KMEM not formatted for blk, magic %lx type %d\n",
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:144:2: note: expanded from macro 'dev_err'
dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:109:3: note: expanded from macro 'dev_printk_index_wrap'
dev_printk_index_emit(level, fmt); \
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:105:2: note: expanded from macro 'dev_printk_index_emit'
printk_index_subsys_emit("%s %s: ", level, fmt)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/printk.h:431:2: note: expanded from macro 'printk_index_subsys_emit'
__printk_index_emit(fmt, level, subsys_fmt_prefix)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/printk.h:397:39: note: expanded from macro '__printk_index_emit'
.fmt = __builtin_constant_p(_fmt) ? (_fmt) : NULL, \
^ ~~~~~~ ~~~~
drivers/dax/kmem_blk.c:479:11: error: incompatible pointer types passing 'char[47]' to parameter of type 'const struct device *' [-Werror,-Wincompatible-pointer-types]
dev_err("KMEM not formatted for blk, magic %lx type %d\n",
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:144:44: note: expanded from macro 'dev_err'
dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__)
^~~
include/linux/dev_printk.h:110:11: note: expanded from macro 'dev_printk_index_wrap'
_p_func(dev, fmt, ##__VA_ARGS__); \
^~~
include/linux/dev_printk.h:50:36: note: passing argument to parameter 'dev' here
void _dev_err(const struct device *dev, const char *fmt, ...);
^
drivers/dax/kmem_blk.c:480:4: error: incompatible integer to pointer conversion passing 'unsigned long' to parameter of type 'const char *' [-Wint-conversion]
super->header.magic, super->header.type);
^~~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:144:57: note: expanded from macro 'dev_err'
dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__)
^~~
include/linux/dev_printk.h:19:22: note: expanded from macro 'dev_fmt'
#define dev_fmt(fmt) fmt
^~~
include/linux/dev_printk.h:110:16: note: expanded from macro 'dev_printk_index_wrap'
_p_func(dev, fmt, ##__VA_ARGS__); \
^~~
include/linux/dev_printk.h:50:53: note: passing argument to parameter 'fmt' here
void _dev_err(const struct device *dev, const char *fmt, ...);
^1 warning and 3 errors generated.
vim +479 drivers/dax/kmem_blk.c
459
460 static int kmem_blk_probe(struct dev_dax *dev_dax, void **persist_data)
461 {
462 struct device *dev = &dev_dax->dev;
463 struct kmem_blk_super *super;
464 unsigned long i;
465 struct kmem_blk_data *data;
466 unsigned long num_pages = dax_kmem_num_pages(dev_dax);
467
468 if (num_pages == 0) {
469 dev_err(dev, "Dax device for KMEM has no pages\n");
470 *persist_data = NULL;
471 return -1;
472 }
473
474 super = kmap(dax_kmem_index_to_page(0, dev_dax));
475
476 /* Validate superblock magic and type */
477 if (super->header.magic != kmem_persist_magic ||
478 super->header.type != KMEM_PERSIST_BLK) {
> 479 dev_err("KMEM not formatted for blk, magic %lx type %d\n", 480 super->header.magic, super->header.type);
481 kunmap(dax_kmem_index_to_page(0, dev_dax));
482 *persist_data = NULL;
483 return -EINVAL;
484 }
485
486 /* Validate superblock index page counts */
487 if (super->num_index_entries <=
488 super->num_index_pages * index_entries_per_page &&
489 1 + super->num_index_pages + super->num_index_entries
490 == num_pages) {
491 dev_info(dev,
492 "Found kmem_blk superblock num_index_entries %llu num_index_pages %llu num_pages %lu\n",
493 super->num_index_entries,
494 super->num_index_pages, num_pages);
495 } else {
496 dev_warn(dev,
497 "Invalid kmem_blk superblock num_index_entries %llu num_index_pages %llu num_pages %lu\n",
498 super->num_index_entries,
499 super->num_index_pages, num_pages);
500 kunmap(dax_kmem_index_to_page(0, dev_dax));
501 *persist_data = NULL;
502 return -EINVAL;
503 }
504
505 data = kzalloc(struct_size(data, index_page, super->num_index_pages),
506 GFP_KERNEL);
507 if (!data) {
508 kunmap(dax_kmem_index_to_page(0, dev_dax));
509 *persist_data = NULL;
510 return -ENOMEM;
511 }
512
513 *persist_data = data;
514 data->dev_dax = dev_dax;
515 data->super = super;
516 spin_lock_init(&data->index_lock);
517
518 for (i = 0; i < super->num_index_pages; i++)
519 data->index_page[i] =
520 kmap(dax_kmem_index_to_page(i + 1, dev_dax));
521
522 kmem_blk_free_unused(data);
523
524 kmem_blk_disk_init(data);
525
526 return 0;
527 }
528
Signature · 2 lines
0-DAY CI Kernel Test Service https://01.org/lkp
Quoted 22 lines · Srinivas Aji
Linux supports adding a DAX driver managed memory region as system memory using the KMEM driver (from version 5.1). We would like to use a persistent addressable memory segment as system memory and simultaneously for storing some persistent data.
Motivation: It is already possible to partition an NVDIMM device for RAM and storage by creating separate regions on the device and using one of them with KMEM and another as fsdax. This patch set is a start to trying to get zero copy snapshots of processes which are using the DAX device as RAM. That requires dynamically sharing pages between process RAM and the storage within a single NVDIMM region.
To do this, we add a layer for handling the persistent data which does the following:
1. When a DAX device is added as KMEM, mark all the memory as allocated and pass it up to a module which is aware of the storage layout.
2. This module scans the memory, identifies the unused parts, and frees those memory pages.
Quoted 8 lines
3. Further memory from this device is allocated using the kernel memory allocation API. The memory allocation API currently allows the allocation to be limited only based on NUMA node. So this feature works only when the DAX device used as KMEM is the only memory from its NUMA node.
4. Discarding of blocks previously used for persistent data results in those blocks being freed to system memory.
Can you explain how "zero copy snapshots of processes" would work, both
a) From a user space POV
b) From a kernel-internal POV
Especially, what I get is that you have a filesystem on that memory region, and all memory that is not used for filesystem blocks can be used as ordinary system RAM (a little like shmem, but restricted to dax memory regions?).
But how does this interact with zero-copy snapshots?
I feel like I am missing one piece where we really need system RAM as part of the bigger picture. Hopefully it's not some hack that converts system RAM to file system blocks :)
Signature · 3 lines
Thanks, David / dhildenb
Quoted 10 lines · David Hildenbrand
Can you explain how "zero copy snapshots of processes" would work, both
a) From a user space POV
b) From a kernel-internal POVEspecially, what I get is that you have a filesystem on that memory region, and all memory that is not used for filesystem blocks can be used as ordinary system RAM (a little like shmem, but restricted to dax memory regions?).
Quoted 5 lines
But how does this interact with zero-copy snapshots?
I feel like I am missing one piece where we really need system RAM as part of the bigger picture. Hopefully it's not some hack that converts system RAM to file system blocks :)
My proposal probably falls into this category. The idea is that if we have the persistent filesystem in the same space as system RAM, we could make most of the process pages part of a snapshot file by holding references to the these pages and making the pages copy-on-write for the process, in about the same way a forked child would. (I still don't have this piece fully worked out. May be there are reasons why this won't work or will make something else difficult, and that is why you are advising against it.)
Regarding the userspace and kernel POV:
The userspace operation would be that the process tries to save or restore its pages using vmsplice(). In the kernel, this would be implemented using a filesystem which shares pages with system RAM and uses a zero-copy COW mechanism for those process pages which can be shared with the filesystem.
I had earlier been thinking of having a different interface to the kernel, which creates a file with only those memory pages which can be saved using COW and also indicates to the caller which pages have actually been saved. But having a vmsplice implementation which does COW as far as possible keeps the userspace process indicating the desired function (saving or restoring memory pages) and the kernel implementation handling the zero copy as an optimization where possible.
Thanks,
Srinivas
Quoted 17 lines · Srinivas Aji
Quoted 10 lines · David Hildenbrand
Can you explain how "zero copy snapshots of processes" would work, both
a) From a user space POV
b) From a kernel-internal POVEspecially, what I get is that you have a filesystem on that memory region, and all memory that is not used for filesystem blocks can be used as ordinary system RAM (a little like shmem, but restricted to dax memory regions?).
Quoted 5 lines
But how does this interact with zero-copy snapshots?
I feel like I am missing one piece where we really need system RAM as part of the bigger picture. Hopefully it's not some hack that converts system RAM to file system blocks :)
Quoted 8 lines
My proposal probably falls into this category. The idea is that if we have the persistent filesystem in the same space as system RAM, we could make most of the process pages part of a snapshot file by holding references to the these pages and making the pages copy-on-write for the process, in about the same way a forked child would. (I still don't have this piece fully worked out. May be there are reasons why this won't work or will make something else difficult, and that is why you are advising against it.)
If I understand the proposal correctly I think you eventually run into situations similar to what killed RDMA+FSDAX support. The filesystem needs to be the ultimate arbiter of the physical address space and this solution seems to want to put part of that control in an agent outside of the filesystem.
Quoted 8 lines
Regarding the userspace and kernel POV:
The userspace operation would be that the process tries to save or restore its pages using
vmsplice(). In the kernel, this would be implemented using a filesystem which shares pages with system RAM and uses a zero-copy COW mechanism for those process pages which can be shared with the filesystem.
Quoted 8 lines
I had earlier been thinking of having a different interface to the kernel, which creates a file with only those memory pages which can be saved using COW and also indicates to the caller which pages have actually been saved. But having a vmsplice implementation which does COW as far as possible keeps the userspace process indicating the desired function (saving or restoring memory pages) and the kernel implementation handling the zero copy as an optimization where possible.
While my initial reaction to hearing about this proposal back at LSF indeed made it sound like an extension to FSDAX semantics, now I am not so sure. This requirement you state, "...we have to get the blocks through the memory allocation API, at an offset not under our control" makes me feel like this is a new memory management facility where the application thinks it is getting page allocations serviced via the typical malloc+mempolicy APIs, but another agent is positioned to trap and service those requests.
Correct me if I am wrong, but is the end goal similar to what an application in a VM experiences when that VM's memory is backed by a file mappping on the VMM side? I.e. the application is accessing a virtual NUMA node, but the faults into physical address space are trapped and serviced by the VMM. If that is the case then the solution starts look more like NUMA "namespacing" than a block-device + file interface. In other words a rough (I mean rough) strawman like:
numactlX --remap=3,/dev/dax0.0 --membind=3 $application
Where memory allocation and refault requests can be trapped by that modified numactl. As far as the application is concerned its memory policy is set to allocate from NUMA node 3, and those page allocation requests are routed to numactlX via userfaultfd-like mechanics to map pages out of /dev/dax0.0 (or any other file for that mattter). Snap shotting would be achieved by telling numactlX to CoW all of the pages that it currently has mapped while the snapshot is taken.