On Mon, 2021-04-26 at 18:54 +0000, Al Viro wrote:
> On Fri, Apr 23, 2021 at 02:28:01PM +0100, David Howells wrote:
> > -#define iterate_all_kinds(i, n, v, I, B, K) {                      \
> > +#define iterate_xarray(i, n, __v, skip, STEP) {            \
> > +   struct page *head = NULL;                               \
> > +   size_t wanted = n, seg, offset;                         \
> > +   loff_t start = i->xarray_start + skip;                  \
> > +   pgoff_t index = start >> PAGE_SHIFT;                    \
> > +   int j;                                                  \
> > +                                                           \
> > +   XA_STATE(xas, i->xarray, index);                        \
> > +                                                           \
> > +   rcu_read_lock();                                                \
> > +   xas_for_each(&xas, head, ULONG_MAX) {                           \
> > +           if (xas_retry(&xas, head))                              \
> > +                   continue;                                       \
> 
> OK, now I'm really confused; what's to guarantee that restart will not have
> you hit the same entry more than once?  STEP might be e.g.
> 
>               memcpy_to_page(v.bv_page, v.bv_offset,
>                              (from += v.bv_len) - v.bv_len, v.bv_len)
> 
> which is clearly not idempotent - from gets incremented, after all.
> What am I missing here?
> 

Not sure I understand the issue you see. If xas_retry returns true,
we'll restart, but we won't have called STEP yet for that entry. I
don't see how we'd retry there and have an issue with idempotency.

> > +           if (WARN_ON(xa_is_value(head)))                         \
> > +                   break;                                          \
> > +           if (WARN_ON(PageHuge(head)))                            \
> > +                   break;                                          \
> > +           for (j = (head->index < index) ? index - head->index : 0; \
> > +                j < thp_nr_pages(head); j++) {                     \
> > +                   __v.bv_page = head + j;                         \
> > +                   offset = (i->xarray_start + skip) & ~PAGE_MASK; \
> > +                   seg = PAGE_SIZE - offset;                       \
> > +                   __v.bv_offset = offset;                         \
> > +                   __v.bv_len = min(n, seg);                       \
> > +                   (void)(STEP);                                   \
> > +                   n -= __v.bv_len;                                \
> > +                   skip += __v.bv_len;                             \
> > +                   if (n == 0)                                     \
> > +                           break;                                  \
> > +           }                                                       \
> > +           if (n == 0)                                             \
> > +                   break;                                          \
> > +   }                                                       \
> > +   rcu_read_unlock();                                      \
> > +   n = wanted - n;                                         \
> > +}


--
Linux-cachefs mailing list
Linux-cachefs@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-cachefs

Reply via email to