Messages in this thread | | | From | Dan Williams <> | Date | Mon, 11 Apr 2022 21:57:36 -0700 | Subject | Re: [PATCH v7 4/6] dax: add DAX_RECOVERY flag and .recovery_write dev_pgmap_ops |
| |
On Tue, Apr 5, 2022 at 12:48 PM Jane Chu <jane.chu@oracle.com> wrote: > > Introduce DAX_RECOVERY flag to dax_direct_access(). The flag is > not set by default in dax_direct_access() such that the helper > does not translate a pmem range to kernel virtual address if the > range contains uncorrectable errors. When the flag is set, > the helper ignores the UEs and return kernel virtual adderss so > that the caller may get on with data recovery via write.
It strikes me that there is likely never going to be any other flags to dax_direct_access() and what this option really is an access type. I also find code changes like this error prone to read:
- rc = dax_direct_access(iter->iomap.dax_dev, pgoff, 1, &kaddr, NULL); + rc = dax_direct_access(iter->iomap.dax_dev, pgoff, 1, 0, &kaddr, NULL);
...i.e. without looking at the prototype, which option is the nr_pages and which is the flags?
So how about change 'int flags' to 'enum dax_access_mode mode' where dax_access_mode is:
/** * enum dax_access_mode - operational mode for dax_direct_access() * @DAX_ACCESS: nominal access, fail / trim access on encountering poison * @DAX_RECOVERY_WRITE: ignore poison and provide a pointer suitable for use with dax_recovery_write() */ enum dax_access_mode { DAX_ACCESS, DAX_RECOVERY_WRITE, };
Then the conversions look like this:
- rc = dax_direct_access(iter->iomap.dax_dev, pgoff, 1, &kaddr, NULL); + rc = dax_direct_access(iter->iomap.dax_dev, pgoff, 1, DAX_ACCESS, &kaddr, NULL);
...and there's less chance of confusion with the @nr_pages argument.
| |