> +static int pmem_pagemap_memory_failure(struct dev_pagemap *pgmap,
> + unsigned long pfn, int flags)
> +{
> + struct pmem_device *pdev;
> + struct gendisk *disk;
> + loff_t disk_offset;
> + int rc = 0;
> + unsigned long size = page_size(pfn_to_page(pfn));
> +
> + pdev = container_of(pgmap, struct pmem_device, pgmap);
> + disk = pdev->disk;
Would be nice to initialize this at the time of declaration:
struct pmem_device *pdev =
container_of(pgmap, struct pmem_device, pgmap);
struct gendisk *disk = pdev->disk
unsigned long size = page_size(pfn_to_page(pfn));
> + if (!disk)
> + return -ENXIO;
> +
> + disk_offset = PFN_PHYS(pfn) - pdev->phys_addr - pdev->data_offset;
> + if (disk->fops->corrupted_range) {
> + rc = disk->fops->corrupted_range(disk, NULL, disk_offset,
> + size, &flags);
> + if (rc == -ENODEV)
> + rc = -ENXIO;
> + } else
> + rc = -EOPNOTSUPP;
Why do we need the disk and fops check here? A pgmap registered by pmem.c
should always have a disk with pmem_fops. And more importantly this
has no business going through the block layer.
Instead the file system should deposit a callback when starting to use
the dax_device using fs_dax_get_by_bdev / dax_get_by_host and a private
data (the superblock), and we avoid all the lookup problems.
> +int mf_generic_kill_procs(unsigned long long pfn, int flags)
This function seems to be only used inside of memory-failure.c, so it
could be marked static. Also I'd name it dax_generic_memory_failure
or something like that to match the naming of the ->memory_failure
pgmap operation.
Also maybe just splitting this out into a helper would be a nice prep
patch.
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
