On Fri, Jul 17, 2020 at 11:17:18AM +0200, Peter Zijlstra wrote:

On Fri, Jul 17, 2020 at 12:20:51AM -0700, ira.weiny@intel.com wrote:

quoted

+void dev_access_disable(void)
+{
+	unsigned long flags;
+
+	if (!static_branch_unlikely(&dev_protection_static_key))
+		return;
+
+	local_irq_save(flags);
+	current->dev_page_access_ref--;
+	if (current->dev_page_access_ref == 0)

	if (!--current->dev_page_access_ref)

It's not my style but I'm ok with it.

quoted

+		pks_update_protection(dev_page_pkey, PKEY_DISABLE_ACCESS);
+	local_irq_restore(flags);
+}
+EXPORT_SYMBOL_GPL(dev_access_disable);
+
+void dev_access_enable(void)
+{
+	unsigned long flags;
+
+	if (!static_branch_unlikely(&dev_protection_static_key))
+		return;
+
+	local_irq_save(flags);
+	/* 0 clears the PKEY_DISABLE_ACCESS bit, allowing access */
+	if (current->dev_page_access_ref == 0)
+		pks_update_protection(dev_page_pkey, 0);
+	current->dev_page_access_ref++;

	if (!current->dev_page_access_ref++)

Sure.

quoted

+	local_irq_restore(flags);
+}
+EXPORT_SYMBOL_GPL(dev_access_enable);

Also, you probably want something like:

static __always_inline devm_access_disable(void)

Yes that is better.

However, again Dan and I agree devm is not the right prefix here.

I've updated.

Thanks!
Ira

{
	if (static_branch_unlikely(&dev_protection_static_key))
		__devm_access_disable();
}

static __always_inline devm_access_enable(void)
{
	if (static_branch_unlikely(&dev_protection_static_key))
		__devm_access_enable();
}