| Message ID | 20220223180859.GA1412216@dingwall.me.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC] kernel: xenfs parameter to hide deprecated files | expand |
On 23.02.22 19:08, James Dingwall wrote: > Hi, > > I have been investigating a very intermittent issue we have with xenstore > access hanging. Typically it seems to happen when all domains are stopped > prior to a system reboot. xenstore is running in a stubdom and using the > hypervisor debug keys indicates the domain is still there. Could it be dom0 shutdown handling is unloading some modules which are needed for Xenstore communication? E.g. xen-evtchn? > > I have come across some old list threads which suggested access via > /proc/xen/xenbus could cause problems but it seems patches went in to the > kernel for 4.10. However to eliminate this entirely as a possibility > I came up with this kernel patch to hide deprecated entries in xenfs. I don't see how this patch could help. libxenstore is using /dev/xen/xenbus if it is available. So the only case where your patch would avoid accessing /proc/xen/xenbus would be if /dev/xen/xenbus isn't there. But this wouldn't make Xenstore more reactive, I guess. ;-) > I found this old thread for a similar change where the entries were made > conditional on kernel config options instead of a module parameter but > this was never merged. > > https://lkml.org/lkml/2015/11/30/761 > > If this would be a useful feature I would welcome feedback. I'm not sure how helpful it is to let the user specify a boot parameter for hiding the files. It will probably not get used a lot. Juergen
Hi Juergen, On Fri, Feb 25, 2022 at 03:09:05PM +0100, Juergen Gross wrote: > On 23.02.22 19:08, James Dingwall wrote: > > Hi, > > > > I have been investigating a very intermittent issue we have with xenstore > > access hanging. Typically it seems to happen when all domains are stopped > > prior to a system reboot. xenstore is running in a stubdom and using the > > hypervisor debug keys indicates the domain is still there. > > Could it be dom0 shutdown handling is unloading some modules which are > needed for Xenstore communication? E.g. xen-evtchn? > > > > > I have come across some old list threads which suggested access via > > /proc/xen/xenbus could cause problems but it seems patches went in to the > > kernel for 4.10. However to eliminate this entirely as a possibility > > I came up with this kernel patch to hide deprecated entries in xenfs. > > I don't see how this patch could help. > > libxenstore is using /dev/xen/xenbus if it is available. So the only > case where your patch would avoid accessing /proc/xen/xenbus would be > if /dev/xen/xenbus isn't there. But this wouldn't make Xenstore more > reactive, I guess. ;-) > > > I found this old thread for a similar change where the entries were made > > conditional on kernel config options instead of a module parameter but > > this was never merged. > > > > https://lkml.org/lkml/2015/11/30/761 > > > > If this would be a useful feature I would welcome feedback. > > I'm not sure how helpful it is to let the user specify a boot parameter > for hiding the files. It will probably not get used a lot. Thank you for taking the time to look this over. I did suspect it might not be relevant for most people. I'll keep it in our build for now to see if we improve our xenstore stability. Thank you also for your suggestions about why we might be having a xenstore problem. Next time we encounter that I'll check the status of the loaded modules. Regards, James
diff --git a/drivers/xen/xenfs/super.c b/drivers/xen/xenfs/super.c index d7d64235010d..d02c451f6a4d 100644 --- a/drivers/xen/xenfs/super.c +++ b/drivers/xen/xenfs/super.c @@ -3,6 +3,11 @@ * xenfs.c - a filesystem for passing info between the a domain and * the hypervisor. * + * 2022-02-12 James Dingwall Introduce hide_deprecated module parameter to + * mask: + * - xenbus (deprecated in xen 4.6.0) + * - privcmd (deprecated in xen 4.7.0) + * * 2008-10-07 Alex Zeffertt Replaced /proc/xen/xenbus with xenfs filesystem * and /proc/xen compatibility mount point. * Turned xenfs into a loadable module. @@ -28,6 +33,13 @@ MODULE_DESCRIPTION("Xen filesystem"); MODULE_LICENSE("GPL"); +static bool __read_mostly hide_deprecated = 0; +module_param(hide_deprecated, bool, 0444); +MODULE_PARM_DESC(hide_deprecated, + "Allow deprecated files to be hidden in xenfs.\n"\ + " 0 - (default) show deprecated xenfs files."\ + " 1 - hide deprecated xenfs files [xenbus, privcmd].\n"); + static ssize_t capabilities_read(struct file *file, char __user *buf, size_t size, loff_t *off) { @@ -69,8 +81,32 @@ static int xenfs_fill_super(struct super_block *sb, struct fs_context *fc) xen_initial_domain() ? xenfs_init_files : xenfs_files); } +static int xenfs_fill_super_hide_deprecated(struct super_block *sb, struct fs_context *fc) +{ + static const struct tree_descr xenfs_files[] = { + [2] = { "capabilities", &capabilities_file_ops, S_IRUGO }, + {""}, + }; + + static const struct tree_descr xenfs_init_files[] = { + [2] = { "capabilities", &capabilities_file_ops, S_IRUGO }, + { "xsd_kva", &xsd_kva_file_ops, S_IRUSR|S_IWUSR}, + { "xsd_port", &xsd_port_file_ops, S_IRUSR|S_IWUSR}, +#ifdef CONFIG_XEN_SYMS + { "xensyms", &xensyms_ops, S_IRUSR}, +#endif + {""}, + }; + + return simple_fill_super(sb, XENFS_SUPER_MAGIC, + xen_initial_domain() ? xenfs_init_files : xenfs_files); +} + static int xenfs_get_tree(struct fs_context *fc) { + if(hide_deprecated) + return get_tree_single(fc, xenfs_fill_super_hide_deprecated); + return get_tree_single(fc, xenfs_fill_super); }