On Fri, May 24, 2024 at 08:35:42PM +0100, Matthew Wilcox wrote:
> 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?
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);
> > }
> >
>
