lkml.org 
[lkml]   [2021]   [Feb]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v3 06/11] mm, pmem: Implement ->memory_failure() in pmem driver
    > +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.

    \
     
     \ /
      Last update: 2021-02-10 14:42    [W:5.647 / U:0.016 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site