| Message ID | 20220128213150.1333552-7-jane.chu@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | DAX poison recovery | expand |
On Fri, Jan 28, 2022 at 02:31:49PM -0700, Jane Chu wrote: > +typedef size_t (*iter_func_t)(struct dax_device *dax_dev, pgoff_t pgoff, > + void *addr, size_t bytes, struct iov_iter *i); > static loff_t dax_iomap_iter(const struct iomap_iter *iomi, > struct iov_iter *iter) > { > @@ -1210,6 +1212,7 @@ static loff_t dax_iomap_iter(const struct iomap_iter *iomi, > ssize_t ret = 0; > size_t xfer; > int id; > + iter_func_t write_func = dax_copy_from_iter; This use of a function pointer causes indirect call overhead. A simple "bool in_recovery" or do_recovery does the trick in a way that is both more readable and generates faster code. > + if ((map_len == -EIO) && (iov_iter_rw(iter) == WRITE)) { No need for the braces. > if (iov_iter_rw(iter) == WRITE) > - xfer = dax_copy_from_iter(dax_dev, pgoff, kaddr, > - map_len, iter); > + xfer = write_func(dax_dev, pgoff, kaddr, map_len, iter); > else > xfer = dax_copy_to_iter(dax_dev, pgoff, kaddr, > map_len, iter); i.e. if (iov_iter_rw(iter) == READ) xfer = dax_copy_to_iter(dax_dev, pgoff, kaddr, map_len, iter); else if (unlikely(do_recovery)) xfer = dax_recovery_write(dax_dev, pgoff, kaddr, map_len, iter); else xfer = dax_copy_from_iter(dax_dev, pgoff, kaddr, map_len, iter);
On 2/2/2022 5:44 AM, Christoph Hellwig wrote: > On Fri, Jan 28, 2022 at 02:31:49PM -0700, Jane Chu wrote: >> +typedef size_t (*iter_func_t)(struct dax_device *dax_dev, pgoff_t pgoff, >> + void *addr, size_t bytes, struct iov_iter *i); >> static loff_t dax_iomap_iter(const struct iomap_iter *iomi, >> struct iov_iter *iter) >> { >> @@ -1210,6 +1212,7 @@ static loff_t dax_iomap_iter(const struct iomap_iter *iomi, >> ssize_t ret = 0; >> size_t xfer; >> int id; >> + iter_func_t write_func = dax_copy_from_iter; > > This use of a function pointer causes indirect call overhead. A simple > "bool in_recovery" or do_recovery does the trick in a way that is > both more readable and generates faster code. Good point, thanks! > >> + if ((map_len == -EIO) && (iov_iter_rw(iter) == WRITE)) { > > No need for the braces. Did you mean the outer "( )" ? > >> if (iov_iter_rw(iter) == WRITE) >> - xfer = dax_copy_from_iter(dax_dev, pgoff, kaddr, >> - map_len, iter); >> + xfer = write_func(dax_dev, pgoff, kaddr, map_len, iter); >> else >> xfer = dax_copy_to_iter(dax_dev, pgoff, kaddr, >> map_len, iter); > > i.e. > > if (iov_iter_rw(iter) == READ) > xfer = dax_copy_to_iter(dax_dev, pgoff, kaddr, > map_len, iter); > else if (unlikely(do_recovery)) > xfer = dax_recovery_write(dax_dev, pgoff, kaddr, > map_len, iter); > else > xfer = dax_copy_from_iter(dax_dev, pgoff, kaddr, > map_len, iter); > Will do. Thanks a lot! -jane
diff --git a/fs/dax.c b/fs/dax.c index cd03485867a7..236675bd5946 100644 --- a/fs/dax.c +++ b/fs/dax.c @@ -1199,6 +1199,8 @@ int dax_truncate_page(struct inode *inode, loff_t pos, bool *did_zero, } EXPORT_SYMBOL_GPL(dax_truncate_page); +typedef size_t (*iter_func_t)(struct dax_device *dax_dev, pgoff_t pgoff, + void *addr, size_t bytes, struct iov_iter *i); static loff_t dax_iomap_iter(const struct iomap_iter *iomi, struct iov_iter *iter) { @@ -1210,6 +1212,7 @@ static loff_t dax_iomap_iter(const struct iomap_iter *iomi, ssize_t ret = 0; size_t xfer; int id; + iter_func_t write_func = dax_copy_from_iter; if (iov_iter_rw(iter) == READ) { end = min(end, i_size_read(iomi->inode)); @@ -1249,6 +1252,17 @@ static loff_t dax_iomap_iter(const struct iomap_iter *iomi, map_len = dax_direct_access(dax_dev, pgoff, PHYS_PFN(size), &kaddr, NULL); + if ((map_len == -EIO) && (iov_iter_rw(iter) == WRITE)) { + if (dax_prep_recovery(dax_dev, &kaddr) < 0) { + ret = map_len; + break; + } + map_len = dax_direct_access(dax_dev, pgoff, + PHYS_PFN(size), &kaddr, NULL); + if (map_len > 0) + write_func = dax_recovery_write; + } + if (map_len < 0) { ret = map_len; break; @@ -1261,12 +1275,17 @@ static loff_t dax_iomap_iter(const struct iomap_iter *iomi, map_len = end - pos; if (iov_iter_rw(iter) == WRITE) - xfer = dax_copy_from_iter(dax_dev, pgoff, kaddr, - map_len, iter); + xfer = write_func(dax_dev, pgoff, kaddr, map_len, iter); else xfer = dax_copy_to_iter(dax_dev, pgoff, kaddr, map_len, iter); + if (xfer == (ssize_t) -EIO) { + pr_warn("dax_ioma_iter: write_func returns -EIO\n"); + ret = -EIO; + break; + } + pos += xfer; length -= xfer; done += xfer;
dax_iomap_iter() fails if the destination range contains poison. Add recovery_write to the failure code path. Signed-off-by: Jane Chu <jane.chu@oracle.com> --- fs/dax.c | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-)