libfs,xenfs: replace /proc/xen/xenbus with a symlink
Subsystems (2)
v3 cover libfs,xenfs: replace /proc/xen/xenbus with a symlink
Using /proc/xen/xenbus can cause deadlocks on the atomic file position mutex since this file should behave like a character device and not a regular file. This is easiest to achive by making it a symlink to the existing /dev/xen/xenbus device.
This requires extending simple_fill_super() to add symlinks as well as regular files.
Changes in v3:
- Rebased on v4.7-rc5.
David
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel
PATCH v3 1/2 libfs: allow simple_fill_super() to add symlinks
simple_fill_super() will add symlinks if an entry has mode & S_IFLNK. The target is provided in the new "link" field.
---
v2:
- simplified.
--- fs/libfs.c | 15 +++++++++++++-- include/linux/fs.h | 2 +- 2 files changed, 14 insertions(+), 3 deletions(-)
fs/libfs.c +13 -2
@@ -512,9 +512,20 @@ int simple_fill_super(struct super_block *s, unsigned long magic,dput (dentry); goto out; } - inode->i_mode =S_IFREG | files->mode; +if (files->mode &S_IFLNK ) { + inode->i_mode = files->mode; + inode->i_op = &simple_symlink_inode_operations; + inode->i_link =kstrdup (files->link,GFP_KERNEL ); +if (!inode->i_link) { +iput (inode); +dput (dentry); + goto out; + } + }else { + inode->i_mode =S_IFREG | files->mode; + inode->i_fop = files->ops; + } inode->i_atime = inode->i_mtime = inode->i_ctime =CURRENT_TIME ; - inode->i_fop = files->ops; inode->i_ino = i;d_add (dentry, inode); }
include/linux/fs.h +1 -1
@@ -2953,7 +2953,7 @@ extern const struct file_operations simple_dir_operations;extern const struct inode_operations simple_dir_inode_operations;extern void make_empty_dir_inode (struct inode *inode);extern bool is_empty_dir_inode (struct inode *inode); -struct tree_descr {char *name;const struct file_operations *ops;int mode; }; +struct tree_descr {char *name;const struct file_operations *ops;int mode;char *link; };struct dentry *d_alloc_name (struct dentry *,const char *);extern int simple_fill_super (struct super_block *,unsigned long ,struct tree_descr *);extern int simple_pin_fs (struct file_system_type *,struct vfsmount **mount,int *count);
Signature · 1 line
2.1.4
Re: [Xen-devel] [PATCHv3 1/2] libfs: allow simple_fill_super() to add symlinks
Quoted 2 lines · David Vrabel
simple_fill_super()will add symlinks if an entry has mode &S_IFLNK. The target is provided in the new "link" field.
Can I get an ack for this, please? So it can go into 4.8 via the Xen tree.
David
Quoted 51 lines
Signed-off-by David Vrabel <[email protected]>---
v2:
- simplified.--- fs/libfs.c | 15 +++++++++++++-- include/linux/fs.h | 2 +- 2 files changed, 14 insertions(+), 3 deletions(-)fs/libfs.c +13
-2@@ -512,9 +512,20 @@ int simple_fill_super(struct super_block *s, unsigned long magic,dput (dentry); goto out; } - inode->i_mode =S_IFREG | files->mode; +if (files->mode &S_IFLNK ) { + inode->i_mode = files->mode; + inode->i_op = &simple_symlink_inode_operations; + inode->i_link =kstrdup (files->link,GFP_KERNEL ); +if (!inode->i_link) { +iput (inode); +dput (dentry); + goto out; + } + }else { + inode->i_mode =S_IFREG | files->mode; + inode->i_fop = files->ops; + } inode->i_atime = inode->i_mtime = inode->i_ctime =CURRENT_TIME ; - inode->i_fop = files->ops; inode->i_ino = i;d_add (dentry, inode); }include/linux/fs.h +1
-1@@ -2953,7 +2953,7 @@ extern const struct file_operations simple_dir_operations;extern const struct inode_operations simple_dir_inode_operations;extern void make_empty_dir_inode (struct inode *inode);extern bool is_empty_dir_inode (struct inode *inode); -struct tree_descr {char *name;const struct file_operations *ops;int mode; }; +struct tree_descr {char *name;const struct file_operations *ops;int mode;char *link; };struct dentry *d_alloc_name (struct dentry *,const char *);extern int simple_fill_super (struct super_block *,unsigned long ,struct tree_descr *);extern int simple_pin_fs (struct file_system_type *,struct vfsmount **mount,int *count);
Re: [PATCHv3 1/2] libfs: allow simple_fill_super() to add symlinks
In reply to David VrabelQuoted 2 lines · David Vrabel
simple_fill_super()will add symlinks if an entry has mode &S_IFLNK. The target is provided in the new "link" field.
Can I get an ack for this, please? So it can go into 4.8 via the Xen tree.
David
Quoted 43 lines
Signed-off-by David Vrabel <[email protected]>---
v2:
- simplified.--- fs/libfs.c | 15 +++++++++++++-- include/linux/fs.h | 2 +- 2 files changed, 14 insertions(+), 3 deletions(-)fs/libfs.c +13
-2@@ -512,9 +512,20 @@ int simple_fill_super(struct super_block *s, unsigned long magic,dput (dentry); goto out; } - inode->i_mode =S_IFREG | files->mode; +if (files->mode &S_IFLNK ) { + inode->i_mode = files->mode; + inode->i_op = &simple_symlink_inode_operations; + inode->i_link =kstrdup (files->link,GFP_KERNEL ); +if (!inode->i_link) { +iput (inode); +dput (dentry); + goto out; + } + }else { + inode->i_mode =S_IFREG | files->mode; + inode->i_fop = files->ops; + } inode->i_atime = inode->i_mtime = inode->i_ctime =CURRENT_TIME ; - inode->i_fop = files->ops; inode->i_ino = i;d_add (dentry, inode); }include/linux/fs.h +0
-0
include/linux/fs.h extern const struct file_operations simple_dir_operations; · line 2959
Quoted 8 lines
include/linux/fs.h +1
-1@@ -2953,7 +2953,7 @@ extern const struct file_operations simple_dir_operations;extern const struct inode_operations simple_dir_inode_operations;extern void make_empty_dir_inode (struct inode *inode);extern bool is_empty_dir_inode (struct inode *inode); -struct tree_descr {char *name;const struct file_operations *ops;int mode; }; +struct tree_descr {char *name;const struct file_operations *ops;int mode;char *link; };struct dentry *d_alloc_name (struct dentry *,const char *);extern int simple_fill_super (struct super_block *,unsigned long ,struct tree_descr *);extern int simple_pin_fs (struct file_system_type *,struct vfsmount **mount,int *count);
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel
PATCH v3 2/2 xenfs: replace xenbus and privcmd with symlinks
In reply to David Vrabel/proc/xen/xenbus does not work correctly. A read blocked waiting for a xenstore message holds the mutex needed for atomic file position updates. This blocks any writes on the same file handle, which can deadlock if the write is needed to unblock the read.
/proc/xen/xenbus is supposed to be identical to the character device /dev/xen/xenbus so replace the file with a symlink.
Similarly, replace /proc/xen/privcmd with a symlink since it should be the same as /dev/xen/privcmd.
---
v2:
- remove unneeded includes
--- drivers/xen/xenfs/super.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)
drivers/xen/xenfs/super.c +4 -6
@@ -18,8 +18,6 @@#include <xen/xen.h> #include "xenfs.h" -#include "../privcmd.h" -#include "../xenbus/xenbus_comms.h" #include <asm/xen/hypervisor.h>
@@ -45,16 +43,16 @@ static const struct file_operations capabilities_file_ops = {static int xenfs_fill_super (struct super_block *sb,void *data,int silent) {static struct tree_descr xenfs_files[] = { - [2 ] = {"xenbus" , &xen_xenbus_fops,S_IRUSR |S_IWUSR }, + [2 ] = {"xenbus" ,NULL ,S_IFLNK |S_IRWXUGO ,"/dev/xen/xenbus" }, {"capabilities" , &capabilities_file_ops,S_IRUGO }, - {"privcmd" , &xen_privcmd_fops,S_IRUSR |S_IWUSR }, + {"privcmd" ,NULL ,S_IFLNK |S_IRWXUGO ,"/dev/xen/privcmd" }, {"" }, };static struct tree_descr xenfs_init_files[] = { - [2 ] = {"xenbus" , &xen_xenbus_fops,S_IRUSR |S_IWUSR }, + [2 ] = {"xenbus" ,NULL ,S_IFLNK |S_IRWXUGO ,"/dev/xen/xenbus" }, {"capabilities" , &capabilities_file_ops,S_IRUGO }, - {"privcmd" , &xen_privcmd_fops,S_IRUSR |S_IWUSR }, + {"privcmd" ,NULL ,S_IFLNK |S_IRWXUGO ,"/dev/xen/privcmd" }, {"xsd_kva" , &xsd_kva_file_ops,S_IRUSR |S_IWUSR }, {"xsd_port" , &xsd_port_file_ops,S_IRUSR |S_IWUSR }, #ifdef CONFIG_XEN_SYMS
Signature · 7 lines
2.1.4 _______________________________________________ Xen-devel mailing list [email protected] http://lists.xen.org/xen-devel
Juergen Gross
maintainerRe: [PATCHv3 2/2] xenfs: replace xenbus and privcmd with symlinks
Quoted 8 lines · David Vrabel
/proc/xen/xenbusdoes not work correctly. A read blocked waiting for a xenstore message holds the mutex needed for atomic file position updates. This blocks any writes on the same file handle, which can deadlock if the write is needed to unblock the read.
/proc/xen/xenbusis supposed to be identical to the character device/dev/xen/xenbusso replace the file with a symlink.
Quoted 8 lines
Similarly, replace
/proc/xen/privcmdwith a symlink since it should be the same as/dev/xen/privcmd.Signed-off-by David Vrabel <[email protected]>---
v2:
- remove unneeded includes
---
I think you should make xen_xenbus_fops and xen_privcmd_fops static now that they are no longer referenced by super.c
Juergen
Quoted 38 lines
drivers/xen/xenfs/super.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)drivers/xen/xenfs/super.c +4
-6@@ -18,8 +18,6 @@#include <xen/xen.h> #include "xenfs.h" -#include "../privcmd.h" -#include "../xenbus/xenbus_comms.h" #include <asm/xen/hypervisor.h> @@ -45,16 +43,16 @@ static const struct file_operations capabilities_file_ops = {static int xenfs_fill_super (struct super_block *sb,void *data,int silent) {static struct tree_descr xenfs_files[] = { - [2 ] = {"xenbus" , &xen_xenbus_fops,S_IRUSR |S_IWUSR }, + [2 ] = {"xenbus" ,NULL ,S_IFLNK |S_IRWXUGO ,"/dev/xen/xenbus" }, {"capabilities" , &capabilities_file_ops,S_IRUGO }, - {"privcmd" , &xen_privcmd_fops,S_IRUSR |S_IWUSR }, + {"privcmd" ,NULL ,S_IFLNK |S_IRWXUGO ,"/dev/xen/privcmd" }, {"" }, };static struct tree_descr xenfs_init_files[] = { - [2 ] = {"xenbus" , &xen_xenbus_fops,S_IRUSR |S_IWUSR }, + [2 ] = {"xenbus" ,NULL ,S_IFLNK |S_IRWXUGO ,"/dev/xen/xenbus" }, {"capabilities" , &capabilities_file_ops,S_IRUGO }, - {"privcmd" , &xen_privcmd_fops,S_IRUSR |S_IWUSR }, + {"privcmd" ,NULL ,S_IFLNK |S_IRWXUGO ,"/dev/xen/privcmd" }, {"xsd_kva" , &xsd_kva_file_ops,S_IRUSR |S_IWUSR }, {"xsd_port" , &xsd_port_file_ops,S_IRUSR |S_IWUSR }, #ifdef CONFIG_XEN_SYMS
Juergen Gross
maintainerRe: [PATCHv3 2/2] xenfs: replace xenbus and privcmd with symlinks
In reply to David VrabelQuoted 8 lines · David Vrabel
/proc/xen/xenbusdoes not work correctly. A read blocked waiting for a xenstore message holds the mutex needed for atomic file position updates. This blocks any writes on the same file handle, which can deadlock if the write is needed to unblock the read.
/proc/xen/xenbusis supposed to be identical to the character device/dev/xen/xenbusso replace the file with a symlink.
Quoted 8 lines
Similarly, replace
/proc/xen/privcmdwith a symlink since it should be the same as/dev/xen/privcmd.Signed-off-by David Vrabel <[email protected]>---
v2:
- remove unneeded includes
---
I think you should make xen_xenbus_fops and xen_privcmd_fops static now that they are no longer referenced by super.c
Juergen
Quoted 28 lines
drivers/xen/xenfs/super.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)drivers/xen/xenfs/super.c +2
-4@@ -18,8 +18,6 @@#include <xen/xen.h> #include "xenfs.h" -#include "../privcmd.h" -#include "../xenbus/xenbus_comms.h" #include <asm/xen/hypervisor.h> @@ -45,16 +43,16 @@ static const struct file_operations capabilities_file_ops = {static int xenfs_fill_super (struct super_block *sb,void *data,int silent) {static struct tree_descr xenfs_files[] = { - [2 ] = {"xenbus" , &xen_xenbus_fops,S_IRUSR |S_IWUSR }, + [2 ] = {"xenbus" ,NULL ,S_IFLNK |S_IRWXUGO ,"/dev/xen/xenbus" }, {"capabilities" , &capabilities_file_ops,S_IRUGO }, - {"privcmd" , &xen_privcmd_fops,S_IRUSR |S_IWUSR }, + {"privcmd" ,NULL ,S_IFLNK |S_IRWXUGO ,"/dev/xen/privcmd" }, {"" }, };
drivers/xen/xenfs/super.c static const struct file_operations capabilities_file_ops = { · line 58
Quoted 10 lines
drivers/xen/xenfs/super.c +2
-2static struct tree_descr xenfs_init_files[] = { - [2 ] = {"xenbus" , &xen_xenbus_fops,S_IRUSR |S_IWUSR }, + [2 ] = {"xenbus" ,NULL ,S_IFLNK |S_IRWXUGO ,"/dev/xen/xenbus" }, {"capabilities" , &capabilities_file_ops,S_IRUGO }, - {"privcmd" , &xen_privcmd_fops,S_IRUSR |S_IWUSR }, + {"privcmd" ,NULL ,S_IFLNK |S_IRWXUGO ,"/dev/xen/privcmd" }, {"xsd_kva" , &xsd_kva_file_ops,S_IRUSR |S_IWUSR }, {"xsd_port" , &xsd_port_file_ops,S_IRUSR |S_IWUSR }, #ifdef CONFIG_XEN_SYMS
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCHv3 0/2] libfs, xenfs: replace /proc/xen/xenbus with a symlink
In reply to David VrabelQuoted 5 lines · David Vrabel
Using
/proc/xen/xenbuscan cause deadlocks on the atomic file position mutex since this file should behave like a character device and not a regular file. This is easiest to achive by making it a symlink to the existing/dev/xen/xenbusdevice.
Quoted 7 lines
This requires extending
simple_fill_super()to add symlinks as well as regular files.Changes in v3:
- Rebased on v4.7-rc5.David
Just a bit of a nudge to see where the status of this is at? We're at the point of potentially having yet another kernel release go out the door where the default behavior with Xen 4.6 and earlier is to deadlock domUs.
Signature · 1 line
Doug Goldstein
Re: [PATCHv3 0/2] libfs, xenfs: replace /proc/xen/xenbus with a symlink
In reply to David VrabelQuoted 5 lines · David Vrabel
Using
/proc/xen/xenbuscan cause deadlocks on the atomic file position mutex since this file should behave like a character device and not a regular file. This is easiest to achive by making it a symlink to the existing/dev/xen/xenbusdevice.
Quoted 7 lines
This requires extending
simple_fill_super()to add symlinks as well as regular files.Changes in v3:
- Rebased on v4.7-rc5.David
Just a bit of a nudge to see where the status of this is at? We're at the point of potentially having yet another kernel release go out the door where the default behavior with Xen 4.6 and earlier is to deadlock domUs.
Signature · 1 line
Doug Goldstein
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel