Re: [RFC][PATCH 1/3] radix priority search tree - objrmap complexity fix

Andrea Arcangeli
2004-03-29T18:01:09Z

On Mon, Mar 29, 2004 at 12:50:20PM -0500, Rajesh Venkatasubramanian wrote:
>
>
> > #define VN_MAPPED(vp) \
> > - (!list_empty(&(LINVFS_GET_IP(vp)->i_mapping->i_mmap)) || \
> > - (!list_empty(&(LINVFS_GET_IP(vp)->i_mapping->i_mmap_shared))))
> > + (!prio_tree_empty(&(LINVFS_GET_IP(vp)->i_mapping->i_mmap)) || \
> > + (!prio_tree_empty(&(LINVFS_GET_IP(vp)->i_mapping->i_mmap_shared))))
>
> I think we will need the following too:
> (!list_empty(&(LINVFS_GET_IP(vp)->i_mmaping->i_mmap_nonlinear)
>
>
> > down(&mapping->i_shared_sem);
> > - list_for_each_entry(vma, &mapping->i_mmap_shared, shared) {
> > + vma = __vma_prio_tree_first(&mapping->i_mmap_shared, &iter, 0, ULONG_MAX);
> > + while (vma) {
> > if (!(vma->vm_flags & VM_DENYWRITE)) {
> > prohibited |= (1 << DM_EVENT_WRITE);
> > break;
> > }
> > +
> > + vma = __vma_prio_tree_next(vma, &mapping->i_mmap_shared, &iter, 0, ULONG_MAX);
> > }
>
> This part looks fine. But, I am not sure whether you have to handle
> nonlinear maps here.
>
> list_for_each_entry(vma, &mapping->i_mmap_nonlinear, shared) {
> ...
> }
>

I agree we should handle the nonlinear maps. since nobody uses nonlinear
this isn't a big issue for the short term.

There's now also a screwup in the writeback -mm changes for swapsuspend,
it bugs out in radix tree tag, I believe it's because it doesn't
insert the page in the radix tree before doing writeback I/O on it. This
is my first attempt to cure it.

--- sles/mm/page_io.c.~1~ 2004-03-29 19:05:50.014588464 +0200
+++ sles/mm/page_io.c 2004-03-29 19:46:14.282043792 +0200
@@ -151,8 +151,15 @@ int rw_swap_page_sync(int rw, swp_entry_
lock_page(page);

BUG_ON(page_mapping(page));
+ BUG_ON(PageSwapCache(page));
SetPageSwapCache(page);
page->private = entry.val;
+ ret = radix_tree_insert(&page_mapping(page)->page_tree, page->private, page);
+ if (unlikely(ret)) {
+ ClearPageSwapCache(page);
+ unlock_page(page);
+ return ret;
+ }

if (rw == READ) {
ret = swap_readpage(NULL, page);
@@ -161,7 +168,10 @@ int rw_swap_page_sync(int rw, swp_entry_
ret = swap_writepage(page, &swap_wbc);
wait_on_page_writeback(page);
}
+
+ radix_tree_delete(&page_mapping(page)->page_tree, page->private);
ClearPageSwapCache(page);
+
if (ret == 0 && (!PageUptodate(page) || PageError(page)))
ret = -EIO;
return ret;

then there's x86-64 bombing too:

--- sles/arch/x86_64/ia32/ia32_binfmt.c.~1~ 2004-03-29 19:05:50.516512160 +0200
+++ sles/arch/x86_64/ia32/ia32_binfmt.c 2004-03-29 19:53:20.031320064 +0200
@@ -366,7 +366,7 @@ int setup_arg_pages(struct linux_binprm
mpnt->vm_pgoff = mpnt->vm_start >> PAGE_SHIFT;
mpnt->vm_file = NULL;
mpol_set_vma_default(mpnt);
- INIT_LIST_HEAD(&mpnt->shared);
+ INIT_VMA_SHARED(mpnt);
/* insert_vm_struct takes care of anon_vma_node */
mpnt->anon_vma = NULL;
mpnt->vm_private_data = (void *) 0;

the writeback part will require testing, so I'll postpone further
updates until I get confirmation that swapsuspend works again (this is
all low prio stuff anyways, the previous xfs list_empty miscompilation
was scary instead so I update it quickly, since people actually uses
MAP_SHARED/MAP_PRIVATE).