Hi all
-> It seems to me this patch is a nothing-urgency one, but I hope it helps
While reading linux-2.4.0-test2/fs/proc/inode.c:de_put()
I saw a --FIXME: comment, I thought I could made a patch for it,
wich is attached below
Here are some notes about it
Note 0) 2.4.0-test2/include/linux/proc_fs.h
changed proc_dir_entry's count field from unsigned int to atomic_t
wich is signed int
Note 1) 2.4.0-test2/drivers/pci/proc.c:pci_proc_detach_device()
does not use the defered feature, it is a bug or a feature?
Something similar happens with nubus
Note 2) 2.4.0-test2/fs/proc/generic.c:remove_proc_entry()
IMHO:
In order to free_proc_entry be exclusive, the remove_proc_entry call
should be made exclusive, hopefully remove_proc_entry it's always called
in a exclusive way, at least for each proc_dir_entry.
It seems there is one exception: pci_remove_device(), which calls
pci_proc_detach_device(), and this finally calls remove_proc_entry().
I have not found any references to this function but an EXPORT_SYMBOL...
An authoritative person should aprove this or not.
The afected functions are:
i2o_proc_remove_controller()
i2o_proc_remove_device()
destroy_i2o_procfs()
proc_get_inode()
proc_delete_inode()
nubus_proc_detach_device()
pci_proc_detach_device()
remove_proc_entry()
unregister_proc_table()
I't not clear for me how test this right, that is make some tests that
say something useful. Any comments will be greatly appreciated.
Anyway the mofifications I made are rather trivial, so It seems for me it's
correct, also I stressed a system(2uP) without no problems,
so I decided to post it here to hear your feedback
Please CC me your comments and flames to uaca@alumni.uv.es
Regards
Ulisses
Debian/GNU Linux: a dream come true
-----------------------------------------------------------------------------
"Computers are useless. They can only give answers." Pablo Picasso
---> Visita http://www.valux.org/ para saber acerca de la <---
---> Asociación Valenciana de Usuarios de Linux <---
diff -10 -u --recursive --exclude *~ linux-2.4.0-test2/drivers/i2o/i2o_proc.c linux-2.4.0-test2-modified/drivers/i2o/i2o_proc.c
--- linux-2.4.0-test2/drivers/i2o/i2o_proc.c Wed Apr 12 18:38:53 2000
+++ linux-2.4.0-test2-modified/drivers/i2o/i2o_proc.c Thu Jul 13 07:30:03 2000
@@ -3230,41 +3230,41 @@
static void i2o_proc_remove_controller(struct i2o_controller *pctrl,
struct proc_dir_entry *parent)
{
char buff[10];
struct i2o_device *dev;
/* Remove unused device entries */
for(dev=pctrl->devices; dev; dev=dev->next)
i2o_proc_remove_device(dev);
- if(!pctrl->proc_entry->count)
+ if(!atomic_read(&pctrl->proc_entry->count))
{
sprintf(buff, "iop%d", pctrl->unit);
i2o_proc_remove_entries(generic_iop_entries, pctrl->proc_entry);
remove_proc_entry(buff, parent);
pctrl->proc_entry = NULL;
}
}
void i2o_proc_remove_device(struct i2o_device *dev)
{
struct proc_dir_entry *de=dev->proc_entry;
char dev_id[10];
sprintf(dev_id, "%0#5x", dev->lct_data.tid);
i2o_device_notify_off(dev, &i2o_proc_handler);
/* Would it be safe to remove _files_ even if they are in use? */
- if((de) && (!de->count))
+ if((de) && (!atomic_read(&de->count)))
{
i2o_proc_remove_entries(generic_dev_entries, de);
switch(dev->lct_data.class_id)
{
case I2O_CLASS_SCSI_PERIPHERAL:
case I2O_CLASS_RANDOM_BLOCK_STORAGE:
i2o_proc_remove_entries(rbs_dev_entries, de);
break;
case I2O_CLASS_LAN:
{
@@ -3327,21 +3327,21 @@
for(i = 0; i < MAX_I2O_CONTROLLERS; i++)
{
pctrl = i2o_find_controller(i);
if(pctrl)
{
i2o_proc_remove_controller(pctrl, i2o_proc_dir_root);
i2o_unlock_controller(pctrl);
}
}
- if(!i2o_proc_dir_root->count)
+ if(!atomic_read(&i2o_proc_dir_root->count))
remove_proc_entry("i2o", 0);
else
return -1;
return 0;
}
#ifdef MODULE
#define i2o_proc_init init_module
#endif
diff -10 -u --recursive --exclude *~ linux-2.4.0-test2/drivers/nubus/proc.c linux-2.4.0-test2-modified/drivers/nubus/proc.c
--- linux-2.4.0-test2/drivers/nubus/proc.c Sun Nov 28 00:27:48 1999
+++ linux-2.4.0-test2-modified/drivers/nubus/proc.c Thu Jul 13 07:27:44 2000
@@ -141,21 +141,21 @@
return 0;
}
/* FIXME: this is certainly broken! */
int nubus_proc_detach_device(struct nubus_dev *dev)
{
struct proc_dir_entry *e;
if ((e = dev->procdir)) {
- if (e->count)
+ if (atomic_read(&e->count))
return -EBUSY;
remove_proc_entry(e->name, proc_bus_nubus_dir);
dev->procdir = NULL;
}
return 0;
}
void __init proc_bus_nubus_add_devices(void)
{
struct nubus_dev *dev;
diff -10 -u --recursive --exclude *~ linux-2.4.0-test2/drivers/pci/proc.c linux-2.4.0-test2-modified/drivers/pci/proc.c
--- linux-2.4.0-test2/drivers/pci/proc.c Sat Apr 29 19:19:57 2000
+++ linux-2.4.0-test2-modified/drivers/pci/proc.c Thu Jul 13 09:22:08 2000
@@ -265,21 +265,21 @@
e->data = dev;
e->size = PCI_CFG_SPACE_SIZE;
return 0;
}
int pci_proc_detach_device(struct pci_dev *dev)
{
struct proc_dir_entry *e;
if ((e = dev->procent)) {
- if (e->count)
+ if (atomic_read(&e->count))
return -EBUSY;
remove_proc_entry(e->name, dev->bus->procdir);
dev->procent = NULL;
}
return 0;
}
/*
* Backward compatible /proc/pci interface.
diff -10 -u --recursive --exclude *~ linux-2.4.0-test2/fs/proc/generic.c linux-2.4.0-test2-modified/fs/proc/generic.c
--- linux-2.4.0-test2/fs/proc/generic.c Thu Jun 22 16:09:45 2000
+++ linux-2.4.0-test2-modified/fs/proc/generic.c Thu Jul 13 09:10:36 2000
@@ -549,38 +549,49 @@
/*
* Remove a /proc entry and free it if it's not currently in use.
* If it is in use, we set the 'deleted' flag.
*/
void remove_proc_entry(const char *name, struct proc_dir_entry *parent)
{
struct proc_dir_entry **p;
struct proc_dir_entry *de;
const char *fn = name;
int len;
+ int count;
if (!parent && xlate_proc_name(name, &parent, &fn) != 0)
goto out;
len = strlen(fn);
for (p = &parent->subdir; *p; p=&(*p)->next ) {
if (!proc_match(len, fn, *p))
continue;
de = *p;
*p = de->next;
de->next = NULL;
if (S_ISDIR(de->mode))
parent->nlink--;
clear_bit(de->low_ino-PROC_DYNAMIC_FIRST,
(void *) proc_alloc_map);
proc_kill_inodes(de);
de->nlink = 0;
de->deleted = 1;
- if (!de->count)
+
+ /* IMHO:
+ *
+ * Seems that in order to free_proc_entry be exclusive,
+ * this remove_proc_entry call should be made exclusive
+ *
+ * Hopefully remove_proc_entry it's always called in a exclusive way,
+ * at least for each proc_dir_entry. An authoritative person should
+ * aprove this or not.
+ */
+ if (!(count= atomic_read(&de->count)))
free_proc_entry(de);
else {
printk("remove_proc_entry: %s/%s busy, count=%d\n",
- parent->name, de->name, de->count);
+ parent->name, de->name, count);
}
break;
}
out:
return;
}
diff -10 -u --recursive --exclude *~ linux-2.4.0-test2/fs/proc/inode.c linux-2.4.0-test2-modified/fs/proc/inode.c
--- linux-2.4.0-test2/fs/proc/inode.c Wed Jun 21 16:25:17 2000
+++ linux-2.4.0-test2-modified/fs/proc/inode.c Thu Jul 13 23:01:27 2000
@@ -12,50 +12,50 @@
#include <linux/stat.h>
#include <linux/file.h>
#include <linux/locks.h>
#include <linux/limits.h>
#define __NO_VERSION__
#include <linux/module.h>
#include <linux/smp_lock.h>
#include <asm/system.h>
#include <asm/uaccess.h>
+#include <asm/atomic.h>
extern void free_proc_entry(struct proc_dir_entry *);
struct proc_dir_entry * de_get(struct proc_dir_entry *de)
{
if (de)
- de->count++;
+ atomic_inc(&de->count);
return de;
}
/*
* Decrements the use count and checks for deferred deletion.
*/
void de_put(struct proc_dir_entry *de)
{
if (de) {
- lock_kernel(); /* FIXME: count should be atomic_t */
- if (!de->count) {
+ if (!atomic_read(&de->count)) {
printk("de_put: entry %s already free!\n", de->name);
return;
}
- if (!--de->count) {
+ /* Only one will test de->deleted */
+ if (!atomic_dec_and_test(&de->count)) {
if (de->deleted) {
printk("de_put: deferred delete of %s\n",
de->name);
free_proc_entry(de);
}
}
- unlock_kernel();
}
}
/*
* Decrement the use count of the proc_dir_entry.
*/
static void proc_delete_inode(struct inode *inode)
{
struct proc_dir_entry *de = inode->u.generic_ip;
@@ -132,21 +132,21 @@
{
struct inode * inode;
/*
* Increment the use count so the dir entry can't disappear.
*/
de_get(de);
#if 1
/* shouldn't ever happen */
if (de && de->deleted)
-printk("proc_iget: using deleted entry %s, count=%d\n", de->name, de->count);
+printk("proc_iget: using deleted entry %s, count=%d\n", de->name, atomic_read(&de->count));
#endif
inode = iget(sb, ino);
if (!inode)
goto out_fail;
inode->u.generic_ip = (void *) de;
if (de) {
if (de->mode) {
inode->i_mode = de->mode;
Only in linux-2.4.0-test2/include/asm-i386: log
diff -10 -u --recursive --exclude *~ linux-2.4.0-test2/include/linux/proc_fs.h linux-2.4.0-test2-modified/include/linux/proc_fs.h
--- linux-2.4.0-test2/include/linux/proc_fs.h Sun Jul 9 12:47:42 2000
+++ linux-2.4.0-test2-modified/include/linux/proc_fs.h Thu Jul 13 13:27:04 2000
@@ -1,16 +1,21 @@
#ifndef _LINUX_PROC_FS_H
#define _LINUX_PROC_FS_H
#include <linux/config.h>
#include <linux/malloc.h>
+/* Changed count field of proc_dir_entry to be atomic_t
+ * and modified references to it.
+ * Ulisses Alonso Camaró <uaca@alumni.uv.es> July, 11 2000
+ */
+
/*
* The proc filesystem constants/structures
*/
/*
* Offset of the first process in the /proc root directory..
*/
#define FIRST_PROCESS_ENTRY 256
@@ -60,21 +65,21 @@
gid_t gid;
unsigned long size;
struct inode_operations * proc_iops;
struct file_operations * proc_fops;
get_info_t *get_info;
struct module *owner;
struct proc_dir_entry *next, *parent, *subdir;
void *data;
read_proc_t *read_proc;
write_proc_t *write_proc;
- unsigned int count; /* use count */
+ atomic_t count; /* use count, changed from unsigned int to signed, does it will hurt? */
int deleted; /* delete flag */
kdev_t rdev;
};
#define PROC_INODE_PROPER(inode) ((inode)->i_ino & ~0xffff)
#ifdef CONFIG_PROC_FS
extern struct proc_dir_entry proc_root;
extern struct proc_dir_entry *proc_root_fs;
diff -10 -u --recursive --exclude *~ linux-2.4.0-test2/kernel/sysctl.c linux-2.4.0-test2-modified/kernel/sysctl.c
--- linux-2.4.0-test2/kernel/sysctl.c Sat Jun 24 06:06:37 2000
+++ linux-2.4.0-test2-modified/kernel/sysctl.c Thu Jul 13 10:21:29 2000
@@ -550,21 +550,21 @@
continue;
}
unregister_proc_table(table->child, de);
/* Don't unregister directories which still have entries.. */
if (de->subdir)
continue;
}
/* Don't unregister proc entries that are still being used.. */
- if (de->count)
+ if (atomic_read(&de->count))
continue;
table->de = NULL;
remove_proc_entry(table->procname, root);
}
}
static ssize_t do_rw_proc(int write, struct file * file, char * buf,
size_t count, loff_t *ppos)
{
-
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/
This archive was generated by hypermail 2b29 : Sat Jul 15 2000 - 21:00:19 EST