On Tue, 29 Oct 2002, Christoph Hellwig wrote:
| On Mon, Oct 28, 2002 at 09:36:44PM -0800, Randy.Dunlap wrote:
| >
| > Hi,
| >
| > This patch to 2.5.44 converts /proc/swaps to use seq_file.
| >
| > It's basically the same patch that I posted a few days ago
| > with locking added [using swap_list_lock() and
| > swap_list_unlock(), as directed by Al].
| >
| > Any comments on this version?
|
| Looks fine. Any chance you could move proc_swaps_operations and
| the entry creating to swapfile.c so when uclinux makes this file
| conditional on CONFIG_SWAP we don't need ifdefs in proc_misc.c?
Are you and/or Greg U. going to do a CONFIG_SWAP option?
Sure, some (small) chance of moving it. I just tried that exercise,
and the results aren't very interesting, at least not to me, and
here's why.
I moved the proc_swaps_operations to swapfile.c like you asked.
I like that part of this patch.
But it makes more sense to me to move the create_seq_entry() call
to mm/swap.c:: in its __init swap_setup() function
(or add another __init function in swapfile.c to do this ?).
That meant that I had to duplicate create_seq_entry() or
export it. For now I have duplicated it, and that would make
3 copies of it in the kernel -- and that's bad IMO, so it would
need to be exported (but I didn't do that for now).
And then there's the issue of do we want to keep the
/proc misc entries closely located (in source code) or not...
I kinda like having them in one place.
And there's the question of __init ordering: when will the __init
function mm/swap.c::swap_setup(), calling create_seq_entry(),
happen in relation to create_proc_entry() being ready to work?
I.e., is there a chance that create_seq_entry() could fail
even though it shouldn't? This should be solvable by using
initcall levels (subsystem levels), but I didn't look closely
at that yet.
So yeah, it's do-able. I just don't see that it's worth it.
Patch below (to previous patch, which is already in BK) compiles.
Not tested.
-- ~Randy--- ./fs/proc/proc_misc.c%swap2 Mon Oct 21 20:11:48 2002 +++ ./fs/proc/proc_misc.c Tue Oct 29 20:17:11 2002 @@ -295,18 +295,6 @@ .release = seq_release, };
-extern struct seq_operations swaps_op; -static int swaps_open(struct inode *inode, struct file *file) -{ - return seq_open(file, &swaps_op); -} -static struct file_operations proc_swaps_operations = { - .open = swaps_open, - .read = seq_read, - .llseek = seq_lseek, - .release = seq_release, -}; - #ifdef CONFIG_MODULES extern struct seq_operations modules_op; static int modules_open(struct inode *inode, struct file *file) @@ -636,7 +624,6 @@ entry->proc_fops = &proc_kmsg_operations; create_seq_entry("cpuinfo", 0, &proc_cpuinfo_operations); create_seq_entry("partitions", 0, &proc_partitions_operations); - create_seq_entry("swaps", 0, &proc_swaps_operations); #if !defined(CONFIG_ARCH_S390) create_seq_entry("interrupts", 0, &proc_interrupts_operations); #endif --- ./mm/swapfile.c%swap2 Mon Oct 28 20:56:06 2002 +++ ./mm/swapfile.c Tue Oct 29 20:38:18 2002 @@ -6,6 +6,7 @@ */
#include <linux/config.h> +#include <linux/module.h> #include <linux/mm.h> #include <linux/slab.h> #include <linux/kernel_stat.h> @@ -1127,6 +1128,18 @@ .stop = swap_stop, .show = swap_show }; + +static int swaps_open(struct inode *inode, struct file *file) +{ + return seq_open(file, &swaps_op); +} +struct file_operations proc_swaps_operations = { + .open = swaps_open, + .read = seq_read, + .llseek = seq_lseek, + .release = seq_release, +}; +EXPORT_SYMBOL(proc_swaps_operations); #endif
/* --- ./mm/swap.c%swap2 Fri Oct 18 21:01:49 2002 +++ ./mm/swap.c Tue Oct 29 20:36:53 2002 @@ -13,6 +13,7 @@ * Buffermem limits added 12.3.98, Rik van Riel. */
+#include <linux/config.h> #include <linux/mm.h> #include <linux/kernel_stat.h> #include <linux/swap.h> @@ -22,6 +23,7 @@ #include <linux/mm_inline.h> #include <linux/buffer_head.h> #include <linux/prefetch.h> +#include <linux/proc_fs.h>
/* How many pages do we try to swap or page in/out together? */ int page_cluster; @@ -269,6 +271,18 @@ return pagevec_count(pvec); }
+#ifdef CONFIG_PROC_FS +static void __init create_seq_entry(char *name, mode_t mode, struct file_operations *f) +{ + struct proc_dir_entry *entry; + entry = create_proc_entry(name, mode, NULL); + if (entry) + entry->proc_fops = f; +} + +extern struct file_operations proc_swaps_operations; +#endif + /* * Perform any setup for the swap system */ @@ -285,4 +299,7 @@ * Right now other parts of the system means that we * _really_ don't want to cluster much more */ +#ifdef CONFIG_PROC_FS + create_seq_entry("swaps", 0, &proc_swaps_operations); +#endif } --- ./mm/Makefile%swap2 Fri Oct 18 21:02:00 2002 +++ ./mm/Makefile Tue Oct 29 20:39:20 2002 @@ -2,7 +2,8 @@ # Makefile for the linux memory manager. #
-export-objs := shmem.o filemap.o mempool.o page_alloc.o page-writeback.o +export-objs := shmem.o filemap.o mempool.o page_alloc.o page-writeback.o \ + swapfile.o
obj-y := memory.o mmap.o filemap.o mprotect.o mlock.o mremap.o \ vmalloc.o slab.o bootmem.o swap.o vmscan.o page_io.o \
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Thu Oct 31 2002 - 22:00:46 EST