[PATCH RFC 1/2] Add notifiers for various swap events

[PATCH RFC 1/2] Add notifiers for various swap events

[Nitin Gupta]

Add notifiers for following swap events:
 - Swapon
 - Swapoff
 - When a swap slot is freed

This is required for ramzswap module which implements RAM based block
devices to be used as swap disks. These devices require a notification
on these events to function properly (as shown in patch 2/2).

Currently, I'm not sure if any of these event notifiers have any other
users. However, adding ramzswap specific hooks instead of this generic
approach resulted in a bad/hacky code.

For SWAP_EVENT_SLOT_FREE, callbacks are made under swap_lock. Currently, this
is not a problem since ramzswap is the only user and the callback it registers
can be safely made under this lock. However, if this event finds more users,
we might have to work on reducing contention on this lock.

Signed-off-by: Nitin Gupta <ngupta@vflare.org>

---
 include/linux/swap.h |   12 +++++++++
 mm/swapfile.c        |   67 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 79 insertions(+), 0 deletions(-)

diff --git a/include/linux/swap.h b/include/linux/swap.h
index 7c15334..2873aad 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -127,6 +127,12 @@ enum {
 	SWP_SCANNING	= (1 << 8),	/* refcount in scan_swap_map */
 };
 
+enum swap_event {
+	SWAP_EVENT_SWAPON,
+	SWAP_EVENT_SWAPOFF,
+	SWAP_EVENT_SLOT_FREE,
+};
+
 #define SWAP_CLUSTER_MAX 32
 
 #define SWAP_MAP_MAX	0x7ffe
@@ -155,6 +161,7 @@ struct swap_info_struct {
 	unsigned int max;
 	unsigned int inuse_pages;
 	unsigned int old_block_size;
+	struct atomic_notifier_head slot_free_notify_list;
 };
 
 struct swap_list_t {
@@ -295,8 +302,13 @@ extern sector_t swapdev_block(int, pgoff_t);
 extern struct swap_info_struct *get_swap_info_struct(unsigned);
 extern int reuse_swap_page(struct page *);
 extern int try_to_free_swap(struct page *);
+extern int register_swap_event_notifier(struct notifier_block *nb,
+                                enum swap_event event, unsigned long val);
+extern int unregister_swap_event_notifier(struct notifier_block *nb,
+                                enum swap_event event, unsigned long val);
 struct backing_dev_info;
 
+
 /* linux/mm/thrash.c */
 extern struct mm_struct *swap_token_mm;
 extern void grab_swap_token(struct mm_struct *);
diff --git a/mm/swapfile.c b/mm/swapfile.c
index 74f1102..f63643c 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -52,6 +52,9 @@ static struct swap_list_t swap_list = {-1, -1};
 static struct swap_info_struct swap_info[MAX_SWAPFILES];
 
 static DEFINE_MUTEX(swapon_mutex);
+static BLOCKING_NOTIFIER_HEAD(swapon_notify_list);
+static BLOCKING_NOTIFIER_HEAD(swapoff_notify_list);
+
 
 /* For reference count accounting in swap_map */
 /* enum for swap_map[] handling. internal use only */
@@ -585,6 +588,8 @@ static int swap_entry_free(struct swap_info_struct *p,
 			swap_list.next = p - swap_info;
 		nr_swap_pages++;
 		p->inuse_pages--;
+		atomic_notifier_call_chain(&p->slot_free_notify_list,
+					offset, p->swap_file);
 	}
 	if (!swap_count(count))
 		mem_cgroup_uncharge_swap(ent);
@@ -1626,6 +1631,7 @@ SYSCALL_DEFINE1(swapoff, const char __user *, specialfile)
 	p->swap_map = NULL;
 	p->flags = 0;
 	spin_unlock(&swap_lock);
+	blocking_notifier_call_chain(&swapoff_notify_list, type, swap_file);
 	mutex_unlock(&swapon_mutex);
 	vfree(swap_map);
 	/* Destroy swap account informatin */
@@ -2014,7 +2020,9 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
 	} else {
 		swap_info[prev].next = p - swap_info;
 	}
+	ATOMIC_INIT_NOTIFIER_HEAD(&p->slot_free_notify_list);
 	spin_unlock(&swap_lock);
+	blocking_notifier_call_chain(&swapon_notify_list, type, swap_file);
 	mutex_unlock(&swapon_mutex);
 	error = 0;
 	goto out;
@@ -2216,3 +2224,62 @@ int valid_swaphandles(swp_entry_t entry, unsigned long *offset)
 	*offset = ++toff;
 	return nr_pages? ++nr_pages: 0;
 }
+
+int register_swap_event_notifier(struct notifier_block *nb,
+				enum swap_event event, unsigned long val)
+{
+	switch (event) {
+	case SWAP_EVENT_SWAPON:
+		return blocking_notifier_chain_register(
+					&swapon_notify_list, nb);
+	case SWAP_EVENT_SWAPOFF:
+		return blocking_notifier_chain_register(
+					&swapoff_notify_list, nb);
+	case SWAP_EVENT_SLOT_FREE:
+		{
+		struct swap_info_struct *sis;
+
+		if (val > nr_swapfiles)
+			goto out;
+		sis = get_swap_info_struct(val);
+		return atomic_notifier_chain_register(
+				&sis->slot_free_notify_list, nb);
+		}
+	default:
+		pr_err("Invalid swap event: %d\n", event);
+	};
+
+out:
+	return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(register_swap_event_notifier);
+
+int unregister_swap_event_notifier(struct notifier_block *nb,
+				enum swap_event event, unsigned long val)
+{
+	switch (event) {
+	case SWAP_EVENT_SWAPON:
+		return blocking_notifier_chain_unregister(
+					&swapon_notify_list, nb);
+	case SWAP_EVENT_SWAPOFF:
+		return blocking_notifier_chain_unregister(
+					&swapoff_notify_list, nb);
+	case SWAP_EVENT_SLOT_FREE:
+		{
+		struct swap_info_struct *sis;
+
+		if (val > nr_swapfiles)
+			goto out;
+		sis = get_swap_info_struct(val);
+		return atomic_notifier_chain_unregister(
+				&sis->slot_free_notify_list, nb);
+		}
+	default:
+		pr_err("Invalid swap event: %d\n", event);
+	};
+
+out:
+	return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(unregister_swap_event_notifier);
+

[PATCH RFC 1/2] Add notifiers for various swap events

[Nitin Gupta]

Add notifiers for following swap events:
 - Swapon
 - Swapoff
 - When a swap slot is freed

This is required for ramzswap module which implements RAM based block
devices to be used as swap disks. These devices require a notification
on these events to function properly (as shown in patch 2/2).

Currently, I'm not sure if any of these event notifiers have any other
users. However, adding ramzswap specific hooks instead of this generic
approach resulted in a bad/hacky code.

For SWAP_EVENT_SLOT_FREE, callbacks are made under swap_lock. Currently, this
is not a problem since ramzswap is the only user and the callback it registers
can be safely made under this lock. However, if this event finds more users,
we might have to work on reducing contention on this lock.

Signed-off-by: Nitin Gupta <ngupta@vflare.org>

---
 include/linux/swap.h |   12 +++++++++
 mm/swapfile.c        |   67 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 79 insertions(+), 0 deletions(-)

diff --git a/include/linux/swap.h b/include/linux/swap.h
index 7c15334..2873aad 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -127,6 +127,12 @@ enum {
 	SWP_SCANNING	= (1 << 8),	/* refcount in scan_swap_map */
 };
 
+enum swap_event {
+	SWAP_EVENT_SWAPON,
+	SWAP_EVENT_SWAPOFF,
+	SWAP_EVENT_SLOT_FREE,
+};
+
 #define SWAP_CLUSTER_MAX 32
 
 #define SWAP_MAP_MAX	0x7ffe
@@ -155,6 +161,7 @@ struct swap_info_struct {
 	unsigned int max;
 	unsigned int inuse_pages;
 	unsigned int old_block_size;
+	struct atomic_notifier_head slot_free_notify_list;
 };
 
 struct swap_list_t {
@@ -295,8 +302,13 @@ extern sector_t swapdev_block(int, pgoff_t);
 extern struct swap_info_struct *get_swap_info_struct(unsigned);
 extern int reuse_swap_page(struct page *);
 extern int try_to_free_swap(struct page *);
+extern int register_swap_event_notifier(struct notifier_block *nb,
+                                enum swap_event event, unsigned long val);
+extern int unregister_swap_event_notifier(struct notifier_block *nb,
+                                enum swap_event event, unsigned long val);
 struct backing_dev_info;
 
+
 /* linux/mm/thrash.c */
 extern struct mm_struct *swap_token_mm;
 extern void grab_swap_token(struct mm_struct *);
diff --git a/mm/swapfile.c b/mm/swapfile.c
index 74f1102..f63643c 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -52,6 +52,9 @@ static struct swap_list_t swap_list = {-1, -1};
 static struct swap_info_struct swap_info[MAX_SWAPFILES];
 
 static DEFINE_MUTEX(swapon_mutex);
+static BLOCKING_NOTIFIER_HEAD(swapon_notify_list);
+static BLOCKING_NOTIFIER_HEAD(swapoff_notify_list);
+
 
 /* For reference count accounting in swap_map */
 /* enum for swap_map[] handling. internal use only */
@@ -585,6 +588,8 @@ static int swap_entry_free(struct swap_info_struct *p,
 			swap_list.next = p - swap_info;
 		nr_swap_pages++;
 		p->inuse_pages--;
+		atomic_notifier_call_chain(&p->slot_free_notify_list,
+					offset, p->swap_file);
 	}
 	if (!swap_count(count))
 		mem_cgroup_uncharge_swap(ent);
@@ -1626,6 +1631,7 @@ SYSCALL_DEFINE1(swapoff, const char __user *, specialfile)
 	p->swap_map = NULL;
 	p->flags = 0;
 	spin_unlock(&swap_lock);
+	blocking_notifier_call_chain(&swapoff_notify_list, type, swap_file);
 	mutex_unlock(&swapon_mutex);
 	vfree(swap_map);
 	/* Destroy swap account informatin */
@@ -2014,7 +2020,9 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
 	} else {
 		swap_info[prev].next = p - swap_info;
 	}
+	ATOMIC_INIT_NOTIFIER_HEAD(&p->slot_free_notify_list);
 	spin_unlock(&swap_lock);
+	blocking_notifier_call_chain(&swapon_notify_list, type, swap_file);
 	mutex_unlock(&swapon_mutex);
 	error = 0;
 	goto out;
@@ -2216,3 +2224,62 @@ int valid_swaphandles(swp_entry_t entry, unsigned long *offset)
 	*offset = ++toff;
 	return nr_pages? ++nr_pages: 0;
 }
+
+int register_swap_event_notifier(struct notifier_block *nb,
+				enum swap_event event, unsigned long val)
+{
+	switch (event) {
+	case SWAP_EVENT_SWAPON:
+		return blocking_notifier_chain_register(
+					&swapon_notify_list, nb);
+	case SWAP_EVENT_SWAPOFF:
+		return blocking_notifier_chain_register(
+					&swapoff_notify_list, nb);
+	case SWAP_EVENT_SLOT_FREE:
+		{
+		struct swap_info_struct *sis;
+
+		if (val > nr_swapfiles)
+			goto out;
+		sis = get_swap_info_struct(val);
+		return atomic_notifier_chain_register(
+				&sis->slot_free_notify_list, nb);
+		}
+	default:
+		pr_err("Invalid swap event: %d\n", event);
+	};
+
+out:
+	return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(register_swap_event_notifier);
+
+int unregister_swap_event_notifier(struct notifier_block *nb,
+				enum swap_event event, unsigned long val)
+{
+	switch (event) {
+	case SWAP_EVENT_SWAPON:
+		return blocking_notifier_chain_unregister(
+					&swapon_notify_list, nb);
+	case SWAP_EVENT_SWAPOFF:
+		return blocking_notifier_chain_unregister(
+					&swapoff_notify_list, nb);
+	case SWAP_EVENT_SLOT_FREE:
+		{
+		struct swap_info_struct *sis;
+
+		if (val > nr_swapfiles)
+			goto out;
+		sis = get_swap_info_struct(val);
+		return atomic_notifier_chain_unregister(
+				&sis->slot_free_notify_list, nb);
+		}
+	default:
+		pr_err("Invalid swap event: %d\n", event);
+	};
+
+out:
+	return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(unregister_swap_event_notifier);
+

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH RFC 2/2] example usage of swap notifiers in ramzswap

[Nitin Gupta]

This patch is against a version of ramzswap which uses module sepcific hacks
to register a callback for swap slot free event and does not use any notifier.

It shows improvments in the ramzswap module (in terms of code flow) that
resulted from swap notifier support added in patch 1/2.

Signed-off-by: Nitin Gupta <ngupta@vflare.org>

---

diff --git a/drivers/staging/ramzswap/ramzswap_drv.c b/drivers/staging/ramzswap/ramzswap_drv.c
index 1a7167f..1c5326e 100644
--- a/drivers/staging/ramzswap/ramzswap_drv.c
+++ b/drivers/staging/ramzswap/ramzswap_drv.c
@@ -647,19 +647,6 @@ out:
 	rzs->table[index].offset = 0;
 }
 
-/*
- * callback function called when swap_map[offset] == 0
- * i.e page at this swap offset is no longer used
- */
-static void ramzswap_free_notify(struct block_device *bdev,
-				unsigned long index)
-{
-	struct ramzswap *rzs = bdev->bd_disk->private_data;
-
-	ramzswap_free_page(rzs, index);
-	stat_inc(rzs->stats.notify_free);
-}
-
 static int handle_zero_page(struct bio *bio)
 {
 	void *user_mem;
@@ -760,11 +747,6 @@ static int ramzswap_read(struct ramzswap *rzs, struct bio *bio)
 	page = bio->bi_io_vec[0].bv_page;
 	index = bio->bi_sector >> SECTORS_PER_PAGE_SHIFT;
 
-	if (unlikely(!rzs->init_notify_callback) && PageSwapCache(page)) {
-		set_ramzswap_free_notify(bio->bi_bdev, ramzswap_free_notify);
-		rzs->init_notify_callback = 1;
-	}
-
 	if (rzs_test_flag(rzs, index, RZS_ZERO))
 		return handle_zero_page(bio);
 
@@ -1318,13 +1300,6 @@ static int ramzswap_ioctl(struct block_device *bdev, fmode_t mode,
 			goto out;
 		}
 		ret = ramzswap_ioctl_reset_device(rzs);
-		/*
-		 * Racy! Device has already been swapoff'ed.  Bad things
-		 * can happen if another swapon is done before this reset.
-		 * TODO: A callback from swapoff() will solve this problem.
-		 */
-		set_ramzswap_free_notify(bdev, NULL);
-		rzs->init_notify_callback = 0;
 		break;
 
 	default:
@@ -1395,13 +1370,62 @@ static void destroy_device(struct ramzswap *rzs)
 		blk_cleanup_queue(rzs->queue);
 }
 
+static int ramzswap_slot_free_notify(struct notifier_block *self,
+			unsigned long index, void *swap_file)
+{
+	struct block_device *bdev;
+	struct inode *inode;
+	struct ramzswap *rzs;
+
+	inode = ((struct file *)swap_file)->f_mapping->host;
+	bdev = I_BDEV(inode);
+	rzs = bdev->bd_disk->private_data;
+
+	ramzswap_free_page(rzs, index);
+	stat_inc(rzs->stats.notify_free);
+	return 0;
+}
+
+static struct notifier_block ramzswap_slot_free_nb = {
+	.notifier_call = ramzswap_slot_free_notify
+};
+
+static int ramzswap_swapon_notify(struct notifier_block *self,
+			unsigned long swap_id, void *swap_file)
+{
+	int ret = 0;
+
+	ret = register_swap_event_notifier(&ramzswap_slot_free_nb,
+				SWAP_EVENT_SLOT_FREE, swap_id);
+	if (ret)
+		pr_err("Error registering swap free notifier\n");
+	return ret;
+}
+
+static int ramzswap_swapoff_notify(struct notifier_block *self,
+			unsigned long swap_id, void *swap_file)
+{
+	unregister_swap_event_notifier(&ramzswap_slot_free_nb,
+				SWAP_EVENT_SLOT_FREE, swap_id);
+	return 0;
+}
+
+
+static struct notifier_block ramzswap_swapon_nb = {
+	.notifier_call = ramzswap_swapon_notify
+};
+
+static struct notifier_block ramzswap_swapoff_nb = {
+	.notifier_call = ramzswap_swapoff_notify
+};
+
 static int __init ramzswap_init(void)
 {
-	int i;
+	int i, ret;
 
 	if (num_devices > max_num_devices) {
 		pr_warning("Invalid value for num_devices: %u\n",
-							num_devices);
+				num_devices);
 		return -EINVAL;
 	}
 
@@ -1419,17 +1443,32 @@ static int __init ramzswap_init(void)
 	/* Allocate the device array and initialize each one */
 	pr_info("Creating %u devices ...\n", num_devices);
 	devices = kzalloc(num_devices * sizeof(struct ramzswap), GFP_KERNEL);
-	if (!devices)
+	if (!devices) {
+		ret = -ENOMEM;
 		goto out;
+	}
 
 	for (i = 0; i < num_devices; i++)
 		create_device(&devices[i], i);
 
+	ret = register_swap_event_notifier(&ramzswap_swapon_nb,
+				SWAP_EVENT_SWAPON, 0);
+	if (ret) {
+		pr_err("Error registering swapon notifier\n");
+		goto out;
+	}
+
+	ret = register_swap_event_notifier(&ramzswap_swapoff_nb,
+				SWAP_EVENT_SWAPOFF, 0);
+	if (ret) {
+		pr_err("Error registering swapoff notifier\n");
+		goto out;
+	}
 	return 0;
 
 out:
 	unregister_blkdev(ramzswap_major, "ramzswap");
-	return -ENOMEM;
+	return ret;
 }
 
 static void __exit ramzswap_exit(void)
@@ -1437,6 +1476,11 @@ static void __exit ramzswap_exit(void)
 	int i;
 	struct ramzswap *rzs;
 
+	unregister_swap_event_notifier(&ramzswap_swapon_nb,
+				SWAP_EVENT_SWAPON, 0);
+	unregister_swap_event_notifier(&ramzswap_swapoff_nb,
+				SWAP_EVENT_SWAPOFF, 0);
+
 	for (i = 0; i < num_devices; i++) {
 		rzs = &devices[i];
 
diff --git a/drivers/staging/ramzswap/ramzswap_drv.h b/drivers/staging/ramzswap/ramzswap_drv.h
index f7f273f..350db81 100644
--- a/drivers/staging/ramzswap/ramzswap_drv.h
+++ b/drivers/staging/ramzswap/ramzswap_drv.h
@@ -143,7 +143,6 @@ struct ramzswap {
 	struct request_queue *queue;
 	struct gendisk *disk;
 	int init_done;
-	int init_notify_callback;
 	/*
 	 * This is limit on compressed data size (stats.compr_size)
 	 * Its applicable only when backing swap device is present.
@@ -162,6 +161,7 @@ struct ramzswap {
 	struct ramzswap_backing_extent *curr_extent;
 	struct list_head backing_swap_extent_list;
 	unsigned long num_extents;
+	struct notifier_block *slot_free_nb;
 	char backing_swap_name[MAX_SWAP_NAME_LEN];
 	struct block_device *backing_swap;
 	struct file *swap_file;

[PATCH RFC 2/2] example usage of swap notifiers in ramzswap

[Nitin Gupta]

This patch is against a version of ramzswap which uses module sepcific hacks
to register a callback for swap slot free event and does not use any notifier.

It shows improvments in the ramzswap module (in terms of code flow) that
resulted from swap notifier support added in patch 1/2.

Signed-off-by: Nitin Gupta <ngupta@vflare.org>

---

diff --git a/drivers/staging/ramzswap/ramzswap_drv.c b/drivers/staging/ramzswap/ramzswap_drv.c
index 1a7167f..1c5326e 100644
--- a/drivers/staging/ramzswap/ramzswap_drv.c
+++ b/drivers/staging/ramzswap/ramzswap_drv.c
@@ -647,19 +647,6 @@ out:
 	rzs->table[index].offset = 0;
 }
 
-/*
- * callback function called when swap_map[offset] == 0
- * i.e page at this swap offset is no longer used
- */
-static void ramzswap_free_notify(struct block_device *bdev,
-				unsigned long index)
-{
-	struct ramzswap *rzs = bdev->bd_disk->private_data;
-
-	ramzswap_free_page(rzs, index);
-	stat_inc(rzs->stats.notify_free);
-}
-
 static int handle_zero_page(struct bio *bio)
 {
 	void *user_mem;
@@ -760,11 +747,6 @@ static int ramzswap_read(struct ramzswap *rzs, struct bio *bio)
 	page = bio->bi_io_vec[0].bv_page;
 	index = bio->bi_sector >> SECTORS_PER_PAGE_SHIFT;
 
-	if (unlikely(!rzs->init_notify_callback) && PageSwapCache(page)) {
-		set_ramzswap_free_notify(bio->bi_bdev, ramzswap_free_notify);
-		rzs->init_notify_callback = 1;
-	}
-
 	if (rzs_test_flag(rzs, index, RZS_ZERO))
 		return handle_zero_page(bio);
 
@@ -1318,13 +1300,6 @@ static int ramzswap_ioctl(struct block_device *bdev, fmode_t mode,
 			goto out;
 		}
 		ret = ramzswap_ioctl_reset_device(rzs);
-		/*
-		 * Racy! Device has already been swapoff'ed.  Bad things
-		 * can happen if another swapon is done before this reset.
-		 * TODO: A callback from swapoff() will solve this problem.
-		 */
-		set_ramzswap_free_notify(bdev, NULL);
-		rzs->init_notify_callback = 0;
 		break;
 
 	default:
@@ -1395,13 +1370,62 @@ static void destroy_device(struct ramzswap *rzs)
 		blk_cleanup_queue(rzs->queue);
 }
 
+static int ramzswap_slot_free_notify(struct notifier_block *self,
+			unsigned long index, void *swap_file)
+{
+	struct block_device *bdev;
+	struct inode *inode;
+	struct ramzswap *rzs;
+
+	inode = ((struct file *)swap_file)->f_mapping->host;
+	bdev = I_BDEV(inode);
+	rzs = bdev->bd_disk->private_data;
+
+	ramzswap_free_page(rzs, index);
+	stat_inc(rzs->stats.notify_free);
+	return 0;
+}
+
+static struct notifier_block ramzswap_slot_free_nb = {
+	.notifier_call = ramzswap_slot_free_notify
+};
+
+static int ramzswap_swapon_notify(struct notifier_block *self,
+			unsigned long swap_id, void *swap_file)
+{
+	int ret = 0;
+
+	ret = register_swap_event_notifier(&ramzswap_slot_free_nb,
+				SWAP_EVENT_SLOT_FREE, swap_id);
+	if (ret)
+		pr_err("Error registering swap free notifier\n");
+	return ret;
+}
+
+static int ramzswap_swapoff_notify(struct notifier_block *self,
+			unsigned long swap_id, void *swap_file)
+{
+	unregister_swap_event_notifier(&ramzswap_slot_free_nb,
+				SWAP_EVENT_SLOT_FREE, swap_id);
+	return 0;
+}
+
+
+static struct notifier_block ramzswap_swapon_nb = {
+	.notifier_call = ramzswap_swapon_notify
+};
+
+static struct notifier_block ramzswap_swapoff_nb = {
+	.notifier_call = ramzswap_swapoff_notify
+};
+
 static int __init ramzswap_init(void)
 {
-	int i;
+	int i, ret;
 
 	if (num_devices > max_num_devices) {
 		pr_warning("Invalid value for num_devices: %u\n",
-							num_devices);
+				num_devices);
 		return -EINVAL;
 	}
 
@@ -1419,17 +1443,32 @@ static int __init ramzswap_init(void)
 	/* Allocate the device array and initialize each one */
 	pr_info("Creating %u devices ...\n", num_devices);
 	devices = kzalloc(num_devices * sizeof(struct ramzswap), GFP_KERNEL);
-	if (!devices)
+	if (!devices) {
+		ret = -ENOMEM;
 		goto out;
+	}
 
 	for (i = 0; i < num_devices; i++)
 		create_device(&devices[i], i);
 
+	ret = register_swap_event_notifier(&ramzswap_swapon_nb,
+				SWAP_EVENT_SWAPON, 0);
+	if (ret) {
+		pr_err("Error registering swapon notifier\n");
+		goto out;
+	}
+
+	ret = register_swap_event_notifier(&ramzswap_swapoff_nb,
+				SWAP_EVENT_SWAPOFF, 0);
+	if (ret) {
+		pr_err("Error registering swapoff notifier\n");
+		goto out;
+	}
 	return 0;
 
 out:
 	unregister_blkdev(ramzswap_major, "ramzswap");
-	return -ENOMEM;
+	return ret;
 }
 
 static void __exit ramzswap_exit(void)
@@ -1437,6 +1476,11 @@ static void __exit ramzswap_exit(void)
 	int i;
 	struct ramzswap *rzs;
 
+	unregister_swap_event_notifier(&ramzswap_swapon_nb,
+				SWAP_EVENT_SWAPON, 0);
+	unregister_swap_event_notifier(&ramzswap_swapoff_nb,
+				SWAP_EVENT_SWAPOFF, 0);
+
 	for (i = 0; i < num_devices; i++) {
 		rzs = &devices[i];
 
diff --git a/drivers/staging/ramzswap/ramzswap_drv.h b/drivers/staging/ramzswap/ramzswap_drv.h
index f7f273f..350db81 100644
--- a/drivers/staging/ramzswap/ramzswap_drv.h
+++ b/drivers/staging/ramzswap/ramzswap_drv.h
@@ -143,7 +143,6 @@ struct ramzswap {
 	struct request_queue *queue;
 	struct gendisk *disk;
 	int init_done;
-	int init_notify_callback;
 	/*
 	 * This is limit on compressed data size (stats.compr_size)
 	 * Its applicable only when backing swap device is present.
@@ -162,6 +161,7 @@ struct ramzswap {
 	struct ramzswap_backing_extent *curr_extent;
 	struct list_head backing_swap_extent_list;
 	unsigned long num_extents;
+	struct notifier_block *slot_free_nb;
 	char backing_swap_name[MAX_SWAP_NAME_LEN];
 	struct block_device *backing_swap;
 	struct file *swap_file;

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH RFC 1/2] Add notifiers for various swap events

[KAMEZAWA Hiroyuki]

On Mon, 21 Sep 2009 19:03:59 +0530
Nitin Gupta <ngupta@vflare.org> wrote:

> Add notifiers for following swap events:
>  - Swapon
>  - Swapoff
>  - When a swap slot is freed
> 
> This is required for ramzswap module which implements RAM based block
> devices to be used as swap disks. These devices require a notification
> on these events to function properly (as shown in patch 2/2).
> 
> Currently, I'm not sure if any of these event notifiers have any other
> users. However, adding ramzswap specific hooks instead of this generic
> approach resulted in a bad/hacky code.
> 
Hmm ? if it's not necessary to make ramzswap as module, for-ramzswap-only
code is much easier to read..



> For SWAP_EVENT_SLOT_FREE, callbacks are made under swap_lock. Currently, this
> is not a problem since ramzswap is the only user and the callback it registers
> can be safely made under this lock. However, if this event finds more users,
> we might have to work on reducing contention on this lock.
> 
> Signed-off-by: Nitin Gupta <ngupta@vflare.org>
> 

In general, notifier chain codes allowed to return NOTIFY_BAD.
But this patch just assumes all chains should return NOTIFY_OK or
just ignore return code.

That's not good as generic interface, I think.

Thanks,
-Kame


> ---
>  include/linux/swap.h |   12 +++++++++
>  mm/swapfile.c        |   67 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 79 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/swap.h b/include/linux/swap.h
> index 7c15334..2873aad 100644
> --- a/include/linux/swap.h
> +++ b/include/linux/swap.h
> @@ -127,6 +127,12 @@ enum {
>  	SWP_SCANNING	= (1 << 8),	/* refcount in scan_swap_map */
>  };
>  
> +enum swap_event {
> +	SWAP_EVENT_SWAPON,
> +	SWAP_EVENT_SWAPOFF,
> +	SWAP_EVENT_SLOT_FREE,
> +};
> +
>  #define SWAP_CLUSTER_MAX 32
>  
>  #define SWAP_MAP_MAX	0x7ffe
> @@ -155,6 +161,7 @@ struct swap_info_struct {
>  	unsigned int max;
>  	unsigned int inuse_pages;
>  	unsigned int old_block_size;
> +	struct atomic_notifier_head slot_free_notify_list;
>  };
>  
>  struct swap_list_t {
> @@ -295,8 +302,13 @@ extern sector_t swapdev_block(int, pgoff_t);
>  extern struct swap_info_struct *get_swap_info_struct(unsigned);
>  extern int reuse_swap_page(struct page *);
>  extern int try_to_free_swap(struct page *);
> +extern int register_swap_event_notifier(struct notifier_block *nb,
> +                                enum swap_event event, unsigned long val);
> +extern int unregister_swap_event_notifier(struct notifier_block *nb,
> +                                enum swap_event event, unsigned long val);
>  struct backing_dev_info;
>  
> +
>  /* linux/mm/thrash.c */
>  extern struct mm_struct *swap_token_mm;
>  extern void grab_swap_token(struct mm_struct *);
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index 74f1102..f63643c 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -52,6 +52,9 @@ static struct swap_list_t swap_list = {-1, -1};
>  static struct swap_info_struct swap_info[MAX_SWAPFILES];
>  
>  static DEFINE_MUTEX(swapon_mutex);
> +static BLOCKING_NOTIFIER_HEAD(swapon_notify_list);
> +static BLOCKING_NOTIFIER_HEAD(swapoff_notify_list);
> +
>  
>  /* For reference count accounting in swap_map */
>  /* enum for swap_map[] handling. internal use only */
> @@ -585,6 +588,8 @@ static int swap_entry_free(struct swap_info_struct *p,
>  			swap_list.next = p - swap_info;
>  		nr_swap_pages++;
>  		p->inuse_pages--;
> +		atomic_notifier_call_chain(&p->slot_free_notify_list,
> +					offset, p->swap_file);
>  	}
>  	if (!swap_count(count))
>  		mem_cgroup_uncharge_swap(ent);
> @@ -1626,6 +1631,7 @@ SYSCALL_DEFINE1(swapoff, const char __user *, specialfile)
>  	p->swap_map = NULL;
>  	p->flags = 0;
>  	spin_unlock(&swap_lock);
> +	blocking_notifier_call_chain(&swapoff_notify_list, type, swap_file);
>  	mutex_unlock(&swapon_mutex);
>  	vfree(swap_map);
>  	/* Destroy swap account informatin */
> @@ -2014,7 +2020,9 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
>  	} else {
>  		swap_info[prev].next = p - swap_info;
>  	}
> +	ATOMIC_INIT_NOTIFIER_HEAD(&p->slot_free_notify_list);
>  	spin_unlock(&swap_lock);
> +	blocking_notifier_call_chain(&swapon_notify_list, type, swap_file);
>  	mutex_unlock(&swapon_mutex);
>  	error = 0;
>  	goto out;
> @@ -2216,3 +2224,62 @@ int valid_swaphandles(swp_entry_t entry, unsigned long *offset)
>  	*offset = ++toff;
>  	return nr_pages? ++nr_pages: 0;
>  }
> +
> +int register_swap_event_notifier(struct notifier_block *nb,
> +				enum swap_event event, unsigned long val)
> +{
> +	switch (event) {
> +	case SWAP_EVENT_SWAPON:
> +		return blocking_notifier_chain_register(
> +					&swapon_notify_list, nb);
> +	case SWAP_EVENT_SWAPOFF:
> +		return blocking_notifier_chain_register(
> +					&swapoff_notify_list, nb);
> +	case SWAP_EVENT_SLOT_FREE:
> +		{
> +		struct swap_info_struct *sis;
> +
> +		if (val > nr_swapfiles)
> +			goto out;
> +		sis = get_swap_info_struct(val);
> +		return atomic_notifier_chain_register(
> +				&sis->slot_free_notify_list, nb);
> +		}
> +	default:
> +		pr_err("Invalid swap event: %d\n", event);
> +	};
> +
> +out:
> +	return -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(register_swap_event_notifier);
> +
> +int unregister_swap_event_notifier(struct notifier_block *nb,
> +				enum swap_event event, unsigned long val)
> +{
> +	switch (event) {
> +	case SWAP_EVENT_SWAPON:
> +		return blocking_notifier_chain_unregister(
> +					&swapon_notify_list, nb);
> +	case SWAP_EVENT_SWAPOFF:
> +		return blocking_notifier_chain_unregister(
> +					&swapoff_notify_list, nb);
> +	case SWAP_EVENT_SLOT_FREE:
> +		{
> +		struct swap_info_struct *sis;
> +
> +		if (val > nr_swapfiles)
> +			goto out;
> +		sis = get_swap_info_struct(val);
> +		return atomic_notifier_chain_unregister(
> +				&sis->slot_free_notify_list, nb);
> +		}
> +	default:
> +		pr_err("Invalid swap event: %d\n", event);
> +	};
> +
> +out:
> +	return -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(unregister_swap_event_notifier);
> +
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
> 

Re: [PATCH RFC 1/2] Add notifiers for various swap events

[KAMEZAWA Hiroyuki]

On Mon, 21 Sep 2009 19:03:59 +0530
Nitin Gupta <ngupta@vflare.org> wrote:

> Add notifiers for following swap events:
>  - Swapon
>  - Swapoff
>  - When a swap slot is freed
> 
> This is required for ramzswap module which implements RAM based block
> devices to be used as swap disks. These devices require a notification
> on these events to function properly (as shown in patch 2/2).
> 
> Currently, I'm not sure if any of these event notifiers have any other
> users. However, adding ramzswap specific hooks instead of this generic
> approach resulted in a bad/hacky code.
> 
Hmm ? if it's not necessary to make ramzswap as module, for-ramzswap-only
code is much easier to read..



> For SWAP_EVENT_SLOT_FREE, callbacks are made under swap_lock. Currently, this
> is not a problem since ramzswap is the only user and the callback it registers
> can be safely made under this lock. However, if this event finds more users,
> we might have to work on reducing contention on this lock.
> 
> Signed-off-by: Nitin Gupta <ngupta@vflare.org>
> 

In general, notifier chain codes allowed to return NOTIFY_BAD.
But this patch just assumes all chains should return NOTIFY_OK or
just ignore return code.

That's not good as generic interface, I think.

Thanks,
-Kame


> ---
>  include/linux/swap.h |   12 +++++++++
>  mm/swapfile.c        |   67 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 79 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/swap.h b/include/linux/swap.h
> index 7c15334..2873aad 100644
> --- a/include/linux/swap.h
> +++ b/include/linux/swap.h
> @@ -127,6 +127,12 @@ enum {
>  	SWP_SCANNING	= (1 << 8),	/* refcount in scan_swap_map */
>  };
>  
> +enum swap_event {
> +	SWAP_EVENT_SWAPON,
> +	SWAP_EVENT_SWAPOFF,
> +	SWAP_EVENT_SLOT_FREE,
> +};
> +
>  #define SWAP_CLUSTER_MAX 32
>  
>  #define SWAP_MAP_MAX	0x7ffe
> @@ -155,6 +161,7 @@ struct swap_info_struct {
>  	unsigned int max;
>  	unsigned int inuse_pages;
>  	unsigned int old_block_size;
> +	struct atomic_notifier_head slot_free_notify_list;
>  };
>  
>  struct swap_list_t {
> @@ -295,8 +302,13 @@ extern sector_t swapdev_block(int, pgoff_t);
>  extern struct swap_info_struct *get_swap_info_struct(unsigned);
>  extern int reuse_swap_page(struct page *);
>  extern int try_to_free_swap(struct page *);
> +extern int register_swap_event_notifier(struct notifier_block *nb,
> +                                enum swap_event event, unsigned long val);
> +extern int unregister_swap_event_notifier(struct notifier_block *nb,
> +                                enum swap_event event, unsigned long val);
>  struct backing_dev_info;
>  
> +
>  /* linux/mm/thrash.c */
>  extern struct mm_struct *swap_token_mm;
>  extern void grab_swap_token(struct mm_struct *);
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index 74f1102..f63643c 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -52,6 +52,9 @@ static struct swap_list_t swap_list = {-1, -1};
>  static struct swap_info_struct swap_info[MAX_SWAPFILES];
>  
>  static DEFINE_MUTEX(swapon_mutex);
> +static BLOCKING_NOTIFIER_HEAD(swapon_notify_list);
> +static BLOCKING_NOTIFIER_HEAD(swapoff_notify_list);
> +
>  
>  /* For reference count accounting in swap_map */
>  /* enum for swap_map[] handling. internal use only */
> @@ -585,6 +588,8 @@ static int swap_entry_free(struct swap_info_struct *p,
>  			swap_list.next = p - swap_info;
>  		nr_swap_pages++;
>  		p->inuse_pages--;
> +		atomic_notifier_call_chain(&p->slot_free_notify_list,
> +					offset, p->swap_file);
>  	}
>  	if (!swap_count(count))
>  		mem_cgroup_uncharge_swap(ent);
> @@ -1626,6 +1631,7 @@ SYSCALL_DEFINE1(swapoff, const char __user *, specialfile)
>  	p->swap_map = NULL;
>  	p->flags = 0;
>  	spin_unlock(&swap_lock);
> +	blocking_notifier_call_chain(&swapoff_notify_list, type, swap_file);
>  	mutex_unlock(&swapon_mutex);
>  	vfree(swap_map);
>  	/* Destroy swap account informatin */
> @@ -2014,7 +2020,9 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
>  	} else {
>  		swap_info[prev].next = p - swap_info;
>  	}
> +	ATOMIC_INIT_NOTIFIER_HEAD(&p->slot_free_notify_list);
>  	spin_unlock(&swap_lock);
> +	blocking_notifier_call_chain(&swapon_notify_list, type, swap_file);
>  	mutex_unlock(&swapon_mutex);
>  	error = 0;
>  	goto out;
> @@ -2216,3 +2224,62 @@ int valid_swaphandles(swp_entry_t entry, unsigned long *offset)
>  	*offset = ++toff;
>  	return nr_pages? ++nr_pages: 0;
>  }
> +
> +int register_swap_event_notifier(struct notifier_block *nb,
> +				enum swap_event event, unsigned long val)
> +{
> +	switch (event) {
> +	case SWAP_EVENT_SWAPON:
> +		return blocking_notifier_chain_register(
> +					&swapon_notify_list, nb);
> +	case SWAP_EVENT_SWAPOFF:
> +		return blocking_notifier_chain_register(
> +					&swapoff_notify_list, nb);
> +	case SWAP_EVENT_SLOT_FREE:
> +		{
> +		struct swap_info_struct *sis;
> +
> +		if (val > nr_swapfiles)
> +			goto out;
> +		sis = get_swap_info_struct(val);
> +		return atomic_notifier_chain_register(
> +				&sis->slot_free_notify_list, nb);
> +		}
> +	default:
> +		pr_err("Invalid swap event: %d\n", event);
> +	};
> +
> +out:
> +	return -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(register_swap_event_notifier);
> +
> +int unregister_swap_event_notifier(struct notifier_block *nb,
> +				enum swap_event event, unsigned long val)
> +{
> +	switch (event) {
> +	case SWAP_EVENT_SWAPON:
> +		return blocking_notifier_chain_unregister(
> +					&swapon_notify_list, nb);
> +	case SWAP_EVENT_SWAPOFF:
> +		return blocking_notifier_chain_unregister(
> +					&swapoff_notify_list, nb);
> +	case SWAP_EVENT_SLOT_FREE:
> +		{
> +		struct swap_info_struct *sis;
> +
> +		if (val > nr_swapfiles)
> +			goto out;
> +		sis = get_swap_info_struct(val);
> +		return atomic_notifier_chain_unregister(
> +				&sis->slot_free_notify_list, nb);
> +		}
> +	default:
> +		pr_err("Invalid swap event: %d\n", event);
> +	};
> +
> +out:
> +	return -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(unregister_swap_event_notifier);
> +
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
> 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH RFC 1/2] Add notifiers for various swap events

[Nitin Gupta]

On 09/24/2009 07:17 AM, KAMEZAWA Hiroyuki wrote:
> On Mon, 21 Sep 2009 19:03:59 +0530
> Nitin Gupta <ngupta@vflare.org> wrote:
> 
>> Add notifiers for following swap events:
>>  - Swapon
>>  - Swapoff
>>  - When a swap slot is freed
>>
>> This is required for ramzswap module which implements RAM based block
>> devices to be used as swap disks. These devices require a notification
>> on these events to function properly (as shown in patch 2/2).
>>
>> Currently, I'm not sure if any of these event notifiers have any other
>> users. However, adding ramzswap specific hooks instead of this generic
>> approach resulted in a bad/hacky code.
>>
> Hmm ? if it's not necessary to make ramzswap as module, for-ramzswap-only
> code is much easier to read..
>

The patches posted earlier (v3 patches) inserts special hooks for swap slot
free event only. In this version, the callback is set when we get first R/W request.
Actually ramzswap needs callback for swapon/swapoff too but I just didn't do it.

Then Pekka posted test patch that allows setting this callback during swapon
itself. Looking that all these patches, I realized its already too messy even
if we just make everything ramzswap specific.
Just FYI, Pekka's test patch:
http://patchwork.kernel.org/patch/48472/

Then I added this generic notifier interface which, compared to earlier version,
looks much cleaner. The code to add these notifiers is also very small.
 
> 
> 
>> For SWAP_EVENT_SLOT_FREE, callbacks are made under swap_lock. Currently, this
>> is not a problem since ramzswap is the only user and the callback it registers
>> can be safely made under this lock. However, if this event finds more users,
>> we might have to work on reducing contention on this lock.
>>
>> Signed-off-by: Nitin Gupta <ngupta@vflare.org>
>>
> 
> In general, notifier chain codes allowed to return NOTIFY_BAD.
> But this patch just assumes all chains should return NOTIFY_OK or
> just ignore return code.
> 
> That's not good as generic interface, I think.


What action we can take here if the notifier_call_chain() returns an error (apart
from maybe printing an error)? Perhaps we can add a warning in case of swapon/off
events but not in case of swap slot free event which is called under swap_lock.



Thanks,
Nitin

Re: [PATCH RFC 1/2] Add notifiers for various swap events

[Nitin Gupta]

On 09/24/2009 07:17 AM, KAMEZAWA Hiroyuki wrote:
> On Mon, 21 Sep 2009 19:03:59 +0530
> Nitin Gupta <ngupta@vflare.org> wrote:
> 
>> Add notifiers for following swap events:
>>  - Swapon
>>  - Swapoff
>>  - When a swap slot is freed
>>
>> This is required for ramzswap module which implements RAM based block
>> devices to be used as swap disks. These devices require a notification
>> on these events to function properly (as shown in patch 2/2).
>>
>> Currently, I'm not sure if any of these event notifiers have any other
>> users. However, adding ramzswap specific hooks instead of this generic
>> approach resulted in a bad/hacky code.
>>
> Hmm ? if it's not necessary to make ramzswap as module, for-ramzswap-only
> code is much easier to read..
>

The patches posted earlier (v3 patches) inserts special hooks for swap slot
free event only. In this version, the callback is set when we get first R/W request.
Actually ramzswap needs callback for swapon/swapoff too but I just didn't do it.

Then Pekka posted test patch that allows setting this callback during swapon
itself. Looking that all these patches, I realized its already too messy even
if we just make everything ramzswap specific.
Just FYI, Pekka's test patch:
http://patchwork.kernel.org/patch/48472/

Then I added this generic notifier interface which, compared to earlier version,
looks much cleaner. The code to add these notifiers is also very small.
 
> 
> 
>> For SWAP_EVENT_SLOT_FREE, callbacks are made under swap_lock. Currently, this
>> is not a problem since ramzswap is the only user and the callback it registers
>> can be safely made under this lock. However, if this event finds more users,
>> we might have to work on reducing contention on this lock.
>>
>> Signed-off-by: Nitin Gupta <ngupta@vflare.org>
>>
> 
> In general, notifier chain codes allowed to return NOTIFY_BAD.
> But this patch just assumes all chains should return NOTIFY_OK or
> just ignore return code.
> 
> That's not good as generic interface, I think.


What action we can take here if the notifier_call_chain() returns an error (apart
from maybe printing an error)? Perhaps we can add a warning in case of swapon/off
events but not in case of swap slot free event which is called under swap_lock.



Thanks,
Nitin

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH RFC 1/2] Add notifiers for various swap events

[KAMEZAWA Hiroyuki]

On Thu, 24 Sep 2009 08:40:56 +0530
Nitin Gupta <ngupta@vflare.org> wrote:

> On 09/24/2009 07:17 AM, KAMEZAWA Hiroyuki wrote:
> > On Mon, 21 Sep 2009 19:03:59 +0530
> > Nitin Gupta <ngupta@vflare.org> wrote:
> > 
> >> Add notifiers for following swap events:
> >>  - Swapon
> >>  - Swapoff
> >>  - When a swap slot is freed
> >>
> >> This is required for ramzswap module which implements RAM based block
> >> devices to be used as swap disks. These devices require a notification
> >> on these events to function properly (as shown in patch 2/2).
> >>
> >> Currently, I'm not sure if any of these event notifiers have any other
> >> users. However, adding ramzswap specific hooks instead of this generic
> >> approach resulted in a bad/hacky code.
> >>
> > Hmm ? if it's not necessary to make ramzswap as module, for-ramzswap-only
> > code is much easier to read..
> >
> 
> The patches posted earlier (v3 patches) inserts special hooks for swap slot
> free event only. In this version, the callback is set when we get first R/W request.
> Actually ramzswap needs callback for swapon/swapoff too but I just didn't do it.
> 
> Then Pekka posted test patch that allows setting this callback during swapon
> itself. Looking that all these patches, I realized its already too messy even
> if we just make everything ramzswap specific.
> Just FYI, Pekka's test patch:
> http://patchwork.kernel.org/patch/48472/
> 
> Then I added this generic notifier interface which, compared to earlier version,
> looks much cleaner. The code to add these notifiers is also very small.
>  
ya, yes. the patch itsels seems clean.

> > 
> > 
> >> For SWAP_EVENT_SLOT_FREE, callbacks are made under swap_lock. Currently, this
> >> is not a problem since ramzswap is the only user and the callback it registers
> >> can be safely made under this lock. However, if this event finds more users,
> >> we might have to work on reducing contention on this lock.
> >>
> >> Signed-off-by: Nitin Gupta <ngupta@vflare.org>
> >>
> > 
> > In general, notifier chain codes allowed to return NOTIFY_BAD.
> > But this patch just assumes all chains should return NOTIFY_OK or
> > just ignore return code.
> > 
> > That's not good as generic interface, I think.
> 
> 
> What action we can take here if the notifier_call_chain() returns an error (apart
> from maybe printing an error)? Perhaps we can add a warning in case of swapon/off
> events but not in case of swap slot free event which is called under swap_lock.
> 
If return code is ignored, please add commentary at least.

I wonder I may able to move memcg's swap_cgroup code for swapon/swapoff onto this
notifier. (swap_cgroup_swapon/swap_cgroup_swapoff) But it seems not.
sorry for bothering you.

Thanks,
-Kame


> 
> 
> Thanks,
> Nitin
> 

Re: [PATCH RFC 1/2] Add notifiers for various swap events

[KAMEZAWA Hiroyuki]

On Thu, 24 Sep 2009 08:40:56 +0530
Nitin Gupta <ngupta@vflare.org> wrote:

> On 09/24/2009 07:17 AM, KAMEZAWA Hiroyuki wrote:
> > On Mon, 21 Sep 2009 19:03:59 +0530
> > Nitin Gupta <ngupta@vflare.org> wrote:
> > 
> >> Add notifiers for following swap events:
> >>  - Swapon
> >>  - Swapoff
> >>  - When a swap slot is freed
> >>
> >> This is required for ramzswap module which implements RAM based block
> >> devices to be used as swap disks. These devices require a notification
> >> on these events to function properly (as shown in patch 2/2).
> >>
> >> Currently, I'm not sure if any of these event notifiers have any other
> >> users. However, adding ramzswap specific hooks instead of this generic
> >> approach resulted in a bad/hacky code.
> >>
> > Hmm ? if it's not necessary to make ramzswap as module, for-ramzswap-only
> > code is much easier to read..
> >
> 
> The patches posted earlier (v3 patches) inserts special hooks for swap slot
> free event only. In this version, the callback is set when we get first R/W request.
> Actually ramzswap needs callback for swapon/swapoff too but I just didn't do it.
> 
> Then Pekka posted test patch that allows setting this callback during swapon
> itself. Looking that all these patches, I realized its already too messy even
> if we just make everything ramzswap specific.
> Just FYI, Pekka's test patch:
> http://patchwork.kernel.org/patch/48472/
> 
> Then I added this generic notifier interface which, compared to earlier version,
> looks much cleaner. The code to add these notifiers is also very small.
>  
ya, yes. the patch itsels seems clean.

> > 
> > 
> >> For SWAP_EVENT_SLOT_FREE, callbacks are made under swap_lock. Currently, this
> >> is not a problem since ramzswap is the only user and the callback it registers
> >> can be safely made under this lock. However, if this event finds more users,
> >> we might have to work on reducing contention on this lock.
> >>
> >> Signed-off-by: Nitin Gupta <ngupta@vflare.org>
> >>
> > 
> > In general, notifier chain codes allowed to return NOTIFY_BAD.
> > But this patch just assumes all chains should return NOTIFY_OK or
> > just ignore return code.
> > 
> > That's not good as generic interface, I think.
> 
> 
> What action we can take here if the notifier_call_chain() returns an error (apart
> from maybe printing an error)? Perhaps we can add a warning in case of swapon/off
> events but not in case of swap slot free event which is called under swap_lock.
> 
If return code is ignored, please add commentary at least.

I wonder I may able to move memcg's swap_cgroup code for swapon/swapoff onto this
notifier. (swap_cgroup_swapon/swap_cgroup_swapoff) But it seems not.
sorry for bothering you.

Thanks,
-Kame


> 
> 
> Thanks,
> Nitin
> 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH RFC 1/2] Add notifiers for various swap events

[Nitin Gupta]

On Thu, Sep 24, 2009 at 9:20 AM, KAMEZAWA Hiroyuki
<kamezawa.hiroyu@jp.fujitsu.com> wrote:

>> >
>> > In general, notifier chain codes allowed to return NOTIFY_BAD.
>> > But this patch just assumes all chains should return NOTIFY_OK or
>> > just ignore return code.
>> >
>> > That's not good as generic interface, I think.
>>
>>
>> What action we can take here if the notifier_call_chain() returns an error (apart
>> from maybe printing an error)? Perhaps we can add a warning in case of swapon/off
>> events but not in case of swap slot free event which is called under swap_lock.
>>
> If return code is ignored, please add commentary at least.
>

okay.

> I wonder I may able to move memcg's swap_cgroup code for swapon/swapoff onto this
> notifier. (swap_cgroup_swapon/swap_cgroup_swapoff) But it seems not.
> sorry for bothering you.
>

Thanks for your comments!

Nitin

Re: [PATCH RFC 1/2] Add notifiers for various swap events

[Nitin Gupta]

On Thu, Sep 24, 2009 at 9:20 AM, KAMEZAWA Hiroyuki
<kamezawa.hiroyu@jp.fujitsu.com> wrote:

>> >
>> > In general, notifier chain codes allowed to return NOTIFY_BAD.
>> > But this patch just assumes all chains should return NOTIFY_OK or
>> > just ignore return code.
>> >
>> > That's not good as generic interface, I think.
>>
>>
>> What action we can take here if the notifier_call_chain() returns an error (apart
>> from maybe printing an error)? Perhaps we can add a warning in case of swapon/off
>> events but not in case of swap slot free event which is called under swap_lock.
>>
> If return code is ignored, please add commentary at least.
>

okay.

> I wonder I may able to move memcg's swap_cgroup code for swapon/swapoff onto this
> notifier. (swap_cgroup_swapon/swap_cgroup_swapoff) But it seems not.
> sorry for bothering you.
>

Thanks for your comments!

Nitin

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>