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
