Re: Filesystem specific information in inode overwritten

Alexander Viro (viro@math.psu.edu)
Thu, 4 Feb 1999 15:49:01 -0500 (EST)


On Thu, 4 Feb 1999, Mikulas Patocka wrote:

> Look at unit 'u' in inode structure in fs.h
> You see that pipe_inode_info and socket share space with filesystem
> specific information.

Ah, one more person who noticed this lossage. Welcome to the club. I've
sent the fix, but it was waay too close to 2.2.0, so... Linus told that
while it looked OK for 2.3 it was *not* a 2.2-pre material.

I'll probably put my tree (-bird) on anon ftp in a few days (again). It's
mostly VFS/filesystems stuff and if you can contact me (at least with
coordinates of your patch) it would be nice. As for the FIFO lossage -
puh-lease. IIRC it's against -pre7, so you may have to rediff it. I didn't
touch socket stuff - I'm still not convinced that it's needed. I'll look
at it, anyway.
Al

--- linux.vanilla/fs/fifo.c Fri Nov 20 09:38:42 1998
+++ linux.bird-stable/fs/fifo.c Mon Jan 11 08:10:18 1999
@@ -5,6 +5,7 @@
*/

#include <linux/mm.h>
+#include <linux/malloc.h>

static int fifo_open(struct inode * inode,struct file * filp)
{
@@ -151,10 +152,17 @@
void init_fifo(struct inode * inode)
{
inode->i_op = &fifo_inode_operations;
+ /* XXX - we should use kmem_cache_alloc */
+ inode->pipe_i=kmalloc(sizeof(struct pipe_inode_info),GFP_KERNEL);
+ if (!inode->pipe_i)
+ goto out_no_mem;
PIPE_LOCK(*inode) = 0;
PIPE_BASE(*inode) = NULL;
PIPE_START(*inode) = PIPE_LEN(*inode) = 0;
PIPE_RD_OPENERS(*inode) = PIPE_WR_OPENERS(*inode) = 0;
PIPE_WAIT(*inode) = NULL;
PIPE_READERS(*inode) = PIPE_WRITERS(*inode) = 0;
+ return;
+out_no_mem:
+ inode->i_op = NULL;
}
--- linux.vanilla/fs/inode.c Mon Jan 11 08:08:32 1999
+++ linux.bird-stable/fs/inode.c Mon Jan 11 08:10:19 1999
@@ -11,6 +11,8 @@
#include <linux/init.h>
#include <linux/quotaops.h>

+#include <linux/malloc.h>
+
/*
* New inode.c implementation.
*
@@ -224,10 +226,23 @@
wait_on_inode(inode);
if (IS_QUOTAINIT(inode))
DQUOT_DROP(inode);
+ /* Sorry, but let's keep the main path clean */
+ if (inode->pipe_i)
+ goto is_fifo;
+do_clear:
if (inode->i_sb && inode->i_sb->s_op && inode->i_sb->s_op->clear_inode)
inode->i_sb->s_op->clear_inode(inode);

inode->i_state = 0;
+ return;
+
+is_fifo:
+ {
+ void *p=inode->pipe_i;
+ inode->pipe_i=NULL;
+ kfree(p);
+ goto do_clear;
+ }
}

/*
@@ -501,6 +516,7 @@
inode->i_size = 0;
memset(&inode->i_dquot, 0, sizeof(inode->i_dquot));
sema_init(&inode->i_sem, 1);
+ inode->pipe_i = NULL;
}

/*
--- linux.vanilla/fs/pipe.c Fri Nov 20 09:38:43 1998
+++ linux.bird-stable/fs/pipe.c Mon Jan 11 08:10:19 1999
@@ -7,6 +7,7 @@
#include <linux/mm.h>
#include <linux/file.h>
#include <linux/poll.h>
+#include <linux/malloc.h>

#include <asm/uaccess.h>

@@ -404,36 +405,48 @@
{
extern struct inode_operations pipe_inode_operations;
struct inode *inode = get_empty_inode();
+ unsigned long page;

- if (inode) {
- unsigned long page = __get_free_page(GFP_USER);
+ if (!inode)
+ goto fail_inode;

- if (!page) {
- iput(inode);
- inode = NULL;
- } else {
- PIPE_BASE(*inode) = (char *) page;
- inode->i_op = &pipe_inode_operations;
- PIPE_WAIT(*inode) = NULL;
- PIPE_START(*inode) = PIPE_LEN(*inode) = 0;
- PIPE_RD_OPENERS(*inode) = PIPE_WR_OPENERS(*inode) = 0;
- PIPE_READERS(*inode) = PIPE_WRITERS(*inode) = 1;
- PIPE_LOCK(*inode) = 0;
- /*
- * Mark the inode dirty from the very beginning,
- * that way it will never be moved to the dirty
- * list because "mark_inode_dirty()" will think
- * that it already _is_ on the dirty list.
- */
- inode->i_state = I_DIRTY;
- inode->i_mode = S_IFIFO | S_IRUSR | S_IWUSR;
- inode->i_uid = current->fsuid;
- inode->i_gid = current->fsgid;
- inode->i_atime = inode->i_mtime = inode->i_ctime = CURRENT_TIME;
- inode->i_blksize = PAGE_SIZE;
- }
- }
+ page = __get_free_page(GFP_USER);
+
+ if (!page)
+ goto fail_iput;
+
+ /* XXX */
+ inode->pipe_i = kmalloc(sizeof(struct pipe_inode_info), GFP_KERNEL);
+ if (!inode->pipe_i)
+ goto fail_pipe_i;
+
+ PIPE_BASE(*inode) = (char *) page;
+ inode->i_op = &pipe_inode_operations;
+ PIPE_WAIT(*inode) = NULL;
+ PIPE_START(*inode) = PIPE_LEN(*inode) = 0;
+ PIPE_RD_OPENERS(*inode) = PIPE_WR_OPENERS(*inode) = 0;
+ PIPE_READERS(*inode) = PIPE_WRITERS(*inode) = 1;
+ PIPE_LOCK(*inode) = 0;
+ /*
+ * Mark the inode dirty from the very beginning,
+ * that way it will never be moved to the dirty
+ * list because "mark_inode_dirty()" will think
+ * that it already _is_ on the dirty list.
+ */
+ inode->i_state = I_DIRTY;
+ inode->i_mode = S_IFIFO | S_IRUSR | S_IWUSR;
+ inode->i_uid = current->fsuid;
+ inode->i_gid = current->fsgid;
+ inode->i_atime = inode->i_mtime = inode->i_ctime = CURRENT_TIME;
+ inode->i_blksize = PAGE_SIZE;
return inode;
+
+fail_pipe_i:
+ free_page(page);
+fail_iput:
+ iput(inode);
+fail_inode:
+ return NULL;
}

struct inode_operations pipe_inode_operations = {
--- linux.vanilla/include/linux/fs.h Sun Jan 3 22:40:08 1999
+++ linux.bird-stable/include/linux/fs.h Mon Jan 11 08:10:19 1999
@@ -375,7 +375,6 @@
int i_writecount;
unsigned int i_attr_flags;
union {
- struct pipe_inode_info pipe_i;
struct minix_inode_info minix_i;
struct ext2_inode_info ext2_i;
struct hpfs_inode_info hpfs_i;
@@ -396,6 +395,7 @@
struct socket socket_i;
void *generic_ip;
} u;
+ struct pipe_inode_info *pipe_i; /* XXX */
};

/* Inode state bits.. */
--- linux.vanilla/include/linux/msdos_fs_i.h Fri Nov 13 17:55:32 1998
+++ linux.bird-stable/include/linux/msdos_fs_i.h Mon Jan 11 08:10:20 1999
@@ -1,10 +1,6 @@
#ifndef _MSDOS_FS_I
#define _MSDOS_FS_I

-#ifndef _LINUX_PIPE_FS_I_H
-#include <linux/pipe_fs_i.h>
-#endif
-
/*
* MS-DOS file system inode data in memory
*/
@@ -24,7 +20,12 @@
a MS-DOS FS driver unaware of UMSDOS and then later to
load a (then incompatible) UMSDOS FS driver.
*/
- struct pipe_inode_info reserved;
+ /*
+ All this pipe_info monstrosity is fixed now. It's kmalloc'ed
+ outside of struct inode and is referenced from outside of
+ ->u field of said struct. Oh, and ext2 inode is going to become
+ *much* smaller, along with the rest of its kin. RIP. AV
+ */
int i_start; /* first cluster or 0 */
int i_logstart; /* logical first cluster */
int i_attrs; /* unused attribute bits */
--- linux.vanilla/include/linux/nfs_fs_i.h Sat Jan 24 13:53:00 1998
+++ linux.bird-stable/include/linux/nfs_fs_i.h Mon Jan 11 08:10:20 1999
@@ -2,7 +2,6 @@
#define _NFS_FS_I

#include <linux/nfs.h>
-#include <linux/pipe_fs_i.h>

/*
* nfs fs inode data in memory
@@ -13,7 +12,9 @@
* work (more or less correctly). This must be first in the
* struct because the data is really accessed via inode->u.pipe_i.
*/
- struct pipe_inode_info pipeinfo;
+ /*
+ * Here WAS a placeholder. RIP.
+ */

/*
* Various flags
--- linux.vanilla/include/linux/pipe_fs_i.h Wed Jul 16 13:26:21 1997
+++ linux.bird-stable/include/linux/pipe_fs_i.h Mon Jan 11 08:10:20 1999
@@ -12,15 +12,15 @@
unsigned int writers;
};

-#define PIPE_WAIT(inode) ((inode).u.pipe_i.wait)
-#define PIPE_BASE(inode) ((inode).u.pipe_i.base)
-#define PIPE_START(inode) ((inode).u.pipe_i.start)
+#define PIPE_WAIT(inode) ((inode).pipe_i->wait)
+#define PIPE_BASE(inode) ((inode).pipe_i->base)
+#define PIPE_START(inode) ((inode).pipe_i->start)
#define PIPE_LEN(inode) ((inode).i_size)
-#define PIPE_RD_OPENERS(inode) ((inode).u.pipe_i.rd_openers)
-#define PIPE_WR_OPENERS(inode) ((inode).u.pipe_i.wr_openers)
-#define PIPE_READERS(inode) ((inode).u.pipe_i.readers)
-#define PIPE_WRITERS(inode) ((inode).u.pipe_i.writers)
-#define PIPE_LOCK(inode) ((inode).u.pipe_i.lock)
+#define PIPE_RD_OPENERS(inode) ((inode).pipe_i->rd_openers)
+#define PIPE_WR_OPENERS(inode) ((inode).pipe_i->wr_openers)
+#define PIPE_READERS(inode) ((inode).pipe_i->readers)
+#define PIPE_WRITERS(inode) ((inode).pipe_i->writers)
+#define PIPE_LOCK(inode) ((inode).pipe_i->lock)
#define PIPE_SIZE(inode) PIPE_LEN(inode)

#define PIPE_EMPTY(inode) (PIPE_SIZE(inode)==0)
--- linux.vanilla/include/linux/umsdos_fs_i.h Fri Nov 13 18:00:35 1998
+++ linux.bird-stable/include/linux/umsdos_fs_i.h Mon Jan 11 08:10:20 1999
@@ -13,6 +13,9 @@
* system. This information is added to the end of the standard struct
* inode. Each file system has its own extension to struct inode,
* so do the umsdos file system.
+ *
+ * (These days it's not true. Goal is to keep those extensions out
+ * of struct inode. Only reference should be kept -- AV)
*
* The strategy is to have the umsdos_inode_info as a superset of
* the msdos_inode_info, since most of the time the job is done
@@ -42,6 +45,8 @@
* space for fifos side by side with msdos_inode_info. This is just
* to for the show, because msdos_inode_info already include the
* pipe_inode_info.
+ *
+ * (Now it doesn't. See comments in msdos_fs_i.h).
*
* The UMSDOS specific extension is placed after the union.
*/
@@ -60,7 +65,6 @@
struct umsdos_inode_info {
union {
struct msdos_inode_info msdos_info;
- struct pipe_inode_info pipe_info;
struct dir_locking_info dir_info;
} u;
int i_patched; /* Inode has been patched */

-
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/