* Re: [PATCH v3] memfd_test: Make it work on 32-bit systems
From: Shuah Khan @ 2014-09-03 21:41 UTC (permalink / raw)
To: Pranith Kumar, David Herrmann, Hugh Dickins, Andrew Morton,
open list:KERNEL SELFTEST F..., open list
Cc: Shuah Khan
In-Reply-To: <1409754679-22667-1-git-send-email-bobby.prani@gmail.com>
On 09/03/2014 08:31 AM, Pranith Kumar wrote:
> This test currently fails on 32-bit systems since we use u64 type to pass the
> flags to fcntl.
>
> This commit changes this to use 'unsigned int' type for flags to fcntl making it
> work on 32-bit systems.
>
> Signed-off-by: Pranith Kumar <bobby.prani@gmail.com>
> Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
> ---
I will queue this up for 3.17-fixes. I am in the process of
setting up kselftest git for fixes.
thanks,
-- Shuah
--
Shuah Khan
Sr. Linux Kernel Developer
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978
^ permalink raw reply
* [PATCH v4 04/11] drivers: base: support cpu cache information interface to userspace via sysfs
From: Sudeep Holla @ 2014-09-03 17:00 UTC (permalink / raw)
To: LKML
Cc: linux-s390, Lorenzo Pieralisi, linux-ia64, Greg Kroah-Hartman,
Sudeep Holla, Heiko Carstens, x86, linux-api, linux390,
linuxppc-dev, Stephen Boyd, linux-arm-kernel
In-Reply-To: <1409763617-17074-1-git-send-email-sudeep.holla@arm.com>
From: Sudeep Holla <sudeep.holla@arm.com>
This patch adds initial support for providing processor cache information
to userspace through sysfs interface. This is based on already existing
implementations(x86, ia64, s390 and powerpc) and hence the interface is
intended to be fully compatible.
The main purpose of this generic support is to avoid further code
duplication to support new architectures and also to unify all the existing
different implementations.
This implementation maintains the hierarchy of cache objects which reflects
the system's cache topology. Cache devices are instantiated as needed as
CPUs come online. The cache information is replicated per-cpu even if they are
shared. A per-cpu array of cache information maintained is used mainly for
sysfs-related book keeping.
It also implements the shared_cpu_map attribute, which is essential for
enabling both kernel and user-space to discover the system's overall cache
topology.
This patch also add the missing ABI documentation for the cacheinfo sysfs
interface already, which is well defined and widely used.
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: linux-api@vger.kernel.org
Cc: linux390@de.ibm.com
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-ia64@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-s390@vger.kernel.org
Cc: x86@kernel.org
---
Documentation/ABI/testing/sysfs-devices-system-cpu | 47 ++
drivers/base/Makefile | 2 +-
drivers/base/cacheinfo.c | 541 +++++++++++++++++++++
include/linux/cacheinfo.h | 100 ++++
4 files changed, 689 insertions(+), 1 deletion(-)
create mode 100644 drivers/base/cacheinfo.c
create mode 100644 include/linux/cacheinfo.h
diff --git a/Documentation/ABI/testing/sysfs-devices-system-cpu b/Documentation/ABI/testing/sysfs-devices-system-cpu
index acb9bfc89b48..99983e67c13c 100644
--- a/Documentation/ABI/testing/sysfs-devices-system-cpu
+++ b/Documentation/ABI/testing/sysfs-devices-system-cpu
@@ -224,3 +224,50 @@ Description: Parameters for the Intel P-state driver
frequency range.
More details can be found in Documentation/cpu-freq/intel-pstate.txt
+
+What: /sys/devices/system/cpu/cpu*/cache/index*/<set_of_attributes_mentioned_below>
+Date: July 2014(documented, existed before August 2008)
+Contact: Sudeep Holla <sudeep.holla@arm.com>
+ Linux kernel mailing list <linux-kernel@vger.kernel.org>
+Description: Parameters for the CPU cache attributes
+
+ allocation_policy:
+ - WriteAllocate: allocate a memory location to a cache line
+ on a cache miss because of a write
+ - ReadAllocate: allocate a memory location to a cache line
+ on a cache miss because of a read
+ - ReadWriteAllocate: both writeallocate and readallocate
+
+ attributes: LEGACY used only on IA64 and is same as write_policy
+
+ coherency_line_size: the minimum amount of data in bytes that gets
+ transferred from memory to cache
+
+ level: the cache hierarcy in the multi-level cache configuration
+
+ number_of_sets: total number of sets in the cache, a set is a
+ collection of cache lines with the same cache index
+
+ physical_line_partition: number of physical cache line per cache tag
+
+ shared_cpu_list: the list of logical cpus sharing the cache
+
+ shared_cpu_map: logical cpu mask containing the list of cpus sharing
+ the cache
+
+ size: the total cache size in kB
+
+ type:
+ - Instruction: cache that only holds instructions
+ - Data: cache that only caches data
+ - Unified: cache that holds both data and instructions
+
+ ways_of_associativity: degree of freedom in placing a particular block
+ of memory in the cache
+
+ write_policy:
+ - WriteThrough: data is written to both the cache line
+ and to the block in the lower-level memory
+ - WriteBack: data is written only to the cache line and
+ the modified cache line is written to main
+ memory only when it is replaced
diff --git a/drivers/base/Makefile b/drivers/base/Makefile
index 4aab26ec0292..f901bc1cffc8 100644
--- a/drivers/base/Makefile
+++ b/drivers/base/Makefile
@@ -4,7 +4,7 @@ obj-y := component.o core.o bus.o dd.o syscore.o \
driver.o class.o platform.o \
cpu.o firmware.o init.o map.o devres.o \
attribute_container.o transport_class.o \
- topology.o container.o
+ topology.o container.o cacheinfo.o
obj-$(CONFIG_DEVTMPFS) += devtmpfs.o
obj-$(CONFIG_DMA_CMA) += dma-contiguous.o
obj-y += power/
diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c
new file mode 100644
index 000000000000..9534044cca79
--- /dev/null
+++ b/drivers/base/cacheinfo.c
@@ -0,0 +1,541 @@
+/*
+ * cacheinfo support - processor cache information via sysfs
+ *
+ * Based on arch/x86/kernel/cpu/intel_cacheinfo.c
+ * Author: Sudeep Holla <sudeep.holla@arm.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+#include <linux/bitops.h>
+#include <linux/cacheinfo.h>
+#include <linux/compiler.h>
+#include <linux/cpu.h>
+#include <linux/device.h>
+#include <linux/init.h>
+#include <linux/of.h>
+#include <linux/sched.h>
+#include <linux/slab.h>
+#include <linux/smp.h>
+#include <linux/sysfs.h>
+
+/* pointer to per cpu cacheinfo */
+static DEFINE_PER_CPU(struct cpu_cacheinfo, ci_cpu_cacheinfo);
+#define ci_cacheinfo(cpu) (&per_cpu(ci_cpu_cacheinfo, cpu))
+#define cache_leaves(cpu) (ci_cacheinfo(cpu)->num_leaves)
+#define per_cpu_cacheinfo(cpu) (ci_cacheinfo(cpu)->info_list)
+
+struct cpu_cacheinfo *get_cpu_cacheinfo(unsigned int cpu)
+{
+ return ci_cacheinfo(cpu);
+}
+
+#ifdef CONFIG_OF
+static int cache_setup_of_node(unsigned int cpu)
+{
+ struct device_node *np;
+ struct cacheinfo *this_leaf;
+ struct device *cpu_dev = get_cpu_device(cpu);
+ struct cpu_cacheinfo *this_cpu_ci = get_cpu_cacheinfo(cpu);
+ unsigned int index = 0;
+
+ /* skip if of_node is already populated */
+ if (this_cpu_ci->info_list->of_node)
+ return 0;
+
+ if (!cpu_dev) {
+ pr_err("No cpu device for CPU %d\n", cpu);
+ return -ENODEV;
+ }
+ np = cpu_dev->of_node;
+ if (!np) {
+ pr_err("Failed to find cpu%d device node\n", cpu);
+ return -ENOENT;
+ }
+
+ while (np && index < cache_leaves(cpu)) {
+ this_leaf = this_cpu_ci->info_list + index;
+ if (this_leaf->level != 1)
+ np = of_find_next_cache_node(np);
+ else
+ np = of_node_get(np);/* cpu node itself */
+ this_leaf->of_node = np;
+ index++;
+ }
+ return 0;
+}
+
+static inline bool cache_leaves_are_shared(struct cacheinfo *this_leaf,
+ struct cacheinfo *sib_leaf)
+{
+ return sib_leaf->of_node == this_leaf->of_node;
+}
+#else
+static inline int cache_setup_of_node(unsigned int cpu) { return 0; }
+static inline bool cache_leaves_are_shared(struct cacheinfo *this_leaf,
+ struct cacheinfo *sib_leaf)
+{
+ /*
+ * For non-DT systems, assume unique level 1 cache, system-wide
+ * shared caches for all other levels. This will be used only if
+ * arch specific code has not populated shared_cpu_map
+ */
+ return !(this_leaf->level == 1);
+}
+#endif
+
+static int cache_shared_cpu_map_setup(unsigned int cpu)
+{
+ struct cpu_cacheinfo *this_cpu_ci = get_cpu_cacheinfo(cpu);
+ struct cacheinfo *this_leaf, *sib_leaf;
+ unsigned int index;
+ int ret;
+
+ ret = cache_setup_of_node(cpu);
+ if (ret)
+ return ret;
+
+ for (index = 0; index < cache_leaves(cpu); index++) {
+ unsigned int i;
+
+ this_leaf = this_cpu_ci->info_list + index;
+ /* skip if shared_cpu_map is already populated */
+ if (!cpumask_empty(&this_leaf->shared_cpu_map))
+ continue;
+
+ cpumask_set_cpu(cpu, &this_leaf->shared_cpu_map);
+ for_each_online_cpu(i) {
+ struct cpu_cacheinfo *sib_cpu_ci = get_cpu_cacheinfo(i);
+
+ if (i == cpu || !sib_cpu_ci->info_list)
+ continue;/* skip if itself or no cacheinfo */
+ sib_leaf = sib_cpu_ci->info_list + index;
+ if (cache_leaves_are_shared(this_leaf, sib_leaf)) {
+ cpumask_set_cpu(cpu, &sib_leaf->shared_cpu_map);
+ cpumask_set_cpu(i, &this_leaf->shared_cpu_map);
+ }
+ }
+ }
+
+ return 0;
+}
+
+static void cache_shared_cpu_map_remove(unsigned int cpu)
+{
+ struct cpu_cacheinfo *this_cpu_ci = get_cpu_cacheinfo(cpu);
+ struct cacheinfo *this_leaf, *sib_leaf;
+ unsigned int sibling, index;
+
+ for (index = 0; index < cache_leaves(cpu); index++) {
+ this_leaf = this_cpu_ci->info_list + index;
+ for_each_cpu(sibling, &this_leaf->shared_cpu_map) {
+ struct cpu_cacheinfo *sib_cpu_ci;
+
+ if (sibling == cpu) /* skip itself */
+ continue;
+ sib_cpu_ci = get_cpu_cacheinfo(sibling);
+ sib_leaf = sib_cpu_ci->info_list + index;
+ cpumask_clear_cpu(cpu, &sib_leaf->shared_cpu_map);
+ cpumask_clear_cpu(sibling, &this_leaf->shared_cpu_map);
+ }
+ of_node_put(this_leaf->of_node);
+ }
+}
+
+static void free_cache_attributes(unsigned int cpu)
+{
+ cache_shared_cpu_map_remove(cpu);
+
+ kfree(per_cpu_cacheinfo(cpu));
+ per_cpu_cacheinfo(cpu) = NULL;
+}
+
+int __weak init_cache_level(unsigned int cpu)
+{
+ return -ENOENT;
+}
+
+int __weak populate_cache_leaves(unsigned int cpu)
+{
+ return -ENOENT;
+}
+
+static int detect_cache_attributes(unsigned int cpu)
+{
+ int ret;
+
+ if (init_cache_level(cpu))
+ return -ENOENT;
+
+ per_cpu_cacheinfo(cpu) = kcalloc(cache_leaves(cpu),
+ sizeof(struct cacheinfo), GFP_KERNEL);
+ if (per_cpu_cacheinfo(cpu) == NULL)
+ return -ENOMEM;
+
+ ret = populate_cache_leaves(cpu);
+ if (ret)
+ goto free_ci;
+ /*
+ * For systems using DT for cache hierarcy, of_node and shared_cpu_map
+ * will be set up here only if they are not populated already
+ */
+ ret = cache_shared_cpu_map_setup(cpu);
+ if (ret)
+ goto free_ci;
+ return 0;
+
+free_ci:
+ free_cache_attributes(cpu);
+ return ret;
+}
+
+/* pointer to cpuX/cache device */
+static DEFINE_PER_CPU(struct device *, ci_cache_dev);
+#define per_cpu_cache_dev(cpu) (per_cpu(ci_cache_dev, cpu))
+
+static cpumask_t cache_dev_map;
+
+/* pointer to array of devices for cpuX/cache/indexY */
+static DEFINE_PER_CPU(struct device **, ci_index_dev);
+#define per_cpu_index_dev(cpu) (per_cpu(ci_index_dev, cpu))
+#define per_cache_index_dev(cpu, idx) ((per_cpu_index_dev(cpu))[idx])
+
+#define show_one(file_name, object) \
+static ssize_t file_name##_show(struct device *dev, \
+ struct device_attribute *attr, char *buf) \
+{ \
+ struct cacheinfo *this_leaf = dev_get_drvdata(dev); \
+ return sprintf(buf, "%u\n", this_leaf->object); \
+}
+
+show_one(level, level);
+show_one(coherency_line_size, coherency_line_size);
+show_one(number_of_sets, number_of_sets);
+show_one(physical_line_partition, physical_line_partition);
+show_one(ways_of_associativity, ways_of_associativity);
+
+static ssize_t size_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct cacheinfo *this_leaf = dev_get_drvdata(dev);
+
+ return sprintf(buf, "%uK\n", this_leaf->size >> 10);
+}
+
+static ssize_t shared_cpumap_show_func(struct device *dev, bool list, char *buf)
+{
+ struct cacheinfo *this_leaf = dev_get_drvdata(dev);
+ const struct cpumask *mask = &this_leaf->shared_cpu_map;
+
+ return cpumap_copy_to_buf(list, mask, buf);
+}
+
+static ssize_t shared_cpu_map_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return shared_cpumap_show_func(dev, false, buf);
+}
+
+static ssize_t shared_cpu_list_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return shared_cpumap_show_func(dev, true, buf);
+}
+
+static ssize_t type_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct cacheinfo *this_leaf = dev_get_drvdata(dev);
+
+ switch (this_leaf->type) {
+ case CACHE_TYPE_DATA:
+ return sprintf(buf, "Data\n");
+ case CACHE_TYPE_INST:
+ return sprintf(buf, "Instruction\n");
+ case CACHE_TYPE_UNIFIED:
+ return sprintf(buf, "Unified\n");
+ default:
+ return -EINVAL;
+ }
+}
+
+static ssize_t allocation_policy_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct cacheinfo *this_leaf = dev_get_drvdata(dev);
+ unsigned int ci_attr = this_leaf->attributes;
+ int n = 0;
+
+ if ((ci_attr & CACHE_READ_ALLOCATE) && (ci_attr & CACHE_WRITE_ALLOCATE))
+ n = sprintf(buf, "ReadWriteAllocate\n");
+ else if (ci_attr & CACHE_READ_ALLOCATE)
+ n = sprintf(buf, "ReadAllocate\n");
+ else if (ci_attr & CACHE_WRITE_ALLOCATE)
+ n = sprintf(buf, "WriteAllocate\n");
+ return n;
+}
+
+static ssize_t write_policy_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct cacheinfo *this_leaf = dev_get_drvdata(dev);
+ unsigned int ci_attr = this_leaf->attributes;
+ int n = 0;
+
+ if (ci_attr & CACHE_WRITE_THROUGH)
+ n = sprintf(buf, "WriteThrough\n");
+ else if (ci_attr & CACHE_WRITE_BACK)
+ n = sprintf(buf, "WriteBack\n");
+ return n;
+}
+
+static DEVICE_ATTR_RO(level);
+static DEVICE_ATTR_RO(type);
+static DEVICE_ATTR_RO(coherency_line_size);
+static DEVICE_ATTR_RO(ways_of_associativity);
+static DEVICE_ATTR_RO(number_of_sets);
+static DEVICE_ATTR_RO(size);
+static DEVICE_ATTR_RO(allocation_policy);
+static DEVICE_ATTR_RO(write_policy);
+static DEVICE_ATTR_RO(shared_cpu_map);
+static DEVICE_ATTR_RO(shared_cpu_list);
+static DEVICE_ATTR_RO(physical_line_partition);
+
+static struct attribute *cache_default_attrs[] = {
+ &dev_attr_type.attr,
+ &dev_attr_level.attr,
+ &dev_attr_shared_cpu_map.attr,
+ &dev_attr_shared_cpu_list.attr,
+ &dev_attr_coherency_line_size.attr,
+ &dev_attr_ways_of_associativity.attr,
+ &dev_attr_number_of_sets.attr,
+ &dev_attr_size.attr,
+ &dev_attr_allocation_policy.attr,
+ &dev_attr_write_policy.attr,
+ &dev_attr_physical_line_partition.attr,
+ NULL
+};
+
+static umode_t
+cache_default_attrs_is_visible(struct kobject *kobj,
+ struct attribute *attr, int unused)
+{
+ struct device *dev = kobj_to_dev(kobj);
+ struct cacheinfo *this_leaf = dev_get_drvdata(dev);
+ const struct cpumask *mask = &this_leaf->shared_cpu_map;
+ umode_t mode = attr->mode;
+
+ if ((attr == &dev_attr_type.attr) && this_leaf->type)
+ return mode;
+ if ((attr == &dev_attr_level.attr) && this_leaf->level)
+ return mode;
+ if ((attr == &dev_attr_shared_cpu_map.attr) && !cpumask_empty(mask))
+ return mode;
+ if ((attr == &dev_attr_shared_cpu_list.attr) && !cpumask_empty(mask))
+ return mode;
+ if ((attr == &dev_attr_coherency_line_size.attr) &&
+ this_leaf->coherency_line_size)
+ return mode;
+ if ((attr == &dev_attr_ways_of_associativity.attr) &&
+ this_leaf->size) /* allow 0 = full associativity */
+ return mode;
+ if ((attr == &dev_attr_number_of_sets.attr) &&
+ this_leaf->number_of_sets)
+ return mode;
+ if ((attr == &dev_attr_size.attr) && this_leaf->size)
+ return mode;
+ if ((attr == &dev_attr_write_policy.attr) &&
+ (this_leaf->attributes & CACHE_WRITE_POLICY_MASK))
+ return mode;
+ if ((attr == &dev_attr_allocation_policy.attr) &&
+ (this_leaf->attributes & CACHE_ALLOCATE_POLICY_MASK))
+ return mode;
+ if ((attr == &dev_attr_physical_line_partition.attr) &&
+ this_leaf->physical_line_partition)
+ return mode;
+
+ return 0;
+}
+
+static const struct attribute_group cache_default_group = {
+ .attrs = cache_default_attrs,
+ .is_visible = cache_default_attrs_is_visible,
+};
+
+static const struct attribute_group *cache_default_groups[] = {
+ &cache_default_group,
+ NULL,
+};
+
+static const struct attribute_group *cache_private_groups[] = {
+ &cache_default_group,
+ NULL, /* Place holder for private group */
+ NULL,
+};
+
+const struct attribute_group *
+__weak cache_get_priv_group(struct cacheinfo *this_leaf)
+{
+ return NULL;
+}
+
+static const struct attribute_group **
+cache_get_attribute_groups(struct cacheinfo *this_leaf)
+{
+ const struct attribute_group *priv_group =
+ cache_get_priv_group(this_leaf);
+
+ if (!priv_group)
+ return cache_default_groups;
+
+ if (!cache_private_groups[1])
+ cache_private_groups[1] = priv_group;
+
+ return cache_private_groups;
+}
+
+/* Add/Remove cache interface for CPU device */
+static void cpu_cache_sysfs_exit(unsigned int cpu)
+{
+ int i;
+ struct device *ci_dev;
+
+ if (per_cpu_index_dev(cpu)) {
+ for (i = 0; i < cache_leaves(cpu); i++) {
+ ci_dev = per_cache_index_dev(cpu, i);
+ if (!ci_dev)
+ continue;
+ device_unregister(ci_dev);
+ }
+ kfree(per_cpu_index_dev(cpu));
+ per_cpu_index_dev(cpu) = NULL;
+ }
+ device_unregister(per_cpu_cache_dev(cpu));
+ per_cpu_cache_dev(cpu) = NULL;
+}
+
+static int cpu_cache_sysfs_init(unsigned int cpu)
+{
+ struct device *dev = get_cpu_device(cpu);
+
+ if (per_cpu_cacheinfo(cpu) == NULL)
+ return -ENOENT;
+
+ per_cpu_cache_dev(cpu) = cpu_device_create(dev, NULL, NULL, "cache");
+ if (IS_ERR(per_cpu_cache_dev(cpu)))
+ return PTR_ERR(per_cpu_cache_dev(cpu));
+
+ /* Allocate all required memory */
+ per_cpu_index_dev(cpu) = kcalloc(cache_leaves(cpu),
+ sizeof(struct device *), GFP_KERNEL);
+ if (unlikely(per_cpu_index_dev(cpu) == NULL))
+ goto err_out;
+
+ return 0;
+
+err_out:
+ cpu_cache_sysfs_exit(cpu);
+ return -ENOMEM;
+}
+
+static int cache_add_dev(unsigned int cpu)
+{
+ unsigned int i;
+ int rc;
+ struct device *ci_dev, *parent;
+ struct cacheinfo *this_leaf;
+ struct cpu_cacheinfo *this_cpu_ci = get_cpu_cacheinfo(cpu);
+ const struct attribute_group **cache_groups;
+
+ rc = cpu_cache_sysfs_init(cpu);
+ if (unlikely(rc < 0))
+ return rc;
+
+ parent = per_cpu_cache_dev(cpu);
+ for (i = 0; i < cache_leaves(cpu); i++) {
+ this_leaf = this_cpu_ci->info_list + i;
+ if (this_leaf->disable_sysfs)
+ continue;
+ cache_groups = cache_get_attribute_groups(this_leaf);
+ ci_dev = cpu_device_create(parent, this_leaf, cache_groups,
+ "index%1u", i);
+ if (IS_ERR(ci_dev)) {
+ rc = PTR_ERR(ci_dev);
+ goto err;
+ }
+ per_cache_index_dev(cpu, i) = ci_dev;
+ }
+ cpumask_set_cpu(cpu, &cache_dev_map);
+
+ return 0;
+err:
+ cpu_cache_sysfs_exit(cpu);
+ return rc;
+}
+
+static void cache_remove_dev(unsigned int cpu)
+{
+ if (!cpumask_test_cpu(cpu, &cache_dev_map))
+ return;
+ cpumask_clear_cpu(cpu, &cache_dev_map);
+
+ cpu_cache_sysfs_exit(cpu);
+}
+
+static int cacheinfo_cpu_callback(struct notifier_block *nfb,
+ unsigned long action, void *hcpu)
+{
+ unsigned int cpu = (unsigned long)hcpu;
+ int rc = 0;
+
+ switch (action & ~CPU_TASKS_FROZEN) {
+ case CPU_ONLINE:
+ rc = detect_cache_attributes(cpu);
+ if (!rc)
+ rc = cache_add_dev(cpu);
+ break;
+ case CPU_DEAD:
+ cache_remove_dev(cpu);
+ if (per_cpu_cacheinfo(cpu))
+ free_cache_attributes(cpu);
+ break;
+ }
+ return notifier_from_errno(rc);
+}
+
+static int __init cacheinfo_sysfs_init(void)
+{
+ int cpu, rc = 0;
+
+ cpu_notifier_register_begin();
+
+ for_each_online_cpu(cpu) {
+ rc = detect_cache_attributes(cpu);
+ if (rc) {
+ pr_err("error detecting cacheinfo..cpu%d\n", cpu);
+ goto out;
+ }
+ rc = cache_add_dev(cpu);
+ if (rc) {
+ free_cache_attributes(cpu);
+ pr_err("error populating cacheinfo..cpu%d\n", cpu);
+ goto out;
+ }
+ }
+ __hotcpu_notifier(cacheinfo_cpu_callback, 0);
+
+out:
+ cpu_notifier_register_done();
+ return rc;
+}
+
+device_initcall(cacheinfo_sysfs_init);
diff --git a/include/linux/cacheinfo.h b/include/linux/cacheinfo.h
new file mode 100644
index 000000000000..3daf5ed392c9
--- /dev/null
+++ b/include/linux/cacheinfo.h
@@ -0,0 +1,100 @@
+#ifndef _LINUX_CACHEINFO_H
+#define _LINUX_CACHEINFO_H
+
+#include <linux/bitops.h>
+#include <linux/cpumask.h>
+#include <linux/smp.h>
+
+struct device_node;
+struct attribute;
+
+enum cache_type {
+ CACHE_TYPE_NOCACHE = 0,
+ CACHE_TYPE_INST = BIT(0),
+ CACHE_TYPE_DATA = BIT(1),
+ CACHE_TYPE_SEPARATE = CACHE_TYPE_INST | CACHE_TYPE_DATA,
+ CACHE_TYPE_UNIFIED = BIT(2),
+};
+
+/**
+ * struct cacheinfo - represent a cache leaf node
+ * @type: type of the cache - data, inst or unified
+ * @level: represents the hierarcy in the multi-level cache
+ * @coherency_line_size: size of each cache line usually representing
+ * the minimum amount of data that gets transferred from memory
+ * @number_of_sets: total number of sets, a set is a collection of cache
+ * lines sharing the same index
+ * @ways_of_associativity: number of ways in which a particular memory
+ * block can be placed in the cache
+ * @physical_line_partition: number of physical cache lines sharing the
+ * same cachetag
+ * @size: Total size of the cache
+ * @shared_cpu_map: logical cpumask representing all the cpus sharing
+ * this cache node
+ * @attributes: bitfield representing various cache attributes
+ * @of_node: if devicetree is used, this represents either the cpu node in
+ * case there's no explicit cache node or the cache node itself in the
+ * device tree
+ * @disable_sysfs: indicates whether this node is visible to the user via
+ * sysfs or not
+ * @priv: pointer to any private data structure specific to particular
+ * cache design
+ *
+ * While @of_node, @disable_sysfs and @priv are used for internal book
+ * keeping, the remaining members form the core properties of the cache
+ */
+struct cacheinfo {
+ enum cache_type type;
+ unsigned int level;
+ unsigned int coherency_line_size;
+ unsigned int number_of_sets;
+ unsigned int ways_of_associativity;
+ unsigned int physical_line_partition;
+ unsigned int size;
+ cpumask_t shared_cpu_map;
+ unsigned int attributes;
+#define CACHE_WRITE_THROUGH BIT(0)
+#define CACHE_WRITE_BACK BIT(1)
+#define CACHE_WRITE_POLICY_MASK \
+ (CACHE_WRITE_THROUGH | CACHE_WRITE_BACK)
+#define CACHE_READ_ALLOCATE BIT(2)
+#define CACHE_WRITE_ALLOCATE BIT(3)
+#define CACHE_ALLOCATE_POLICY_MASK \
+ (CACHE_READ_ALLOCATE | CACHE_WRITE_ALLOCATE)
+
+ struct device_node *of_node;
+ bool disable_sysfs;
+ void *priv;
+};
+
+struct cpu_cacheinfo {
+ struct cacheinfo *info_list;
+ unsigned int num_levels;
+ unsigned int num_leaves;
+};
+
+/*
+ * Helpers to make sure "func" is executed on the cpu whose cache
+ * attributes are being detected
+ */
+#define DEFINE_SMP_CALL_CACHE_FUNCTION(func) \
+static inline void _##func(void *ret) \
+{ \
+ int cpu = smp_processor_id(); \
+ *(int *)ret = __##func(cpu); \
+} \
+ \
+int func(unsigned int cpu) \
+{ \
+ int ret; \
+ smp_call_function_single(cpu, _##func, &ret, true); \
+ return ret; \
+}
+
+struct cpu_cacheinfo *get_cpu_cacheinfo(unsigned int cpu);
+int init_cache_level(unsigned int cpu);
+int populate_cache_leaves(unsigned int cpu);
+
+const struct attribute_group *cache_get_priv_group(struct cacheinfo *this_leaf);
+
+#endif /* _LINUX_CACHEINFO_H */
--
1.8.3.2
^ permalink raw reply related
* Re: [PATCH] locks: Ability to test for flock presence on fd
From: Andy Lutomirski @ 2014-09-03 16:57 UTC (permalink / raw)
To: Jeff Layton
Cc: Michael Kerrisk,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Alexander Viro, Pavel Emelyanov, J. Bruce Fields, linux-fsdevel,
Linux API
In-Reply-To: <20140903120321.604f9039-9yPaYZwiELC+kQycOl6kW4xkIHaj4LzF@public.gmane.org>
On Sep 3, 2014 9:04 AM, "Jeff Layton" <jlayton-vpEMnDpepFuMZCB2o+C8xQ@public.gmane.org> wrote:
>
> On Wed, 3 Sep 2014 20:00:02 +0400
> Pavel Emelyanov <xemul-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org> wrote:
>
> > On 09/03/2014 07:55 PM, Jeff Layton wrote:
> > > On Wed, 3 Sep 2014 18:38:24 +0400
> > > Pavel Emelyanov <xemul-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org> wrote:
> > >
> > >> On 09/02/2014 11:53 PM, Jeff Layton wrote:
> > >>> On Tue, 2 Sep 2014 15:43:00 -0400
> > >>> "J. Bruce Fields" <bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org> wrote:
> > >>>
> > >>>> On Tue, Sep 02, 2014 at 11:07:14PM +0400, Pavel Emelyanov wrote:
> > >>>>> On 09/02/2014 10:44 PM, J. Bruce Fields wrote:
> > >>>>>> On Tue, Sep 02, 2014 at 09:17:34PM +0400, Pavel Emelyanov wrote:
> > >>>>>>> Hi,
> > >>>>>>>
> > >>>>>>> There's a problem with getting information about who has a flock on
> > >>>>>>> a specific file. The thing is that the "owner" field, that is shown in
> > >>>>>>> /proc/locks is the pid of the task who created the flock, not the one
> > >>>>>>> who _may_ hold it.
> > >>>>>>>
> > >>>>>>> If the flock creator shared the file with some other task (by forking
> > >>>>>>> or via scm_rights) and then died or closed the file, the information
> > >>>>>>> shown in proc no longer corresponds to the reality.
> > >>>>>>>
> > >>>>>>> This is critical for CRIU project, that tries to dump (and restore)
> > >>>>>>> the state of running tasks. For example, let's take two tasks A and B
> > >>>>>>> both opened a file "/foo", one of tasks places a LOCK_SH lock on the
> > >>>>>>> file and then "obfuscated" the owner field in /proc/locks. After this
> > >>>>>>> we have no ways to find out who is the lock holder.
> > >>>>>>>
> > >>>>>>> I'd like to note, that for LOCK_EX this problem is not critical -- we
> > >>>>>>> may go to both tasks and "ask" them to LOCK_EX the file again (we can
> > >>>>>>> do it in CRIU, I can tell more if required). The one who succeeds is
> > >>>>>>> the lock holder.
> > >>>>>>
> > >>>>>> It could be both, actually, right?
> > >>>>>
> > >>>>> Two succeeding with LOCK_EX? AFAIU no. Am I missing something?
> > >>>>
> > >>>> After a fork, two processes "own" the lock, right?:
> > >>>>
> > >>>> int main(int argc, char *argv[])
> > >>>> {
> > >>>> int fd, ret;
> > >>>>
> > >>>> fd = open(argv[1], O_RDWR);
> > >>>> ret = flock(fd, LOCK_EX);
> > >>>> if (ret)
> > >>>> err(1, "flock");
> > >>>> ret = fork();
> > >>>> if (ret == -1)
> > >>>> err(1, "flock");
> > >>>> ret = flock(fd, LOCK_EX);
> > >>>> if (ret)
> > >>>> err(1, "flock");
> > >>>> printf("%d got exclusive lock\n", getpid());
> > >>>> sleep(1000);
> > >>>> }
> > >>>>
> > >>>> $ touch TMP
> > >>>> $ ./test TMP
> > >>>> 15882 got exclusive lock
> > >>>> 15883 got exclusive lock
> > >>>> ^C
> > >>>>
> > >>>> I may misunderstand what you're doing.
> > >>>>
> > >>>
> > >>> Yeah, I don't understand either.
> > >>>
> > >>> Flock locks are owned by the file description. The task that set
> > >>> them is really irrelevant once they are set.
> > >>>
> > >>> In the second flock() call there, you're just "modifying" an existing
> > >>> lock (which turns out to be a noop here).
> > >>>
> > >>> So, I don't quite understand the problem this solves. I get that you're
> > >>> trying to reestablish the flock "state" after a checkpoint/restore
> > >>> event, but why does it matter what task actually sets the locks as long
> > >>> as they're set on the correct set of fds?
> > >>
> > >> Sorry for confusion. Let me try to explain it more clearly.
> > >>
> > >> First, what I meant talking about two LOCK_EX locks. Let's consider
> > >> this scenario:
> > >>
> > >> pid = fork()
> > >> fd = open("/foo"); /* both parent and child has _different_ files */
> > >> if (pid == 0)
> > >> /* child only */
> > >> flock(fd, LOCK_EX);
> > >>
> > >> at this point we have two different files pointing to "/foo" and
> > >> only one of them has LOCK_EX on it. So if try to LOCK_EX it again,
> > >> only at child's file this would succeed. So we can distinguish which
> > >> file is locked using this method.
> > >>
> > >>
> > >>
> > >> Now, what problem this patch is trying to solve. It's quite tricky,
> > >> but still. Let's imagine this scenario:
> > >>
> > >> pid = fork();
> > >> fd = open("/foo"); /* yet again -- two different files */
> > >> if (pid == 0) {
> > >> flock(fd, LOCK_SH);
> > >> pid2 = fork();
> > >> if (pid2 != 0)
> > >> exit(0);
> > >> }
> > >>
> > >> at this point we have:
> > >>
> > >> task A -- the original task with file "/foo" opened
> > >> task B -- the first child, that exited at the end
> > >> task C -- the 2nd child, that "inherited" a file with the lock from B
> > >>
> > >> Note, that file at A and file at C are two different files (struct
> > >> file-s). And it's only the C's one that is locked.
> > >>
> > >> The problem is that the /proc/locks shows the pid of B in this lock's
> > >> owner field. And we have no glue to find out who the real lock owner
> > >> is using the /proc/locks.
> > >>
> > >> If we try to do the trickery like the one we did with LOCK_EX above,
> > >> this is what we would get.
> > >>
> > >> If putting the 2nd LOCK_SH from A and from C, both attempts would succeed,
> > >> so this is not the solution.
> > >>
> > >> If we try to LOCK_EX from A and C, only C would succeed, so this seem
> > >> to be the solution, but it's actually not. If there's another pair of
> > >> A' and C' tasks holding the same "/foo" and having the LOCK_SH on C',
> > >> this trick would stop working as none of the tasks would be able to
> > >> put such lock on this file.
> > >>
> > >>
> > >> Thus, we need some way to find out whether a task X has a lock on file F.
> > >> This patch is one of the ways of doing this.
> > >>
> > >> Hope this explanation is more clear.
> > >>
> > >
> > > Yes, thanks for clarifying.
> > >
> > > I think we do need to be a bit careful when describing this though.
> > >
> > > flock locks are not owned by tasks, but by the file description. So you
> > > can't really tell whether task X has a lock on file F. Several tasks
> > > could have a reference to file F and none of them has any more "claim"
> > > to a lock on that file than another (at least from an API standpoint).
> > >
> > > What your patch really does is tell you whether that file description
> > > has a particular type of lock set on it.
> >
> > Exactly.
> >
> > > Like Bruce, I think this looks fairly reasonable. That said, I had to
> > > go through a bunch of API gyrations recently when getting the OFD lock
> > > patches merged. It would be good to accompany your kernel patch with
> > > glibc and manpage patches as well so we can make sure we have the
> > > design settled before merging anything.
> > >
> > > Sound OK?
> >
> > Sure! But I think glibc and man-pages people would first want the
> > kernel part to get finished, as it's the part that mostly drives the
> > API. Since the linux-api@ is in Cc for this patch, what else would
> > you suggest me to do to keep the process moving?
> >
> > Thanks,
> > Pavel
> >
>
> (cc'ing Michael Kerrisk, the manpages maintainer)
>
> Michael had good suggestions for me when I was doing the OFD lock work.
> I'd also consider cc'ing the glibc development list.
I would suggest writing up a short description of the exact semantics
of your proposal. That way reviewers can decide whether the semantics
make sense and then check whether the code matches the description.
This could take the form of text that would go in the appropriate manpage.
--Andy
^ permalink raw reply
* Re: [PATCH] locks: Ability to test for flock presence on fd
From: Jeff Layton @ 2014-09-03 16:03 UTC (permalink / raw)
To: Pavel Emelyanov
Cc: J. Bruce Fields, Alexander Viro, linux-fsdevel,
Linux Kernel Mailing List, linux-api-u79uwXL29TY76Z2rM5mHXA,
Michael Kerrisk (man-pages)
In-Reply-To: <54073B02.2060707-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
On Wed, 3 Sep 2014 20:00:02 +0400
Pavel Emelyanov <xemul-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org> wrote:
> On 09/03/2014 07:55 PM, Jeff Layton wrote:
> > On Wed, 3 Sep 2014 18:38:24 +0400
> > Pavel Emelyanov <xemul-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org> wrote:
> >
> >> On 09/02/2014 11:53 PM, Jeff Layton wrote:
> >>> On Tue, 2 Sep 2014 15:43:00 -0400
> >>> "J. Bruce Fields" <bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org> wrote:
> >>>
> >>>> On Tue, Sep 02, 2014 at 11:07:14PM +0400, Pavel Emelyanov wrote:
> >>>>> On 09/02/2014 10:44 PM, J. Bruce Fields wrote:
> >>>>>> On Tue, Sep 02, 2014 at 09:17:34PM +0400, Pavel Emelyanov wrote:
> >>>>>>> Hi,
> >>>>>>>
> >>>>>>> There's a problem with getting information about who has a flock on
> >>>>>>> a specific file. The thing is that the "owner" field, that is shown in
> >>>>>>> /proc/locks is the pid of the task who created the flock, not the one
> >>>>>>> who _may_ hold it.
> >>>>>>>
> >>>>>>> If the flock creator shared the file with some other task (by forking
> >>>>>>> or via scm_rights) and then died or closed the file, the information
> >>>>>>> shown in proc no longer corresponds to the reality.
> >>>>>>>
> >>>>>>> This is critical for CRIU project, that tries to dump (and restore)
> >>>>>>> the state of running tasks. For example, let's take two tasks A and B
> >>>>>>> both opened a file "/foo", one of tasks places a LOCK_SH lock on the
> >>>>>>> file and then "obfuscated" the owner field in /proc/locks. After this
> >>>>>>> we have no ways to find out who is the lock holder.
> >>>>>>>
> >>>>>>> I'd like to note, that for LOCK_EX this problem is not critical -- we
> >>>>>>> may go to both tasks and "ask" them to LOCK_EX the file again (we can
> >>>>>>> do it in CRIU, I can tell more if required). The one who succeeds is
> >>>>>>> the lock holder.
> >>>>>>
> >>>>>> It could be both, actually, right?
> >>>>>
> >>>>> Two succeeding with LOCK_EX? AFAIU no. Am I missing something?
> >>>>
> >>>> After a fork, two processes "own" the lock, right?:
> >>>>
> >>>> int main(int argc, char *argv[])
> >>>> {
> >>>> int fd, ret;
> >>>>
> >>>> fd = open(argv[1], O_RDWR);
> >>>> ret = flock(fd, LOCK_EX);
> >>>> if (ret)
> >>>> err(1, "flock");
> >>>> ret = fork();
> >>>> if (ret == -1)
> >>>> err(1, "flock");
> >>>> ret = flock(fd, LOCK_EX);
> >>>> if (ret)
> >>>> err(1, "flock");
> >>>> printf("%d got exclusive lock\n", getpid());
> >>>> sleep(1000);
> >>>> }
> >>>>
> >>>> $ touch TMP
> >>>> $ ./test TMP
> >>>> 15882 got exclusive lock
> >>>> 15883 got exclusive lock
> >>>> ^C
> >>>>
> >>>> I may misunderstand what you're doing.
> >>>>
> >>>
> >>> Yeah, I don't understand either.
> >>>
> >>> Flock locks are owned by the file description. The task that set
> >>> them is really irrelevant once they are set.
> >>>
> >>> In the second flock() call there, you're just "modifying" an existing
> >>> lock (which turns out to be a noop here).
> >>>
> >>> So, I don't quite understand the problem this solves. I get that you're
> >>> trying to reestablish the flock "state" after a checkpoint/restore
> >>> event, but why does it matter what task actually sets the locks as long
> >>> as they're set on the correct set of fds?
> >>
> >> Sorry for confusion. Let me try to explain it more clearly.
> >>
> >> First, what I meant talking about two LOCK_EX locks. Let's consider
> >> this scenario:
> >>
> >> pid = fork()
> >> fd = open("/foo"); /* both parent and child has _different_ files */
> >> if (pid == 0)
> >> /* child only */
> >> flock(fd, LOCK_EX);
> >>
> >> at this point we have two different files pointing to "/foo" and
> >> only one of them has LOCK_EX on it. So if try to LOCK_EX it again,
> >> only at child's file this would succeed. So we can distinguish which
> >> file is locked using this method.
> >>
> >>
> >>
> >> Now, what problem this patch is trying to solve. It's quite tricky,
> >> but still. Let's imagine this scenario:
> >>
> >> pid = fork();
> >> fd = open("/foo"); /* yet again -- two different files */
> >> if (pid == 0) {
> >> flock(fd, LOCK_SH);
> >> pid2 = fork();
> >> if (pid2 != 0)
> >> exit(0);
> >> }
> >>
> >> at this point we have:
> >>
> >> task A -- the original task with file "/foo" opened
> >> task B -- the first child, that exited at the end
> >> task C -- the 2nd child, that "inherited" a file with the lock from B
> >>
> >> Note, that file at A and file at C are two different files (struct
> >> file-s). And it's only the C's one that is locked.
> >>
> >> The problem is that the /proc/locks shows the pid of B in this lock's
> >> owner field. And we have no glue to find out who the real lock owner
> >> is using the /proc/locks.
> >>
> >> If we try to do the trickery like the one we did with LOCK_EX above,
> >> this is what we would get.
> >>
> >> If putting the 2nd LOCK_SH from A and from C, both attempts would succeed,
> >> so this is not the solution.
> >>
> >> If we try to LOCK_EX from A and C, only C would succeed, so this seem
> >> to be the solution, but it's actually not. If there's another pair of
> >> A' and C' tasks holding the same "/foo" and having the LOCK_SH on C',
> >> this trick would stop working as none of the tasks would be able to
> >> put such lock on this file.
> >>
> >>
> >> Thus, we need some way to find out whether a task X has a lock on file F.
> >> This patch is one of the ways of doing this.
> >>
> >> Hope this explanation is more clear.
> >>
> >
> > Yes, thanks for clarifying.
> >
> > I think we do need to be a bit careful when describing this though.
> >
> > flock locks are not owned by tasks, but by the file description. So you
> > can't really tell whether task X has a lock on file F. Several tasks
> > could have a reference to file F and none of them has any more "claim"
> > to a lock on that file than another (at least from an API standpoint).
> >
> > What your patch really does is tell you whether that file description
> > has a particular type of lock set on it.
>
> Exactly.
>
> > Like Bruce, I think this looks fairly reasonable. That said, I had to
> > go through a bunch of API gyrations recently when getting the OFD lock
> > patches merged. It would be good to accompany your kernel patch with
> > glibc and manpage patches as well so we can make sure we have the
> > design settled before merging anything.
> >
> > Sound OK?
>
> Sure! But I think glibc and man-pages people would first want the
> kernel part to get finished, as it's the part that mostly drives the
> API. Since the linux-api@ is in Cc for this patch, what else would
> you suggest me to do to keep the process moving?
>
> Thanks,
> Pavel
>
(cc'ing Michael Kerrisk, the manpages maintainer)
Michael had good suggestions for me when I was doing the OFD lock work.
I'd also consider cc'ing the glibc development list.
Cheers,
--
Jeff Layton <jlayton-vpEMnDpepFuMZCB2o+C8xQ@public.gmane.org>
^ permalink raw reply
* Re: [PATCH] locks: Ability to test for flock presence on fd
From: Pavel Emelyanov @ 2014-09-03 16:00 UTC (permalink / raw)
To: Jeff Layton
Cc: J. Bruce Fields, Alexander Viro, linux-fsdevel,
Linux Kernel Mailing List, linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20140903115504.63a7ae6f-9yPaYZwiELC+kQycOl6kW4xkIHaj4LzF@public.gmane.org>
On 09/03/2014 07:55 PM, Jeff Layton wrote:
> On Wed, 3 Sep 2014 18:38:24 +0400
> Pavel Emelyanov <xemul-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org> wrote:
>
>> On 09/02/2014 11:53 PM, Jeff Layton wrote:
>>> On Tue, 2 Sep 2014 15:43:00 -0400
>>> "J. Bruce Fields" <bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org> wrote:
>>>
>>>> On Tue, Sep 02, 2014 at 11:07:14PM +0400, Pavel Emelyanov wrote:
>>>>> On 09/02/2014 10:44 PM, J. Bruce Fields wrote:
>>>>>> On Tue, Sep 02, 2014 at 09:17:34PM +0400, Pavel Emelyanov wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> There's a problem with getting information about who has a flock on
>>>>>>> a specific file. The thing is that the "owner" field, that is shown in
>>>>>>> /proc/locks is the pid of the task who created the flock, not the one
>>>>>>> who _may_ hold it.
>>>>>>>
>>>>>>> If the flock creator shared the file with some other task (by forking
>>>>>>> or via scm_rights) and then died or closed the file, the information
>>>>>>> shown in proc no longer corresponds to the reality.
>>>>>>>
>>>>>>> This is critical for CRIU project, that tries to dump (and restore)
>>>>>>> the state of running tasks. For example, let's take two tasks A and B
>>>>>>> both opened a file "/foo", one of tasks places a LOCK_SH lock on the
>>>>>>> file and then "obfuscated" the owner field in /proc/locks. After this
>>>>>>> we have no ways to find out who is the lock holder.
>>>>>>>
>>>>>>> I'd like to note, that for LOCK_EX this problem is not critical -- we
>>>>>>> may go to both tasks and "ask" them to LOCK_EX the file again (we can
>>>>>>> do it in CRIU, I can tell more if required). The one who succeeds is
>>>>>>> the lock holder.
>>>>>>
>>>>>> It could be both, actually, right?
>>>>>
>>>>> Two succeeding with LOCK_EX? AFAIU no. Am I missing something?
>>>>
>>>> After a fork, two processes "own" the lock, right?:
>>>>
>>>> int main(int argc, char *argv[])
>>>> {
>>>> int fd, ret;
>>>>
>>>> fd = open(argv[1], O_RDWR);
>>>> ret = flock(fd, LOCK_EX);
>>>> if (ret)
>>>> err(1, "flock");
>>>> ret = fork();
>>>> if (ret == -1)
>>>> err(1, "flock");
>>>> ret = flock(fd, LOCK_EX);
>>>> if (ret)
>>>> err(1, "flock");
>>>> printf("%d got exclusive lock\n", getpid());
>>>> sleep(1000);
>>>> }
>>>>
>>>> $ touch TMP
>>>> $ ./test TMP
>>>> 15882 got exclusive lock
>>>> 15883 got exclusive lock
>>>> ^C
>>>>
>>>> I may misunderstand what you're doing.
>>>>
>>>
>>> Yeah, I don't understand either.
>>>
>>> Flock locks are owned by the file description. The task that set
>>> them is really irrelevant once they are set.
>>>
>>> In the second flock() call there, you're just "modifying" an existing
>>> lock (which turns out to be a noop here).
>>>
>>> So, I don't quite understand the problem this solves. I get that you're
>>> trying to reestablish the flock "state" after a checkpoint/restore
>>> event, but why does it matter what task actually sets the locks as long
>>> as they're set on the correct set of fds?
>>
>> Sorry for confusion. Let me try to explain it more clearly.
>>
>> First, what I meant talking about two LOCK_EX locks. Let's consider
>> this scenario:
>>
>> pid = fork()
>> fd = open("/foo"); /* both parent and child has _different_ files */
>> if (pid == 0)
>> /* child only */
>> flock(fd, LOCK_EX);
>>
>> at this point we have two different files pointing to "/foo" and
>> only one of them has LOCK_EX on it. So if try to LOCK_EX it again,
>> only at child's file this would succeed. So we can distinguish which
>> file is locked using this method.
>>
>>
>>
>> Now, what problem this patch is trying to solve. It's quite tricky,
>> but still. Let's imagine this scenario:
>>
>> pid = fork();
>> fd = open("/foo"); /* yet again -- two different files */
>> if (pid == 0) {
>> flock(fd, LOCK_SH);
>> pid2 = fork();
>> if (pid2 != 0)
>> exit(0);
>> }
>>
>> at this point we have:
>>
>> task A -- the original task with file "/foo" opened
>> task B -- the first child, that exited at the end
>> task C -- the 2nd child, that "inherited" a file with the lock from B
>>
>> Note, that file at A and file at C are two different files (struct
>> file-s). And it's only the C's one that is locked.
>>
>> The problem is that the /proc/locks shows the pid of B in this lock's
>> owner field. And we have no glue to find out who the real lock owner
>> is using the /proc/locks.
>>
>> If we try to do the trickery like the one we did with LOCK_EX above,
>> this is what we would get.
>>
>> If putting the 2nd LOCK_SH from A and from C, both attempts would succeed,
>> so this is not the solution.
>>
>> If we try to LOCK_EX from A and C, only C would succeed, so this seem
>> to be the solution, but it's actually not. If there's another pair of
>> A' and C' tasks holding the same "/foo" and having the LOCK_SH on C',
>> this trick would stop working as none of the tasks would be able to
>> put such lock on this file.
>>
>>
>> Thus, we need some way to find out whether a task X has a lock on file F.
>> This patch is one of the ways of doing this.
>>
>> Hope this explanation is more clear.
>>
>
> Yes, thanks for clarifying.
>
> I think we do need to be a bit careful when describing this though.
>
> flock locks are not owned by tasks, but by the file description. So you
> can't really tell whether task X has a lock on file F. Several tasks
> could have a reference to file F and none of them has any more "claim"
> to a lock on that file than another (at least from an API standpoint).
>
> What your patch really does is tell you whether that file description
> has a particular type of lock set on it.
Exactly.
> Like Bruce, I think this looks fairly reasonable. That said, I had to
> go through a bunch of API gyrations recently when getting the OFD lock
> patches merged. It would be good to accompany your kernel patch with
> glibc and manpage patches as well so we can make sure we have the
> design settled before merging anything.
>
> Sound OK?
Sure! But I think glibc and man-pages people would first want the
kernel part to get finished, as it's the part that mostly drives the
API. Since the linux-api@ is in Cc for this patch, what else would
you suggest me to do to keep the process moving?
Thanks,
Pavel
^ permalink raw reply
* Re: [PATCH] locks: Ability to test for flock presence on fd
From: Jeff Layton @ 2014-09-03 15:55 UTC (permalink / raw)
To: Pavel Emelyanov
Cc: J. Bruce Fields, Alexander Viro, linux-fsdevel,
Linux Kernel Mailing List, linux-api
In-Reply-To: <540727E0.6030005@parallels.com>
On Wed, 3 Sep 2014 18:38:24 +0400
Pavel Emelyanov <xemul@parallels.com> wrote:
> On 09/02/2014 11:53 PM, Jeff Layton wrote:
> > On Tue, 2 Sep 2014 15:43:00 -0400
> > "J. Bruce Fields" <bfields@fieldses.org> wrote:
> >
> >> On Tue, Sep 02, 2014 at 11:07:14PM +0400, Pavel Emelyanov wrote:
> >>> On 09/02/2014 10:44 PM, J. Bruce Fields wrote:
> >>>> On Tue, Sep 02, 2014 at 09:17:34PM +0400, Pavel Emelyanov wrote:
> >>>>> Hi,
> >>>>>
> >>>>> There's a problem with getting information about who has a flock on
> >>>>> a specific file. The thing is that the "owner" field, that is shown in
> >>>>> /proc/locks is the pid of the task who created the flock, not the one
> >>>>> who _may_ hold it.
> >>>>>
> >>>>> If the flock creator shared the file with some other task (by forking
> >>>>> or via scm_rights) and then died or closed the file, the information
> >>>>> shown in proc no longer corresponds to the reality.
> >>>>>
> >>>>> This is critical for CRIU project, that tries to dump (and restore)
> >>>>> the state of running tasks. For example, let's take two tasks A and B
> >>>>> both opened a file "/foo", one of tasks places a LOCK_SH lock on the
> >>>>> file and then "obfuscated" the owner field in /proc/locks. After this
> >>>>> we have no ways to find out who is the lock holder.
> >>>>>
> >>>>> I'd like to note, that for LOCK_EX this problem is not critical -- we
> >>>>> may go to both tasks and "ask" them to LOCK_EX the file again (we can
> >>>>> do it in CRIU, I can tell more if required). The one who succeeds is
> >>>>> the lock holder.
> >>>>
> >>>> It could be both, actually, right?
> >>>
> >>> Two succeeding with LOCK_EX? AFAIU no. Am I missing something?
> >>
> >> After a fork, two processes "own" the lock, right?:
> >>
> >> int main(int argc, char *argv[])
> >> {
> >> int fd, ret;
> >>
> >> fd = open(argv[1], O_RDWR);
> >> ret = flock(fd, LOCK_EX);
> >> if (ret)
> >> err(1, "flock");
> >> ret = fork();
> >> if (ret == -1)
> >> err(1, "flock");
> >> ret = flock(fd, LOCK_EX);
> >> if (ret)
> >> err(1, "flock");
> >> printf("%d got exclusive lock\n", getpid());
> >> sleep(1000);
> >> }
> >>
> >> $ touch TMP
> >> $ ./test TMP
> >> 15882 got exclusive lock
> >> 15883 got exclusive lock
> >> ^C
> >>
> >> I may misunderstand what you're doing.
> >>
> >
> > Yeah, I don't understand either.
> >
> > Flock locks are owned by the file description. The task that set
> > them is really irrelevant once they are set.
> >
> > In the second flock() call there, you're just "modifying" an existing
> > lock (which turns out to be a noop here).
> >
> > So, I don't quite understand the problem this solves. I get that you're
> > trying to reestablish the flock "state" after a checkpoint/restore
> > event, but why does it matter what task actually sets the locks as long
> > as they're set on the correct set of fds?
>
> Sorry for confusion. Let me try to explain it more clearly.
>
> First, what I meant talking about two LOCK_EX locks. Let's consider
> this scenario:
>
> pid = fork()
> fd = open("/foo"); /* both parent and child has _different_ files */
> if (pid == 0)
> /* child only */
> flock(fd, LOCK_EX);
>
> at this point we have two different files pointing to "/foo" and
> only one of them has LOCK_EX on it. So if try to LOCK_EX it again,
> only at child's file this would succeed. So we can distinguish which
> file is locked using this method.
>
>
>
> Now, what problem this patch is trying to solve. It's quite tricky,
> but still. Let's imagine this scenario:
>
> pid = fork();
> fd = open("/foo"); /* yet again -- two different files */
> if (pid == 0) {
> flock(fd, LOCK_SH);
> pid2 = fork();
> if (pid2 != 0)
> exit(0);
> }
>
> at this point we have:
>
> task A -- the original task with file "/foo" opened
> task B -- the first child, that exited at the end
> task C -- the 2nd child, that "inherited" a file with the lock from B
>
> Note, that file at A and file at C are two different files (struct
> file-s). And it's only the C's one that is locked.
>
> The problem is that the /proc/locks shows the pid of B in this lock's
> owner field. And we have no glue to find out who the real lock owner
> is using the /proc/locks.
>
> If we try to do the trickery like the one we did with LOCK_EX above,
> this is what we would get.
>
> If putting the 2nd LOCK_SH from A and from C, both attempts would succeed,
> so this is not the solution.
>
> If we try to LOCK_EX from A and C, only C would succeed, so this seem
> to be the solution, but it's actually not. If there's another pair of
> A' and C' tasks holding the same "/foo" and having the LOCK_SH on C',
> this trick would stop working as none of the tasks would be able to
> put such lock on this file.
>
>
> Thus, we need some way to find out whether a task X has a lock on file F.
> This patch is one of the ways of doing this.
>
> Hope this explanation is more clear.
>
Yes, thanks for clarifying.
I think we do need to be a bit careful when describing this though.
flock locks are not owned by tasks, but by the file description. So you
can't really tell whether task X has a lock on file F. Several tasks
could have a reference to file F and none of them has any more "claim"
to a lock on that file than another (at least from an API standpoint).
What your patch really does is tell you whether that file description
has a particular type of lock set on it.
Like Bruce, I think this looks fairly reasonable. That said, I had to
go through a bunch of API gyrations recently when getting the OFD lock
patches merged. It would be good to accompany your kernel patch with
glibc and manpage patches as well so we can make sure we have the
design settled before merging anything.
Sound OK?
--
Jeff Layton <jlayton@poochiereds.net>
^ permalink raw reply
* Re: [PATCH] locks: Ability to test for flock presence on fd
From: Pavel Emelyanov @ 2014-09-03 15:47 UTC (permalink / raw)
To: J. Bruce Fields
Cc: Jeff Layton, Alexander Viro, linux-fsdevel,
Linux Kernel Mailing List, linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20140903154434.GC22731-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
On 09/03/2014 07:44 PM, J. Bruce Fields wrote:
...
>> Hope this explanation is more clear.
>
> Thanks, I think I understand.
>
> Remind me how you figure out which file descriptors point to the same
> file description (struct file)?
We do it with the help of kcmp() syscall:
https://github.com/cyrillos/linux-2.6/commit/951bc60c6a
Thanks,
Pavel
^ permalink raw reply
* Re: [PATCH v9 net-next 2/4] net: filter: split filter.h and expose eBPF to user space
From: Daniel Borkmann @ 2014-09-03 15:46 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: David S. Miller, Ingo Molnar, Linus Torvalds, Andy Lutomirski,
Steven Rostedt, Hannes Frederic Sowa, Chema Gonzalez,
Eric Dumazet, Peter Zijlstra, H. Peter Anvin, Andrew Morton,
Kees Cook, linux-api-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1409714246-31054-3-git-send-email-ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
On 09/03/2014 05:17 AM, Alexei Starovoitov wrote:
> allow user space to generate eBPF programs
>
> uapi/linux/bpf.h: eBPF instruction set definition
>
> linux/filter.h: the rest
>
> This patch only moves macro definitions, but practically it freezes existing
> eBPF instruction set, though new instructions can still be added in the future.
>
> These eBPF definitions cannot go into uapi/linux/filter.h, since the names
> may conflict with existing applications.
>
> Full eBPF ISA description is in Documentation/networking/filter.txt
>
> Signed-off-by: Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
Ok, given you post the remaining two RFCs later on this window as
you indicate, I have no objections:
Acked-by: Daniel Borkmann <dborkman-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
^ permalink raw reply
* Re: [PATCH] locks: Ability to test for flock presence on fd
From: J. Bruce Fields @ 2014-09-03 15:44 UTC (permalink / raw)
To: Pavel Emelyanov
Cc: Jeff Layton, Alexander Viro, linux-fsdevel,
Linux Kernel Mailing List, linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <540727E0.6030005-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
On Wed, Sep 03, 2014 at 06:38:24PM +0400, Pavel Emelyanov wrote:
> On 09/02/2014 11:53 PM, Jeff Layton wrote:
> > On Tue, 2 Sep 2014 15:43:00 -0400
> > "J. Bruce Fields" <bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org> wrote:
> >
> >> On Tue, Sep 02, 2014 at 11:07:14PM +0400, Pavel Emelyanov wrote:
> >>> On 09/02/2014 10:44 PM, J. Bruce Fields wrote:
> >>>> On Tue, Sep 02, 2014 at 09:17:34PM +0400, Pavel Emelyanov wrote:
> >>>>> Hi,
> >>>>>
> >>>>> There's a problem with getting information about who has a flock on
> >>>>> a specific file. The thing is that the "owner" field, that is shown in
> >>>>> /proc/locks is the pid of the task who created the flock, not the one
> >>>>> who _may_ hold it.
> >>>>>
> >>>>> If the flock creator shared the file with some other task (by forking
> >>>>> or via scm_rights) and then died or closed the file, the information
> >>>>> shown in proc no longer corresponds to the reality.
> >>>>>
> >>>>> This is critical for CRIU project, that tries to dump (and restore)
> >>>>> the state of running tasks. For example, let's take two tasks A and B
> >>>>> both opened a file "/foo", one of tasks places a LOCK_SH lock on the
> >>>>> file and then "obfuscated" the owner field in /proc/locks. After this
> >>>>> we have no ways to find out who is the lock holder.
> >>>>>
> >>>>> I'd like to note, that for LOCK_EX this problem is not critical -- we
> >>>>> may go to both tasks and "ask" them to LOCK_EX the file again (we can
> >>>>> do it in CRIU, I can tell more if required). The one who succeeds is
> >>>>> the lock holder.
> >>>>
> >>>> It could be both, actually, right?
> >>>
> >>> Two succeeding with LOCK_EX? AFAIU no. Am I missing something?
> >>
> >> After a fork, two processes "own" the lock, right?:
> >>
> >> int main(int argc, char *argv[])
> >> {
> >> int fd, ret;
> >>
> >> fd = open(argv[1], O_RDWR);
> >> ret = flock(fd, LOCK_EX);
> >> if (ret)
> >> err(1, "flock");
> >> ret = fork();
> >> if (ret == -1)
> >> err(1, "flock");
> >> ret = flock(fd, LOCK_EX);
> >> if (ret)
> >> err(1, "flock");
> >> printf("%d got exclusive lock\n", getpid());
> >> sleep(1000);
> >> }
> >>
> >> $ touch TMP
> >> $ ./test TMP
> >> 15882 got exclusive lock
> >> 15883 got exclusive lock
> >> ^C
> >>
> >> I may misunderstand what you're doing.
> >>
> >
> > Yeah, I don't understand either.
> >
> > Flock locks are owned by the file description. The task that set
> > them is really irrelevant once they are set.
> >
> > In the second flock() call there, you're just "modifying" an existing
> > lock (which turns out to be a noop here).
> >
> > So, I don't quite understand the problem this solves. I get that you're
> > trying to reestablish the flock "state" after a checkpoint/restore
> > event, but why does it matter what task actually sets the locks as long
> > as they're set on the correct set of fds?
>
> Sorry for confusion. Let me try to explain it more clearly.
>
> First, what I meant talking about two LOCK_EX locks. Let's consider
> this scenario:
>
> pid = fork()
> fd = open("/foo"); /* both parent and child has _different_ files */
> if (pid == 0)
> /* child only */
> flock(fd, LOCK_EX);
>
> at this point we have two different files pointing to "/foo" and
> only one of them has LOCK_EX on it. So if try to LOCK_EX it again,
> only at child's file this would succeed. So we can distinguish which
> file is locked using this method.
>
>
>
> Now, what problem this patch is trying to solve. It's quite tricky,
> but still. Let's imagine this scenario:
>
> pid = fork();
> fd = open("/foo"); /* yet again -- two different files */
> if (pid == 0) {
> flock(fd, LOCK_SH);
> pid2 = fork();
> if (pid2 != 0)
> exit(0);
> }
>
> at this point we have:
>
> task A -- the original task with file "/foo" opened
> task B -- the first child, that exited at the end
> task C -- the 2nd child, that "inherited" a file with the lock from B
>
> Note, that file at A and file at C are two different files (struct
> file-s). And it's only the C's one that is locked.
>
> The problem is that the /proc/locks shows the pid of B in this lock's
> owner field. And we have no glue to find out who the real lock owner
> is using the /proc/locks.
>
> If we try to do the trickery like the one we did with LOCK_EX above,
> this is what we would get.
>
> If putting the 2nd LOCK_SH from A and from C, both attempts would succeed,
> so this is not the solution.
>
> If we try to LOCK_EX from A and C, only C would succeed, so this seem
> to be the solution, but it's actually not. If there's another pair of
> A' and C' tasks holding the same "/foo" and having the LOCK_SH on C',
> this trick would stop working as none of the tasks would be able to
> put such lock on this file.
>
>
> Thus, we need some way to find out whether a task X has a lock on file F.
> This patch is one of the ways of doing this.
>
> Hope this explanation is more clear.
Thanks, I think I understand.
Remind me how you figure out which file descriptors point to the same
file description (struct file)?
--b.
^ permalink raw reply
* Re: [PATCH] locks: Ability to test for flock presence on fd
From: Pavel Emelyanov @ 2014-09-03 14:38 UTC (permalink / raw)
To: Jeff Layton, J. Bruce Fields
Cc: Alexander Viro, linux-fsdevel, Linux Kernel Mailing List,
linux-api
In-Reply-To: <20140902155351.79183446@tlielax.poochiereds.net>
On 09/02/2014 11:53 PM, Jeff Layton wrote:
> On Tue, 2 Sep 2014 15:43:00 -0400
> "J. Bruce Fields" <bfields@fieldses.org> wrote:
>
>> On Tue, Sep 02, 2014 at 11:07:14PM +0400, Pavel Emelyanov wrote:
>>> On 09/02/2014 10:44 PM, J. Bruce Fields wrote:
>>>> On Tue, Sep 02, 2014 at 09:17:34PM +0400, Pavel Emelyanov wrote:
>>>>> Hi,
>>>>>
>>>>> There's a problem with getting information about who has a flock on
>>>>> a specific file. The thing is that the "owner" field, that is shown in
>>>>> /proc/locks is the pid of the task who created the flock, not the one
>>>>> who _may_ hold it.
>>>>>
>>>>> If the flock creator shared the file with some other task (by forking
>>>>> or via scm_rights) and then died or closed the file, the information
>>>>> shown in proc no longer corresponds to the reality.
>>>>>
>>>>> This is critical for CRIU project, that tries to dump (and restore)
>>>>> the state of running tasks. For example, let's take two tasks A and B
>>>>> both opened a file "/foo", one of tasks places a LOCK_SH lock on the
>>>>> file and then "obfuscated" the owner field in /proc/locks. After this
>>>>> we have no ways to find out who is the lock holder.
>>>>>
>>>>> I'd like to note, that for LOCK_EX this problem is not critical -- we
>>>>> may go to both tasks and "ask" them to LOCK_EX the file again (we can
>>>>> do it in CRIU, I can tell more if required). The one who succeeds is
>>>>> the lock holder.
>>>>
>>>> It could be both, actually, right?
>>>
>>> Two succeeding with LOCK_EX? AFAIU no. Am I missing something?
>>
>> After a fork, two processes "own" the lock, right?:
>>
>> int main(int argc, char *argv[])
>> {
>> int fd, ret;
>>
>> fd = open(argv[1], O_RDWR);
>> ret = flock(fd, LOCK_EX);
>> if (ret)
>> err(1, "flock");
>> ret = fork();
>> if (ret == -1)
>> err(1, "flock");
>> ret = flock(fd, LOCK_EX);
>> if (ret)
>> err(1, "flock");
>> printf("%d got exclusive lock\n", getpid());
>> sleep(1000);
>> }
>>
>> $ touch TMP
>> $ ./test TMP
>> 15882 got exclusive lock
>> 15883 got exclusive lock
>> ^C
>>
>> I may misunderstand what you're doing.
>>
>
> Yeah, I don't understand either.
>
> Flock locks are owned by the file description. The task that set
> them is really irrelevant once they are set.
>
> In the second flock() call there, you're just "modifying" an existing
> lock (which turns out to be a noop here).
>
> So, I don't quite understand the problem this solves. I get that you're
> trying to reestablish the flock "state" after a checkpoint/restore
> event, but why does it matter what task actually sets the locks as long
> as they're set on the correct set of fds?
Sorry for confusion. Let me try to explain it more clearly.
First, what I meant talking about two LOCK_EX locks. Let's consider
this scenario:
pid = fork()
fd = open("/foo"); /* both parent and child has _different_ files */
if (pid == 0)
/* child only */
flock(fd, LOCK_EX);
at this point we have two different files pointing to "/foo" and
only one of them has LOCK_EX on it. So if try to LOCK_EX it again,
only at child's file this would succeed. So we can distinguish which
file is locked using this method.
Now, what problem this patch is trying to solve. It's quite tricky,
but still. Let's imagine this scenario:
pid = fork();
fd = open("/foo"); /* yet again -- two different files */
if (pid == 0) {
flock(fd, LOCK_SH);
pid2 = fork();
if (pid2 != 0)
exit(0);
}
at this point we have:
task A -- the original task with file "/foo" opened
task B -- the first child, that exited at the end
task C -- the 2nd child, that "inherited" a file with the lock from B
Note, that file at A and file at C are two different files (struct
file-s). And it's only the C's one that is locked.
The problem is that the /proc/locks shows the pid of B in this lock's
owner field. And we have no glue to find out who the real lock owner
is using the /proc/locks.
If we try to do the trickery like the one we did with LOCK_EX above,
this is what we would get.
If putting the 2nd LOCK_SH from A and from C, both attempts would succeed,
so this is not the solution.
If we try to LOCK_EX from A and C, only C would succeed, so this seem
to be the solution, but it's actually not. If there's another pair of
A' and C' tasks holding the same "/foo" and having the LOCK_SH on C',
this trick would stop working as none of the tasks would be able to
put such lock on this file.
Thus, we need some way to find out whether a task X has a lock on file F.
This patch is one of the ways of doing this.
Hope this explanation is more clear.
>>>>> With LOCK_SH this doesn't work. Trying to drop the
>>>>> lock doesn't work either, as flock(LOCK_UN) reports 0 in both cases:
>>>>> if the file is locked and if it is not.
>>>>>
>>>>> That said, I'd like to propose the LOCK_TEST flag to the flock call,
>>>>> that would check whether the lock of the given type (LOCK_SH or LOCK_EX)
>>>>> exists on the file we test. It's not the same as the existing in-kernel
>>>>> FL_ACCESS flag, which checks whether the new lock is possible, but
>>>>> it's a new FL_TEST flag, that checks whether the existing lock is there.
>>>>>
>>>>> What do you think?
>>>>
>>>> I guess I can't see anything really wrong with it.
>>>>
>>>> It ignores the (poorly documented) LOCK_MAND case, but maybe that's OK.
>>>
>>> I actually checked it and it seemed to work. What problems do
>>> you see with this case?
>>
>> On its own it just doesn't tell you whether or not LOCK_MAND is set.
>> But I guess you can still get that out of /proc/locks.
>>
>> To be honest I don't really know whether LOCK_MAND works or is used.
>>
>> --b.
>
>
^ permalink raw reply
* [PATCH v3] memfd_test: Make it work on 32-bit systems
From: Pranith Kumar @ 2014-09-03 14:31 UTC (permalink / raw)
To: Shuah Khan, David Herrmann, Hugh Dickins, Andrew Morton,
open list:KERNEL SELFTEST F..., open list
This test currently fails on 32-bit systems since we use u64 type to pass the
flags to fcntl.
This commit changes this to use 'unsigned int' type for flags to fcntl making it
work on 32-bit systems.
Signed-off-by: Pranith Kumar <bobby.prani-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Reviewed-by: David Herrmann <dh.herrmann-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
tools/testing/selftests/memfd/memfd_test.c | 34 ++++++++++++++----------------
1 file changed, 16 insertions(+), 18 deletions(-)
diff --git a/tools/testing/selftests/memfd/memfd_test.c b/tools/testing/selftests/memfd/memfd_test.c
index 3634c90..cb5001b 100644
--- a/tools/testing/selftests/memfd/memfd_test.c
+++ b/tools/testing/selftests/memfd/memfd_test.c
@@ -59,9 +59,9 @@ static void mfd_fail_new(const char *name, unsigned int flags)
}
}
-static __u64 mfd_assert_get_seals(int fd)
+static unsigned int mfd_assert_get_seals(int fd)
{
- long r;
+ int r;
r = fcntl(fd, F_GET_SEALS);
if (r < 0) {
@@ -69,50 +69,48 @@ static __u64 mfd_assert_get_seals(int fd)
abort();
}
- return r;
+ return (unsigned int)r;
}
-static void mfd_assert_has_seals(int fd, __u64 seals)
+static void mfd_assert_has_seals(int fd, unsigned int seals)
{
- __u64 s;
+ unsigned int s;
s = mfd_assert_get_seals(fd);
if (s != seals) {
- printf("%llu != %llu = GET_SEALS(%d)\n",
- (unsigned long long)seals, (unsigned long long)s, fd);
+ printf("%u != %u = GET_SEALS(%d)\n", seals, s, fd);
abort();
}
}
-static void mfd_assert_add_seals(int fd, __u64 seals)
+static void mfd_assert_add_seals(int fd, unsigned int seals)
{
- long r;
- __u64 s;
+ int r;
+ unsigned int s;
s = mfd_assert_get_seals(fd);
r = fcntl(fd, F_ADD_SEALS, seals);
if (r < 0) {
- printf("ADD_SEALS(%d, %llu -> %llu) failed: %m\n",
- fd, (unsigned long long)s, (unsigned long long)seals);
+ printf("ADD_SEALS(%d, %u -> %u) failed: %m\n", fd, s, seals);
abort();
}
}
-static void mfd_fail_add_seals(int fd, __u64 seals)
+static void mfd_fail_add_seals(int fd, unsigned int seals)
{
- long r;
- __u64 s;
+ int r;
+ unsigned int s;
r = fcntl(fd, F_GET_SEALS);
if (r < 0)
s = 0;
else
- s = r;
+ s = (unsigned int)r;
r = fcntl(fd, F_ADD_SEALS, seals);
if (r >= 0) {
- printf("ADD_SEALS(%d, %llu -> %llu) didn't fail as expected\n",
- fd, (unsigned long long)s, (unsigned long long)seals);
+ printf("ADD_SEALS(%d, %u -> %u) didn't fail as expected\n",
+ fd, s, seals);
abort();
}
}
--
2.1.0
^ permalink raw reply related
* [PATCH] shm: add memfd.h to UAPI export list
From: David Drysdale @ 2014-09-03 13:28 UTC (permalink / raw)
To: David Herrmann, Hugh Dickins
Cc: Linus Torvalds, Andrew Morton, linux-kernel, linux-api,
David Drysdale
New header file memfd.h from commit 9183df25fe7b194563db
("shm: add memfd_create() syscall") should be exported.
Signed-off-by: David Drysdale <drysdale@google.com>
Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
---
Against v3.17-rc3, for inclusion in v3.17.
include/uapi/linux/Kbuild | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
index c1c8e4d0ce2a..51b0e9df3e31 100644
--- a/include/uapi/linux/Kbuild
+++ b/include/uapi/linux/Kbuild
@@ -241,6 +241,7 @@ header-y += matroxfb.h
header-y += mdio.h
header-y += media.h
header-y += mei.h
+header-y += memfd.h
header-y += mempolicy.h
header-y += meye.h
header-y += mic_common.h
--
1.9.1
^ permalink raw reply related
* [PATCH RFC v9 net-next 4/4] net: filter: move common defines into bpf_common.h
From: Alexei Starovoitov @ 2014-09-03 3:17 UTC (permalink / raw)
To: David S. Miller
Cc: Ingo Molnar, Linus Torvalds, Andy Lutomirski, Steven Rostedt,
Daniel Borkmann, Hannes Frederic Sowa, Chema Gonzalez,
Eric Dumazet, Peter Zijlstra, H. Peter Anvin, Andrew Morton,
Kees Cook, linux-api-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1409714246-31054-1-git-send-email-ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
userspace programs that wish to use eBPF instruction macros
(like BPF_MOV64_REG or BPF_ALU32_IMM) need to include two files:
uapi/linux/filter.h and uapi/linux/bpf.h
Move common macro definitions that are shared between
classic BPF and eBPF into uapi/linux/bpf_common.h
Signed-off-by: Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
---
include/uapi/linux/Kbuild | 1 +
include/uapi/linux/bpf.h | 1 +
include/uapi/linux/bpf_common.h | 55 ++++++++++++++++++++++++++++++++++++++
include/uapi/linux/filter.h | 56 +--------------------------------------
4 files changed, 58 insertions(+), 55 deletions(-)
create mode 100644 include/uapi/linux/bpf_common.h
diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
index fb3f7b675229..abd0a85cedc7 100644
--- a/include/uapi/linux/Kbuild
+++ b/include/uapi/linux/Kbuild
@@ -68,6 +68,7 @@ header-y += binfmts.h
header-y += blkpg.h
header-y += blktrace_api.h
header-y += bpf.h
+header-y += bpf_common.h
header-y += bpqether.h
header-y += bsg.h
header-y += btrfs.h
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 97edd23622fc..df9c9bdaa8a0 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -8,6 +8,7 @@
#define _UAPI__LINUX_BPF_H__
#include <linux/types.h>
+#include <linux/bpf_common.h>
/* Extended instruction set based on top of classic BPF */
diff --git a/include/uapi/linux/bpf_common.h b/include/uapi/linux/bpf_common.h
new file mode 100644
index 000000000000..e2d8b1ca3730
--- /dev/null
+++ b/include/uapi/linux/bpf_common.h
@@ -0,0 +1,55 @@
+#ifndef _UAPI__LINUX_BPF_COMMON_H__
+#define _UAPI__LINUX_BPF_COMMON_H__
+
+/* Instruction classes */
+#define BPF_CLASS(code) ((code) & 0x07)
+#define BPF_LD 0x00
+#define BPF_LDX 0x01
+#define BPF_ST 0x02
+#define BPF_STX 0x03
+#define BPF_ALU 0x04
+#define BPF_JMP 0x05
+#define BPF_RET 0x06
+#define BPF_MISC 0x07
+
+/* ld/ldx fields */
+#define BPF_SIZE(code) ((code) & 0x18)
+#define BPF_W 0x00
+#define BPF_H 0x08
+#define BPF_B 0x10
+#define BPF_MODE(code) ((code) & 0xe0)
+#define BPF_IMM 0x00
+#define BPF_ABS 0x20
+#define BPF_IND 0x40
+#define BPF_MEM 0x60
+#define BPF_LEN 0x80
+#define BPF_MSH 0xa0
+
+/* alu/jmp fields */
+#define BPF_OP(code) ((code) & 0xf0)
+#define BPF_ADD 0x00
+#define BPF_SUB 0x10
+#define BPF_MUL 0x20
+#define BPF_DIV 0x30
+#define BPF_OR 0x40
+#define BPF_AND 0x50
+#define BPF_LSH 0x60
+#define BPF_RSH 0x70
+#define BPF_NEG 0x80
+#define BPF_MOD 0x90
+#define BPF_XOR 0xa0
+
+#define BPF_JA 0x00
+#define BPF_JEQ 0x10
+#define BPF_JGT 0x20
+#define BPF_JGE 0x30
+#define BPF_JSET 0x40
+#define BPF_SRC(code) ((code) & 0x08)
+#define BPF_K 0x00
+#define BPF_X 0x08
+
+#ifndef BPF_MAXINSNS
+#define BPF_MAXINSNS 4096
+#endif
+
+#endif /* _UAPI__LINUX_BPF_COMMON_H__ */
diff --git a/include/uapi/linux/filter.h b/include/uapi/linux/filter.h
index 253b4d42cf2b..47785d5ecf17 100644
--- a/include/uapi/linux/filter.h
+++ b/include/uapi/linux/filter.h
@@ -7,7 +7,7 @@
#include <linux/compiler.h>
#include <linux/types.h>
-
+#include <linux/bpf_common.h>
/*
* Current version of the filter code architecture.
@@ -32,56 +32,6 @@ struct sock_fprog { /* Required for SO_ATTACH_FILTER. */
struct sock_filter __user *filter;
};
-/*
- * Instruction classes
- */
-
-#define BPF_CLASS(code) ((code) & 0x07)
-#define BPF_LD 0x00
-#define BPF_LDX 0x01
-#define BPF_ST 0x02
-#define BPF_STX 0x03
-#define BPF_ALU 0x04
-#define BPF_JMP 0x05
-#define BPF_RET 0x06
-#define BPF_MISC 0x07
-
-/* ld/ldx fields */
-#define BPF_SIZE(code) ((code) & 0x18)
-#define BPF_W 0x00
-#define BPF_H 0x08
-#define BPF_B 0x10
-#define BPF_MODE(code) ((code) & 0xe0)
-#define BPF_IMM 0x00
-#define BPF_ABS 0x20
-#define BPF_IND 0x40
-#define BPF_MEM 0x60
-#define BPF_LEN 0x80
-#define BPF_MSH 0xa0
-
-/* alu/jmp fields */
-#define BPF_OP(code) ((code) & 0xf0)
-#define BPF_ADD 0x00
-#define BPF_SUB 0x10
-#define BPF_MUL 0x20
-#define BPF_DIV 0x30
-#define BPF_OR 0x40
-#define BPF_AND 0x50
-#define BPF_LSH 0x60
-#define BPF_RSH 0x70
-#define BPF_NEG 0x80
-#define BPF_MOD 0x90
-#define BPF_XOR 0xa0
-
-#define BPF_JA 0x00
-#define BPF_JEQ 0x10
-#define BPF_JGT 0x20
-#define BPF_JGE 0x30
-#define BPF_JSET 0x40
-#define BPF_SRC(code) ((code) & 0x08)
-#define BPF_K 0x00
-#define BPF_X 0x08
-
/* ret - BPF_K and BPF_X also apply */
#define BPF_RVAL(code) ((code) & 0x18)
#define BPF_A 0x10
@@ -91,10 +41,6 @@ struct sock_fprog { /* Required for SO_ATTACH_FILTER. */
#define BPF_TAX 0x00
#define BPF_TXA 0x80
-#ifndef BPF_MAXINSNS
-#define BPF_MAXINSNS 4096
-#endif
-
/*
* Macros for filter block array initializers.
*/
--
1.7.9.5
^ permalink raw reply related
* [PATCH RFC v9 net-next 3/4] net: filter: move eBPF instruction macros
From: Alexei Starovoitov @ 2014-09-03 3:17 UTC (permalink / raw)
To: David S. Miller
Cc: Ingo Molnar, Linus Torvalds, Andy Lutomirski, Steven Rostedt,
Daniel Borkmann, Hannes Frederic Sowa, Chema Gonzalez,
Eric Dumazet, Peter Zijlstra, H. Peter Anvin, Andrew Morton,
Kees Cook, linux-api, netdev, linux-kernel
In-Reply-To: <1409714246-31054-1-git-send-email-ast@plumgrid.com>
move instruction macros (like BPF_MOV64_REG or BPF_ALU32_IMM)
from linux/filter.h into uapi/linux/bpf.h
so that userspace programs can use them.
verifier testsuite (in later patches) will be using them.
Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
---
include/linux/filter.h | 226 ----------------------------------------------
include/uapi/linux/bpf.h | 226 ++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 226 insertions(+), 226 deletions(-)
diff --git a/include/linux/filter.h b/include/linux/filter.h
index ff77842af3e1..c220b27b10df 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -37,232 +37,6 @@ struct bpf_prog_info;
/* BPF program can access up to 512 bytes of stack space. */
#define MAX_BPF_STACK 512
-/* Helper macros for filter block array initializers. */
-
-/* ALU ops on registers, bpf_add|sub|...: dst_reg += src_reg */
-
-#define BPF_ALU64_REG(OP, DST, SRC) \
- ((struct bpf_insn) { \
- .code = BPF_ALU64 | BPF_OP(OP) | BPF_X, \
- .dst_reg = DST, \
- .src_reg = SRC, \
- .off = 0, \
- .imm = 0 })
-
-#define BPF_ALU32_REG(OP, DST, SRC) \
- ((struct bpf_insn) { \
- .code = BPF_ALU | BPF_OP(OP) | BPF_X, \
- .dst_reg = DST, \
- .src_reg = SRC, \
- .off = 0, \
- .imm = 0 })
-
-/* ALU ops on immediates, bpf_add|sub|...: dst_reg += imm32 */
-
-#define BPF_ALU64_IMM(OP, DST, IMM) \
- ((struct bpf_insn) { \
- .code = BPF_ALU64 | BPF_OP(OP) | BPF_K, \
- .dst_reg = DST, \
- .src_reg = 0, \
- .off = 0, \
- .imm = IMM })
-
-#define BPF_ALU32_IMM(OP, DST, IMM) \
- ((struct bpf_insn) { \
- .code = BPF_ALU | BPF_OP(OP) | BPF_K, \
- .dst_reg = DST, \
- .src_reg = 0, \
- .off = 0, \
- .imm = IMM })
-
-/* Endianess conversion, cpu_to_{l,b}e(), {l,b}e_to_cpu() */
-
-#define BPF_ENDIAN(TYPE, DST, LEN) \
- ((struct bpf_insn) { \
- .code = BPF_ALU | BPF_END | BPF_SRC(TYPE), \
- .dst_reg = DST, \
- .src_reg = 0, \
- .off = 0, \
- .imm = LEN })
-
-/* Short form of mov, dst_reg = src_reg */
-
-#define BPF_MOV64_REG(DST, SRC) \
- ((struct bpf_insn) { \
- .code = BPF_ALU64 | BPF_MOV | BPF_X, \
- .dst_reg = DST, \
- .src_reg = SRC, \
- .off = 0, \
- .imm = 0 })
-
-#define BPF_MOV32_REG(DST, SRC) \
- ((struct bpf_insn) { \
- .code = BPF_ALU | BPF_MOV | BPF_X, \
- .dst_reg = DST, \
- .src_reg = SRC, \
- .off = 0, \
- .imm = 0 })
-
-/* Short form of mov, dst_reg = imm32 */
-
-#define BPF_MOV64_IMM(DST, IMM) \
- ((struct bpf_insn) { \
- .code = BPF_ALU64 | BPF_MOV | BPF_K, \
- .dst_reg = DST, \
- .src_reg = 0, \
- .off = 0, \
- .imm = IMM })
-
-#define BPF_MOV32_IMM(DST, IMM) \
- ((struct bpf_insn) { \
- .code = BPF_ALU | BPF_MOV | BPF_K, \
- .dst_reg = DST, \
- .src_reg = 0, \
- .off = 0, \
- .imm = IMM })
-
-/* BPF_LD_IMM64 macro encodes single 'load 64-bit immediate' insn */
-#define BPF_LD_IMM64(DST, IMM) \
- BPF_LD_IMM64_RAW(DST, 0, IMM)
-
-#define BPF_LD_IMM64_RAW(DST, SRC, IMM) \
- ((struct bpf_insn) { \
- .code = BPF_LD | BPF_DW | BPF_IMM, \
- .dst_reg = DST, \
- .src_reg = SRC, \
- .off = 0, \
- .imm = (__u32) (IMM) }), \
- ((struct bpf_insn) { \
- .code = 0, /* zero is reserved opcode */ \
- .dst_reg = 0, \
- .src_reg = 0, \
- .off = 0, \
- .imm = ((__u64) (IMM)) >> 32 })
-
-#define BPF_PSEUDO_MAP_FD 1
-
-/* pseudo BPF_LD_IMM64 insn used to refer to process-local map_fd */
-#define BPF_LD_MAP_FD(DST, MAP_FD) \
- BPF_LD_IMM64_RAW(DST, BPF_PSEUDO_MAP_FD, MAP_FD)
-
-/* Short form of mov based on type, BPF_X: dst_reg = src_reg, BPF_K: dst_reg = imm32 */
-
-#define BPF_MOV64_RAW(TYPE, DST, SRC, IMM) \
- ((struct bpf_insn) { \
- .code = BPF_ALU64 | BPF_MOV | BPF_SRC(TYPE), \
- .dst_reg = DST, \
- .src_reg = SRC, \
- .off = 0, \
- .imm = IMM })
-
-#define BPF_MOV32_RAW(TYPE, DST, SRC, IMM) \
- ((struct bpf_insn) { \
- .code = BPF_ALU | BPF_MOV | BPF_SRC(TYPE), \
- .dst_reg = DST, \
- .src_reg = SRC, \
- .off = 0, \
- .imm = IMM })
-
-/* Direct packet access, R0 = *(uint *) (skb->data + imm32) */
-
-#define BPF_LD_ABS(SIZE, IMM) \
- ((struct bpf_insn) { \
- .code = BPF_LD | BPF_SIZE(SIZE) | BPF_ABS, \
- .dst_reg = 0, \
- .src_reg = 0, \
- .off = 0, \
- .imm = IMM })
-
-/* Indirect packet access, R0 = *(uint *) (skb->data + src_reg + imm32) */
-
-#define BPF_LD_IND(SIZE, SRC, IMM) \
- ((struct bpf_insn) { \
- .code = BPF_LD | BPF_SIZE(SIZE) | BPF_IND, \
- .dst_reg = 0, \
- .src_reg = SRC, \
- .off = 0, \
- .imm = IMM })
-
-/* Memory load, dst_reg = *(uint *) (src_reg + off16) */
-
-#define BPF_LDX_MEM(SIZE, DST, SRC, OFF) \
- ((struct bpf_insn) { \
- .code = BPF_LDX | BPF_SIZE(SIZE) | BPF_MEM, \
- .dst_reg = DST, \
- .src_reg = SRC, \
- .off = OFF, \
- .imm = 0 })
-
-/* Memory store, *(uint *) (dst_reg + off16) = src_reg */
-
-#define BPF_STX_MEM(SIZE, DST, SRC, OFF) \
- ((struct bpf_insn) { \
- .code = BPF_STX | BPF_SIZE(SIZE) | BPF_MEM, \
- .dst_reg = DST, \
- .src_reg = SRC, \
- .off = OFF, \
- .imm = 0 })
-
-/* Memory store, *(uint *) (dst_reg + off16) = imm32 */
-
-#define BPF_ST_MEM(SIZE, DST, OFF, IMM) \
- ((struct bpf_insn) { \
- .code = BPF_ST | BPF_SIZE(SIZE) | BPF_MEM, \
- .dst_reg = DST, \
- .src_reg = 0, \
- .off = OFF, \
- .imm = IMM })
-
-/* Conditional jumps against registers, if (dst_reg 'op' src_reg) goto pc + off16 */
-
-#define BPF_JMP_REG(OP, DST, SRC, OFF) \
- ((struct bpf_insn) { \
- .code = BPF_JMP | BPF_OP(OP) | BPF_X, \
- .dst_reg = DST, \
- .src_reg = SRC, \
- .off = OFF, \
- .imm = 0 })
-
-/* Conditional jumps against immediates, if (dst_reg 'op' imm32) goto pc + off16 */
-
-#define BPF_JMP_IMM(OP, DST, IMM, OFF) \
- ((struct bpf_insn) { \
- .code = BPF_JMP | BPF_OP(OP) | BPF_K, \
- .dst_reg = DST, \
- .src_reg = 0, \
- .off = OFF, \
- .imm = IMM })
-
-/* Function call */
-
-#define BPF_EMIT_CALL(FUNC) \
- ((struct bpf_insn) { \
- .code = BPF_JMP | BPF_CALL, \
- .dst_reg = 0, \
- .src_reg = 0, \
- .off = 0, \
- .imm = ((FUNC) - __bpf_call_base) })
-
-/* Raw code statement block */
-
-#define BPF_RAW_INSN(CODE, DST, SRC, OFF, IMM) \
- ((struct bpf_insn) { \
- .code = CODE, \
- .dst_reg = DST, \
- .src_reg = SRC, \
- .off = OFF, \
- .imm = IMM })
-
-/* Program exit */
-
-#define BPF_EXIT_INSN() \
- ((struct bpf_insn) { \
- .code = BPF_JMP | BPF_EXIT, \
- .dst_reg = 0, \
- .src_reg = 0, \
- .off = 0, \
- .imm = 0 })
-
#define bytes_to_bpf_size(bytes) \
({ \
int bpf_size = -EINVAL; \
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 13bd4bf3b100..97edd23622fc 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -54,6 +54,232 @@ enum {
/* BPF has 10 general purpose 64-bit registers and stack frame. */
#define MAX_BPF_REG __MAX_BPF_REG
+/* Helper macros for filter block array initializers. */
+
+/* ALU ops on registers, bpf_add|sub|...: dst_reg += src_reg */
+
+#define BPF_ALU64_REG(OP, DST, SRC) \
+ ((struct bpf_insn) { \
+ .code = BPF_ALU64 | BPF_OP(OP) | BPF_X, \
+ .dst_reg = DST, \
+ .src_reg = SRC, \
+ .off = 0, \
+ .imm = 0 })
+
+#define BPF_ALU32_REG(OP, DST, SRC) \
+ ((struct bpf_insn) { \
+ .code = BPF_ALU | BPF_OP(OP) | BPF_X, \
+ .dst_reg = DST, \
+ .src_reg = SRC, \
+ .off = 0, \
+ .imm = 0 })
+
+/* ALU ops on immediates, bpf_add|sub|...: dst_reg += imm32 */
+
+#define BPF_ALU64_IMM(OP, DST, IMM) \
+ ((struct bpf_insn) { \
+ .code = BPF_ALU64 | BPF_OP(OP) | BPF_K, \
+ .dst_reg = DST, \
+ .src_reg = 0, \
+ .off = 0, \
+ .imm = IMM })
+
+#define BPF_ALU32_IMM(OP, DST, IMM) \
+ ((struct bpf_insn) { \
+ .code = BPF_ALU | BPF_OP(OP) | BPF_K, \
+ .dst_reg = DST, \
+ .src_reg = 0, \
+ .off = 0, \
+ .imm = IMM })
+
+/* Endianess conversion, cpu_to_{l,b}e(), {l,b}e_to_cpu() */
+
+#define BPF_ENDIAN(TYPE, DST, LEN) \
+ ((struct bpf_insn) { \
+ .code = BPF_ALU | BPF_END | BPF_SRC(TYPE), \
+ .dst_reg = DST, \
+ .src_reg = 0, \
+ .off = 0, \
+ .imm = LEN })
+
+/* Short form of mov, dst_reg = src_reg */
+
+#define BPF_MOV64_REG(DST, SRC) \
+ ((struct bpf_insn) { \
+ .code = BPF_ALU64 | BPF_MOV | BPF_X, \
+ .dst_reg = DST, \
+ .src_reg = SRC, \
+ .off = 0, \
+ .imm = 0 })
+
+#define BPF_MOV32_REG(DST, SRC) \
+ ((struct bpf_insn) { \
+ .code = BPF_ALU | BPF_MOV | BPF_X, \
+ .dst_reg = DST, \
+ .src_reg = SRC, \
+ .off = 0, \
+ .imm = 0 })
+
+/* Short form of mov, dst_reg = imm32 */
+
+#define BPF_MOV64_IMM(DST, IMM) \
+ ((struct bpf_insn) { \
+ .code = BPF_ALU64 | BPF_MOV | BPF_K, \
+ .dst_reg = DST, \
+ .src_reg = 0, \
+ .off = 0, \
+ .imm = IMM })
+
+#define BPF_MOV32_IMM(DST, IMM) \
+ ((struct bpf_insn) { \
+ .code = BPF_ALU | BPF_MOV | BPF_K, \
+ .dst_reg = DST, \
+ .src_reg = 0, \
+ .off = 0, \
+ .imm = IMM })
+
+/* BPF_LD_IMM64 macro encodes single 'load 64-bit immediate' insn */
+#define BPF_LD_IMM64(DST, IMM) \
+ BPF_LD_IMM64_RAW(DST, 0, IMM)
+
+#define BPF_LD_IMM64_RAW(DST, SRC, IMM) \
+ ((struct bpf_insn) { \
+ .code = BPF_LD | BPF_DW | BPF_IMM, \
+ .dst_reg = DST, \
+ .src_reg = SRC, \
+ .off = 0, \
+ .imm = (__u32) (IMM) }), \
+ ((struct bpf_insn) { \
+ .code = 0, /* zero is reserved opcode */ \
+ .dst_reg = 0, \
+ .src_reg = 0, \
+ .off = 0, \
+ .imm = ((__u64) (IMM)) >> 32 })
+
+#define BPF_PSEUDO_MAP_FD 1
+
+/* pseudo BPF_LD_IMM64 insn used to refer to process-local map_fd */
+#define BPF_LD_MAP_FD(DST, MAP_FD) \
+ BPF_LD_IMM64_RAW(DST, BPF_PSEUDO_MAP_FD, MAP_FD)
+
+/* Short form of mov based on type, BPF_X: dst_reg = src_reg, BPF_K: dst_reg = imm32 */
+
+#define BPF_MOV64_RAW(TYPE, DST, SRC, IMM) \
+ ((struct bpf_insn) { \
+ .code = BPF_ALU64 | BPF_MOV | BPF_SRC(TYPE), \
+ .dst_reg = DST, \
+ .src_reg = SRC, \
+ .off = 0, \
+ .imm = IMM })
+
+#define BPF_MOV32_RAW(TYPE, DST, SRC, IMM) \
+ ((struct bpf_insn) { \
+ .code = BPF_ALU | BPF_MOV | BPF_SRC(TYPE), \
+ .dst_reg = DST, \
+ .src_reg = SRC, \
+ .off = 0, \
+ .imm = IMM })
+
+/* Direct packet access, R0 = *(uint *) (skb->data + imm32) */
+
+#define BPF_LD_ABS(SIZE, IMM) \
+ ((struct bpf_insn) { \
+ .code = BPF_LD | BPF_SIZE(SIZE) | BPF_ABS, \
+ .dst_reg = 0, \
+ .src_reg = 0, \
+ .off = 0, \
+ .imm = IMM })
+
+/* Indirect packet access, R0 = *(uint *) (skb->data + src_reg + imm32) */
+
+#define BPF_LD_IND(SIZE, SRC, IMM) \
+ ((struct bpf_insn) { \
+ .code = BPF_LD | BPF_SIZE(SIZE) | BPF_IND, \
+ .dst_reg = 0, \
+ .src_reg = SRC, \
+ .off = 0, \
+ .imm = IMM })
+
+/* Memory load, dst_reg = *(uint *) (src_reg + off16) */
+
+#define BPF_LDX_MEM(SIZE, DST, SRC, OFF) \
+ ((struct bpf_insn) { \
+ .code = BPF_LDX | BPF_SIZE(SIZE) | BPF_MEM, \
+ .dst_reg = DST, \
+ .src_reg = SRC, \
+ .off = OFF, \
+ .imm = 0 })
+
+/* Memory store, *(uint *) (dst_reg + off16) = src_reg */
+
+#define BPF_STX_MEM(SIZE, DST, SRC, OFF) \
+ ((struct bpf_insn) { \
+ .code = BPF_STX | BPF_SIZE(SIZE) | BPF_MEM, \
+ .dst_reg = DST, \
+ .src_reg = SRC, \
+ .off = OFF, \
+ .imm = 0 })
+
+/* Memory store, *(uint *) (dst_reg + off16) = imm32 */
+
+#define BPF_ST_MEM(SIZE, DST, OFF, IMM) \
+ ((struct bpf_insn) { \
+ .code = BPF_ST | BPF_SIZE(SIZE) | BPF_MEM, \
+ .dst_reg = DST, \
+ .src_reg = 0, \
+ .off = OFF, \
+ .imm = IMM })
+
+/* Conditional jumps against registers, if (dst_reg 'op' src_reg) goto pc + off16 */
+
+#define BPF_JMP_REG(OP, DST, SRC, OFF) \
+ ((struct bpf_insn) { \
+ .code = BPF_JMP | BPF_OP(OP) | BPF_X, \
+ .dst_reg = DST, \
+ .src_reg = SRC, \
+ .off = OFF, \
+ .imm = 0 })
+
+/* Conditional jumps against immediates, if (dst_reg 'op' imm32) goto pc + off16 */
+
+#define BPF_JMP_IMM(OP, DST, IMM, OFF) \
+ ((struct bpf_insn) { \
+ .code = BPF_JMP | BPF_OP(OP) | BPF_K, \
+ .dst_reg = DST, \
+ .src_reg = 0, \
+ .off = OFF, \
+ .imm = IMM })
+
+/* Function call */
+
+#define BPF_EMIT_CALL(FUNC) \
+ ((struct bpf_insn) { \
+ .code = BPF_JMP | BPF_CALL, \
+ .dst_reg = 0, \
+ .src_reg = 0, \
+ .off = 0, \
+ .imm = ((FUNC) - __bpf_call_base) })
+
+/* Raw code statement block */
+
+#define BPF_RAW_INSN(CODE, DST, SRC, OFF, IMM) \
+ ((struct bpf_insn) { \
+ .code = CODE, \
+ .dst_reg = DST, \
+ .src_reg = SRC, \
+ .off = OFF, \
+ .imm = IMM })
+
+/* Program exit */
+
+#define BPF_EXIT_INSN() \
+ ((struct bpf_insn) { \
+ .code = BPF_JMP | BPF_EXIT, \
+ .dst_reg = 0, \
+ .src_reg = 0, \
+ .off = 0, \
+ .imm = 0 })
+
struct bpf_insn {
__u8 code; /* opcode */
__u8 dst_reg:4; /* dest register */
--
1.7.9.5
^ permalink raw reply related
* [PATCH v9 net-next 2/4] net: filter: split filter.h and expose eBPF to user space
From: Alexei Starovoitov @ 2014-09-03 3:17 UTC (permalink / raw)
To: David S. Miller
Cc: Ingo Molnar, Linus Torvalds, Andy Lutomirski, Steven Rostedt,
Daniel Borkmann, Hannes Frederic Sowa, Chema Gonzalez,
Eric Dumazet, Peter Zijlstra, H. Peter Anvin, Andrew Morton,
Kees Cook, linux-api, netdev, linux-kernel
In-Reply-To: <1409714246-31054-1-git-send-email-ast@plumgrid.com>
allow user space to generate eBPF programs
uapi/linux/bpf.h: eBPF instruction set definition
linux/filter.h: the rest
This patch only moves macro definitions, but practically it freezes existing
eBPF instruction set, though new instructions can still be added in the future.
These eBPF definitions cannot go into uapi/linux/filter.h, since the names
may conflict with existing applications.
Full eBPF ISA description is in Documentation/networking/filter.txt
Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
---
include/linux/filter.h | 56 +-------------------------------------
include/uapi/linux/Kbuild | 1 +
include/uapi/linux/bpf.h | 65 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 67 insertions(+), 55 deletions(-)
create mode 100644 include/uapi/linux/bpf.h
diff --git a/include/linux/filter.h b/include/linux/filter.h
index bf323da77950..8f82ef3f1cdd 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -10,58 +10,12 @@
#include <linux/workqueue.h>
#include <uapi/linux/filter.h>
#include <asm/cacheflush.h>
+#include <uapi/linux/bpf.h>
struct sk_buff;
struct sock;
struct seccomp_data;
-/* Internally used and optimized filter representation with extended
- * instruction set based on top of classic BPF.
- */
-
-/* instruction classes */
-#define BPF_ALU64 0x07 /* alu mode in double word width */
-
-/* ld/ldx fields */
-#define BPF_DW 0x18 /* double word */
-#define BPF_XADD 0xc0 /* exclusive add */
-
-/* alu/jmp fields */
-#define BPF_MOV 0xb0 /* mov reg to reg */
-#define BPF_ARSH 0xc0 /* sign extending arithmetic shift right */
-
-/* change endianness of a register */
-#define BPF_END 0xd0 /* flags for endianness conversion: */
-#define BPF_TO_LE 0x00 /* convert to little-endian */
-#define BPF_TO_BE 0x08 /* convert to big-endian */
-#define BPF_FROM_LE BPF_TO_LE
-#define BPF_FROM_BE BPF_TO_BE
-
-#define BPF_JNE 0x50 /* jump != */
-#define BPF_JSGT 0x60 /* SGT is signed '>', GT in x86 */
-#define BPF_JSGE 0x70 /* SGE is signed '>=', GE in x86 */
-#define BPF_CALL 0x80 /* function call */
-#define BPF_EXIT 0x90 /* function return */
-
-/* Register numbers */
-enum {
- BPF_REG_0 = 0,
- BPF_REG_1,
- BPF_REG_2,
- BPF_REG_3,
- BPF_REG_4,
- BPF_REG_5,
- BPF_REG_6,
- BPF_REG_7,
- BPF_REG_8,
- BPF_REG_9,
- BPF_REG_10,
- __MAX_BPF_REG,
-};
-
-/* BPF has 10 general purpose 64-bit registers and stack frame. */
-#define MAX_BPF_REG __MAX_BPF_REG
-
/* ArgX, context and stack frame pointer register positions. Note,
* Arg1, Arg2, Arg3, etc are used as argument mappings of function
* calls in BPF_CALL instruction.
@@ -322,14 +276,6 @@ enum {
#define SK_RUN_FILTER(filter, ctx) \
(*filter->prog->bpf_func)(ctx, filter->prog->insnsi)
-struct bpf_insn {
- __u8 code; /* opcode */
- __u8 dst_reg:4; /* dest register */
- __u8 src_reg:4; /* source register */
- __s16 off; /* signed offset */
- __s32 imm; /* signed immediate constant */
-};
-
#ifdef CONFIG_COMPAT
/* A struct sock_filter is architecture independent. */
struct compat_sock_fprog {
diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
index 24e9033f8b3f..fb3f7b675229 100644
--- a/include/uapi/linux/Kbuild
+++ b/include/uapi/linux/Kbuild
@@ -67,6 +67,7 @@ header-y += bfs_fs.h
header-y += binfmts.h
header-y += blkpg.h
header-y += blktrace_api.h
+header-y += bpf.h
header-y += bpqether.h
header-y += bsg.h
header-y += btrfs.h
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
new file mode 100644
index 000000000000..479ed0b6be16
--- /dev/null
+++ b/include/uapi/linux/bpf.h
@@ -0,0 +1,65 @@
+/* Copyright (c) 2011-2014 PLUMgrid, http://plumgrid.com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of version 2 of the GNU General Public
+ * License as published by the Free Software Foundation.
+ */
+#ifndef _UAPI__LINUX_BPF_H__
+#define _UAPI__LINUX_BPF_H__
+
+#include <linux/types.h>
+
+/* Extended instruction set based on top of classic BPF */
+
+/* instruction classes */
+#define BPF_ALU64 0x07 /* alu mode in double word width */
+
+/* ld/ldx fields */
+#define BPF_DW 0x18 /* double word */
+#define BPF_XADD 0xc0 /* exclusive add */
+
+/* alu/jmp fields */
+#define BPF_MOV 0xb0 /* mov reg to reg */
+#define BPF_ARSH 0xc0 /* sign extending arithmetic shift right */
+
+/* change endianness of a register */
+#define BPF_END 0xd0 /* flags for endianness conversion: */
+#define BPF_TO_LE 0x00 /* convert to little-endian */
+#define BPF_TO_BE 0x08 /* convert to big-endian */
+#define BPF_FROM_LE BPF_TO_LE
+#define BPF_FROM_BE BPF_TO_BE
+
+#define BPF_JNE 0x50 /* jump != */
+#define BPF_JSGT 0x60 /* SGT is signed '>', GT in x86 */
+#define BPF_JSGE 0x70 /* SGE is signed '>=', GE in x86 */
+#define BPF_CALL 0x80 /* function call */
+#define BPF_EXIT 0x90 /* function return */
+
+/* Register numbers */
+enum {
+ BPF_REG_0 = 0,
+ BPF_REG_1,
+ BPF_REG_2,
+ BPF_REG_3,
+ BPF_REG_4,
+ BPF_REG_5,
+ BPF_REG_6,
+ BPF_REG_7,
+ BPF_REG_8,
+ BPF_REG_9,
+ BPF_REG_10,
+ __MAX_BPF_REG,
+};
+
+/* BPF has 10 general purpose 64-bit registers and stack frame. */
+#define MAX_BPF_REG __MAX_BPF_REG
+
+struct bpf_insn {
+ __u8 code; /* opcode */
+ __u8 dst_reg:4; /* dest register */
+ __u8 src_reg:4; /* source register */
+ __s16 off; /* signed offset */
+ __s32 imm; /* signed immediate constant */
+};
+
+#endif /* _UAPI__LINUX_BPF_H__ */
--
1.7.9.5
^ permalink raw reply related
* [PATCH v9 net-next 1/4] net: filter: add "load 64-bit immediate" eBPF instruction
From: Alexei Starovoitov @ 2014-09-03 3:17 UTC (permalink / raw)
To: David S. Miller
Cc: Ingo Molnar, Linus Torvalds, Andy Lutomirski, Steven Rostedt,
Daniel Borkmann, Hannes Frederic Sowa, Chema Gonzalez,
Eric Dumazet, Peter Zijlstra, H. Peter Anvin, Andrew Morton,
Kees Cook, linux-api, netdev, linux-kernel
In-Reply-To: <1409714246-31054-1-git-send-email-ast@plumgrid.com>
add BPF_LD_IMM64 instruction to load 64-bit immediate value into a register.
All previous instructions were 8-byte. This is first 16-byte instruction.
Two consecutive 'struct bpf_insn' blocks are interpreted as single instruction:
insn[0].code = BPF_LD | BPF_DW | BPF_IMM
insn[0].dst_reg = destination register
insn[0].imm = lower 32-bit
insn[1].code = 0
insn[1].imm = upper 32-bit
All unused fields must be zero.
Classic BPF has similar instruction: BPF_LD | BPF_W | BPF_IMM
which loads 32-bit immediate value into a register.
x64 JITs it as single 'movabsq %rax, imm64'
arm64 may JIT as sequence of four 'movk x0, #imm16, lsl #shift' insn
Note that old eBPF programs are binary compatible with new interpreter.
It helps eBPF programs load 64-bit constant into a register with one
instruction instead of using two registers and 4 instructions:
BPF_MOV32_IMM(R1, imm32)
BPF_ALU64_IMM(BPF_LSH, R1, 32)
BPF_MOV32_IMM(R2, imm32)
BPF_ALU64_REG(BPF_OR, R1, R2)
User space generated programs will use this instruction to load constants only.
To tell kernel that user space needs a pointer the _pseudo_ variant of
this instruction may be added later, which will use extra bits of encoding
to indicate what type of pointer user space is asking kernel to provide.
For example 'off' or 'src_reg' fields can be used for such purpose.
src_reg = 1 could mean that user space is asking kernel to validate and
load in-kernel map pointer.
src_reg = 2 could mean that user space needs readonly data section pointer
src_reg = 3 could mean that user space needs a pointer to per-cpu local data
All such future pseudo instructions will not be carrying the actual pointer
as part of the instruction, but rather will be treated as a request to kernel
to provide one. The kernel will verify the request_for_a_pointer, then
will drop _pseudo_ marking and will store actual internal pointer inside
the instruction, so the end result is the interpreter and JITs never
see pseudo BPF_LD_IMM64 insns and only operate on generic BPF_LD_IMM64 that
loads 64-bit immediate into a register. User space never operates on direct
pointers and verifier can easily recognize request_for_pointer vs other
instructions.
Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
---
Documentation/networking/filter.txt | 8 +++++++-
arch/x86/net/bpf_jit_comp.c | 17 +++++++++++++++++
include/linux/filter.h | 18 ++++++++++++++++++
kernel/bpf/core.c | 5 +++++
lib/test_bpf.c | 21 +++++++++++++++++++++
5 files changed, 68 insertions(+), 1 deletion(-)
diff --git a/Documentation/networking/filter.txt b/Documentation/networking/filter.txt
index c48a9704bda8..81916ab5d96f 100644
--- a/Documentation/networking/filter.txt
+++ b/Documentation/networking/filter.txt
@@ -951,7 +951,7 @@ Size modifier is one of ...
Mode modifier is one of:
- BPF_IMM 0x00 /* classic BPF only, reserved in eBPF */
+ BPF_IMM 0x00 /* used for 32-bit mov in classic BPF and 64-bit in eBPF */
BPF_ABS 0x20
BPF_IND 0x40
BPF_MEM 0x60
@@ -995,6 +995,12 @@ BPF_XADD | BPF_DW | BPF_STX: lock xadd *(u64 *)(dst_reg + off16) += src_reg
Where size is one of: BPF_B or BPF_H or BPF_W or BPF_DW. Note that 1 and
2 byte atomic increments are not supported.
+eBPF has one 16-byte instruction: BPF_LD | BPF_DW | BPF_IMM which consists
+of two consecutive 'struct bpf_insn' 8-byte blocks and interpreted as single
+instruction that loads 64-bit immediate value into a dst_reg.
+Classic BPF has similar instruction: BPF_LD | BPF_W | BPF_IMM which loads
+32-bit immediate value into a register.
+
Testing
-------
diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 39ccfbb4a723..06f8c17f5484 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -393,6 +393,23 @@ static int do_jit(struct bpf_prog *bpf_prog, int *addrs, u8 *image,
EMIT1_off32(add_1reg(0xB8, dst_reg), imm32);
break;
+ case BPF_LD | BPF_IMM | BPF_DW:
+ if (insn[1].code != 0 || insn[1].src_reg != 0 ||
+ insn[1].dst_reg != 0 || insn[1].off != 0) {
+ /* verifier must catch invalid insns */
+ pr_err("invalid BPF_LD_IMM64 insn\n");
+ return -EINVAL;
+ }
+
+ /* movabsq %rax, imm64 */
+ EMIT2(add_1mod(0x48, dst_reg), add_1reg(0xB8, dst_reg));
+ EMIT(insn[0].imm, 4);
+ EMIT(insn[1].imm, 4);
+
+ insn++;
+ i++;
+ break;
+
/* dst %= src, dst /= src, dst %= imm32, dst /= imm32 */
case BPF_ALU | BPF_MOD | BPF_X:
case BPF_ALU | BPF_DIV | BPF_X:
diff --git a/include/linux/filter.h b/include/linux/filter.h
index c78994593355..bf323da77950 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -166,6 +166,24 @@ enum {
.off = 0, \
.imm = IMM })
+/* BPF_LD_IMM64 macro encodes single 'load 64-bit immediate' insn */
+#define BPF_LD_IMM64(DST, IMM) \
+ BPF_LD_IMM64_RAW(DST, 0, IMM)
+
+#define BPF_LD_IMM64_RAW(DST, SRC, IMM) \
+ ((struct bpf_insn) { \
+ .code = BPF_LD | BPF_DW | BPF_IMM, \
+ .dst_reg = DST, \
+ .src_reg = SRC, \
+ .off = 0, \
+ .imm = (__u32) (IMM) }), \
+ ((struct bpf_insn) { \
+ .code = 0, /* zero is reserved opcode */ \
+ .dst_reg = 0, \
+ .src_reg = 0, \
+ .off = 0, \
+ .imm = ((__u64) (IMM)) >> 32 })
+
/* Short form of mov based on type, BPF_X: dst_reg = src_reg, BPF_K: dst_reg = imm32 */
#define BPF_MOV64_RAW(TYPE, DST, SRC, IMM) \
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index b54bb2c2e494..2c2bfaacce66 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -242,6 +242,7 @@ static unsigned int __bpf_prog_run(void *ctx, const struct bpf_insn *insn)
[BPF_LD | BPF_IND | BPF_W] = &&LD_IND_W,
[BPF_LD | BPF_IND | BPF_H] = &&LD_IND_H,
[BPF_LD | BPF_IND | BPF_B] = &&LD_IND_B,
+ [BPF_LD | BPF_IMM | BPF_DW] = &&LD_IMM_DW,
};
void *ptr;
int off;
@@ -301,6 +302,10 @@ select_insn:
ALU64_MOV_K:
DST = IMM;
CONT;
+ LD_IMM_DW:
+ DST = (u64) (u32) insn[0].imm | ((u64) (u32) insn[1].imm) << 32;
+ insn++;
+ CONT;
ALU64_ARSH_X:
(*(s64 *) &DST) >>= SRC;
CONT;
diff --git a/lib/test_bpf.c b/lib/test_bpf.c
index 9a67456ba29a..413890815d3e 100644
--- a/lib/test_bpf.c
+++ b/lib/test_bpf.c
@@ -1735,6 +1735,27 @@ static struct bpf_test tests[] = {
{ },
{ { 1, 0 } },
},
+ {
+ "load 64-bit immediate",
+ .u.insns_int = {
+ BPF_LD_IMM64(R1, 0x567800001234L),
+ BPF_MOV64_REG(R2, R1),
+ BPF_MOV64_REG(R3, R2),
+ BPF_ALU64_IMM(BPF_RSH, R2, 32),
+ BPF_ALU64_IMM(BPF_LSH, R3, 32),
+ BPF_ALU64_IMM(BPF_RSH, R3, 32),
+ BPF_ALU64_IMM(BPF_MOV, R0, 0),
+ BPF_JMP_IMM(BPF_JEQ, R2, 0x5678, 1),
+ BPF_EXIT_INSN(),
+ BPF_JMP_IMM(BPF_JEQ, R3, 0x1234, 1),
+ BPF_EXIT_INSN(),
+ BPF_ALU64_IMM(BPF_MOV, R0, 1),
+ BPF_EXIT_INSN(),
+ },
+ INTERNAL,
+ { },
+ { { 0, 1 } }
+ },
};
static struct net_device dev;
--
1.7.9.5
^ permalink raw reply related
* [PATCH v9 net-next 0/4] load imm64 insn and uapi/linux/bpf.h
From: Alexei Starovoitov @ 2014-09-03 3:17 UTC (permalink / raw)
To: David S. Miller
Cc: Ingo Molnar, Linus Torvalds, Andy Lutomirski, Steven Rostedt,
Daniel Borkmann, Hannes Frederic Sowa, Chema Gonzalez,
Eric Dumazet, Peter Zijlstra, H. Peter Anvin, Andrew Morton,
Kees Cook, linux-api-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Hi,
V8->V9
- rebase on top of Hannes's patch in the same area [1]
- no changes to patches 1 and 2
- added patches 3 and 4 as RFC to address Daniel's concern.
patch 3 moves eBPF instruction macros and
patch 4 split macros which are shared between classic and eBPF
into bpf_common.h
3 and 4 will be used by verifier testsuite and will be reposted later
during stage III
V8 thread with 'why' reasoning and end goal:
https://lkml.org/lkml/2014/8/27/628
Original set [2] of ~28 patches I'm planning to present in 4 stages:
I. this 2 patches to fork off llvm upstreaming
II. bpf syscall with manpage and map implementation
III. bpf program load/unload with verifier testsuite (1st user of
instruction macros from bpf.h and 1st user of load imm64 insn)
IV. tracing, etc
[1] http://patchwork.ozlabs.org/patch/385266/
[2] https://lkml.org/lkml/2014/8/26/859
^ permalink raw reply
* Re: [PATCH V4 1/8] namespaces: assign each namespace instance a serial number
From: Richard Guy Briggs @ 2014-09-02 21:40 UTC (permalink / raw)
To: Eric W. Biederman
Cc: linux-api-u79uwXL29TY76Z2rM5mHXA,
containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Andy Lutomirski,
linux-audit-H+wXaHxf7aLQT0dZR+AlfA
In-Reply-To: <87tx4wmlcj.fsf-JOvCrm2gF+uungPnsOpG7nhyD016LWXt@public.gmane.org>
On 14/08/28, Eric W. Biederman wrote:
> Richard Guy Briggs <rgb-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> writes:
> > On 14/08/23, Eric W. Biederman wrote:
> >> Richard Guy Briggs <rgb-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> writes:
> >>
> >> > Generate and assign a serial number per namespace instance since boot.
> >> >
> >> > Use a serial number per namespace (unique across one boot of one kernel)
> >> > instead of the inode number (which is claimed to have had the right to change
> >> > reserved and is not necessarily unique if there is more than one proc fs) to
> >> > uniquely identify it per kernel boot.
> >>
> >> This approach is just broken.
> >>
> >> For this to work with migration (aka criu) you need to implement a
> >> namespace of namespaces. You haven't done this, and therefore
> >> such an interface will break existing userspace.
> >>
> >> Inside of audit I can understand not caring about these issues,
> >> but you go foward and expose these serial numbers in proc,
> >> and generally make this infrastructure available to others.
> >>
> >> The deep issue with migration is that we move tasks from one machine
> >> from another and on the destination machine we need to have all of the
> >> same global identifiers for software to function properly.
> >>
> >> My weasel words around the proc inode numbers is to preserve to allow us
> >> room to be able to restore those ids if it every becomes relevant for
> >> migration.
> >
> > What do you do if the inode number is already in use on the target
> > host?
>
> Since the inode numbers are relative to a superblock or a pid namespace
> the numbers that are in use can be restored on the target system
> by creating them in the appropriate namespace.
So you seem to be advocating for a namespace of namespaces, since
neither host can create a new namespace without consulting the others in
its pool for a new free number.
> The support does not exist in the kernel today for doing that because no
> one has cared but as architected the support can be added if needed to
> support migration.
>
> >> That is the proc inode numbers (technically) live in a pid namespace,
> >> (aka a mount of proc). So depending on the pid namespace you are in
> >> or the mount of proc you look in the numbers could change.
> >>
> >> Qualifications like that must exist to have a prayer of ever supporting
> >> process migration in the crazy corner cases where people start caring
> >> about inode numbers.
> >>
> >> We currently don't and inode numbers for a namespace will never change
> >> after a namespace is created. So I think you really are ok using the
> >> proc inode numbers. I am happy declaring by fiat that the inode numbers
> >> that audit uses are the numbers connected to the initial pid namespace.
> >
> > But once a namespace/container is migrated, it is a different audit that
> > is looking at it (unless we create an audit manager or entity that
> > functions at the level of a container manager), so audit should not care.
>
> These numbers were exported to everyone as a general purpose facility in
> proc. If audit is global and audit doesn't migrate you are right it
> doesn't matter. However if these numbers are used by anyone else for
> anything else it causes a problem.
So let us restrict their use to audit, by removing them from
/proc/<pid>/ns/ and only exposing them via netlink calls to audit gated
by CAP_AUDIT_WRITE or CAP_AUDIT_CONTROL.
> Further given that people run entire distributions in containers we may
> reach the point where we wish to run auditd in a container in the
> future. I would hate to paint ourselves into a corner with a design
> that could never allow audit to migrate. Support that case someday
> seems a valid naive desire.
Agreed. That is an option we do not want to rule out at this point.
I'll need to think about this one more.
> >> At a fairly basic level anything that is used to identify namespaces for
> >> any general purpose use needs to have most if not all of the same
> >> properties of the proc inode numbers. The most important of which is
> >> being tied to some context/namespace so there is a ability if we ever
> >> need it to migrate those numbers from one machine to another.
> >
> > Sooo... does it make any sense to have those inode or serial numbers be
> > blank inside the namespace/container itself, but only visible to its
> > manager outside the container (unless it is the initial namespace)?
>
> Mostly I think it makes sense to use the inode numbers from the initial
> pid namespace. They already exist. They already are unique. (Which
> means I don't need to maintain more code and more special cases). And
> the do what you need now.
Will inode numbers never be re-used once they are freed? Guaranteed?
> I probably haven't followed closely enough but I don't see what makes
> inode numbers undesirable.
This posting:
https://www.redhat.com/archives/linux-audit/2013-March/msg00032.html
> Eric
- RGB
--
Richard Guy Briggs <rbriggs-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545
^ permalink raw reply
* Re: [PATCH] locks: Ability to test for flock presence on fd
From: Jeff Layton @ 2014-09-02 19:53 UTC (permalink / raw)
To: J. Bruce Fields
Cc: Pavel Emelyanov, Alexander Viro, linux-fsdevel,
Linux Kernel Mailing List, linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20140902194300.GE31793-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
On Tue, 2 Sep 2014 15:43:00 -0400
"J. Bruce Fields" <bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org> wrote:
> On Tue, Sep 02, 2014 at 11:07:14PM +0400, Pavel Emelyanov wrote:
> > On 09/02/2014 10:44 PM, J. Bruce Fields wrote:
> > > On Tue, Sep 02, 2014 at 09:17:34PM +0400, Pavel Emelyanov wrote:
> > >> Hi,
> > >>
> > >> There's a problem with getting information about who has a flock on
> > >> a specific file. The thing is that the "owner" field, that is shown in
> > >> /proc/locks is the pid of the task who created the flock, not the one
> > >> who _may_ hold it.
> > >>
> > >> If the flock creator shared the file with some other task (by forking
> > >> or via scm_rights) and then died or closed the file, the information
> > >> shown in proc no longer corresponds to the reality.
> > >>
> > >> This is critical for CRIU project, that tries to dump (and restore)
> > >> the state of running tasks. For example, let's take two tasks A and B
> > >> both opened a file "/foo", one of tasks places a LOCK_SH lock on the
> > >> file and then "obfuscated" the owner field in /proc/locks. After this
> > >> we have no ways to find out who is the lock holder.
> > >>
> > >> I'd like to note, that for LOCK_EX this problem is not critical -- we
> > >> may go to both tasks and "ask" them to LOCK_EX the file again (we can
> > >> do it in CRIU, I can tell more if required). The one who succeeds is
> > >> the lock holder.
> > >
> > > It could be both, actually, right?
> >
> > Two succeeding with LOCK_EX? AFAIU no. Am I missing something?
>
> After a fork, two processes "own" the lock, right?:
>
> int main(int argc, char *argv[])
> {
> int fd, ret;
>
> fd = open(argv[1], O_RDWR);
> ret = flock(fd, LOCK_EX);
> if (ret)
> err(1, "flock");
> ret = fork();
> if (ret == -1)
> err(1, "flock");
> ret = flock(fd, LOCK_EX);
> if (ret)
> err(1, "flock");
> printf("%d got exclusive lock\n", getpid());
> sleep(1000);
> }
>
> $ touch TMP
> $ ./test TMP
> 15882 got exclusive lock
> 15883 got exclusive lock
> ^C
>
> I may misunderstand what you're doing.
>
Yeah, I don't understand either.
Flock locks are owned by the file description. The task that set
them is really irrelevant once they are set.
In the second flock() call there, you're just "modifying" an existing
lock (which turns out to be a noop here).
So, I don't quite understand the problem this solves. I get that you're
trying to reestablish the flock "state" after a checkpoint/restore
event, but why does it matter what task actually sets the locks as long
as they're set on the correct set of fds?
> > >> With LOCK_SH this doesn't work. Trying to drop the
> > >> lock doesn't work either, as flock(LOCK_UN) reports 0 in both cases:
> > >> if the file is locked and if it is not.
> > >>
> > >> That said, I'd like to propose the LOCK_TEST flag to the flock call,
> > >> that would check whether the lock of the given type (LOCK_SH or LOCK_EX)
> > >> exists on the file we test. It's not the same as the existing in-kernel
> > >> FL_ACCESS flag, which checks whether the new lock is possible, but
> > >> it's a new FL_TEST flag, that checks whether the existing lock is there.
> > >>
> > >> What do you think?
> > >
> > > I guess I can't see anything really wrong with it.
> > >
> > > It ignores the (poorly documented) LOCK_MAND case, but maybe that's OK.
> >
> > I actually checked it and it seemed to work. What problems do
> > you see with this case?
>
> On its own it just doesn't tell you whether or not LOCK_MAND is set.
> But I guess you can still get that out of /proc/locks.
>
> To be honest I don't really know whether LOCK_MAND works or is used.
>
> --b.
--
Jeff Layton <jlayton-vpEMnDpepFuMZCB2o+C8xQ@public.gmane.org>
^ permalink raw reply
* Re: [PATCH] locks: Ability to test for flock presence on fd
From: J. Bruce Fields @ 2014-09-02 19:43 UTC (permalink / raw)
To: Pavel Emelyanov
Cc: Jeff Layton, Alexander Viro, linux-fsdevel,
Linux Kernel Mailing List, linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <54061562.4080306-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
On Tue, Sep 02, 2014 at 11:07:14PM +0400, Pavel Emelyanov wrote:
> On 09/02/2014 10:44 PM, J. Bruce Fields wrote:
> > On Tue, Sep 02, 2014 at 09:17:34PM +0400, Pavel Emelyanov wrote:
> >> Hi,
> >>
> >> There's a problem with getting information about who has a flock on
> >> a specific file. The thing is that the "owner" field, that is shown in
> >> /proc/locks is the pid of the task who created the flock, not the one
> >> who _may_ hold it.
> >>
> >> If the flock creator shared the file with some other task (by forking
> >> or via scm_rights) and then died or closed the file, the information
> >> shown in proc no longer corresponds to the reality.
> >>
> >> This is critical for CRIU project, that tries to dump (and restore)
> >> the state of running tasks. For example, let's take two tasks A and B
> >> both opened a file "/foo", one of tasks places a LOCK_SH lock on the
> >> file and then "obfuscated" the owner field in /proc/locks. After this
> >> we have no ways to find out who is the lock holder.
> >>
> >> I'd like to note, that for LOCK_EX this problem is not critical -- we
> >> may go to both tasks and "ask" them to LOCK_EX the file again (we can
> >> do it in CRIU, I can tell more if required). The one who succeeds is
> >> the lock holder.
> >
> > It could be both, actually, right?
>
> Two succeeding with LOCK_EX? AFAIU no. Am I missing something?
After a fork, two processes "own" the lock, right?:
int main(int argc, char *argv[])
{
int fd, ret;
fd = open(argv[1], O_RDWR);
ret = flock(fd, LOCK_EX);
if (ret)
err(1, "flock");
ret = fork();
if (ret == -1)
err(1, "flock");
ret = flock(fd, LOCK_EX);
if (ret)
err(1, "flock");
printf("%d got exclusive lock\n", getpid());
sleep(1000);
}
$ touch TMP
$ ./test TMP
15882 got exclusive lock
15883 got exclusive lock
^C
I may misunderstand what you're doing.
> >> With LOCK_SH this doesn't work. Trying to drop the
> >> lock doesn't work either, as flock(LOCK_UN) reports 0 in both cases:
> >> if the file is locked and if it is not.
> >>
> >> That said, I'd like to propose the LOCK_TEST flag to the flock call,
> >> that would check whether the lock of the given type (LOCK_SH or LOCK_EX)
> >> exists on the file we test. It's not the same as the existing in-kernel
> >> FL_ACCESS flag, which checks whether the new lock is possible, but
> >> it's a new FL_TEST flag, that checks whether the existing lock is there.
> >>
> >> What do you think?
> >
> > I guess I can't see anything really wrong with it.
> >
> > It ignores the (poorly documented) LOCK_MAND case, but maybe that's OK.
>
> I actually checked it and it seemed to work. What problems do
> you see with this case?
On its own it just doesn't tell you whether or not LOCK_MAND is set.
But I guess you can still get that out of /proc/locks.
To be honest I don't really know whether LOCK_MAND works or is used.
--b.
^ permalink raw reply
* Re: [PATCH] locks: Ability to test for flock presence on fd
From: Pavel Emelyanov @ 2014-09-02 19:07 UTC (permalink / raw)
To: J. Bruce Fields
Cc: Jeff Layton, Alexander Viro, linux-fsdevel,
Linux Kernel Mailing List, linux-api
In-Reply-To: <20140902184456.GC31793@fieldses.org>
On 09/02/2014 10:44 PM, J. Bruce Fields wrote:
> On Tue, Sep 02, 2014 at 09:17:34PM +0400, Pavel Emelyanov wrote:
>> Hi,
>>
>> There's a problem with getting information about who has a flock on
>> a specific file. The thing is that the "owner" field, that is shown in
>> /proc/locks is the pid of the task who created the flock, not the one
>> who _may_ hold it.
>>
>> If the flock creator shared the file with some other task (by forking
>> or via scm_rights) and then died or closed the file, the information
>> shown in proc no longer corresponds to the reality.
>>
>> This is critical for CRIU project, that tries to dump (and restore)
>> the state of running tasks. For example, let's take two tasks A and B
>> both opened a file "/foo", one of tasks places a LOCK_SH lock on the
>> file and then "obfuscated" the owner field in /proc/locks. After this
>> we have no ways to find out who is the lock holder.
>>
>> I'd like to note, that for LOCK_EX this problem is not critical -- we
>> may go to both tasks and "ask" them to LOCK_EX the file again (we can
>> do it in CRIU, I can tell more if required). The one who succeeds is
>> the lock holder.
>
> It could be both, actually, right?
Two succeeding with LOCK_EX? AFAIU no. Am I missing something?
>> With LOCK_SH this doesn't work. Trying to drop the
>> lock doesn't work either, as flock(LOCK_UN) reports 0 in both cases:
>> if the file is locked and if it is not.
>>
>> That said, I'd like to propose the LOCK_TEST flag to the flock call,
>> that would check whether the lock of the given type (LOCK_SH or LOCK_EX)
>> exists on the file we test. It's not the same as the existing in-kernel
>> FL_ACCESS flag, which checks whether the new lock is possible, but
>> it's a new FL_TEST flag, that checks whether the existing lock is there.
>>
>> What do you think?
>
> I guess I can't see anything really wrong with it.
>
> It ignores the (poorly documented) LOCK_MAND case, but maybe that's OK.
I actually checked it and it seemed to work. What problems do
you see with this case?
> Would it make sense to return the lock type held instead, so you could
> do one flock(fd, LOCK_TEST) instead of flock(fd, LOCK_TEST|LOCK_SH) and
> flock(fd, LOCK_TEST|LOCK_EX) ?
Well, in our case we parse /proc/locks anyway to see what
files at least to test for being locked. But what you propose
looks even better. I'll look what can be done here.
> It'd be nice if we could fix /proc/locks. (You'd think I'd know better,
> but I've certainly been confused before when /proc/locks told me a lock
> was owned by a nonexistant PID.) But as long as multiple PIDs can "own"
> a flock and as long as there's no simple ID we can use to refer to a
> given struct file, I don't see an easy solution.
>
> --b.
>
>
>>
>> Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
>>
>> ---
>>
>> diff --git a/fs/locks.c b/fs/locks.c
>> index bb08857..50842bf 100644
>> --- a/fs/locks.c
>> +++ b/fs/locks.c
>> @@ -830,7 +830,7 @@ static int flock_lock_file(struct file *filp, struct file_lock *request)
>> int found = 0;
>> LIST_HEAD(dispose);
>>
>> - if (!(request->fl_flags & FL_ACCESS) && (request->fl_type != F_UNLCK)) {
>> + if (!(request->fl_flags & (FL_ACCESS|FL_TEST)) && (request->fl_type != F_UNLCK)) {
>> new_fl = locks_alloc_lock();
>> if (!new_fl)
>> return -ENOMEM;
>> @@ -850,11 +850,18 @@ static int flock_lock_file(struct file *filp, struct file_lock *request)
>> continue;
>> if (request->fl_type == fl->fl_type)
>> goto out;
>> + if (request->fl_flags & FL_TEST)
>> + break;
>> found = 1;
>> locks_delete_lock(before, &dispose);
>> break;
>> }
>>
>> + if (request->fl_flags & FL_TEST) {
>> + error = -ENOENT;
>> + goto out;
>> + }
>> +
>> if (request->fl_type == F_UNLCK) {
>> if ((request->fl_flags & FL_EXISTS) && !found)
>> error = -ENOENT;
>> @@ -1852,15 +1859,16 @@ SYSCALL_DEFINE2(flock, unsigned int, fd, unsigned int, cmd)
>> {
>> struct fd f = fdget(fd);
>> struct file_lock *lock;
>> - int can_sleep, unlock;
>> + int can_sleep, unlock, test;
>> int error;
>>
>> error = -EBADF;
>> if (!f.file)
>> goto out;
>>
>> + test = (cmd & LOCK_TEST);
>> can_sleep = !(cmd & LOCK_NB);
>> - cmd &= ~LOCK_NB;
>> + cmd &= ~(LOCK_NB|LOCK_TEST);
>> unlock = (cmd == LOCK_UN);
>>
>> if (!unlock && !(cmd & LOCK_MAND) &&
>> @@ -1872,6 +1880,8 @@ SYSCALL_DEFINE2(flock, unsigned int, fd, unsigned int, cmd)
>> goto out_putf;
>> if (can_sleep)
>> lock->fl_flags |= FL_SLEEP;
>> + if (test)
>> + lock->fl_flags |= FL_TEST;
>>
>> error = security_file_lock(f.file, lock->fl_type);
>> if (error)
>> diff --git a/include/linux/fs.h b/include/linux/fs.h
>> index 9418772..9230e1d 100644
>> --- a/include/linux/fs.h
>> +++ b/include/linux/fs.h
>> @@ -844,6 +844,7 @@ static inline struct file *get_file(struct file *f)
>> #define FL_DOWNGRADE_PENDING 256 /* Lease is being downgraded */
>> #define FL_UNLOCK_PENDING 512 /* Lease is being broken */
>> #define FL_OFDLCK 1024 /* lock is "owned" by struct file */
>> +#define FL_TEST 2048
>>
>> /*
>> * Special return value from posix_lock_file() and vfs_lock_file() for
>> diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h
>> index 7543b3e..7302e36 100644
>> --- a/include/uapi/asm-generic/fcntl.h
>> +++ b/include/uapi/asm-generic/fcntl.h
>> @@ -184,6 +184,7 @@ struct f_owner_ex {
>> #define LOCK_READ 64 /* which allows concurrent read operations */
>> #define LOCK_WRITE 128 /* which allows concurrent write operations */
>> #define LOCK_RW 192 /* which allows concurrent read & write ops */
>> +#define LOCK_TEST 256 /* check for my SH|EX locks present */
>>
>> #define F_LINUX_SPECIFIC_BASE 1024
>>
>>
> .
>
^ permalink raw reply
* Re: [PATCH] locks: Ability to test for flock presence on fd
From: J. Bruce Fields @ 2014-09-02 18:44 UTC (permalink / raw)
To: Pavel Emelyanov
Cc: Jeff Layton, Alexander Viro, linux-fsdevel,
Linux Kernel Mailing List, linux-api
In-Reply-To: <5405FBAE.5020303@parallels.com>
On Tue, Sep 02, 2014 at 09:17:34PM +0400, Pavel Emelyanov wrote:
> Hi,
>
> There's a problem with getting information about who has a flock on
> a specific file. The thing is that the "owner" field, that is shown in
> /proc/locks is the pid of the task who created the flock, not the one
> who _may_ hold it.
>
> If the flock creator shared the file with some other task (by forking
> or via scm_rights) and then died or closed the file, the information
> shown in proc no longer corresponds to the reality.
>
> This is critical for CRIU project, that tries to dump (and restore)
> the state of running tasks. For example, let's take two tasks A and B
> both opened a file "/foo", one of tasks places a LOCK_SH lock on the
> file and then "obfuscated" the owner field in /proc/locks. After this
> we have no ways to find out who is the lock holder.
>
> I'd like to note, that for LOCK_EX this problem is not critical -- we
> may go to both tasks and "ask" them to LOCK_EX the file again (we can
> do it in CRIU, I can tell more if required). The one who succeeds is
> the lock holder.
It could be both, actually, right?
> With LOCK_SH this doesn't work. Trying to drop the
> lock doesn't work either, as flock(LOCK_UN) reports 0 in both cases:
> if the file is locked and if it is not.
>
> That said, I'd like to propose the LOCK_TEST flag to the flock call,
> that would check whether the lock of the given type (LOCK_SH or LOCK_EX)
> exists on the file we test. It's not the same as the existing in-kernel
> FL_ACCESS flag, which checks whether the new lock is possible, but
> it's a new FL_TEST flag, that checks whether the existing lock is there.
>
> What do you think?
I guess I can't see anything really wrong with it.
It ignores the (poorly documented) LOCK_MAND case, but maybe that's OK.
Would it make sense to return the lock type held instead, so you could
do one flock(fd, LOCK_TEST) instead of flock(fd, LOCK_TEST|LOCK_SH) and
flock(fd, LOCK_TEST|LOCK_EX) ?
It'd be nice if we could fix /proc/locks. (You'd think I'd know better,
but I've certainly been confused before when /proc/locks told me a lock
was owned by a nonexistant PID.) But as long as multiple PIDs can "own"
a flock and as long as there's no simple ID we can use to refer to a
given struct file, I don't see an easy solution.
--b.
>
> Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
>
> ---
>
> diff --git a/fs/locks.c b/fs/locks.c
> index bb08857..50842bf 100644
> --- a/fs/locks.c
> +++ b/fs/locks.c
> @@ -830,7 +830,7 @@ static int flock_lock_file(struct file *filp, struct file_lock *request)
> int found = 0;
> LIST_HEAD(dispose);
>
> - if (!(request->fl_flags & FL_ACCESS) && (request->fl_type != F_UNLCK)) {
> + if (!(request->fl_flags & (FL_ACCESS|FL_TEST)) && (request->fl_type != F_UNLCK)) {
> new_fl = locks_alloc_lock();
> if (!new_fl)
> return -ENOMEM;
> @@ -850,11 +850,18 @@ static int flock_lock_file(struct file *filp, struct file_lock *request)
> continue;
> if (request->fl_type == fl->fl_type)
> goto out;
> + if (request->fl_flags & FL_TEST)
> + break;
> found = 1;
> locks_delete_lock(before, &dispose);
> break;
> }
>
> + if (request->fl_flags & FL_TEST) {
> + error = -ENOENT;
> + goto out;
> + }
> +
> if (request->fl_type == F_UNLCK) {
> if ((request->fl_flags & FL_EXISTS) && !found)
> error = -ENOENT;
> @@ -1852,15 +1859,16 @@ SYSCALL_DEFINE2(flock, unsigned int, fd, unsigned int, cmd)
> {
> struct fd f = fdget(fd);
> struct file_lock *lock;
> - int can_sleep, unlock;
> + int can_sleep, unlock, test;
> int error;
>
> error = -EBADF;
> if (!f.file)
> goto out;
>
> + test = (cmd & LOCK_TEST);
> can_sleep = !(cmd & LOCK_NB);
> - cmd &= ~LOCK_NB;
> + cmd &= ~(LOCK_NB|LOCK_TEST);
> unlock = (cmd == LOCK_UN);
>
> if (!unlock && !(cmd & LOCK_MAND) &&
> @@ -1872,6 +1880,8 @@ SYSCALL_DEFINE2(flock, unsigned int, fd, unsigned int, cmd)
> goto out_putf;
> if (can_sleep)
> lock->fl_flags |= FL_SLEEP;
> + if (test)
> + lock->fl_flags |= FL_TEST;
>
> error = security_file_lock(f.file, lock->fl_type);
> if (error)
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 9418772..9230e1d 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -844,6 +844,7 @@ static inline struct file *get_file(struct file *f)
> #define FL_DOWNGRADE_PENDING 256 /* Lease is being downgraded */
> #define FL_UNLOCK_PENDING 512 /* Lease is being broken */
> #define FL_OFDLCK 1024 /* lock is "owned" by struct file */
> +#define FL_TEST 2048
>
> /*
> * Special return value from posix_lock_file() and vfs_lock_file() for
> diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h
> index 7543b3e..7302e36 100644
> --- a/include/uapi/asm-generic/fcntl.h
> +++ b/include/uapi/asm-generic/fcntl.h
> @@ -184,6 +184,7 @@ struct f_owner_ex {
> #define LOCK_READ 64 /* which allows concurrent read operations */
> #define LOCK_WRITE 128 /* which allows concurrent write operations */
> #define LOCK_RW 192 /* which allows concurrent read & write ops */
> +#define LOCK_TEST 256 /* check for my SH|EX locks present */
>
> #define F_LINUX_SPECIFIC_BASE 1024
>
>
^ permalink raw reply
* [PATCH] locks: Ability to test for flock presence on fd
From: Pavel Emelyanov @ 2014-09-02 17:17 UTC (permalink / raw)
To: Jeff Layton, J. Bruce Fields, Alexander Viro, linux-fsdevel,
Linux Kernel Mailing List, linux-api-u79uwXL29TY76Z2rM5mHXA
Hi,
There's a problem with getting information about who has a flock on
a specific file. The thing is that the "owner" field, that is shown in
/proc/locks is the pid of the task who created the flock, not the one
who _may_ hold it.
If the flock creator shared the file with some other task (by forking
or via scm_rights) and then died or closed the file, the information
shown in proc no longer corresponds to the reality.
This is critical for CRIU project, that tries to dump (and restore)
the state of running tasks. For example, let's take two tasks A and B
both opened a file "/foo", one of tasks places a LOCK_SH lock on the
file and then "obfuscated" the owner field in /proc/locks. After this
we have no ways to find out who is the lock holder.
I'd like to note, that for LOCK_EX this problem is not critical -- we
may go to both tasks and "ask" them to LOCK_EX the file again (we can
do it in CRIU, I can tell more if required). The one who succeeds is
the lock holder. With LOCK_SH this doesn't work. Trying to drop the
lock doesn't work either, as flock(LOCK_UN) reports 0 in both cases:
if the file is locked and if it is not.
That said, I'd like to propose the LOCK_TEST flag to the flock call,
that would check whether the lock of the given type (LOCK_SH or LOCK_EX)
exists on the file we test. It's not the same as the existing in-kernel
FL_ACCESS flag, which checks whether the new lock is possible, but
it's a new FL_TEST flag, that checks whether the existing lock is there.
What do you think?
Signed-off-by: Pavel Emelyanov <xemul-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
---
diff --git a/fs/locks.c b/fs/locks.c
index bb08857..50842bf 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -830,7 +830,7 @@ static int flock_lock_file(struct file *filp, struct file_lock *request)
int found = 0;
LIST_HEAD(dispose);
- if (!(request->fl_flags & FL_ACCESS) && (request->fl_type != F_UNLCK)) {
+ if (!(request->fl_flags & (FL_ACCESS|FL_TEST)) && (request->fl_type != F_UNLCK)) {
new_fl = locks_alloc_lock();
if (!new_fl)
return -ENOMEM;
@@ -850,11 +850,18 @@ static int flock_lock_file(struct file *filp, struct file_lock *request)
continue;
if (request->fl_type == fl->fl_type)
goto out;
+ if (request->fl_flags & FL_TEST)
+ break;
found = 1;
locks_delete_lock(before, &dispose);
break;
}
+ if (request->fl_flags & FL_TEST) {
+ error = -ENOENT;
+ goto out;
+ }
+
if (request->fl_type == F_UNLCK) {
if ((request->fl_flags & FL_EXISTS) && !found)
error = -ENOENT;
@@ -1852,15 +1859,16 @@ SYSCALL_DEFINE2(flock, unsigned int, fd, unsigned int, cmd)
{
struct fd f = fdget(fd);
struct file_lock *lock;
- int can_sleep, unlock;
+ int can_sleep, unlock, test;
int error;
error = -EBADF;
if (!f.file)
goto out;
+ test = (cmd & LOCK_TEST);
can_sleep = !(cmd & LOCK_NB);
- cmd &= ~LOCK_NB;
+ cmd &= ~(LOCK_NB|LOCK_TEST);
unlock = (cmd == LOCK_UN);
if (!unlock && !(cmd & LOCK_MAND) &&
@@ -1872,6 +1880,8 @@ SYSCALL_DEFINE2(flock, unsigned int, fd, unsigned int, cmd)
goto out_putf;
if (can_sleep)
lock->fl_flags |= FL_SLEEP;
+ if (test)
+ lock->fl_flags |= FL_TEST;
error = security_file_lock(f.file, lock->fl_type);
if (error)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 9418772..9230e1d 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -844,6 +844,7 @@ static inline struct file *get_file(struct file *f)
#define FL_DOWNGRADE_PENDING 256 /* Lease is being downgraded */
#define FL_UNLOCK_PENDING 512 /* Lease is being broken */
#define FL_OFDLCK 1024 /* lock is "owned" by struct file */
+#define FL_TEST 2048
/*
* Special return value from posix_lock_file() and vfs_lock_file() for
diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h
index 7543b3e..7302e36 100644
--- a/include/uapi/asm-generic/fcntl.h
+++ b/include/uapi/asm-generic/fcntl.h
@@ -184,6 +184,7 @@ struct f_owner_ex {
#define LOCK_READ 64 /* which allows concurrent read operations */
#define LOCK_WRITE 128 /* which allows concurrent write operations */
#define LOCK_RW 192 /* which allows concurrent read & write ops */
+#define LOCK_TEST 256 /* check for my SH|EX locks present */
#define F_LINUX_SPECIFIC_BASE 1024
^ permalink raw reply related
* Re: [PATCH] usb: gadget: f_fs: add usb_functionfs_descs_head_v2
From: Michal Nazarewicz @ 2014-09-02 12:45 UTC (permalink / raw)
To: balbi, mgautam, andrzej.p
Cc: jin.can.zhuang, linux-api, linux-kernel, david.a.cohen, hang.yuan
In-Reply-To: <20140902065836.GA23094@intel.com>
On Tue, Sep 02 2014, Zhuang Jin Can <jin.can.zhuang@intel.com> wrote:
> Add usb_functionfs_descs_head_v2 structure for the new layout of
> descriptors.
NAK. It's a duplicate of <https://lkml.org/lkml/2014/6/6/128> and even
more importantly, the format of the header is not fixed past the flags
field (e.g. fs_count may be missing), so fs_count, hs_count and ss_count
cannot be in the structure.
> Signed-off-by: Zhuang Jin Can <jin.can.zhuang@intel.com>
> ---
> include/uapi/linux/usb/functionfs.h | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/include/uapi/linux/usb/functionfs.h b/include/uapi/linux/usb/functionfs.h
> index 0154b28..0b3f9fc 100644
> --- a/include/uapi/linux/usb/functionfs.h
> +++ b/include/uapi/linux/usb/functionfs.h
> @@ -32,6 +32,15 @@ struct usb_endpoint_descriptor_no_audio {
> __u8 bInterval;
> } __attribute__((packed));
>
> +struct usb_functionfs_descs_head_v2 {
> + __le32 magic;
> + __le32 length;
> + __le32 flags;
> + __le32 fs_count;
> + __le32 hs_count;
> + __le32 ss_count;
> +} __attribute__((packed));
> +
> /* Legacy format, deprecated as of 3.14. */
> struct usb_functionfs_descs_head {
> __le32 magic;
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michał “mina86” Nazarewicz (o o)
ooo +--<mpn@google.com>--<xmpp:mina86@jabber.org>--ooO--(_)--Ooo--
^ permalink raw reply
* [PATCH] usb: gadget: f_fs: add usb_functionfs_descs_head_v2
From: Zhuang Jin Can @ 2014-09-02 6:58 UTC (permalink / raw)
To: mina86-deATy8a+UHjQT0dZR+AlfA, balbi-l0cyMroinI0,
mgautam-sgV2jX0FEOL9JmXXK+q4OQ, andrzej.p-Sze3O3UU22JBDgjK7y7TUQ
Cc: jin.can.zhuang-ral2JQCrhuEAvxtiuMwx3w,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
david.a.cohen-ral2JQCrhuEAvxtiuMwx3w,
hang.yuan-ral2JQCrhuEAvxtiuMwx3w
Add usb_functionfs_descs_head_v2 structure for the new layout of
descriptors.
Signed-off-by: Zhuang Jin Can <jin.can.zhuang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
include/uapi/linux/usb/functionfs.h | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/include/uapi/linux/usb/functionfs.h b/include/uapi/linux/usb/functionfs.h
index 0154b28..0b3f9fc 100644
--- a/include/uapi/linux/usb/functionfs.h
+++ b/include/uapi/linux/usb/functionfs.h
@@ -32,6 +32,15 @@ struct usb_endpoint_descriptor_no_audio {
__u8 bInterval;
} __attribute__((packed));
+struct usb_functionfs_descs_head_v2 {
+ __le32 magic;
+ __le32 length;
+ __le32 flags;
+ __le32 fs_count;
+ __le32 hs_count;
+ __le32 ss_count;
+} __attribute__((packed));
+
/* Legacy format, deprecated as of 3.14. */
struct usb_functionfs_descs_head {
__le32 magic;
--
1.7.9.5
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox