Linux-kernel mailing list archive 1999-41,: Re: is lock_kernel() required for lookup_swap_cache() and swap_free()?

# Re: is lock_kernel() required for lookup_swap_cache() and swap_free()?

Andrea Arcangeli (andrea@suse.de)
Thu, 14 Oct 1999 16:38:21 +0200 (CEST)

- Messages sorted by: [ date ][ thread ][ subject ][ author ]
- Next message: Jamie Lokier: "2.1.20: e820 crash data"
- Previous message: Andrea Arcangeli: "Re: [PATCH] scheduler cleanup"

On Thu, 14 Oct 1999, Manfred Spraul wrote:

>* do_swap_page() calls lookup_swap_cache() without lock_kernel(), but

lookup_swap_cache doesn't need the kernel lock.

>* shrink_mmap() calls __delete_from_swap_cache() without any special
>locking. __delete_from_swap_cache() calls swap_free(), and it seems that
>this function is not thread-safe.

No. You just hold the per page lock and the pagecache lock before calling
__delete_from_swap_cache. That's the only thing you need.

shrink_mmap is completly SMP threaded (I made it SMP threaded while
replacing the sloww clock algorithm with a real page-LRU). It only rely on
the per-page lock and on the pagecache_lock when necessary (to avoid
somebody to find the page under us).

>[__delete_from_swap_cache() also increases a non-atomic global variable,
>but this looks like debug code].

There's also a missing lock kernel around swap_free. Here the obviously
right fix I did some time ago (it applyed cleanly on the top of 2.3.21).

--- 2.3.18ac2/mm/swap_state.c Wed Sep 8 00:26:08 1999
+++ /tmp/swap_state.c Mon Sep 13 18:49:18 1999
@@ -13,6 +13,7 @@
#include <linux/swapctl.h>
#include <linux/init.h>
#include <linux/pagemap.h>
+#include <linux/smp_lock.h>
#include <asm/pgtable.h>
@@ -234,7 +235,9 @@
page_address(page), page_count(page), entry);
#endif
remove_from_swap_cache (page);
+ lock_kernel();
swap_free (entry);
+ unlock_kernel();
}
static void delete_from_swap_cache_nolock(struct page *page)

Andrea

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/

- Next message: Jamie Lokier: "2.1.20: e820 crash data"
- Previous message: Andrea Arcangeli: "Re: [PATCH] scheduler cleanup"