Re: [PATCH] s390: Hypervisor File System

From: Michael Holzheu
Date: Fri Apr 28 2006 - 13:37:01 EST

Hi Andrew,

Here the patch with the bugfixes for your and Pekka's comments:

- move some globals into the superblock
- remove some inlines
- fix some error handling functions
- implement diag204_get_buffer() helper function
- "hypfs" prefix for global functions
- use uid_t and gid_t
- use kstrdup()
- Cosmetics

Signed-off-by: Michael Holzheu <holzheu@xxxxxxxxxx>

---

arch/s390/Kconfig | 2
arch/s390/hypfs/hypfs_diag.c | 127 +++++++++++++++++++++++++++----------------
arch/s390/hypfs/hypfs_diag.h | 6 +-
arch/s390/hypfs/inode.c | 107 ++++++++++++++++++++----------------
4 files changed, 145 insertions(+), 97 deletions(-)

diff -urpN linux-2.6.16-hypfs-2006-04-28/arch/s390/Kconfig linux-2.6.16-hypfs-2006-04-28-FIXED/arch/s390/Kconfig
--- linux-2.6.16-hypfs-2006-04-28/arch/s390/Kconfig 2006-04-28 19:16:56.000000000 +0200
+++ linux-2.6.16-hypfs-2006-04-28-FIXED/arch/s390/Kconfig 2006-04-28 19:17:37.000000000 +0200
@@ -447,7 +447,7 @@ config HYPFS_FS
default y
help
This is a virtual file system intended to provide accounting
- information in a s390 hypervisor environment.
+ information in an s390 hypervisor environment.

config KEXEC
bool "kexec system call (EXPERIMENTAL)"
diff -urpN linux-2.6.16-hypfs-2006-04-28/arch/s390/hypfs/hypfs_diag.c linux-2.6.16-hypfs-2006-04-28-FIXED/arch/s390/hypfs/hypfs_diag.c
--- linux-2.6.16-hypfs-2006-04-28/arch/s390/hypfs/hypfs_diag.c 2006-04-28 19:16:56.000000000 +0200
+++ linux-2.6.16-hypfs-2006-04-28-FIXED/arch/s390/hypfs/hypfs_diag.c 2006-04-28 19:17:51.000000000 +0200
@@ -322,7 +322,7 @@ static inline __u64 phys_cpu__ctidx(enum

/* Diagnose 204 functions */

-static inline int diag204(unsigned long subcode, unsigned long size, void *addr)
+static int diag204(unsigned long subcode, unsigned long size, void *addr)
{
register unsigned long _subcode asm("0") = subcode;
register unsigned long _size asm("1") = size;
@@ -345,6 +345,25 @@ static inline int diag204(unsigned long
return _size;
}

+static void *diag204_get_buffer(enum diag204_format fmt, int *pages)
+{
+ void *buf;
+
+ if (fmt == INFO_SIMPLE)
+ *pages = 1;
+ else
+ *pages = diag204(SUBC_RSI | fmt, 0, 0);
+
+ if (*pages <= 0)
+ return ERR_PTR(-ENOSYS);
+
+ buf = kmalloc(*pages * PAGE_SIZE, GFP_KERNEL);
+ if (!buf)
+ return ERR_PTR(-ENOMEM);
+
+ return buf;
+}
+
/*
* diag204_probe() has to find out, which type of diagnose 204 implementation
* we have on our machine. Currently there are three possible scanarios:
@@ -364,13 +383,14 @@ static int diag204_probe(void)
void *buf;
int pages, rc;

- pages = diag204(SUBC_RSI | INFO_EXT, 0, 0);
- if (pages > 0) {
- buf = kmalloc(pages * PAGE_SIZE, GFP_KERNEL);
- if (!buf) {
- rc = -ENOMEM;
- goto err_out;
- }
+ /*
+ * We first try to get the number of pages needed to store the extended
+ * data format. If this already fails, our machine does not support
+ * subcode 6 and 7. We then continue checking for subcode 4.
+ */
+
+ buf = diag204_get_buffer(INFO_EXT, &pages);
+ if (!IS_ERR(buf)) {
if (diag204(SUBC_STIB7 | INFO_EXT, pages, buf) >= 0) {
diag204_store_sc = SUBC_STIB7;
diag204_info_type = INFO_EXT;
@@ -382,10 +402,19 @@ static int diag204_probe(void)
goto out;
}
kfree(buf);
+ } else if (PTR_ERR(buf) != -ENOSYS) {
+ /* other error than 'not implemented'. Give up! */
+ return PTR_ERR(buf);
}
- buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
- if (!buf) {
- rc = -ENOMEM;
+
+ /*
+ * subcode 6 and 7 did not work. Now try subcode 4 with the simple
+ * data format.
+ */
+
+ buf = diag204_get_buffer(INFO_SIMPLE, &pages);
+ if (IS_ERR(buf)) {
+ rc = PTR_ERR(buf);
goto err_out;
}
if (diag204(SUBC_STIB4 | INFO_SIMPLE, pages, buf) >= 0) {
@@ -396,9 +425,9 @@ static int diag204_probe(void)
rc = -ENOSYS;
goto err_out;
}
- out:
+out:
rc = 0;
- err_out:
+err_out:
kfree(buf);
return rc;
}
@@ -408,34 +437,26 @@ static void *diag204_store(void)
void *buf;
int pages;

- if (diag204_store_sc == SUBC_STIB4)
- pages = 1;
- else
- pages = diag204(SUBC_RSI | diag204_info_type, 0, 0);
-
- if (pages < 0)
- return ERR_PTR(-ENOSYS);
-
- buf = kmalloc(pages * PAGE_SIZE, GFP_KERNEL);
- if (!buf)
- return ERR_PTR(-ENOMEM);
-
+ buf = diag204_get_buffer(diag204_info_type, &pages);
+ if (IS_ERR(buf))
+ goto out;
if (diag204(diag204_store_sc | diag204_info_type, pages, buf) < 0) {
kfree(buf);
return ERR_PTR(-ENOSYS);
}
+out:
return buf;
}

/* Diagnose 224 functions */

-static inline void diag224(void *ptr)
+static void diag224(void *ptr)
{
asm volatile(" diag %0,%1,0x224\n"
: :"d" (0), "d"(ptr) : "memory");
}

-static inline int diag224_get_name_table(void)
+static int diag224_get_name_table(void)
{
/* memory must be below 2GB */
diag224_cpu_names = kmalloc(PAGE_SIZE, GFP_KERNEL | GFP_DMA);
@@ -446,7 +467,7 @@ static inline int diag224_get_name_table
return 0;
}

-static inline void diag224_delete_name_table(void)
+static void diag224_delete_name_table(void)
{
kfree(diag224_cpu_names);
}
@@ -459,7 +480,7 @@ static int diag224_idx2name(int index, c
return 0;
}

-int diag_init(void)
+__init int hypfs_diag_init(void)
{
int rc;

@@ -475,7 +496,7 @@ int diag_init(void)
return rc;
}

-void diag_exit(void)
+__exit void hypfs_diag_exit(void)
{
diag224_delete_name_table();
}
@@ -590,39 +611,51 @@ static void *hypfs_create_phys_files(str
return cpu_info;
}

-int diag_create_files(struct super_block *sb, struct dentry *root)
+int hypfs_diag_create_files(struct super_block *sb, struct dentry *root)
{
struct dentry *systems_dir, *hyp_dir;
void *time_hdr, *part_hdr;
- int i;
- void *buffer, *rc;
+ int i, rc;
+ void *buffer, *ptr;

buffer = diag204_store();
if (IS_ERR(buffer))
return PTR_ERR(buffer);

systems_dir = hypfs_mkdir(sb, root, "systems");
- if (IS_ERR(systems_dir))
- return PTR_ERR(systems_dir);
+ if (IS_ERR(systems_dir)) {
+ rc = PTR_ERR(systems_dir);
+ goto err_out;
+ }
time_hdr = (struct x_info_blk_hdr *)buffer;
part_hdr = time_hdr + info_blk_hdr__size(diag204_info_type);
for (i = 0; i < info_blk_hdr__npar(diag204_info_type, time_hdr); i++) {
part_hdr = hypfs_create_lpar_files(sb, systems_dir, part_hdr);
- if (IS_ERR(part_hdr))
- return PTR_ERR(part_hdr);
+ if (IS_ERR(part_hdr)) {
+ rc = PTR_ERR(part_hdr);
+ goto err_out;
+ }
}
if (info_blk_hdr__flags(diag204_info_type, time_hdr) & LPAR_PHYS_FLG) {
- void *rc;
- rc = hypfs_create_phys_files(sb, root, part_hdr);
- if (IS_ERR(rc))
- return PTR_ERR(rc);
+ ptr = hypfs_create_phys_files(sb, root, part_hdr);
+ if (IS_ERR(ptr)) {
+ rc = PTR_ERR(ptr);
+ goto err_out;
+ }
}
hyp_dir = hypfs_mkdir(sb, root, "hyp");
- if (IS_ERR(hyp_dir))
- return PTR_ERR(hyp_dir);
- rc = hypfs_create_str(sb, hyp_dir, "type", "LPAR Hypervisor");
- if (IS_ERR(rc))
- return PTR_ERR(rc);
+ if (IS_ERR(hyp_dir)) {
+ rc = PTR_ERR(hyp_dir);
+ goto err_out;
+ }
+ ptr = hypfs_create_str(sb, hyp_dir, "type", "LPAR Hypervisor");
+ if (IS_ERR(ptr)) {
+ rc = PTR_ERR(ptr);
+ goto err_out;
+ }
+ rc = 0;
+
+err_out:
kfree(buffer);
- return 0;
+ return rc;
}
diff -urpN linux-2.6.16-hypfs-2006-04-28/arch/s390/hypfs/hypfs_diag.h linux-2.6.16-hypfs-2006-04-28-FIXED/arch/s390/hypfs/hypfs_diag.h
--- linux-2.6.16-hypfs-2006-04-28/arch/s390/hypfs/hypfs_diag.h 2006-04-28 19:16:56.000000000 +0200
+++ linux-2.6.16-hypfs-2006-04-28-FIXED/arch/s390/hypfs/hypfs_diag.h 2006-04-28 19:17:51.000000000 +0200
@@ -9,8 +9,8 @@
#ifndef _HYPFS_DIAG_H_
#define _HYPFS_DIAG_H_

-extern int diag_init(void);
-extern void diag_exit(void);
-extern int diag_create_files(struct super_block *sb, struct dentry *root);
+extern int hypfs_diag_init(void);
+extern void hypfs_diag_exit(void);
+extern int hypfs_diag_create_files(struct super_block *sb, struct dentry *root);

#endif /* _HYPFS_DIAG_H_ */
diff -urpN linux-2.6.16-hypfs-2006-04-28/arch/s390/hypfs/inode.c linux-2.6.16-hypfs-2006-04-28-FIXED/arch/s390/hypfs/inode.c
--- linux-2.6.16-hypfs-2006-04-28/arch/s390/hypfs/inode.c 2006-04-28 19:16:56.000000000 +0200
+++ linux-2.6.16-hypfs-2006-04-28-FIXED/arch/s390/hypfs/inode.c 2006-04-28 19:17:51.000000000 +0200
@@ -28,25 +28,26 @@ static struct dentry *hypfs_create_updat
struct dentry *dir);

struct hypfs_sb_info {
- int uid; /* uid used for files and dirs */
- int gid; /* gid used for files and dirs */
+ uid_t uid; /* uid used for files and dirs */
+ gid_t gid; /* gid used for files and dirs */
+ struct dentry *update_file; /* file to trigger update */
+ time_t last_update; /* last update time in secs since 1970 */
+ struct mutex lock; /* lock to protect update process */
};

-static struct dentry *update_file_dentry;
static struct file_operations hypfs_file_ops;
static struct file_system_type hypfs_type;
static struct super_operations hypfs_s_ops;
-static time_t last_update_time = 0; /* update time in seconds since 1970 */
-static DEFINE_MUTEX(hypfs_lock);

/* start of list of all dentries, which have to be deleted on update */
static struct dentry *hypfs_last_dentry;

-static void hypfs_update_update(void)
+static void hypfs_update_update(struct super_block *sb)
{
- struct inode* inode = update_file_dentry->d_inode;
+ struct hypfs_sb_info *sb_info = sb->s_fs_info;
+ struct inode *inode = sb_info->update_file->d_inode;

- last_update_time = get_seconds();
+ sb_info->last_update = get_seconds();
inode->i_atime = inode->i_mtime = inode->i_ctime = CURRENT_TIME;
}

@@ -103,7 +104,8 @@ static void hypfs_drop_inode(struct inod

static int hypfs_open(struct inode *inode, struct file *filp)
{
- char *data = (char *)filp->f_dentry->d_inode->u.generic_ip;
+ char *data = filp->f_dentry->d_inode->u.generic_ip;
+ struct hypfs_sb_info *fs_info;

if (filp->f_mode & FMODE_WRITE) {
if (!(inode->i_mode & S_IWUGO))
@@ -113,14 +115,17 @@ static int hypfs_open(struct inode *inod
if (!(inode->i_mode & S_IRUGO))
return -EACCES;
}
- mutex_lock(&hypfs_lock);
- filp->private_data = kmalloc(strlen(data) + 1, GFP_KERNEL);
- if (!filp->private_data) {
- mutex_unlock(&hypfs_lock);
- return -ENOMEM;
+
+ fs_info = inode->i_sb->s_fs_info;
+ if(data) {
+ mutex_lock(&fs_info->lock);
+ filp->private_data = kstrdup(data, GFP_KERNEL);
+ if (!filp->private_data) {
+ mutex_unlock(&fs_info->lock);
+ return -ENOMEM;
+ }
+ mutex_unlock(&fs_info->lock);
}
- strcpy(filp->private_data, data);
- mutex_unlock(&hypfs_lock);
return 0;
}

@@ -131,7 +136,7 @@ static ssize_t hypfs_aio_read(struct kio
size_t len;
struct file *filp = iocb->ki_filp;

- data = (char *)filp->private_data;
+ data = filp->private_data;
len = strlen(data);
if (offset > len) {
count = 0;
@@ -145,7 +150,7 @@ static ssize_t hypfs_aio_read(struct kio
}
iocb->ki_pos += count;
file_accessed(filp);
- out:
+out:
return count;
}
static ssize_t hypfs_aio_write(struct kiocb *iocb, const char __user *buf,
@@ -153,24 +158,36 @@ static ssize_t hypfs_aio_write(struct ki
{
int rc;
struct super_block *sb;
+ struct hypfs_sb_info *fs_info;

- mutex_lock(&hypfs_lock);
sb = iocb->ki_filp->f_dentry->d_inode->i_sb;
- if (last_update_time == get_seconds()) {
+ fs_info = sb->s_fs_info;
+ /*
+ * Currently we only allow one update per second for two reasons:
+ * 1. diag 204 is VERY expensive
+ * 2. If several processes do updates in parallel and then read the
+ * hypfs data, the likelihood of collisions is reduced, if we restrict
+ * the minimum update interval. A collision occurs, if during the
+ * data gathering of one process another process triggers an update
+ * If the first process wants to ensure consistent data, it has
+ * to restart data collection in this case.
+ */
+ mutex_lock(&fs_info->lock);
+ if (fs_info->last_update == get_seconds()) {
rc = -EBUSY;
goto out;
}
hypfs_delete_tree(sb->s_root);
- rc = diag_create_files(sb, sb->s_root);
+ rc = hypfs_diag_create_files(sb, sb->s_root);
if (rc) {
printk(KERN_ERR "hypfs: Update failed\n");
hypfs_delete_tree(sb->s_root);
goto out;
}
- hypfs_update_update();
+ hypfs_update_update(sb);
rc = count;
- out:
- mutex_unlock(&hypfs_lock);
+out:
+ mutex_unlock(&fs_info->lock);
return rc;
}

@@ -229,9 +246,10 @@ static int hypfs_fill_super(struct super
int rc = 0;
struct hypfs_sb_info *sbi;

- sbi = kmalloc(sizeof(struct hypfs_sb_info), GFP_KERNEL);
+ sbi = kzalloc(sizeof(struct hypfs_sb_info), GFP_KERNEL);
if (!sbi)
return -ENOMEM;
+ mutex_init(&sbi->lock);
sbi->uid = current->uid;
sbi->gid = current->gid;
sb->s_fs_info = sbi;
@@ -255,23 +273,23 @@ static int hypfs_fill_super(struct super
rc = -ENOMEM;
goto err_inode;
}
- rc = diag_create_files(sb, sb->s_root);
+ rc = hypfs_diag_create_files(sb, sb->s_root);
if (rc)
goto err_tree;
- update_file_dentry = hypfs_create_update_file(sb, sb->s_root);
- if (IS_ERR(update_file_dentry)) {
- rc = PTR_ERR(update_file_dentry);
+ sbi->update_file = hypfs_create_update_file(sb, sb->s_root);
+ if (IS_ERR(sbi->update_file)) {
+ rc = PTR_ERR(sbi->update_file);
goto err_tree;
}
- hypfs_update_update();
+ hypfs_update_update(sb);
return 0;

- err_tree:
+err_tree:
hypfs_delete_tree(sb->s_root);
dput(sb->s_root);
- err_inode:
+err_inode:
iput(root_inode);
- err_alloc:
+err_alloc:
kfree(sbi);
return rc;
}
@@ -309,7 +327,7 @@ static struct dentry *hypfs_create_file(
}
if (mode & S_IFREG) {
inode->i_fop = &hypfs_file_ops;
- if(data)
+ if (data)
inode->i_size = strlen(data);
else
inode->i_size = 0;
@@ -344,8 +362,6 @@ static struct dentry *hypfs_create_updat

dentry = hypfs_create_file(sb, dir, "update", NULL,
S_IFREG | UPDATE_FILE_MODE);
- if (!dentry)
- return ERR_PTR(-ENOMEM);
/*
* We do not put the update file on the 'delete' list with
* hypfs_add_dentry(), since it should not be removed when the tree
@@ -362,13 +378,12 @@ struct dentry *hypfs_create_u64(struct s
struct dentry *dentry;

snprintf(tmp, TMP_SIZE, "%lld\n", (unsigned long long int)value);
- buffer = kmalloc(strlen(tmp) + 1, GFP_KERNEL);
+ buffer = kstrdup(tmp, GFP_KERNEL);
if (!buffer)
return ERR_PTR(-ENOMEM);
- strcpy(buffer, tmp);
dentry =
hypfs_create_file(sb, dir, name, buffer, S_IFREG | REG_FILE_MODE);
- if (!dentry) {
+ if (IS_ERR(dentry)) {
kfree(buffer);
return ERR_PTR(-ENOMEM);
}
@@ -388,7 +403,7 @@ struct dentry *hypfs_create_str(struct s
sprintf(buffer, "%s\n", string);
dentry =
hypfs_create_file(sb, dir, name, buffer, S_IFREG | REG_FILE_MODE);
- if (!dentry) {
+ if (IS_ERR(dentry)) {
kfree(buffer);
return ERR_PTR(-ENOMEM);
}
@@ -426,7 +441,7 @@ static int __init hypfs_init(void)

if (MACHINE_IS_VM)
return -ENODATA;
- if (diag_init()) {
+ if (hypfs_diag_init()) {
rc = -ENODATA;
goto err_msg;
}
@@ -438,18 +453,18 @@ static int __init hypfs_init(void)
goto err_sysfs;
return 0;

- err_sysfs:
+err_sysfs:
subsystem_unregister(&hypervisor_subsys);
- err_diag:
- diag_exit();
- err_msg:
+err_diag:
+ hypfs_diag_exit();
+err_msg:
printk(KERN_ERR "hypfs: Initialization failed with rc = %i.\n", rc);
return rc;
}

static void __exit hypfs_exit(void)
{
- diag_exit();
+ hypfs_diag_exit();
unregister_filesystem(&hypfs_type);
subsystem_unregister(&hypervisor_subsys);
}

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/