On Mon, May 06, 2024 at 05:14:21AM +0100, Matthew Wilcox wrote:
> On Sun, May 05, 2024 at 01:25:58PM +0100, Matthew Wilcox wrote:
> > Here's a fun bug that's not obvious:
> >
> > hfs_bnode_move:
> > dst_ptr = kmap_local_page(*dst_page);
> > src_ptr = kmap_local_page(*src_page);
> > memmove(dst_ptr, src_ptr, src);
>
> OK, so now we know this is the only place with this problem, how are we
> going to fix it?
Fabio?
> I think the obvious thing to do is to revert the kmap -> kmap_local
> conversion in this function. The other functions look fine.
>
> Longer term, hfs_bnode_move() makes my eyes bleed. I really think we
> need to do something stupider. Something like ...
>
> void hfs_bnode_move(struct hfs_bnode *node, int dst, int src, int len)
> {
> void *data;
> int first, last;
>
> if (!len || src == dst)
> return;
> if (src < dst && src + len < dst)
> return hfs_bnode_copy(node, dst, node, src, len);
> if (dst < src && dst + len < src)
> return hfs_bnode_copy(node, dst, node, src, len);
>
> src += node->page_offset;
> dst += node->page_offset;
> first = min(dst, src) / PAGE_SIZE;
> last = max(dst + len, src + len) / PAGE_SIZE;
> data = vmap_folios(bnode->folios + first, last - first + 1);
> src -= first * PAGE_SIZE;
> dst -= first * PAGE_SIZE;
> // maybe an off-by-one in above calculations; check it
> memmove(data + dst, data + src, len);
> vunmap(data);
> }
>
