Messages in this thread | | | From | Jane Chu <> | Subject | Re: [PATCH v5 4/7] dax: add dax_recovery_write to dax_op and dm target type | Date | Wed, 2 Feb 2022 22:03:12 +0000 |
| |
On 2/2/2022 5:34 AM, Christoph Hellwig wrote: > On Fri, Jan 28, 2022 at 02:31:47PM -0700, Jane Chu wrote: >> dax_recovery_write() dax op is only required for DAX device that >> export DAXDEV_RECOVERY indicating its capability to recover from >> poisons. >> >> DM may be nested, if part of the base dax devices forming a DM >> device support dax recovery, the DM device is marked with such >> capability. > > I'd fold this into the previous 2 patches as the flag and method > are clearly very tightly coupled.
Make sense, will do.
> >> +static size_t linear_dax_recovery_write(struct dm_target *ti, pgoff_t pgoff, >> + void *addr, size_t bytes, struct iov_iter *i) > > Function line continuations use two tab indentations or alignment after > the opening brace.
Okay.
> >> +{ >> + struct dax_device *dax_dev = linear_dax_pgoff(ti, &pgoff); >> + >> + if (!dax_recovery_capable(dax_dev)) >> + return (size_t) -EOPNOTSUPP; > > Returning a negativ errno through an unsigned argument looks dangerous.
Sorry, should be (ssize_t) there.
> >> + /* recovery_write: optional operation. */ > > And explanation of what the method is use for might be more useful than > mentioning that is is optional.
Will add substance to comments.
> >> + size_t (*recovery_write)(struct dax_device *, pgoff_t, void *, size_t, >> + struct iov_iter *); > > Spelling out the arguments tends to help readability, but then again > none of the existing methods does it.
Thanks! -jane
| |