Discussion:
[PATCH 3/4] chardev: Don't use i_devices inode field
(too old to reply)
Jan Kara
2014-10-22 20:14:12 UTC
Permalink
Now that character device can be freed only after all inodes referencing
it through i_cdev are gone, we can remove all the tracking of inodes
pointing to a character device.

Signed-off-by: Jan Kara <***@suse.cz>
---
fs/char_dev.c | 22 +---------------------
include/linux/cdev.h | 1 -
2 files changed, 1 insertion(+), 22 deletions(-)

diff --git a/fs/char_dev.c b/fs/char_dev.c
index 21c210dbdba1..08a1404f590b 100644
--- a/fs/char_dev.c
+++ b/fs/char_dev.c
@@ -390,7 +390,6 @@ static int chrdev_open(struct inode *inode, struct file *filp)
if (!p) {
/* Use reference from kobj_lookup for i_cdev ref */
inode->i_cdev = p = new;
- list_add(&inode->i_devices, &p->list);
new = NULL;
}
}
@@ -425,7 +424,6 @@ void cd_forget(struct inode *inode)
struct cdev *cdev;

spin_lock(&cdev_lock);
- list_del_init(&inode->i_devices);
cdev = inode->i_cdev;
inode->i_cdev = NULL;
spin_unlock(&cdev_lock);
@@ -433,18 +431,6 @@ void cd_forget(struct inode *inode)
cdev_put(cdev);
}

-static void cdev_purge(struct cdev *cdev)
-{
- spin_lock(&cdev_lock);
- while (!list_empty(&cdev->list)) {
- struct inode *inode;
- inode = container_of(cdev->list.next, struct inode, i_devices);
- list_del_init(&inode->i_devices);
- inode->i_cdev = NULL;
- }
- spin_unlock(&cdev_lock);
-}
-
/*
* Dummy default file-operations: the only thing this does
* is contain the open that then fills in the correct operations
@@ -515,10 +501,8 @@ void cdev_del(struct cdev *p)

static void cdev_default_release(struct kobject *kobj)
{
- struct cdev *p = container_of(kobj, struct cdev, kobj);
struct kobject *parent = kobj->parent;

- cdev_purge(p);
kobject_put(parent);
}

@@ -527,7 +511,6 @@ static void cdev_dynamic_release(struct kobject *kobj)
struct cdev *p = container_of(kobj, struct cdev, kobj);
struct kobject *parent = kobj->parent;

- cdev_purge(p);
kfree(p);
kobject_put(parent);
}
@@ -548,10 +531,8 @@ static struct kobj_type ktype_cdev_dynamic = {
struct cdev *cdev_alloc(void)
{
struct cdev *p = kzalloc(sizeof(struct cdev), GFP_KERNEL);
- if (p) {
- INIT_LIST_HEAD(&p->list);
+ if (p)
kobject_init(&p->kobj, &ktype_cdev_dynamic);
- }
return p;
}

@@ -566,7 +547,6 @@ struct cdev *cdev_alloc(void)
void cdev_init(struct cdev *cdev, const struct file_operations *fops)
{
memset(cdev, 0, sizeof *cdev);
- INIT_LIST_HEAD(&cdev->list);
kobject_init(&cdev->kobj, &ktype_cdev_default);
cdev->ops = fops;
}
diff --git a/include/linux/cdev.h b/include/linux/cdev.h
index fb4591977b03..fe00138b5106 100644
--- a/include/linux/cdev.h
+++ b/include/linux/cdev.h
@@ -13,7 +13,6 @@ struct cdev {
struct kobject kobj;
struct module *owner;
const struct file_operations *ops;
- struct list_head list;
dev_t dev;
unsigned int count;
};
--
1.8.1.4
Jan Kara
2014-10-22 20:14:13 UTC
Permalink
Noone uses i_devices anymore. Remove it (thus saving two pointers in
every inode).

Signed-off-by: Jan Kara <***@suse.cz>
---
fs/inode.c | 1 -
include/linux/fs.h | 1 -
2 files changed, 2 deletions(-)

diff --git a/fs/inode.c b/fs/inode.c
index 26753ba7b6d6..cb7acd45dce5 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -366,7 +366,6 @@ void inode_init_once(struct inode *inode)
{
memset(inode, 0, sizeof(*inode));
INIT_HLIST_NODE(&inode->i_hash);
- INIT_LIST_HEAD(&inode->i_devices);
INIT_LIST_HEAD(&inode->i_wb_list);
INIT_LIST_HEAD(&inode->i_lru);
address_space_init_once(&inode->i_data);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index d7fd7959a933..8c4f5678e75e 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -595,7 +595,6 @@ struct inode {
#ifdef CONFIG_QUOTA
struct dquot *i_dquot[MAXQUOTAS];
#endif
- struct list_head i_devices;
union {
struct pipe_inode_info *i_pipe;
struct block_device *i_bdev;
--
1.8.1.4
Jan Kara
2014-10-22 20:14:11 UTC
Permalink
Currently i_cdev reference to a character device isn't accounted in the
reference count of the character device. This then requires us to track
all references through a list of all inodes referencing a character
device which is somewhat clumsy and requires list_head in each inode in
the system.

So make i_cdev a reference like any other.

Signed-off-by: Jan Kara <***@suse.cz>
---
fs/char_dev.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/fs/char_dev.c b/fs/char_dev.c
index f77f7702fabe..21c210dbdba1 100644
--- a/fs/char_dev.c
+++ b/fs/char_dev.c
@@ -388,12 +388,13 @@ static int chrdev_open(struct inode *inode, struct file *filp)
we dropped the lock. */
p = inode->i_cdev;
if (!p) {
+ /* Use reference from kobj_lookup for i_cdev ref */
inode->i_cdev = p = new;
list_add(&inode->i_devices, &p->list);
new = NULL;
- } else if (!cdev_get(p))
- ret = -ENXIO;
- } else if (!cdev_get(p))
+ }
+ }
+ if (!cdev_get(p))
ret = -ENXIO;
spin_unlock(&cdev_lock);
cdev_put(new);
@@ -421,10 +422,15 @@ static int chrdev_open(struct inode *inode, struct file *filp)

void cd_forget(struct inode *inode)
{
+ struct cdev *cdev;
+
spin_lock(&cdev_lock);
list_del_init(&inode->i_devices);
+ cdev = inode->i_cdev;
inode->i_cdev = NULL;
spin_unlock(&cdev_lock);
+ if (cdev)
+ cdev_put(cdev);
}

static void cdev_purge(struct cdev *cdev)
--
1.8.1.4
Jan Kara
2014-10-22 20:14:10 UTC
Permalink
Block devices use i_devices inode field to track all inodes that
reference a particular block device (through i_bdev field) so that this
reference can be removed when block device inode is being evicted from
memory. However we get a reference to the block device (in fact an inode
holding the block device structure) when setting up i_bdev in
bd_acquire() and we drop the reference only in bd_forget() when clearing
i_bdev. Thus inode holding block device structure can be evicted only
after all inodes referencing it are evicted and the whole excercise with
i_devices is pointless. Remove the i_devices handling.

Signed-off-by: Jan Kara <***@suse.cz>
---
fs/block_dev.c | 17 +++--------------
include/linux/fs.h | 1 -
2 files changed, 3 insertions(+), 15 deletions(-)

diff --git a/fs/block_dev.c b/fs/block_dev.c
index cc9d4114cda0..493cd69df9a6 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -458,7 +458,6 @@ static void init_once(void *foo)

memset(bdev, 0, sizeof(*bdev));
mutex_init(&bdev->bd_mutex);
- INIT_LIST_HEAD(&bdev->bd_inodes);
INIT_LIST_HEAD(&bdev->bd_list);
#ifdef CONFIG_SYSFS
INIT_LIST_HEAD(&bdev->bd_holder_disks);
@@ -468,24 +467,14 @@ static void init_once(void *foo)
mutex_init(&bdev->bd_fsfreeze_mutex);
}

-static inline void __bd_forget(struct inode *inode)
-{
- list_del_init(&inode->i_devices);
- inode->i_bdev = NULL;
- inode->i_mapping = &inode->i_data;
-}
-
static void bdev_evict_inode(struct inode *inode)
{
struct block_device *bdev = &BDEV_I(inode)->bdev;
- struct list_head *p;
+
truncate_inode_pages_final(&inode->i_data);
invalidate_inode_buffers(inode); /* is it needed here? */
clear_inode(inode);
spin_lock(&bdev_lock);
- while ( (p = bdev->bd_inodes.next) != &bdev->bd_inodes ) {
- __bd_forget(list_entry(p, struct inode, i_devices));
- }
list_del_init(&bdev->bd_list);
spin_unlock(&bdev_lock);
}
@@ -645,7 +634,6 @@ static struct block_device *bd_acquire(struct inode *inode)
ihold(bdev->bd_inode);
inode->i_bdev = bdev;
inode->i_mapping = bdev->bd_inode->i_mapping;
- list_add(&inode->i_devices, &bdev->bd_inodes);
}
spin_unlock(&bdev_lock);
}
@@ -666,7 +654,8 @@ void bd_forget(struct inode *inode)
spin_lock(&bdev_lock);
if (!sb_is_blkdev_sb(inode->i_sb))
bdev = inode->i_bdev;
- __bd_forget(inode);
+ inode->i_bdev = NULL;
+ inode->i_mapping = &inode->i_data;
spin_unlock(&bdev_lock);

if (bdev)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index a957d4366c24..d7fd7959a933 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -413,7 +413,6 @@ struct block_device {
struct inode * bd_inode; /* will die */
struct super_block * bd_super;
struct mutex bd_mutex; /* open/close mutex */
- struct list_head bd_inodes;
void * bd_claiming;
void * bd_holder;
int bd_holders;
--
1.8.1.4
Loading...