* [PATCH RFC v3 04/43] KVM: Stub in ability to disable per-VM memory attribute tracking
From: Ackerley Tng @ 2026-03-13 6:12 UTC (permalink / raw)
To: aik, andrew.jones, binbin.wu, brauner, chao.p.peng, david,
ira.weiny, jmattson, jroedel, jthoughton, michael.roth, oupton,
pankaj.gupta, qperret, rick.p.edgecombe, rientjes, shivankg,
steven.price, tabba, willy, wyihan, yan.y.zhao, forkloop,
pratyush, suzuki.poulose, aneesh.kumar, Paolo Bonzini,
Sean Christopherson, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, Jonathan Corbet, Shuah Khan,
Shuah Khan, Vishal Annapurve, Jason Gunthorpe, Vlastimil Babka
Cc: kvm, linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest,
Ackerley Tng
In-Reply-To: <20260313-gmem-inplace-conversion-v3-0-5fc12a70ec89@google.com>
From: Sean Christopherson <seanjc@google.com>
Introduce the basic infrastructure to allow per-VM memory attribute
tracking to be disabled. This will be built-upon in a later patch, where a
module param can disable per-VM memory attribute tracking.
Split the Kconfig option into a base KVM_MEMORY_ATTRIBUTES and the
existing KVM_VM_MEMORY_ATTRIBUTES. The base option provides the core
plumbing, while the latter enables the full per-VM tracking via an xarray
and the associated ioctls.
kvm_get_memory_attributes() now performs a static call that either looks up
kvm->mem_attr_array with CONFIG_KVM_VM_MEMORY_ATTRIBUTES is enabled, or
just returns 0 otherwise. The static call can be patched depending on
whether per-VM tracking is enabled by the CONFIG.
No functional change intended.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/include/asm/kvm_host.h | 2 +-
include/linux/kvm_host.h | 23 ++++++++++++---------
virt/kvm/Kconfig | 4 ++++
virt/kvm/kvm_main.c | 44 ++++++++++++++++++++++++++++++++++++++++-
4 files changed, 62 insertions(+), 11 deletions(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index cf3d2bdababc7..537f25121e345 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -2329,7 +2329,7 @@ void kvm_configure_mmu(bool enable_tdp, int tdp_forced_root_level,
int tdp_max_root_level, int tdp_huge_page_level);
-#ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES
+#ifdef CONFIG_KVM_MEMORY_ATTRIBUTES
#define kvm_arch_has_private_mem(kvm) ((kvm)->arch.has_private_mem)
#endif
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 4ba42a1278d5f..5393854ca5966 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -2513,19 +2513,15 @@ static inline bool kvm_memslot_is_gmem_only(const struct kvm_memory_slot *slot)
return slot->flags & KVM_MEMSLOT_GMEM_ONLY;
}
-#ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES
+#ifdef CONFIG_KVM_MEMORY_ATTRIBUTES
+typedef unsigned long (kvm_get_memory_attributes_t)(struct kvm *kvm, gfn_t gfn);
+DECLARE_STATIC_CALL(__kvm_get_memory_attributes, kvm_get_memory_attributes_t);
+
static inline unsigned long kvm_get_memory_attributes(struct kvm *kvm, gfn_t gfn)
{
- return xa_to_value(xa_load(&kvm->mem_attr_array, gfn));
+ return static_call(__kvm_get_memory_attributes)(kvm, gfn);
}
-bool kvm_range_has_memory_attributes(struct kvm *kvm, gfn_t start, gfn_t end,
- unsigned long mask, unsigned long attrs);
-bool kvm_arch_pre_set_memory_attributes(struct kvm *kvm,
- struct kvm_gfn_range *range);
-bool kvm_arch_post_set_memory_attributes(struct kvm *kvm,
- struct kvm_gfn_range *range);
-
static inline bool kvm_mem_is_private(struct kvm *kvm, gfn_t gfn)
{
return kvm_get_memory_attributes(kvm, gfn) & KVM_MEMORY_ATTRIBUTE_PRIVATE;
@@ -2535,6 +2531,15 @@ static inline bool kvm_mem_is_private(struct kvm *kvm, gfn_t gfn)
{
return false;
}
+#endif
+
+#ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES
+bool kvm_range_has_memory_attributes(struct kvm *kvm, gfn_t start, gfn_t end,
+ unsigned long mask, unsigned long attrs);
+bool kvm_arch_pre_set_memory_attributes(struct kvm *kvm,
+ struct kvm_gfn_range *range);
+bool kvm_arch_post_set_memory_attributes(struct kvm *kvm,
+ struct kvm_gfn_range *range);
#endif /* CONFIG_KVM_VM_MEMORY_ATTRIBUTES */
#ifdef CONFIG_KVM_GUEST_MEMFD
diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig
index 5119cb37145fc..3fea89c45cfb4 100644
--- a/virt/kvm/Kconfig
+++ b/virt/kvm/Kconfig
@@ -100,7 +100,11 @@ config KVM_ELIDE_TLB_FLUSH_IF_YOUNG
config KVM_MMU_LOCKLESS_AGING
bool
+config KVM_MEMORY_ATTRIBUTES
+ bool
+
config KVM_VM_MEMORY_ATTRIBUTES
+ select KVM_MEMORY_ATTRIBUTES
bool
config KVM_GUEST_MEMFD
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 793a2c8476b09..dbdcef90a4950 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -102,6 +102,17 @@ EXPORT_SYMBOL_FOR_KVM_INTERNAL(halt_poll_ns_shrink);
static bool allow_unsafe_mappings;
module_param(allow_unsafe_mappings, bool, 0444);
+#ifdef CONFIG_KVM_MEMORY_ATTRIBUTES
+#ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES
+static bool vm_memory_attributes = true;
+#else
+#define vm_memory_attributes false
+#endif
+DEFINE_STATIC_CALL_RET0(__kvm_get_memory_attributes, kvm_get_memory_attributes_t);
+EXPORT_SYMBOL_FOR_KVM_INTERNAL(STATIC_CALL_KEY(__kvm_get_memory_attributes));
+EXPORT_SYMBOL_FOR_KVM_INTERNAL(STATIC_CALL_TRAMP(__kvm_get_memory_attributes));
+#endif
+
/*
* Ordering of locks:
*
@@ -2425,7 +2436,7 @@ static int kvm_vm_ioctl_clear_dirty_log(struct kvm *kvm,
}
#endif /* CONFIG_KVM_GENERIC_DIRTYLOG_READ_PROTECT */
-#ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES
+#ifdef CONFIG_KVM_MEMORY_ATTRIBUTES
static u64 kvm_supported_mem_attributes(struct kvm *kvm)
{
#ifdef kvm_arch_has_private_mem
@@ -2436,6 +2447,12 @@ static u64 kvm_supported_mem_attributes(struct kvm *kvm)
return 0;
}
+#ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES
+static unsigned long kvm_get_vm_memory_attributes(struct kvm *kvm, gfn_t gfn)
+{
+ return xa_to_value(xa_load(&kvm->mem_attr_array, gfn));
+}
+
/*
* Returns true if _all_ gfns in the range [@start, @end) have attributes
* such that the bits in @mask match @attrs.
@@ -2632,7 +2649,24 @@ static int kvm_vm_ioctl_set_mem_attributes(struct kvm *kvm,
return kvm_vm_set_mem_attributes(kvm, start, end, attrs->attributes);
}
+#else /* CONFIG_KVM_VM_MEMORY_ATTRIBUTES */
+static unsigned long kvm_get_vm_memory_attributes(struct kvm *kvm, gfn_t gfn)
+{
+ BUILD_BUG_ON(1);
+}
#endif /* CONFIG_KVM_VM_MEMORY_ATTRIBUTES */
+static void kvm_init_memory_attributes(void)
+{
+ if (vm_memory_attributes)
+ static_call_update(__kvm_get_memory_attributes,
+ kvm_get_vm_memory_attributes);
+ else
+ static_call_update(__kvm_get_memory_attributes,
+ (void *)__static_call_return0);
+}
+#else /* CONFIG_KVM_MEMORY_ATTRIBUTES */
+static void kvm_init_memory_attributes(void) { }
+#endif /* CONFIG_KVM_MEMORY_ATTRIBUTES */
struct kvm_memory_slot *gfn_to_memslot(struct kvm *kvm, gfn_t gfn)
{
@@ -4932,6 +4966,9 @@ static int kvm_vm_ioctl_check_extension_generic(struct kvm *kvm, long arg)
return 1;
#ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES
case KVM_CAP_MEMORY_ATTRIBUTES:
+ if (!vm_memory_attributes)
+ return 0;
+
return kvm_supported_mem_attributes(kvm);
#endif
#ifdef CONFIG_KVM_GUEST_MEMFD
@@ -5338,6 +5375,10 @@ static long kvm_vm_ioctl(struct file *filp,
case KVM_SET_MEMORY_ATTRIBUTES: {
struct kvm_memory_attributes attrs;
+ r = -ENOTTY;
+ if (!vm_memory_attributes)
+ goto out;
+
r = -EFAULT;
if (copy_from_user(&attrs, argp, sizeof(attrs)))
goto out;
@@ -6528,6 +6569,7 @@ int kvm_init(unsigned vcpu_size, unsigned vcpu_align, struct module *module)
kvm_preempt_ops.sched_in = kvm_sched_in;
kvm_preempt_ops.sched_out = kvm_sched_out;
+ kvm_init_memory_attributes();
kvm_init_debug();
r = kvm_vfio_ops_init();
--
2.53.0.851.ga537e3e6e9-goog
^ permalink raw reply related
* [PATCH RFC v3 03/43] KVM: Enumerate support for PRIVATE memory iff kvm_arch_has_private_mem is defined
From: Ackerley Tng @ 2026-03-13 6:12 UTC (permalink / raw)
To: aik, andrew.jones, binbin.wu, brauner, chao.p.peng, david,
ira.weiny, jmattson, jroedel, jthoughton, michael.roth, oupton,
pankaj.gupta, qperret, rick.p.edgecombe, rientjes, shivankg,
steven.price, tabba, willy, wyihan, yan.y.zhao, forkloop,
pratyush, suzuki.poulose, aneesh.kumar, Paolo Bonzini,
Sean Christopherson, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, Jonathan Corbet, Shuah Khan,
Shuah Khan, Vishal Annapurve, Jason Gunthorpe, Vlastimil Babka
Cc: kvm, linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest,
Ackerley Tng
In-Reply-To: <20260313-gmem-inplace-conversion-v3-0-5fc12a70ec89@google.com>
From: Sean Christopherson <seanjc@google.com>
Explicitly guard reporting support for KVM_MEMORY_ATTRIBUTE_PRIVATE based
on kvm_arch_has_private_mem being #defined in anticipation of decoupling
kvm_supported_mem_attributes() from CONFIG_KVM_VM_MEMORY_ATTRIBUTES.
guest_memfd support for memory attributes will be unconditional to avoid
yet more macros (all architectures that support guest_memfd are expected to
use per-gmem attributes at some point), at which point enumerating support
KVM_MEMORY_ATTRIBUTE_PRIVATE based solely on memory attributes being
supported _somewhere_ would result in KVM over-reporting support on arm64.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
include/linux/kvm_host.h | 2 +-
virt/kvm/kvm_main.c | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index e75f7295af5d0..4ba42a1278d5f 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -721,7 +721,7 @@ static inline int kvm_arch_vcpu_memslots_id(struct kvm_vcpu *vcpu)
}
#endif
-#ifndef CONFIG_KVM_VM_MEMORY_ATTRIBUTES
+#ifndef kvm_arch_has_private_mem
static inline bool kvm_arch_has_private_mem(struct kvm *kvm)
{
return false;
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 301d7ddac6ba6..793a2c8476b09 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2428,8 +2428,10 @@ static int kvm_vm_ioctl_clear_dirty_log(struct kvm *kvm,
#ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES
static u64 kvm_supported_mem_attributes(struct kvm *kvm)
{
+#ifdef kvm_arch_has_private_mem
if (!kvm || kvm_arch_has_private_mem(kvm))
return KVM_MEMORY_ATTRIBUTE_PRIVATE;
+#endif
return 0;
}
--
2.53.0.851.ga537e3e6e9-goog
^ permalink raw reply related
* [PATCH RFC v3 02/43] KVM: Rename KVM_GENERIC_MEMORY_ATTRIBUTES to KVM_VM_MEMORY_ATTRIBUTES
From: Ackerley Tng @ 2026-03-13 6:12 UTC (permalink / raw)
To: aik, andrew.jones, binbin.wu, brauner, chao.p.peng, david,
ira.weiny, jmattson, jroedel, jthoughton, michael.roth, oupton,
pankaj.gupta, qperret, rick.p.edgecombe, rientjes, shivankg,
steven.price, tabba, willy, wyihan, yan.y.zhao, forkloop,
pratyush, suzuki.poulose, aneesh.kumar, Paolo Bonzini,
Sean Christopherson, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, Jonathan Corbet, Shuah Khan,
Shuah Khan, Vishal Annapurve, Jason Gunthorpe, Vlastimil Babka
Cc: kvm, linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest,
Ackerley Tng
In-Reply-To: <20260313-gmem-inplace-conversion-v3-0-5fc12a70ec89@google.com>
From: Sean Christopherson <seanjc@google.com>
Rename the per-VM memory attributes Kconfig to make it explicitly about
per-VM attributes in anticipation of adding memory attributes support to
guest_memfd, at which point it will be possible (and desirable) to have
memory attributes without the per-VM support, even in x86.
No functional change intended.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/include/asm/kvm_host.h | 2 +-
arch/x86/kvm/Kconfig | 6 +++---
arch/x86/kvm/mmu/mmu.c | 2 +-
arch/x86/kvm/x86.c | 2 +-
include/linux/kvm_host.h | 8 ++++----
include/trace/events/kvm.h | 4 ++--
virt/kvm/Kconfig | 2 +-
virt/kvm/kvm_main.c | 14 +++++++-------
8 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 6e4e3ef9b8c72..cf3d2bdababc7 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -2329,7 +2329,7 @@ void kvm_configure_mmu(bool enable_tdp, int tdp_forced_root_level,
int tdp_max_root_level, int tdp_huge_page_level);
-#ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES
+#ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES
#define kvm_arch_has_private_mem(kvm) ((kvm)->arch.has_private_mem)
#endif
diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
index 801bf9e520db3..26f6afd51bbdc 100644
--- a/arch/x86/kvm/Kconfig
+++ b/arch/x86/kvm/Kconfig
@@ -84,7 +84,7 @@ config KVM_SW_PROTECTED_VM
bool "Enable support for KVM software-protected VMs"
depends on EXPERT
depends on KVM_X86 && X86_64
- select KVM_GENERIC_MEMORY_ATTRIBUTES
+ select KVM_VM_MEMORY_ATTRIBUTES
help
Enable support for KVM software-protected VMs. Currently, software-
protected VMs are purely a development and testing vehicle for
@@ -135,7 +135,7 @@ config KVM_INTEL_TDX
bool "Intel Trust Domain Extensions (TDX) support"
default y
depends on INTEL_TDX_HOST
- select KVM_GENERIC_MEMORY_ATTRIBUTES
+ select KVM_VM_MEMORY_ATTRIBUTES
select HAVE_KVM_ARCH_GMEM_POPULATE
help
Provides support for launching Intel Trust Domain Extensions (TDX)
@@ -159,7 +159,7 @@ config KVM_AMD_SEV
depends on KVM_AMD && X86_64
depends on CRYPTO_DEV_SP_PSP && !(KVM_AMD=y && CRYPTO_DEV_CCP_DD=m)
select ARCH_HAS_CC_PLATFORM
- select KVM_GENERIC_MEMORY_ATTRIBUTES
+ select KVM_VM_MEMORY_ATTRIBUTES
select HAVE_KVM_ARCH_GMEM_PREPARE
select HAVE_KVM_ARCH_GMEM_INVALIDATE
select HAVE_KVM_ARCH_GMEM_POPULATE
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index b922a8b000577..792701b093234 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -7889,7 +7889,7 @@ void kvm_mmu_pre_destroy_vm(struct kvm *kvm)
vhost_task_stop(kvm->arch.nx_huge_page_recovery_thread);
}
-#ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES
+#ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES
static bool hugepage_test_mixed(struct kvm_memory_slot *slot, gfn_t gfn,
int level)
{
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index fd1c4a36b5936..7e133a9da11f0 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -13509,7 +13509,7 @@ static int kvm_alloc_memslot_metadata(struct kvm *kvm,
}
}
-#ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES
+#ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES
kvm_mmu_init_memslot_memory_attributes(kvm, slot);
#endif
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 6b76e7a6f4c22..e75f7295af5d0 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -721,7 +721,7 @@ static inline int kvm_arch_vcpu_memslots_id(struct kvm_vcpu *vcpu)
}
#endif
-#ifndef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES
+#ifndef CONFIG_KVM_VM_MEMORY_ATTRIBUTES
static inline bool kvm_arch_has_private_mem(struct kvm *kvm)
{
return false;
@@ -870,7 +870,7 @@ struct kvm {
#ifdef CONFIG_HAVE_KVM_PM_NOTIFIER
struct notifier_block pm_notifier;
#endif
-#ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES
+#ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES
/* Protected by slots_lock (for writes) and RCU (for reads) */
struct xarray mem_attr_array;
#endif
@@ -2513,7 +2513,7 @@ static inline bool kvm_memslot_is_gmem_only(const struct kvm_memory_slot *slot)
return slot->flags & KVM_MEMSLOT_GMEM_ONLY;
}
-#ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES
+#ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES
static inline unsigned long kvm_get_memory_attributes(struct kvm *kvm, gfn_t gfn)
{
return xa_to_value(xa_load(&kvm->mem_attr_array, gfn));
@@ -2535,7 +2535,7 @@ static inline bool kvm_mem_is_private(struct kvm *kvm, gfn_t gfn)
{
return false;
}
-#endif /* CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES */
+#endif /* CONFIG_KVM_VM_MEMORY_ATTRIBUTES */
#ifdef CONFIG_KVM_GUEST_MEMFD
int kvm_gmem_get_pfn(struct kvm *kvm, struct kvm_memory_slot *slot,
diff --git a/include/trace/events/kvm.h b/include/trace/events/kvm.h
index b282e3a867696..1ba72bd73ea2f 100644
--- a/include/trace/events/kvm.h
+++ b/include/trace/events/kvm.h
@@ -358,7 +358,7 @@ TRACE_EVENT(kvm_dirty_ring_exit,
TP_printk("vcpu %d", __entry->vcpu_id)
);
-#ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES
+#ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES
/*
* @start: Starting address of guest memory range
* @end: End address of guest memory range
@@ -383,7 +383,7 @@ TRACE_EVENT(kvm_vm_set_mem_attributes,
TP_printk("%#016llx -- %#016llx [0x%lx]",
__entry->start, __entry->end, __entry->attr)
);
-#endif /* CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES */
+#endif /* CONFIG_KVM_VM_MEMORY_ATTRIBUTES */
TRACE_EVENT(kvm_unmap_hva_range,
TP_PROTO(unsigned long start, unsigned long end),
diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig
index 794976b88c6f9..5119cb37145fc 100644
--- a/virt/kvm/Kconfig
+++ b/virt/kvm/Kconfig
@@ -100,7 +100,7 @@ config KVM_ELIDE_TLB_FLUSH_IF_YOUNG
config KVM_MMU_LOCKLESS_AGING
bool
-config KVM_GENERIC_MEMORY_ATTRIBUTES
+config KVM_VM_MEMORY_ATTRIBUTES
bool
config KVM_GUEST_MEMFD
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 9093251beb398..301d7ddac6ba6 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1122,7 +1122,7 @@ static struct kvm *kvm_create_vm(unsigned long type, const char *fdname)
spin_lock_init(&kvm->mn_invalidate_lock);
rcuwait_init(&kvm->mn_memslots_update_rcuwait);
xa_init(&kvm->vcpu_array);
-#ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES
+#ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES
xa_init(&kvm->mem_attr_array);
#endif
@@ -1307,7 +1307,7 @@ static void kvm_destroy_vm(struct kvm *kvm)
cleanup_srcu_struct(&kvm->irq_srcu);
srcu_barrier(&kvm->srcu);
cleanup_srcu_struct(&kvm->srcu);
-#ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES
+#ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES
xa_destroy(&kvm->mem_attr_array);
#endif
kvm_arch_free_vm(kvm);
@@ -2425,7 +2425,7 @@ static int kvm_vm_ioctl_clear_dirty_log(struct kvm *kvm,
}
#endif /* CONFIG_KVM_GENERIC_DIRTYLOG_READ_PROTECT */
-#ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES
+#ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES
static u64 kvm_supported_mem_attributes(struct kvm *kvm)
{
if (!kvm || kvm_arch_has_private_mem(kvm))
@@ -2630,7 +2630,7 @@ static int kvm_vm_ioctl_set_mem_attributes(struct kvm *kvm,
return kvm_vm_set_mem_attributes(kvm, start, end, attrs->attributes);
}
-#endif /* CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES */
+#endif /* CONFIG_KVM_VM_MEMORY_ATTRIBUTES */
struct kvm_memory_slot *gfn_to_memslot(struct kvm *kvm, gfn_t gfn)
{
@@ -4928,7 +4928,7 @@ static int kvm_vm_ioctl_check_extension_generic(struct kvm *kvm, long arg)
case KVM_CAP_SYSTEM_EVENT_DATA:
case KVM_CAP_DEVICE_CTRL:
return 1;
-#ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES
+#ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES
case KVM_CAP_MEMORY_ATTRIBUTES:
return kvm_supported_mem_attributes(kvm);
#endif
@@ -5332,7 +5332,7 @@ static long kvm_vm_ioctl(struct file *filp,
break;
}
#endif /* CONFIG_HAVE_KVM_IRQ_ROUTING */
-#ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES
+#ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES
case KVM_SET_MEMORY_ATTRIBUTES: {
struct kvm_memory_attributes attrs;
@@ -5343,7 +5343,7 @@ static long kvm_vm_ioctl(struct file *filp,
r = kvm_vm_ioctl_set_mem_attributes(kvm, &attrs);
break;
}
-#endif /* CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES */
+#endif /* CONFIG_KVM_VM_MEMORY_ATTRIBUTES */
case KVM_CREATE_DEVICE: {
struct kvm_create_device cd;
--
2.53.0.851.ga537e3e6e9-goog
^ permalink raw reply related
* [PATCH RFC v3 01/43] KVM: guest_memfd: Introduce per-gmem attributes, use to guard user mappings
From: Ackerley Tng @ 2026-03-13 6:12 UTC (permalink / raw)
To: aik, andrew.jones, binbin.wu, brauner, chao.p.peng, david,
ira.weiny, jmattson, jroedel, jthoughton, michael.roth, oupton,
pankaj.gupta, qperret, rick.p.edgecombe, rientjes, shivankg,
steven.price, tabba, willy, wyihan, yan.y.zhao, forkloop,
pratyush, suzuki.poulose, aneesh.kumar, Paolo Bonzini,
Sean Christopherson, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, Jonathan Corbet, Shuah Khan,
Shuah Khan, Vishal Annapurve, Jason Gunthorpe, Vlastimil Babka
Cc: kvm, linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest,
Ackerley Tng
In-Reply-To: <20260313-gmem-inplace-conversion-v3-0-5fc12a70ec89@google.com>
From: Sean Christopherson <seanjc@google.com>
Start plumbing in guest_memfd support for in-place private<=>shared
conversions by tracking attributes via a maple tree. KVM currently tracks
private vs. shared attributes on a per-VM basis, which made sense when a
guest_memfd _only_ supported private memory, but tracking per-VM simply
can't work for in-place conversions as the shareability of a given page
needs to be per-gmem_inode, not per-VM.
Use the filemap invalidation lock to protect the maple tree, as taking the
lock for read when faulting in memory (for userspace or the guest) isn't
expected to result in meaningful contention, and using a separate lock
would add significant complexity (avoid deadlock is quite difficult).
Signed-off-by: Sean Christopherson <seanjc@google.com>
Co-developed-by: Ackerley Tng <ackerleytng@google.com>
Signed-off-by: Ackerley Tng <ackerleytng@google.com>
Co-developed-by: Vishal Annapurve <vannapurve@google.com>
Signed-off-by: Vishal Annapurve <vannapurve@google.com>
Co-developed-by: Fuad Tabba <tabba@google.com>
Signed-off-by: Fuad Tabba <tabba@google.com>
---
virt/kvm/guest_memfd.c | 134 +++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 118 insertions(+), 16 deletions(-)
diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
index 017d84a7adf37..67709af03f39d 100644
--- a/virt/kvm/guest_memfd.c
+++ b/virt/kvm/guest_memfd.c
@@ -4,6 +4,7 @@
#include <linux/falloc.h>
#include <linux/fs.h>
#include <linux/kvm_host.h>
+#include <linux/maple_tree.h>
#include <linux/mempolicy.h>
#include <linux/pseudo_fs.h>
#include <linux/pagemap.h>
@@ -32,6 +33,7 @@ struct gmem_inode {
struct inode vfs_inode;
u64 flags;
+ struct maple_tree attributes;
};
static __always_inline struct gmem_inode *GMEM_I(struct inode *inode)
@@ -59,6 +61,31 @@ static pgoff_t kvm_gmem_get_index(struct kvm_memory_slot *slot, gfn_t gfn)
return gfn - slot->base_gfn + slot->gmem.pgoff;
}
+static u64 kvm_gmem_get_attributes(struct inode *inode, pgoff_t index)
+{
+ struct maple_tree *mt = &GMEM_I(inode)->attributes;
+ void *entry = mtree_load(mt, index);
+
+ /*
+ * The lock _must_ be held for lookups, as some maple tree operations,
+ * e.g. append, are unsafe (return inaccurate information) with respect
+ * to concurrent RCU-protected lookups.
+ */
+ lockdep_assert(mt_lock_is_held(mt));
+
+ return WARN_ON_ONCE(!entry) ? 0 : xa_to_value(entry);
+}
+
+static bool kvm_gmem_is_private_mem(struct inode *inode, pgoff_t index)
+{
+ return kvm_gmem_get_attributes(inode, index) & KVM_MEMORY_ATTRIBUTE_PRIVATE;
+}
+
+static bool kvm_gmem_is_shared_mem(struct inode *inode, pgoff_t index)
+{
+ return !kvm_gmem_is_private_mem(inode, index);
+}
+
static int __kvm_gmem_prepare_folio(struct kvm *kvm, struct kvm_memory_slot *slot,
pgoff_t index, struct folio *folio)
{
@@ -397,10 +424,13 @@ static vm_fault_t kvm_gmem_fault_user_mapping(struct vm_fault *vmf)
if (((loff_t)vmf->pgoff << PAGE_SHIFT) >= i_size_read(inode))
return VM_FAULT_SIGBUS;
- if (!(GMEM_I(inode)->flags & GUEST_MEMFD_FLAG_INIT_SHARED))
- return VM_FAULT_SIGBUS;
+ filemap_invalidate_lock_shared(inode->i_mapping);
+ if (kvm_gmem_is_shared_mem(inode, vmf->pgoff))
+ folio = kvm_gmem_get_folio(inode, vmf->pgoff);
+ else
+ folio = ERR_PTR(-EACCES);
+ filemap_invalidate_unlock_shared(inode->i_mapping);
- folio = kvm_gmem_get_folio(inode, vmf->pgoff);
if (IS_ERR(folio)) {
if (PTR_ERR(folio) == -EAGAIN)
return VM_FAULT_RETRY;
@@ -556,6 +586,51 @@ bool __weak kvm_arch_supports_gmem_init_shared(struct kvm *kvm)
return true;
}
+static int kvm_gmem_init_inode(struct inode *inode, loff_t size, u64 flags)
+{
+ struct gmem_inode *gi = GMEM_I(inode);
+ MA_STATE(mas, &gi->attributes, 0, (size >> PAGE_SHIFT) - 1);
+ u64 attrs;
+ int r;
+
+ inode->i_op = &kvm_gmem_iops;
+ inode->i_mapping->a_ops = &kvm_gmem_aops;
+ inode->i_mode |= S_IFREG;
+ inode->i_size = size;
+ mapping_set_gfp_mask(inode->i_mapping, GFP_HIGHUSER);
+
+ /*
+ * guest_memfd memory is neither migratable nor swappable: set
+ * inaccessible to gate off both.
+ */
+ mapping_set_inaccessible(inode->i_mapping);
+ WARN_ON_ONCE(!mapping_unevictable(inode->i_mapping));
+
+ gi->flags = flags;
+
+ mt_set_external_lock(&gi->attributes,
+ &inode->i_mapping->invalidate_lock);
+
+ /*
+ * Store default attributes for the entire gmem instance. Ensuring every
+ * index is represented in the maple tree at all times simplifies the
+ * conversion and merging logic.
+ */
+ attrs = gi->flags & GUEST_MEMFD_FLAG_INIT_SHARED ? 0 : KVM_MEMORY_ATTRIBUTE_PRIVATE;
+
+ /*
+ * Acquire the invalidation lock purely to make lockdep happy. The
+ * maple tree library expects all stores to be protected via the lock,
+ * and the library can't know when the tree is reachable only by the
+ * caller, as is the case here.
+ */
+ filemap_invalidate_lock(inode->i_mapping);
+ r = mas_store_gfp(&mas, xa_mk_value(attrs), GFP_KERNEL);
+ filemap_invalidate_unlock(inode->i_mapping);
+
+ return r;
+}
+
static int __kvm_gmem_create(struct kvm *kvm, loff_t size, u64 flags)
{
static const char *name = "[kvm-gmem]";
@@ -586,16 +661,9 @@ static int __kvm_gmem_create(struct kvm *kvm, loff_t size, u64 flags)
goto err_fops;
}
- inode->i_op = &kvm_gmem_iops;
- inode->i_mapping->a_ops = &kvm_gmem_aops;
- inode->i_mode |= S_IFREG;
- inode->i_size = size;
- mapping_set_gfp_mask(inode->i_mapping, GFP_HIGHUSER);
- mapping_set_inaccessible(inode->i_mapping);
- /* Unmovable mappings are supposed to be marked unevictable as well. */
- WARN_ON_ONCE(!mapping_unevictable(inode->i_mapping));
-
- GMEM_I(inode)->flags = flags;
+ err = kvm_gmem_init_inode(inode, size, flags);
+ if (err)
+ goto err_inode;
file = alloc_file_pseudo(inode, kvm_gmem_mnt, name, O_RDWR, &kvm_gmem_fops);
if (IS_ERR(file)) {
@@ -797,9 +865,13 @@ int kvm_gmem_get_pfn(struct kvm *kvm, struct kvm_memory_slot *slot,
if (!file)
return -EFAULT;
+ filemap_invalidate_lock_shared(file_inode(file)->i_mapping);
+
folio = __kvm_gmem_get_pfn(file, slot, index, pfn, max_order);
- if (IS_ERR(folio))
- return PTR_ERR(folio);
+ if (IS_ERR(folio)) {
+ r = PTR_ERR(folio);
+ goto out;
+ }
if (!folio_test_uptodate(folio)) {
clear_highpage(folio_page(folio, 0));
@@ -815,6 +887,8 @@ int kvm_gmem_get_pfn(struct kvm *kvm, struct kvm_memory_slot *slot,
else
folio_put(folio);
+out:
+ filemap_invalidate_unlock_shared(file_inode(file)->i_mapping);
return r;
}
EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_gmem_get_pfn);
@@ -944,13 +1018,41 @@ static struct inode *kvm_gmem_alloc_inode(struct super_block *sb)
mpol_shared_policy_init(&gi->policy, NULL);
+ /*
+ * Memory attributes are protected by the filemap invalidation lock, but
+ * the lock structure isn't available at this time. Immediately mark
+ * maple tree as using external locking so that accessing the tree
+ * before it's fully initialized results in NULL pointer dereferences
+ * and not more subtle bugs.
+ */
+ mt_init_flags(&gi->attributes, MT_FLAGS_LOCK_EXTERN);
+
gi->flags = 0;
return &gi->vfs_inode;
}
static void kvm_gmem_destroy_inode(struct inode *inode)
{
- mpol_free_shared_policy(&GMEM_I(inode)->policy);
+ struct gmem_inode *gi = GMEM_I(inode);
+
+ mpol_free_shared_policy(&gi->policy);
+
+ /*
+ * Note! Checking for an empty tree is functionally necessary
+ * to avoid explosions if the tree hasn't been fully
+ * initialized, i.e. if the inode is being destroyed before
+ * guest_memfd can set the external lock, lockdep would find
+ * that the tree's internal ma_lock was not held.
+ */
+ if (!mtree_empty(&gi->attributes)) {
+ /*
+ * Acquire the invalidation lock purely to make lockdep happy,
+ * the inode is unreachable at this point.
+ */
+ filemap_invalidate_lock(inode->i_mapping);
+ __mt_destroy(&gi->attributes);
+ filemap_invalidate_unlock(inode->i_mapping);
+ }
}
static void kvm_gmem_free_inode(struct inode *inode)
--
2.53.0.851.ga537e3e6e9-goog
^ permalink raw reply related
* [PATCH RFC v3 00/43] guest_memfd: In-place conversion support
From: Ackerley Tng @ 2026-03-13 6:12 UTC (permalink / raw)
To: aik, andrew.jones, binbin.wu, brauner, chao.p.peng, david,
ira.weiny, jmattson, jroedel, jthoughton, michael.roth, oupton,
pankaj.gupta, qperret, rick.p.edgecombe, rientjes, shivankg,
steven.price, tabba, willy, wyihan, yan.y.zhao, forkloop,
pratyush, suzuki.poulose, aneesh.kumar, Paolo Bonzini,
Sean Christopherson, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, Jonathan Corbet, Shuah Khan,
Shuah Khan, Vishal Annapurve, Jason Gunthorpe, Vlastimil Babka
Cc: kvm, linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest,
Ackerley Tng
Hi,
(Here's the motivation for this series, which I realized was missing from
the earlier revisions of this series)
Up till now, guest_memfd supports the entire inode worth of memory being
used as all-shared, or all-private. CoCo VMs may request guest memory to be
converted between private and shared states, and the only way to support
that currently would be to have the userspace VMM provide two sources of
backing memory from completely different areas of physical memory.
pKVM has a use case for in-place sharing: the guest and host may be
cooperating on given data, and pKVM doesn't protect data through
encryption, so copying that given data between different areas of physical
memory as part of conversions would be unnecessary work.
This series also serves as a foundation for guest_memfd huge page
support. Now, guest_memfd only supports PAGE_SIZE pages, so if two sources
of backing memory are used, the userspace VMM could maintain a steady total
memory utilized by punching out the pages that are not used. When huge
pages are available in guest_memfd, even if the backing memory source
supports hole punching within a huge page, punching out pages to maintain
the total memory utilized by a VM would be introducing lots of
fragmentation.
In-place conversion avoids fragmentation by allowing the same physical
memory to be used for both shared and private memory, with guest_memfd
tracks the shared/private status of all the pages at a per-page
granularity.
The central principle, which guest_memfd continues to uphold, is that any
guest-private page will not be mappable to host userspace. All pages will
be mmap()-able in host userspace, but accesses to guest-private pages (as
tracked by guest_memfd) will result in a SIGBUS.
This series introduces a guest_memfd ioctl (not kvm, vm or vcpu, but
guest_memfd ioctl) that allows userspace to set memory
attributes (shared/private) directly through the guest_memfd. This is the
appropriate interface because shared/private-ness is a property of memory
and hence the request should be sent directly to the memory provider -
guest_memfd.
I'm intending RFC (v3) as a basis for discussion of flags/content
modes (name TBD) to allow userspace to request guarantees on how the memory
contents will look like after setting memory attributes. The last 6 patches
implement content mode support. These patches will be reordered, and some
of them could be absorbed into earlier patches, in later revisions.
Here are the discussion points I can think of (please add on):
1. (Might hopefully resolve soon?) Should ZERO be supported on shared to
private conversions? Discussion is at [6].
2. Do we need a CAP for userspace to query the flags/modes supported?
It seems like there won't be anything dynamic about the flags/modes
supported.
The userspace code can check what platform it is running on, and then
decide ZERO or PRESERVE based on the platform:
If the VM is running on TDX, it would want to specify ZERO all the
time. If the VM were running on pKVM it would want to specify PRESERVE
if it wants to enable in-place sharing, and ZERO if it wants to zero the
memory.
If someday TDX supports PRESERVE, then there's room for discovery of
which algorithm to choose when running the guest. Perhaps that's when
the CAP should be introduced?
3. What do people think of the structure of how various content modes are
checked for support or applied? I used overridable weak functions for
architectures that haven't defined support, and defined overrides for
x86 to show how I think it would work. For CoCo platforms, I only
implemented TDX for illustration purposes and might need help with the
other platforms. Should I have used kvm_x86_ops? I tried and found
myself defining lots of boilerplate.
4. enum for ZERO and PRESERVE?
Pros:
* No way to define both ZERO and PRESERVE (make impossible states
unrepresentable)
* e.g. enum kvm_device_type in __u32 type in struct
kvm_create_device
* But maybe someday some modes can be used together?
* Content modes is a defined axis/aspect of setting memory attributes,
having a separate field avoids having different axes of configuration
in one field. e.g. MAP_HUGETLB for mmap() is on a different axis from
MAP_PRIVATE for example
* I just used flags for this RFC since it's the most common
approach.
TODOs:
+ Let architectures override content mode handlers on a per-inode basis
since per-folio overrides means even no-ops, like ZERO on TDX, would
require iterating all the folios.
Also, in RFC v3, TEST_EXPECT_SIGBUS() is updated to assert that the default
signal handler is installed, so that developers get a clear, explicit
failure if/when something goes wrong.
This series is based on kvm/next, and here's the tree for your convenience:
https://github.com/googleprodkernel/linux-cc/commits/guest_memfd-inplace-conversion-v3
Older series:
+ RFCv2 is at [5]
+ RFCv1 is at [4]
+ Previous versions of this feature, part of other series, are available at
[1][2][3].
[1] https://lore.kernel.org/all/bd163de3118b626d1005aa88e71ef2fb72f0be0f.1726009989.git.ackerleytng@google.com/
[2] https://lore.kernel.org/all/20250117163001.2326672-6-tabba@google.com/
[3] https://lore.kernel.org/all/b784326e9ccae6a08388f1bf39db70a2204bdc51.1747264138.git.ackerleytng@google.com/
[4] https://lore.kernel.org/all/cover.1760731772.git.ackerleytng@google.com/T/
[5] https://lore.kernel.org/all/cover.1770071243.git.ackerleytng@google.com/T/
[6] https://lore.kernel.org/all/CAEvNRgFUc+9xCoN9Yo5NThHrvbccWAhPwp9nNM2fvx7QqrcJsg@mail.gmail.com/
Signed-off-by: Ackerley Tng <ackerleytng@google.com>
---
Ackerley Tng (25):
KVM: guest_memfd: Update kvm_gmem_populate() to use gmem attributes
KVM: Introduce KVM_SET_MEMORY_ATTRIBUTES2
KVM: guest_memfd: Add support for KVM_SET_MEMORY_ATTRIBUTES2
KVM: guest_memfd: Handle lru_add fbatch refcounts during conversion safety check
KVM: selftests: Update framework to use KVM_SET_MEMORY_ATTRIBUTES2
KVM: selftests: Test using guest_memfd for guest private memory
KVM: selftests: Test basic single-page conversion flow
KVM: selftests: Test conversion flow when INIT_SHARED
KVM: selftests: Test indexing in guest_memfd
KVM: selftests: Test conversion before allocation
KVM: selftests: Convert with allocated folios in different layouts
KVM: selftests: Test precision of conversion
KVM: selftests: Test that truncation does not change shared/private status
KVM: selftests: Test conversion with elevated page refcount
KVM: selftests: Reset shared memory after hole-punching
KVM: selftests: Provide function to look up guest_memfd details from gpa
KVM: selftests: Make TEST_EXPECT_SIGBUS thread-safe
KVM: selftests: Update private_mem_conversions_test to mmap() guest_memfd
KVM: selftests: Add script to exercise private_mem_conversions_test
KVM: guest_memfd: Introduce default handlers for content modes
KVM: guest_memfd: Apply content modes while setting memory attributes
KVM: x86: Add support for applying content modes
KVM: x86: Support content mode ZERO for TDX
KVM: selftests: Allow flags to be specified in set_memory_attributes functions
KVM: selftests: Update tests to use flag-enabled library functions
Sean Christopherson (18):
KVM: guest_memfd: Introduce per-gmem attributes, use to guard user mappings
KVM: Rename KVM_GENERIC_MEMORY_ATTRIBUTES to KVM_VM_MEMORY_ATTRIBUTES
KVM: Enumerate support for PRIVATE memory iff kvm_arch_has_private_mem is defined
KVM: Stub in ability to disable per-VM memory attribute tracking
KVM: guest_memfd: Wire up kvm_get_memory_attributes() to per-gmem attributes
KVM: guest_memfd: Enable INIT_SHARED on guest_memfd for x86 Coco VMs
KVM: Move KVM_VM_MEMORY_ATTRIBUTES config definition to x86
KVM: Let userspace disable per-VM mem attributes, enable per-gmem attributes
KVM: selftests: Create gmem fd before "regular" fd when adding memslot
KVM: selftests: Rename guest_memfd{,_offset} to gmem_{fd,offset}
KVM: selftests: Add support for mmap() on guest_memfd in core library
KVM: selftests: Add selftests global for guest memory attributes capability
KVM: selftests: Add helpers for calling ioctls on guest_memfd
KVM: selftests: Test that shared/private status is consistent across processes
KVM: selftests: Provide common function to set memory attributes
KVM: selftests: Check fd/flags provided to mmap() when setting up memslot
KVM: selftests: Update pre-fault test to work with per-guest_memfd attributes
KVM: selftests: Update private memory exits test work with per-gmem attributes
Documentation/virt/kvm/api.rst | 112 ++++-
arch/x86/include/asm/kvm_host.h | 2 +-
arch/x86/kvm/Kconfig | 15 +-
arch/x86/kvm/mmu/mmu.c | 4 +-
arch/x86/kvm/x86.c | 86 +++-
include/linux/kvm_host.h | 62 ++-
include/trace/events/kvm.h | 4 +-
include/uapi/linux/kvm.h | 21 +
tools/testing/selftests/kvm/.gitignore | 1 +
tools/testing/selftests/kvm/Makefile.kvm | 1 +
.../selftests/kvm/guest_memfd_conversions_test.c | 496 +++++++++++++++++++++
tools/testing/selftests/kvm/guest_memfd_test.c | 57 ++-
tools/testing/selftests/kvm/include/kvm_util.h | 136 +++++-
tools/testing/selftests/kvm/include/test_util.h | 32 +-
tools/testing/selftests/kvm/lib/kvm_util.c | 130 +++---
tools/testing/selftests/kvm/lib/test_util.c | 7 -
.../testing/selftests/kvm/pre_fault_memory_test.c | 2 +-
.../kvm/x86/private_mem_conversions_test.c | 54 ++-
.../kvm/x86/private_mem_conversions_test.py | 152 +++++++
.../selftests/kvm/x86/private_mem_kvm_exits_test.c | 36 +-
virt/kvm/Kconfig | 3 +-
virt/kvm/guest_memfd.c | 488 +++++++++++++++++++-
virt/kvm/kvm_main.c | 104 ++++-
23 files changed, 1831 insertions(+), 174 deletions(-)
---
base-commit: d2ea4ff1ce50787a98a3900b3fb1636f3620b7cf
change-id: 20260225-gmem-inplace-conversion-bd0dbd39753a
Best regards,
--
Ackerley Tng <ackerleytng@google.com>
^ permalink raw reply
* Re: [PATCH 06/15] cpufreq: Use trace_invoke_##name() at guarded tracepoint call sites
From: Viresh Kumar @ 2026-03-13 6:07 UTC (permalink / raw)
To: Vineeth Pillai (Google)
Cc: Steven Rostedt, Peter Zijlstra, Huang Rui, Gautham R. Shenoy,
Mario Limonciello, Perry Yuan, Rafael J. Wysocki,
Srinivas Pandruvada, Len Brown, linux-pm, linux-kernel,
linux-trace-kernel
In-Reply-To: <20260312150523.2054552-7-vineeth@bitbyteword.org>
On 12-03-26, 11:05, Vineeth Pillai (Google) wrote:
> Replace trace_foo() with the new trace_invoke_foo() at sites already
> guarded by trace_foo_enabled(), avoiding a redundant
> static_branch_unlikely() re-evaluation inside the tracepoint.
> trace_invoke_foo() calls the tracepoint callbacks directly without
> utilizing the static branch again.
>
> Suggested-by: Steven Rostedt <rostedt@goodmis.org>
> Suggested-by: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Vineeth Pillai (Google) <vineeth@bitbyteword.org>
> Assisted-by: Claude:claude-sonnet-4-6
> ---
> drivers/cpufreq/amd-pstate.c | 10 +++++-----
> drivers/cpufreq/cpufreq.c | 2 +-
> drivers/cpufreq/intel_pstate.c | 2 +-
> 3 files changed, 7 insertions(+), 7 deletions(-)
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
--
viresh
^ permalink raw reply
* [PATCH] bootconfig: Add bootconfig tests about braces
From: Masami Hiramatsu (Google) @ 2026-03-13 4:18 UTC (permalink / raw)
To: Masami Hiramatsu, Steven Rostedt
Cc: Andrew Morton, Josh Law, linux-kernel, linux-trace-kernel
In-Reply-To: <20260312191143.28719-1-objecting@objecting.org>
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Add more bootconfig tests for checking the error message of
non closing brace and max number of nested braces.
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
.../bootconfig/samples/bad-non-closed-brace.bconf | 4 ++++
tools/bootconfig/samples/bad-over-max-brace.bconf | 19 +++++++++++++++++++
.../bootconfig/samples/exp-good-nested-brace.bconf | 1 +
tools/bootconfig/samples/good-nested-brace.bconf | 18 ++++++++++++++++++
tools/bootconfig/test-bootconfig.sh | 9 +++++++++
5 files changed, 51 insertions(+)
create mode 100644 tools/bootconfig/samples/bad-non-closed-brace.bconf
create mode 100644 tools/bootconfig/samples/bad-over-max-brace.bconf
create mode 100644 tools/bootconfig/samples/exp-good-nested-brace.bconf
create mode 100644 tools/bootconfig/samples/good-nested-brace.bconf
diff --git a/tools/bootconfig/samples/bad-non-closed-brace.bconf b/tools/bootconfig/samples/bad-non-closed-brace.bconf
new file mode 100644
index 000000000000..6ed9f3363dde
--- /dev/null
+++ b/tools/bootconfig/samples/bad-non-closed-brace.bconf
@@ -0,0 +1,4 @@
+foo {
+ bar {
+ buz
+ }
diff --git a/tools/bootconfig/samples/bad-over-max-brace.bconf b/tools/bootconfig/samples/bad-over-max-brace.bconf
new file mode 100644
index 000000000000..74b5dc9e21dc
--- /dev/null
+++ b/tools/bootconfig/samples/bad-over-max-brace.bconf
@@ -0,0 +1,19 @@
+key1 {
+key2 {
+key3 {
+key4 {
+key5 {
+key6 {
+key7 {
+key8 {
+key9 {
+key10 {
+key11 {
+key12 {
+key13 {
+key14 {
+key15 {
+key16 {
+key17 {
+}}}}}}}}}}}}}}}}}
+
diff --git a/tools/bootconfig/samples/exp-good-nested-brace.bconf b/tools/bootconfig/samples/exp-good-nested-brace.bconf
new file mode 100644
index 000000000000..19e0f51b4553
--- /dev/null
+++ b/tools/bootconfig/samples/exp-good-nested-brace.bconf
@@ -0,0 +1 @@
+key1.key2.key3.key4.key5.key6.key7.key8.key9.key10.key11.key12.key13.key14.key15.key16;
diff --git a/tools/bootconfig/samples/good-nested-brace.bconf b/tools/bootconfig/samples/good-nested-brace.bconf
new file mode 100644
index 000000000000..980d094f296e
--- /dev/null
+++ b/tools/bootconfig/samples/good-nested-brace.bconf
@@ -0,0 +1,18 @@
+key1 {
+key2 {
+key3 {
+key4 {
+key5 {
+key6 {
+key7 {
+key8 {
+key9 {
+key10 {
+key11 {
+key12 {
+key13 {
+key14 {
+key15 {
+key16 {
+}}}}}}}}}}}}}}}}
+
diff --git a/tools/bootconfig/test-bootconfig.sh b/tools/bootconfig/test-bootconfig.sh
index be9bd18b1d56..fc69f815ce4a 100755
--- a/tools/bootconfig/test-bootconfig.sh
+++ b/tools/bootconfig/test-bootconfig.sh
@@ -171,6 +171,15 @@ $BOOTCONF $INITRD > $OUTFILE
xfail grep -q 'val[[:space:]]' $OUTFILE
xpass grep -q 'val2[[:space:]]' $OUTFILE
+echo "Showing correct line:column of no closing brace"
+cat > $TEMPCONF << EOF
+foo {
+bar {
+}
+EOF
+$BOOTCONF -a $TEMPCONF $INITRD 2> $OUTFILE
+xpass grep -q "1:1" $OUTFILE
+
echo "=== expected failure cases ==="
for i in samples/bad-* ; do
xfail $BOOTCONF -a $i $INITRD
^ permalink raw reply related
* Re: [PATCH v2 2/3] lib/bootconfig: check bounds before writing in __xbc_open_brace()
From: Masami Hiramatsu @ 2026-03-13 4:18 UTC (permalink / raw)
To: Josh Law
Cc: Andrew Morton, Steven Rostedt, Masami Hiramatsu, Josh Law,
linux-kernel, linux-trace-kernel
In-Reply-To: <1bd0e36d-0ea0-4a79-bbee-e38de553f1ea@gmail.com>
On Thu, 12 Mar 2026 21:30:10 +0000
Josh Law <hlcj1234567@gmail.com> wrote:
> 12 Mar 2026 21:28:11 Andrew Morton <akpm@linux-foundation.org>:
>
> > On Thu, 12 Mar 2026 21:09:52 +0000 Josh Law <hlcj1234567@gmail.com> wrote:
> >
> >>> That's a fair point, Steve. Given that brace_index isn't touched elsewhere and the current check effectively prevents the overflow, I agree this isn't strictly necessary. I'll drop this patch and stick with the fix for the off-by-one reporting error instead. Thanks for the feedback!
> >>
> >> Wait Steve,
> >> Thanks for the look. I see your point that it's currently redundant given the call patterns. It looks like Andrew has already merged this into the -mm tree, likely as a 'belt-and-suspenders' safety measure. I'll keep your feedback in mind for future cleanup, but I'm glad we got the other off-by-one fix in as well!
> >
> > Please wordwrap the emails.
> >
> >> And in my opinion, merging it is a decent idea.
> >
> > You've changed your position without explaining why?
>
> Sorry, I think it should be merged because it's better to be safe than sorry, I know there is different methods of implementation, but this one still works... I know it's churn (and I'm sorry)
I would like to keep this original >= because it is safer.
Andrew, I will pick these patches with my test patch.
Thank you,
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH v2 3/3] lib/bootconfig: fix snprintf truncation check in xbc_node_compose_key_after()
From: Masami Hiramatsu @ 2026-03-13 2:26 UTC (permalink / raw)
To: Josh Law; +Cc: Andrew Morton, Josh Law, linux-kernel, linux-trace-kernel
In-Reply-To: <20260312191143.28719-4-objecting@objecting.org>
On Thu, 12 Mar 2026 19:11:43 +0000
Josh Law <hlcj1234567@gmail.com> wrote:
> From: Josh Law <objecting@objecting.org>
>
> snprintf() returns the number of characters that would have been
> written excluding the NUL terminator. Output is truncated when the
> return value is >= the buffer size, not just > the buffer size.
>
> When ret == size, the current code takes the non-truncated path,
> advancing buf by ret and reducing size to 0. This is wrong because
> the output was actually truncated (the last character was replaced by
> NUL). Fix by using >= so the truncation path is taken correctly.
>
OK, but this one is a minor issue, because either way, remaining
size becomes 0.
ret = snprintf(buf, size, "%s%s", xbc_node_get_data(node),
depth ? "." : "");
if (ret < 0)
return ret;
if (ret > size) {
size = 0;
} else {
size -= ret; // if ret == size, the size becomes 0
buf += ret;
}
Anyway, to be clear the correct error case handling, this
should be applied.
Thank you!
> Signed-off-by: Josh Law <objecting@objecting.org>
> ---
> lib/bootconfig.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/bootconfig.c b/lib/bootconfig.c
> index 62b4ed7a0ba6..b0ef1e74e98a 100644
> --- a/lib/bootconfig.c
> +++ b/lib/bootconfig.c
> @@ -316,7 +316,7 @@ int __init xbc_node_compose_key_after(struct xbc_node *root,
> depth ? "." : "");
> if (ret < 0)
> return ret;
> - if (ret > size) {
> + if (ret >= size) {
> size = 0;
> } else {
> size -= ret;
> --
> 2.34.1
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH v2 2/3] lib/bootconfig: check bounds before writing in __xbc_open_brace()
From: Masami Hiramatsu @ 2026-03-13 2:10 UTC (permalink / raw)
To: Josh Law; +Cc: Andrew Morton, Josh Law, linux-kernel, linux-trace-kernel
In-Reply-To: <20260312191143.28719-3-objecting@objecting.org>
On Thu, 12 Mar 2026 19:11:42 +0000
Josh Law <hlcj1234567@gmail.com> wrote:
> From: Josh Law <objecting@objecting.org>
>
> The bounds check for brace_index happens after the array write.
> While the current call pattern prevents an actual out-of-bounds
> access (the previous call would have returned an error), the
> write-before-check pattern is fragile and would become a real
> out-of-bounds write if the error return were ever not propagated.
>
> Move the bounds check before the array write so the function is
> self-contained and safe regardless of caller behavior.
>
Hmm good catch. This is a kind of specification mistake.
If we pass below to the bootconfig tool, currently it fails.
----
key1 {
key2 {
key3 {
key4 {
key5 {
key6 {
key7 {
key8 {
key9 {
key10 {
key11 {
key12 {
key13 {
key14 {
key15 {
key16 {
}}}}}}}}}}}}}}}}
----
But since "open_brace[]" array has 16 entries, it should
accept the 16th brace.
Let me add a good and a bad example for this case too.
Thanks,
> Signed-off-by: Josh Law <objecting@objecting.org>
> ---
> lib/bootconfig.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/bootconfig.c b/lib/bootconfig.c
> index a1e6a2e14b01..62b4ed7a0ba6 100644
> --- a/lib/bootconfig.c
> +++ b/lib/bootconfig.c
> @@ -532,9 +532,9 @@ static char *skip_spaces_until_newline(char *p)
> static int __init __xbc_open_brace(char *p)
> {
> /* Push the last key as open brace */
> - open_brace[brace_index++] = xbc_node_index(last_parent);
> if (brace_index >= XBC_DEPTH_MAX)
> return xbc_parse_error("Exceed max depth of braces", p);
> + open_brace[brace_index++] = xbc_node_index(last_parent);
>
> return 0;
> }
> --
> 2.34.1
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH v2 1/3] lib/bootconfig: fix off-by-one in xbc_verify_tree() unclosed brace error
From: Masami Hiramatsu @ 2026-03-13 1:39 UTC (permalink / raw)
To: Josh Law
Cc: Steven Rostedt, Masami Hiramatsu, Andrew Morton, Josh Law,
linux-kernel, linux-trace-kernel
In-Reply-To: <48aee348-5859-4d13-aa88-d0c21a4c0289@gmail.com>
On Thu, 12 Mar 2026 21:03:52 +0000
Josh Law <hlcj1234567@gmail.com> wrote:
> 12 Mar 2026 21:02:51 Steven Rostedt <rostedt@goodmis.org>:
>
> > On Thu, 12 Mar 2026 19:11:41 +0000
> > Josh Law <hlcj1234567@gmail.com> wrote:
> >
> >> From: Josh Law <objecting@objecting.org>
> >>
> >> __xbc_open_brace() pushes entries with post-increment
> >> (open_brace[brace_index++]), so brace_index always points one past
> >> the last valid entry. xbc_verify_tree() reads open_brace[brace_index]
> >> to report which brace is unclosed, but this is one past the last
> >> pushed entry and contains stale/zero data, causing the error message
> >> to reference the wrong node.
> >>
> >> Use open_brace[brace_index - 1] to correctly identify the unclosed
> >> brace. brace_index is known to be > 0 here since we are inside the
> >> if (brace_index) guard.
> >>
> >> Signed-off-by: Josh Law <objecting@objecting.org>
> >
> > Nice catch. May I ask how you found this.
> >
> > Reviewed-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Thanks for patch and review!
> >
> > -- Steve
> >
> >> ---
> >> lib/bootconfig.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/lib/bootconfig.c b/lib/bootconfig.c
> >> index 2bcd5c2aa87e..a1e6a2e14b01 100644
> >> --- a/lib/bootconfig.c
> >> +++ b/lib/bootconfig.c
> >> @@ -802,7 +802,7 @@ static int __init xbc_verify_tree(void)
> >>
> >> /* Brace closing */
> >> if (brace_index) {
> >> - n = &xbc_nodes[open_brace[brace_index]];
> >> + n = &xbc_nodes[open_brace[brace_index - 1]];
> >> return xbc_parse_error("Brace is not closed",
> >> xbc_node_get_data(n));
> >> }
>
> Hi Steve,
> Thanks for the review!
> I found this while doing a manual audit of the bootconfig parser's error handling. I noticed that the post-increment in __xbc_open_brace() didn't seem to align with how xbc_verify_tree() was accessing the index. I verified it by intentionally passing a
malformed config with an unclosed brace and saw it reporting a 'stale' or incorrect node location
Thanks, I confirmed it with below config.
$ cat samples/bad-non-closed-brace.bconf
foo {
bar {
buz
}
This closed the 2nd `{`, but not close the first one.
Without patch;
$ ./bootconfig samples/bad-non-closed-brace.bconf
Parse Error: Brace is not closed at 2:2
With this fix;
$ ./bootconfig samples/bad-non-closed-brace.bconf
Parse Error: Brace is not closed at 1:1
Than you!
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH 07/12] HID: spi_hid: add ACPI support for SPI over HID
From: Jingyuan Liang @ 2026-03-13 1:24 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-input, linux-doc,
linux-kernel, linux-spi, linux-trace-kernel, devicetree, hbarnor,
Angela Czubak
In-Reply-To: <abD6RJZa5D7LN3x0@google.com>
On Tue, Mar 10, 2026 at 10:27 PM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> On Tue, Mar 03, 2026 at 06:12:59AM +0000, Jingyuan Liang wrote:
> > From: Angela Czubak <acz@semihalf.com>
> >
> > Detect SPI HID devices described in ACPI.
> >
> > Signed-off-by: Angela Czubak <acz@semihalf.com>
> > Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
> > ---
> > drivers/hid/spi-hid/Kconfig | 15 +++
> > drivers/hid/spi-hid/Makefile | 1 +
> > drivers/hid/spi-hid/spi-hid-acpi.c | 253 +++++++++++++++++++++++++++++++++++++
> > drivers/hid/spi-hid/spi-hid-core.c | 27 +---
> > drivers/hid/spi-hid/spi-hid.h | 44 +++++++
> > 5 files changed, 316 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/hid/spi-hid/Kconfig b/drivers/hid/spi-hid/Kconfig
> > index 836fdefe8345..114b1e00da39 100644
> > --- a/drivers/hid/spi-hid/Kconfig
> > +++ b/drivers/hid/spi-hid/Kconfig
> > @@ -10,6 +10,21 @@ menuconfig SPI_HID
> >
> > if SPI_HID
> >
> > +config SPI_HID_ACPI
> > + tristate "HID over SPI transport layer ACPI driver"
> > + depends on ACPI
> > + select SPI_HID_CORE
> > + help
> > + Say Y here if you use a keyboard, a touchpad, a touchscreen, or any
> > + other HID based devices which are connected to your computer via SPI.
> > + This driver supports ACPI-based systems.
> > +
> > + If unsure, say N.
> > +
> > + This support is also available as a module. If so, the module
> > + will be called spi-hid-acpi. It will also build/depend on the
> > + module spi-hid.
> > +
> > config SPI_HID_CORE
> > tristate
> > endif
> > diff --git a/drivers/hid/spi-hid/Makefile b/drivers/hid/spi-hid/Makefile
> > index 92e24cddbfc2..753c7b7a7844 100644
> > --- a/drivers/hid/spi-hid/Makefile
> > +++ b/drivers/hid/spi-hid/Makefile
> > @@ -7,3 +7,4 @@
> >
> > obj-$(CONFIG_SPI_HID_CORE) += spi-hid.o
> > spi-hid-objs = spi-hid-core.o
> > +obj-$(CONFIG_SPI_HID_ACPI) += spi-hid-acpi.o
> > diff --git a/drivers/hid/spi-hid/spi-hid-acpi.c b/drivers/hid/spi-hid/spi-hid-acpi.c
> > new file mode 100644
> > index 000000000000..612e74fe72f9
> > --- /dev/null
> > +++ b/drivers/hid/spi-hid/spi-hid-acpi.c
> > @@ -0,0 +1,253 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * HID over SPI protocol, ACPI related code
> > + *
> > + * Copyright (c) 2021 Microsoft Corporation
> > + * Copyright (c) 2026 Google LLC
> > + *
> > + * This code was forked out of the HID over SPI core code, which is partially
> > + * based on "HID over I2C protocol implementation:
> > + *
> > + * Copyright (c) 2012 Benjamin Tissoires <benjamin.tissoires@gmail.com>
> > + * Copyright (c) 2012 Ecole Nationale de l'Aviation Civile, France
> > + * Copyright (c) 2012 Red Hat, Inc
> > + *
> > + * which in turn is partially based on "USB HID support for Linux":
> > + *
> > + * Copyright (c) 1999 Andreas Gal
> > + * Copyright (c) 2000-2005 Vojtech Pavlik <vojtech@suse.cz>
> > + * Copyright (c) 2005 Michael Haboustak <mike-@cinci.rr.com> for Concept2, Inc
> > + * Copyright (c) 2007-2008 Oliver Neukum
> > + * Copyright (c) 2006-2010 Jiri Kosina
> > + */
> > +
> > +#include <linux/acpi.h>
> > +#include <linux/delay.h>
> > +#include <linux/device.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/reset.h>
> > +#include <linux/uuid.h>
> > +
> > +#include "spi-hid.h"
> > +
> > +/* Config structure is filled with data from ACPI */
> > +struct spi_hid_acpi_config {
> > + struct spihid_ops ops;
> > +
> > + struct spi_hid_conf property_conf;
> > + u32 post_power_on_delay_ms;
> > + u32 minimal_reset_delay_ms;
> > + struct acpi_device *adev;
> > +};
> > +
> > +/* HID SPI Device: 6e2ac436-0fcf41af-a265-b32a220dcfab */
> > +static guid_t spi_hid_guid =
> > + GUID_INIT(0x6E2AC436, 0x0FCF, 0x41AF,
> > + 0xA2, 0x65, 0xB3, 0x2A, 0x22, 0x0D, 0xCF, 0xAB);
> > +
> > +static int spi_hid_acpi_populate_config(struct spi_hid_acpi_config *conf,
> > + struct acpi_device *adev)
> > +{
> > + acpi_handle handle = acpi_device_handle(adev);
> > + union acpi_object *obj;
> > +
> > + conf->adev = adev;
> > +
> > + /* Revision 3 for HID over SPI V1, see specification. */
> > + obj = acpi_evaluate_dsm_typed(handle, &spi_hid_guid, 3, 1, NULL,
> > + ACPI_TYPE_INTEGER);
> > + if (!obj) {
> > + acpi_handle_err(handle,
> > + "Error _DSM call to get HID input report header address failed");
> > + return -ENODEV;
> > + }
> > + conf->property_conf.input_report_header_address = obj->integer.value;
> > + ACPI_FREE(obj);
> > +
> > + obj = acpi_evaluate_dsm_typed(handle, &spi_hid_guid, 3, 2, NULL,
> > + ACPI_TYPE_INTEGER);
> > + if (!obj) {
> > + acpi_handle_err(handle,
> > + "Error _DSM call to get HID input report body address failed");
> > + return -ENODEV;
> > + }
> > + conf->property_conf.input_report_body_address = obj->integer.value;
> > + ACPI_FREE(obj);
> > +
> > + obj = acpi_evaluate_dsm_typed(handle, &spi_hid_guid, 3, 3, NULL,
> > + ACPI_TYPE_INTEGER);
> > + if (!obj) {
> > + acpi_handle_err(handle,
> > + "Error _DSM call to get HID output report header address failed");
> > + return -ENODEV;
> > + }
> > + conf->property_conf.output_report_address = obj->integer.value;
> > + ACPI_FREE(obj);
> > +
> > + obj = acpi_evaluate_dsm_typed(handle, &spi_hid_guid, 3, 4, NULL,
> > + ACPI_TYPE_BUFFER);
> > + if (!obj) {
> > + acpi_handle_err(handle,
> > + "Error _DSM call to get HID read opcode failed");
> > + return -ENODEV;
> > + }
> > + if (obj->buffer.length == 1) {
> > + conf->property_conf.read_opcode = obj->buffer.pointer[0];
> > + } else {
> > + acpi_handle_err(handle,
> > + "Error _DSM call to get HID read opcode, too long buffer");
> > + ACPI_FREE(obj);
> > + return -ENODEV;
> > + }
> > + ACPI_FREE(obj);
> > +
> > + obj = acpi_evaluate_dsm_typed(handle, &spi_hid_guid, 3, 5, NULL,
> > + ACPI_TYPE_BUFFER);
> > + if (!obj) {
> > + acpi_handle_err(handle,
> > + "Error _DSM call to get HID write opcode failed");
> > + return -ENODEV;
> > + }
> > + if (obj->buffer.length == 1) {
> > + conf->property_conf.write_opcode = obj->buffer.pointer[0];
> > + } else {
> > + acpi_handle_err(handle,
> > + "Error _DSM call to get HID write opcode, too long buffer");
> > + ACPI_FREE(obj);
> > + return -ENODEV;
> > + }
> > + ACPI_FREE(obj);
> > +
> > + /* Value not provided in ACPI,*/
> > + conf->post_power_on_delay_ms = 5;
> > + conf->minimal_reset_delay_ms = 150;
> > +
> > + if (!acpi_has_method(handle, "_RST")) {
> > + acpi_handle_err(handle, "No reset method for acpi handle");
> > + return -ENODEV;
>
> I would return -EINVAL as we have the device with right _DSM but without
> mandated by the spec _RST.
Thanks! Will fix this in v2.
>
> > + }
> > +
> > + /* FIXME: not reading hid-over-spi-flags, multi-SPI not supported */
> > +
> > + return 0;
> > +}
> > +
> > +static int spi_hid_acpi_power_none(struct spihid_ops *ops)
> > +{
> > + return 0;
> > +}
> > +
> > +static int spi_hid_acpi_power_down(struct spihid_ops *ops)
> > +{
> > + struct spi_hid_acpi_config *conf = container_of(ops,
> > + struct spi_hid_acpi_config,
> > + ops);
> > +
> > + return acpi_device_set_power(conf->adev, ACPI_STATE_D3);
> > +}
> > +
> > +static int spi_hid_acpi_power_up(struct spihid_ops *ops)
> > +{
> > + struct spi_hid_acpi_config *conf = container_of(ops,
> > + struct spi_hid_acpi_config,
> > + ops);
> > + int error;
> > +
> > + error = acpi_device_set_power(conf->adev, ACPI_STATE_D0);
> > + if (error) {
> > + dev_err(&conf->adev->dev, "Error could not power up ACPI device: %d.", error);
> > + return error;
> > + }
> > +
> > + if (conf->post_power_on_delay_ms)
> > + msleep(conf->post_power_on_delay_ms);
> > +
> > + return 0;
> > +}
> > +
> > +static int spi_hid_acpi_assert_reset(struct spihid_ops *ops)
> > +{
> > + return 0;
> > +}
> > +
> > +static int spi_hid_acpi_deassert_reset(struct spihid_ops *ops)
> > +{
> > + struct spi_hid_acpi_config *conf = container_of(ops,
> > + struct spi_hid_acpi_config,
> > + ops);
> > +
> > + return device_reset(&conf->adev->dev);
> > +}
> > +
> > +static void spi_hid_acpi_sleep_minimal_reset_delay(struct spihid_ops *ops)
> > +{
> > + struct spi_hid_acpi_config *conf = container_of(ops,
> > + struct spi_hid_acpi_config,
> > + ops);
> > + usleep_range(1000 * conf->minimal_reset_delay_ms,
> > + 1000 * (conf->minimal_reset_delay_ms + 1));
>
> I'd probably use "fsleep(conf->minimal_reset_delay_ms * 1000)".
I will fix this in v2. And do the same for the of driver.
>
> > +}
> > +
> > +static int spi_hid_acpi_probe(struct spi_device *spi)
> > +{
> > + struct device *dev = &spi->dev;
> > + struct acpi_device *adev;
> > + struct spi_hid_acpi_config *config;
> > + int error;
> > +
> > + adev = ACPI_COMPANION(dev);
> > + if (!adev) {
> > + dev_err(dev, "Error could not get ACPI device.");
> > + return -ENODEV;
> > + }
> > +
> > + config = devm_kzalloc(dev, sizeof(struct spi_hid_acpi_config),
> > + GFP_KERNEL);
> > + if (!config)
> > + return -ENOMEM;
> > +
> > + if (acpi_device_power_manageable(adev)) {
> > + config->ops.power_up = spi_hid_acpi_power_up;
> > + config->ops.power_down = spi_hid_acpi_power_down;
> > + } else {
> > + config->ops.power_up = spi_hid_acpi_power_none;
> > + config->ops.power_down = spi_hid_acpi_power_none;
> > + }
> > + config->ops.assert_reset = spi_hid_acpi_assert_reset;
> > + config->ops.deassert_reset = spi_hid_acpi_deassert_reset;
> > + config->ops.sleep_minimal_reset_delay =
> > + spi_hid_acpi_sleep_minimal_reset_delay;
> > +
> > + error = spi_hid_acpi_populate_config(config, adev);
> > + if (error) {
> > + dev_err(dev, "%s: unable to populate config data.", __func__);
> > + return error;
> > + }
>
> I would add a blank line.
Sure! Will fix this in v2.
>
> > + return spi_hid_core_probe(spi, &config->ops, &config->property_conf);
> > +}
> > +
> > +static const struct acpi_device_id spi_hid_acpi_match[] = {
> > + { "ACPI0C51", 0 },
> > + { "PNP0C51", 0 },
> > + { },
>
> No comma on sentinels.
Will fix this in v2.
>
> > +};
> > +MODULE_DEVICE_TABLE(acpi, spi_hid_acpi_match);
> > +
> > +static struct spi_driver spi_hid_acpi_driver = {
> > + .driver = {
> > + .name = "spi_hid_acpi",
> > + .owner = THIS_MODULE,
> > + .acpi_match_table = ACPI_PTR(spi_hid_acpi_match),
>
> This is dependent on ACPI, so no need to sue ACPI_PTR().
Will fix this in v2 and remove of_match_ptr in the of driver as well.
>
> > + .probe_type = PROBE_PREFER_ASYNCHRONOUS,
> > + .dev_groups = spi_hid_groups,
> > + },
> > + .probe = spi_hid_acpi_probe,
> > + .remove = spi_hid_core_remove,
> > +};
> > +
> > +module_spi_driver(spi_hid_acpi_driver);
> > +
> > +MODULE_DESCRIPTION("HID over SPI ACPI transport driver");
> > +MODULE_AUTHOR("Angela Czubak <aczubak@google.com>");
> > +MODULE_LICENSE("GPL");
> > diff --git a/drivers/hid/spi-hid/spi-hid-core.c b/drivers/hid/spi-hid/spi-hid-core.c
> > index e3273846267e..02beb209a92d 100644
> > --- a/drivers/hid/spi-hid/spi-hid-core.c
> > +++ b/drivers/hid/spi-hid/spi-hid-core.c
> > @@ -43,6 +43,9 @@
> > #include <linux/wait.h>
> > #include <linux/workqueue.h>
> >
> > +#include "spi-hid.h"
> > +#include "spi-hid-core.h"
> > +
> > /* Protocol constants */
> > #define SPI_HID_READ_APPROVAL_CONSTANT 0xff
> > #define SPI_HID_INPUT_HEADER_SYNC_BYTE 0x5a
> > @@ -105,30 +108,6 @@ struct spi_hid_output_report {
> > u8 *content;
> > };
> >
> > -/* struct spi_hid_conf - Conf provided to the core */
> > -struct spi_hid_conf {
> > - u32 input_report_header_address;
> > - u32 input_report_body_address;
> > - u32 output_report_address;
> > - u8 read_opcode;
> > - u8 write_opcode;
> > -};
> > -
> > -/**
> > - * struct spihid_ops - Ops provided to the core
> > - * @power_up: do sequencing to power up the device
> > - * @power_down: do sequencing to power down the device
> > - * @assert_reset: do sequencing to assert the reset line
> > - * @deassert_reset: do sequencing to deassert the reset line
> > - */
> > -struct spihid_ops {
> > - int (*power_up)(struct spihid_ops *ops);
> > - int (*power_down)(struct spihid_ops *ops);
> > - int (*assert_reset)(struct spihid_ops *ops);
> > - int (*deassert_reset)(struct spihid_ops *ops);
> > - void (*sleep_minimal_reset_delay)(struct spihid_ops *ops);
> > -};
> > -
> > static struct hid_ll_driver spi_hid_ll_driver;
> >
> > static void spi_hid_populate_read_approvals(const struct spi_hid_conf *conf,
> > diff --git a/drivers/hid/spi-hid/spi-hid.h b/drivers/hid/spi-hid/spi-hid.h
> > new file mode 100644
> > index 000000000000..1fdd45262647
> > --- /dev/null
> > +++ b/drivers/hid/spi-hid/spi-hid.h
> > @@ -0,0 +1,44 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Copyright (c) 2021 Microsoft Corporation
> > + * Copyright (c) 2026 Google LLC
> > + */
> > +
> > +#ifndef SPI_HID_H
> > +#define SPI_HID_H
> > +
> > +#include <linux/spi/spi.h>
> > +#include <linux/sysfs.h>
> > +
> > +/* struct spi_hid_conf - Conf provided to the core */
> > +struct spi_hid_conf {
> > + u32 input_report_header_address;
> > + u32 input_report_body_address;
> > + u32 output_report_address;
> > + u8 read_opcode;
> > + u8 write_opcode;
> > +};
> > +
> > +/**
> > + * struct spihid_ops - Ops provided to the core
> > + * @power_up: do sequencing to power up the device
> > + * @power_down: do sequencing to power down the device
> > + * @assert_reset: do sequencing to assert the reset line
> > + * @deassert_reset: do sequencing to deassert the reset line
> > + */
> > +struct spihid_ops {
> > + int (*power_up)(struct spihid_ops *ops);
> > + int (*power_down)(struct spihid_ops *ops);
> > + int (*assert_reset)(struct spihid_ops *ops);
> > + int (*deassert_reset)(struct spihid_ops *ops);
> > + void (*sleep_minimal_reset_delay)(struct spihid_ops *ops);
> > +};
> > +
> > +int spi_hid_core_probe(struct spi_device *spi, struct spihid_ops *ops,
> > + struct spi_hid_conf *conf);
> > +
> > +void spi_hid_core_remove(struct spi_device *spi);
> > +
> > +extern const struct attribute_group *spi_hid_groups[];
> > +
> > +#endif /* SPI_HID_H */
>
> I am not sure if this belongs to this patch or if it should be better in
> the patch introducing the main driver from the beginning.
These definitions were in spi-hid-core.c in the previous patch introducing
the main driver because it was only used in one .c file. This patch introduces
spi-hid-acpi.c and now two .c files need it so I created a separate .h
file here.
>
> For the ACPI part:
>
> Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>
> Thanks.
>
> --
> Dmitry
^ permalink raw reply
* Re: [PATCH 09/12] dt-bindings: input: Document hid-over-spi DT schema
From: Jingyuan Liang @ 2026-03-13 1:14 UTC (permalink / raw)
To: Rob Herring
Cc: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Krzysztof Kozlowski, Conor Dooley, linux-input,
linux-doc, LKML, linux-spi, linux-trace-kernel, devicetree,
Henry Barnor, Dmitry Antipov, Jarrett Schultz
In-Reply-To: <CAEe3GZE_79Lqtzv4ZRi6zWuV_DgkCREBDcZJjpR+X9OaGZCA2g@mail.gmail.com>
On Wed, Mar 11, 2026 at 5:58 PM Jingyuan Liang <jingyliang@chromium.org> wrote:
>
> (Resending to the list. Apologies, I accidentally dropped the CCs on
> my initial reply!)
>
> On Tue, Mar 3, 2026 at 5:53 AM Rob Herring <robh@kernel.org> wrote:
> >
> > On Tue, Mar 3, 2026 at 12:14 AM Jingyuan Liang <jingyliang@chromium.org> wrote:
> > >
> > > Documentation describes the required and optional properties for
> > > implementing Device Tree for a Microsoft G6 Touch Digitizer that
> > > supports HID over SPI Protocol 1.0 specification.
> > >
> > > The properties are common to HID over SPI.
> > >
> > > Signed-off-by: Dmitry Antipov <dmanti@microsoft.com>
> > > Signed-off-by: Jarrett Schultz <jaschultz@microsoft.com>
> > > Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
> > > ---
> > > .../devicetree/bindings/input/hid-over-spi.yaml | 153 +++++++++++++++++++++
> > > 1 file changed, 153 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/input/hid-over-spi.yaml b/Documentation/devicetree/bindings/input/hid-over-spi.yaml
> > > new file mode 100644
> > > index 000000000000..b623629ed9d3
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/input/hid-over-spi.yaml
> > > @@ -0,0 +1,153 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/input/hid-over-spi.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: HID over SPI Devices
> > > +
> > > +maintainers:
> > > + - Benjamin Tissoires <benjamin.tissoires@redhat.com>
> > > + - Jiri Kosina <jkosina@suse.cz>
> > > +
> > > +description: |+
> > > + HID over SPI provides support for various Human Interface Devices over the
> > > + SPI bus. These devices can be for example touchpads, keyboards, touch screens
> > > + or sensors.
> > > +
> > > + The specification has been written by Microsoft and is currently available here:
> > > + https://www.microsoft.com/en-us/download/details.aspx?id=103325
> > > +
> > > + If this binding is used, the kernel module spi-hid will handle the communication
> > > + with the device and the generic hid core layer will handle the protocol.
> > > +
> > > +allOf:
> > > + - $ref: /schemas/input/touchscreen/touchscreen.yaml#
> > > +
> > > +properties:
> > > + compatible:
> > > + oneOf:
> > > + - items:
> > > + - enum:
> > > + - microsoft,g6-touch-digitizer
> > > + - const: hid-over-spi
> > > + - description: Just "hid-over-spi" alone is allowed, but not recommended.
> > > + const: hid-over-spi
> > > +
> > > + reg:
> > > + maxItems: 1
> > > +
> > > + interrupts:
> > > + maxItems: 1
> > > +
> > > + reset-gpios:
> > > + maxItems: 1
> > > + description:
> > > + GPIO specifier for the digitizer's reset pin (active low). The line must
> > > + be flagged with GPIO_ACTIVE_LOW.
> > > +
> > > + vdd-supply:
> > > + description:
> > > + Regulator for the VDD supply voltage.
> >
> > Is this part of the spec? This won't scale for multiple devices with
> > different power rails.
>
> This is not part of the spec but is needed for power management. Is it okay I
> mark it as optional? Thank you.
>
> >
> > > +
> > > + input-report-header-address:
> > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > + minimum: 0
> > > + maximum: 0xffffff
> > > + description:
> > > + A value to be included in the Read Approval packet, listing an address of
> > > + the input report header to be put on the SPI bus. This address has 24
> > > + bits.
> > > +
> > > + input-report-body-address:
> > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > + minimum: 0
> > > + maximum: 0xffffff
> > > + description:
> > > + A value to be included in the Read Approval packet, listing an address of
> > > + the input report body to be put on the SPI bus. This address has 24 bits.
> > > +
> > > + output-report-address:
> > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > + minimum: 0
> > > + maximum: 0xffffff
> > > + description:
> > > + A value to be included in the Output Report sent by the host, listing an
> > > + address where the output report on the SPI bus is to be written to. This
> > > + address has 24 bits.
> > > +
> > > + post-power-on-delay-ms:
> > > + description:
> > > + Optional time in ms required by the device after enabling its regulators
> > > + or powering it on, before it is ready for communication.
> >
> > Drop. This should be implied by the compatible.
>
> Thank you, I will fix this in v2.
>
> >
> > > +
> > > + minimal-reset-delay-ms:
> > > + description:
> > > + Optional minimum amount of time in ms that device needs to be in reset
> > > + state for the reset to take effect.
> >
> > Drop. This should be implied by the compatible.
>
> I will fix this in v2.
>
> >
> > > +
> > > + read-opcode:
> > > + $ref: /schemas/types.yaml#/definitions/uint8
> > > + description:
> > > + Value to be used in Read Approval packets. 1 byte.
> > > +
> > > + write-opcode:
> > > + $ref: /schemas/types.yaml#/definitions/uint8
> > > + description:
> > > + Value to be used in Write Approval packets. 1 byte.
> >
> > Why are these and the address properties above not defined by the
> > spec? Do they vary for a specific device? If not, then they should be
> > implied by the compatible.
>
> These properties are not defined by the spec:
>
> "The Input Report Address (header or body) and READ opcode are retrieved
> from ACPI."
>
> Same for the output report address and write opcode. I will drop these in v2.
Hi Rob,
Please disregard my previous reply about dropping the read/write opcodes and
input/output addresses. The spec does not define these fields. Instead, it says
these values are retrieved from ACPI _DSM methods. If we remove them from
the binding, the driver is not very generic. Would it be okay to keep them?
>
> >
> > > +
> > > + hid-over-spi-flags:
> > > + $ref: /schemas/types.yaml#/definitions/uint16
> > > + description:
> > > + 16 bits.
> > > + Bits 0-12 - Reserved (must be 0)
> > > + Bit 13 - SPI Write Mode. Possible values -
> > > + * 0b0- Writes are carried out in Single-SPI mode
> > > + * 0b1- Writes are carried out in the Multi-SPI mode specified by bits
> > > + 14-15
> > > + Bits 14-15 - Multi-SPI Mode. Possible values -
> > > + * 0b00- Single SPI
> > > + * 0b01- Dual SPI
> > > + * 0b10- Quad SPI
> >
> > We already have SPI properties to define the bus width for read and write.
>
> Will fix this in v2.
>
> >
> > > +
> > > +required:
> > > + - compatible
> > > + - interrupts
> > > + - reset-gpios
> > > + - vdd-supply
> > > + - input-report-header-address
> > > + - input-report-body-address
> > > + - output-report-address
> > > + - read-opcode
> > > + - write-opcode
> > > + - hid-over-spi-flags
> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > + - |
> > > + #include <dt-bindings/interrupt-controller/irq.h>
> > > + #include <dt-bindings/gpio/gpio.h>
> > > +
> > > + spi {
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > +
> > > + hid@0 {
> > > + compatible = "hid-over-spi";
> > > + reg = <0x0>;
> > > + interrupts-extended = <&gpio 42 IRQ_TYPE_EDGE_FALLING>;
> > > + reset-gpios = <&gpio 27 GPIO_ACTIVE_LOW>;
> > > + vdd-supply = <&pm8350c_l3>;
> > > + pinctrl-names = "default";
> > > + pinctrl-0 = <&ts_d6_reset_assert &ts_d6_int_bias>;
> > > + input-report-header-address = <0x1000>;
> > > + input-report-body-address = <0x1004>;
> > > + output-report-address = <0x2000>;
> > > + read-opcode = <0x0b>;
> > > + write-opcode = <0x02>;
> > > + hid-over-spi-flags = <0x0000>;
> > > + post-power-on-delay-ms = <5>;
> > > + minimal-reset-delay-ms = <5>;
> > > + };
> > > + };
> > > \ No newline at end of file
> >
> > Fix this.
>
> Will fix this in v2.
>
> >
> > Rob
^ permalink raw reply
* Re: [PATCH 09/12] dt-bindings: input: Document hid-over-spi DT schema
From: Jingyuan Liang @ 2026-03-13 1:00 UTC (permalink / raw)
To: Val Packett
Cc: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-input, linux-doc, linux-kernel, linux-spi,
linux-trace-kernel, devicetree, hbarnor, Dmitry Antipov,
Jarrett Schultz
In-Reply-To: <1cc6de61-8b56-492e-ab78-e3aa448f58ad@packett.cool>
On Fri, Mar 6, 2026 at 11:25 PM Val Packett <val@packett.cool> wrote:
>
>
> On 3/3/26 3:13 AM, Jingyuan Liang wrote:
> > Documentation describes the required and optional properties for
> > implementing Device Tree for a Microsoft G6 Touch Digitizer that
> > supports HID over SPI Protocol 1.0 specification.
> > […]
> > +properties:
> > + compatible:
> > + oneOf:
> > + - items:
> > + - enum:
> > + - microsoft,g6-touch-digitizer
> > + - const: hid-over-spi
> > + - description: Just "hid-over-spi" alone is allowed, but not recommended.
> > […]
> > +required:
> > + - compatible
> > + - interrupts
> > + - reset-gpios
>
> Why is reset required? Is it so implausible on some device implementing
> the spec there wouldn't be a reset gpio?
>
> > + - vdd-supply
> Linux makes up a dummy regulator if DT doesn't provide one, so can
> regulators even be required?
> > […]
> > + compatible = "hid-over-spi";
> Not following your own recommendation from above :)
Thanks! I will fix this in v2.
> > + reg = <0x0>;
> > + interrupts-extended = <&gpio 42 IRQ_TYPE_EDGE_FALLING>;
> > + reset-gpios = <&gpio 27 GPIO_ACTIVE_LOW>;
> > + vdd-supply = <&pm8350c_l3>;
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&ts_d6_reset_assert &ts_d6_int_bias>;
>
> Heh, "reset_assert" is a name implying it would actually set the value
> from the pinctrl properties, which is what had to be done before
> reset-gpios were supported. But now reset-gpios are supported.
Taken from the original patch. Will fix this in v2.
>
>
> Thanks,
> ~val
>
>
> P.S. happy to see work on this happen again!
>
^ permalink raw reply
* [PATCH 19/53] afs: use d_time instead of d_fsdata
From: NeilBrown @ 2026-03-12 21:12 UTC (permalink / raw)
To: Linus Torvalds, Alexander Viro, Christian Brauner, Jan Kara,
Jeff Layton, Trond Myklebust, Anna Schumaker, Carlos Maiolino,
Miklos Szeredi, Amir Goldstein, Jan Harkes, Hugh Dickins,
Baolin Wang, David Howells, Marc Dionne, Steve French,
Namjae Jeon, Sungjong Seo, Yuezhang Mo, Andreas Hindborg,
Breno Leitao, Theodore Ts'o, Andreas Dilger, Steven Rostedt,
Masami Hiramatsu, Ilya Dryomov, Alex Markuze, Viacheslav Dubeyko,
Tyler Hicks, Andreas Gruenbacher, Richard Weinberger,
Anton Ivanov, Johannes Berg, Jeremy Kerr, Ard Biesheuvel
Cc: linux-fsdevel, linux-nfs, linux-xfs, linux-unionfs, coda,
linux-mm, linux-afs, linux-cifs, linux-ext4, linux-kernel,
linux-trace-kernel, ceph-devel, ecryptfs, gfs2, linux-um,
linux-efi
In-Reply-To: <20260312214330.3885211-1-neilb@ownmail.net>
From: NeilBrown <neil@brown.name>
afs uses ->d_fsdata to store version information for the parent
directory. ->d_time is arguably a better field to store this
information as the version is like a time stamp, and ->d_time is an
unsigned long, while ->d_fsdata is a void *.
This will leave ->d_fsdata free for a different use ... which
admittedly is also not a void*, but is certainly not at all a time.
Interesting the value stored in ->d_time or d_fsdata is u64 which is a
different size of 32 bit hosts. Maybe that doesn't matter.
Signed-off-by: NeilBrown <neil@brown.name>
---
fs/afs/dir.c | 18 +++++++++---------
fs/afs/internal.h | 8 ++++----
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/fs/afs/dir.c b/fs/afs/dir.c
index 78caef3f1338..a0417292314c 100644
--- a/fs/afs/dir.c
+++ b/fs/afs/dir.c
@@ -808,7 +808,7 @@ static struct inode *afs_do_lookup(struct inode *dir, struct dentry *dentry)
afs_dir_iterate(dir, &cookie->ctx, NULL, &data_version);
}
- dentry->d_fsdata = (void *)(unsigned long)data_version;
+ dentry->d_time = (unsigned long)data_version;
/* Check to see if we already have an inode for the primary fid. */
inode = ilookup5(dir->i_sb, cookie->fids[1].vnode,
@@ -895,9 +895,9 @@ static struct inode *afs_do_lookup(struct inode *dir, struct dentry *dentry)
}
if (op->file[0].scb.have_status)
- dentry->d_fsdata = (void *)(unsigned long)op->file[0].scb.status.data_version;
+ dentry->d_time = (unsigned long)op->file[0].scb.status.data_version;
else
- dentry->d_fsdata = (void *)(unsigned long)op->file[0].dv_before;
+ dentry->d_time = (unsigned long)op->file[0].dv_before;
ret = afs_put_operation(op);
out:
kfree(cookie);
@@ -1010,7 +1010,7 @@ static struct dentry *afs_lookup(struct inode *dir, struct dentry *dentry,
_debug("splice %p", dentry->d_inode);
d = d_splice_alias(inode, dentry);
if (!IS_ERR_OR_NULL(d)) {
- d->d_fsdata = dentry->d_fsdata;
+ d->d_time = dentry->d_time;
trace_afs_lookup(dvnode, &d->d_name, &fid);
} else {
trace_afs_lookup(dvnode, &dentry->d_name, &fid);
@@ -1040,7 +1040,7 @@ static int afs_d_revalidate_rcu(struct afs_vnode *dvnode, struct dentry *dentry)
* version.
*/
dir_version = (long)READ_ONCE(dvnode->status.data_version);
- de_version = (long)READ_ONCE(dentry->d_fsdata);
+ de_version = (long)READ_ONCE(dentry->d_time);
if (de_version != dir_version) {
dir_version = (long)READ_ONCE(dvnode->invalid_before);
if (de_version - dir_version < 0)
@@ -1100,7 +1100,7 @@ static int afs_d_revalidate(struct inode *parent_dir, const struct qstr *name,
* version.
*/
dir_version = dir->status.data_version;
- de_version = (long)dentry->d_fsdata;
+ de_version = (long)dentry->d_time;
if (de_version == (long)dir_version)
goto out_valid_noupdate;
@@ -1161,7 +1161,7 @@ static int afs_d_revalidate(struct inode *parent_dir, const struct qstr *name,
}
out_valid:
- dentry->d_fsdata = (void *)(unsigned long)dir_version;
+ dentry->d_time = (unsigned long)dir_version;
out_valid_noupdate:
key_put(key);
_leave(" = 1 [valid]");
@@ -1931,7 +1931,7 @@ static void afs_rename_edit_dir(struct afs_operation *op)
spin_unlock(&new_inode->i_lock);
}
- /* Now we can update d_fsdata on the dentries to reflect their
+ /* Now we can update d_time on the dentries to reflect their
* new parent's data_version.
*/
afs_update_dentry_version(op, new_dvp, op->dentry);
@@ -2167,7 +2167,7 @@ static int afs_rename(struct mnt_idmap *idmap, struct inode *old_dir,
}
/* This bit is potentially nasty as there's a potential race with
- * afs_d_revalidate{,_rcu}(). We have to change d_fsdata on the dentry
+ * afs_d_revalidate{,_rcu}(). We have to change d_time_ on the dentry
* to reflect it's new parent's new data_version after the op, but
* d_revalidate may see old_dentry between the op having taken place
* and the version being updated.
diff --git a/fs/afs/internal.h b/fs/afs/internal.h
index 009064b8d661..106a7fe06b56 100644
--- a/fs/afs/internal.h
+++ b/fs/afs/internal.h
@@ -1746,17 +1746,17 @@ static inline struct inode *AFS_VNODE_TO_I(struct afs_vnode *vnode)
}
/*
- * Note that a dentry got changed. We need to set d_fsdata to the data version
+ * Note that a dentry got changed. We need to set d_time to the data version
* number derived from the result of the operation. It doesn't matter if
- * d_fsdata goes backwards as we'll just revalidate.
+ * d_time goes backwards as we'll just revalidate.
*/
static inline void afs_update_dentry_version(struct afs_operation *op,
struct afs_vnode_param *dir_vp,
struct dentry *dentry)
{
if (!op->cumul_error.error)
- dentry->d_fsdata =
- (void *)(unsigned long)dir_vp->scb.status.data_version;
+ dentry->d_time =
+ (unsigned long)dir_vp->scb.status.data_version;
}
/*
--
2.50.0.107.gf914562f5916.dirty
^ permalink raw reply related
* [PATCH 20/53] afs: don't unhash/rehash dentries during unlink/rename
From: NeilBrown @ 2026-03-12 21:12 UTC (permalink / raw)
To: Linus Torvalds, Alexander Viro, Christian Brauner, Jan Kara,
Jeff Layton, Trond Myklebust, Anna Schumaker, Carlos Maiolino,
Miklos Szeredi, Amir Goldstein, Jan Harkes, Hugh Dickins,
Baolin Wang, David Howells, Marc Dionne, Steve French,
Namjae Jeon, Sungjong Seo, Yuezhang Mo, Andreas Hindborg,
Breno Leitao, Theodore Ts'o, Andreas Dilger, Steven Rostedt,
Masami Hiramatsu, Ilya Dryomov, Alex Markuze, Viacheslav Dubeyko,
Tyler Hicks, Andreas Gruenbacher, Richard Weinberger,
Anton Ivanov, Johannes Berg, Jeremy Kerr, Ard Biesheuvel
Cc: linux-fsdevel, linux-nfs, linux-xfs, linux-unionfs, coda,
linux-mm, linux-afs, linux-cifs, linux-ext4, linux-kernel,
linux-trace-kernel, ceph-devel, ecryptfs, gfs2, linux-um,
linux-efi
In-Reply-To: <20260312214330.3885211-1-neilb@ownmail.net>
From: NeilBrown <neil@brown.name>
afs needs to block lookup of dentries during unlink and rename.
There are two reasons:
1/ If the target is to be removed, not silly-renamed, the subsequent
opens cannot be allowed as the file won't exist on the server.
2/ If the rename source is being moved between directories a lookup,
particularly d_revalidate, might change ->d_time asynchronously
with rename changing ->d_time with possible incorrect results.
afs current unhashes the dentry to force a lookup which will wait on the
directory lock, and rehashes afterwards. This is incompatible with
proposed changed to directory locking which will require a dentry to
remain hashed throughout rename/unlink/etc operations.
This patch copies a mechanism developed for NFS. ->d_fsdata which is
currently unused is now set to a non-NULL value when lookups must be
blocked. d_revalidate checks for this value, and waits for it to become
NULL.
->d_lock is used to ensure d_revalidate never updates ->d_time while
->d_fsdata is set.
Signed-off-by: NeilBrown <neil@brown.name>
---
fs/afs/afs.h | 7 ++++++
fs/afs/dir.c | 64 +++++++++++++++++++++++++++++------------------
fs/afs/internal.h | 5 +---
3 files changed, 47 insertions(+), 29 deletions(-)
diff --git a/fs/afs/afs.h b/fs/afs/afs.h
index ec3db00bd081..019e77b08458 100644
--- a/fs/afs/afs.h
+++ b/fs/afs/afs.h
@@ -26,6 +26,13 @@ typedef u64 afs_volid_t;
typedef u64 afs_vnodeid_t;
typedef u64 afs_dataversion_t;
+/* This is stored in ->d_fsdata to stop d_revalidate looking at,
+ * and possibly changing, ->d_time on a dentry which is being moved
+ * between directories, and to block lookup for dentry that is
+ * being removed without silly-rename.
+ */
+#define AFS_FSDATA_BLOCKED ((void*)1)
+
typedef enum {
AFSVL_RWVOL, /* read/write volume */
AFSVL_ROVOL, /* read-only volume */
diff --git a/fs/afs/dir.c b/fs/afs/dir.c
index a0417292314c..9c57614feccf 100644
--- a/fs/afs/dir.c
+++ b/fs/afs/dir.c
@@ -1034,6 +1034,10 @@ static int afs_d_revalidate_rcu(struct afs_vnode *dvnode, struct dentry *dentry)
if (!afs_check_validity(dvnode))
return -ECHILD;
+ /* A rename/unlink is pending */
+ if (dentry->d_fsdata)
+ return -ECHILD;
+
/* We only need to invalidate a dentry if the server's copy changed
* behind our back. If we made the change, it's no problem. Note that
* on a 32-bit system, we only have 32 bits in the dentry to store the
@@ -1069,6 +1073,10 @@ static int afs_d_revalidate(struct inode *parent_dir, const struct qstr *name,
if (flags & LOOKUP_RCU)
return afs_d_revalidate_rcu(dir, dentry);
+ /* Wait for rename/unlink to complete */
+wait_for_rename:
+ wait_var_event(&dentry->d_fsdata, dentry->d_fsdata == NULL);
+
if (d_really_is_positive(dentry)) {
vnode = AFS_FS_I(d_inode(dentry));
_enter("{v={%llx:%llu} n=%pd fl=%lx},",
@@ -1161,7 +1169,13 @@ static int afs_d_revalidate(struct inode *parent_dir, const struct qstr *name,
}
out_valid:
+ spin_lock(&dentry->d_lock);
+ if (dentry->d_fsdata) {
+ spin_unlock(&dentry->d_lock);
+ goto wait_for_rename;
+ }
dentry->d_time = (unsigned long)dir_version;
+ spin_unlock(&dentry->d_lock);
out_valid_noupdate:
key_put(key);
_leave(" = 1 [valid]");
@@ -1536,8 +1550,7 @@ static void afs_unlink_edit_dir(struct afs_operation *op)
static void afs_unlink_put(struct afs_operation *op)
{
_enter("op=%08x", op->debug_id);
- if (op->unlink.need_rehash && afs_op_error(op) < 0 && afs_op_error(op) != -ENOENT)
- d_rehash(op->dentry);
+ store_release_wake_up(&op->dentry->d_fsdata, NULL);
}
static const struct afs_operation_ops afs_unlink_operation = {
@@ -1591,11 +1604,7 @@ static int afs_unlink(struct inode *dir, struct dentry *dentry)
afs_op_set_error(op, afs_sillyrename(dvnode, vnode, dentry, op->key));
goto error;
}
- if (!d_unhashed(dentry)) {
- /* Prevent a race with RCU lookup. */
- __d_drop(dentry);
- op->unlink.need_rehash = true;
- }
+ dentry->d_fsdata = AFS_FSDATA_BLOCKED;
spin_unlock(&dentry->d_lock);
op->file[1].vnode = vnode;
@@ -1885,9 +1894,10 @@ static void afs_rename_edit_dir(struct afs_operation *op)
_enter("op=%08x", op->debug_id);
- if (op->rename.rehash) {
- d_rehash(op->rename.rehash);
- op->rename.rehash = NULL;
+ if (op->rename.unblock) {
+ /* Rename has finished, so unlocks lookups to target */
+ store_release_wake_up(&op->rename.unblock->d_fsdata, NULL);
+ op->rename.unblock = NULL;
}
fscache_begin_write_operation(&orig_cres, afs_vnode_cache(orig_dvnode));
@@ -1970,6 +1980,9 @@ static void afs_rename_exchange_edit_dir(struct afs_operation *op)
d_exchange(old_dentry, new_dentry);
up_write(&orig_dvnode->validate_lock);
+ /* dentry has been moved, so d_validate can safely proceed */
+ store_release_wake_up(&old_dentry->d_fsdata, NULL);
+
} else {
down_write(&orig_dvnode->validate_lock);
if (test_bit(AFS_VNODE_DIR_VALID, &orig_dvnode->flags) &&
@@ -2009,11 +2022,10 @@ static void afs_rename_exchange_edit_dir(struct afs_operation *op)
static void afs_rename_put(struct afs_operation *op)
{
_enter("op=%08x", op->debug_id);
- if (op->rename.rehash)
- d_rehash(op->rename.rehash);
+ if (op->rename.unblock)
+ store_release_wake_up(&op->rename.unblock->d_fsdata, NULL);
+ store_release_wake_up(&op->dentry->d_fsdata, NULL);
dput(op->rename.tmp);
- if (afs_op_error(op))
- d_rehash(op->dentry);
}
static const struct afs_operation_ops afs_rename_operation = {
@@ -2121,7 +2133,6 @@ static int afs_rename(struct mnt_idmap *idmap, struct inode *old_dir,
op->ops = &afs_rename_noreplace_operation;
} else if (flags & RENAME_EXCHANGE) {
op->ops = &afs_rename_exchange_operation;
- d_drop(new_dentry);
} else {
/* If we might displace the target, we might need to do silly
* rename.
@@ -2135,14 +2146,12 @@ static int afs_rename(struct mnt_idmap *idmap, struct inode *old_dir,
*/
if (d_is_positive(new_dentry) && !d_is_dir(new_dentry)) {
/* To prevent any new references to the target during
- * the rename, we unhash the dentry in advance.
+ * the rename, we set d_fsdata which afs_d_revalidate will wait for.
+ * d_lock ensures d_count() and ->d_fsdata are consistent.
*/
- if (!d_unhashed(new_dentry)) {
- d_drop(new_dentry);
- op->rename.rehash = new_dentry;
- }
-
+ spin_lock(&new_dentry->d_lock);
if (d_count(new_dentry) > 2) {
+ spin_unlock(&new_dentry->d_lock);
/* copy the target dentry's name */
op->rename.tmp = d_alloc(new_dentry->d_parent,
&new_dentry->d_name);
@@ -2160,8 +2169,12 @@ static int afs_rename(struct mnt_idmap *idmap, struct inode *old_dir,
}
op->dentry_2 = op->rename.tmp;
- op->rename.rehash = NULL;
op->rename.new_negative = true;
+ } else {
+ /* Block any lookups to target until the rename completes */
+ new_dentry->d_fsdata = AFS_FSDATA_BLOCKED;
+ op->rename.unblock = new_dentry;
+ spin_unlock(&new_dentry->d_lock);
}
}
}
@@ -2172,10 +2185,11 @@ static int afs_rename(struct mnt_idmap *idmap, struct inode *old_dir,
* d_revalidate may see old_dentry between the op having taken place
* and the version being updated.
*
- * So drop the old_dentry for now to make other threads go through
- * lookup instead - which we hold a lock against.
+ * So block revalidate on the old_dentry until the rename completes.
*/
- d_drop(old_dentry);
+ spin_lock(&old_dentry->d_lock);
+ old_dentry->d_fsdata = AFS_FSDATA_BLOCKED;
+ spin_unlock(&old_dentry->d_lock);
ret = afs_do_sync_operation(op);
if (ret == -ENOTSUPP)
diff --git a/fs/afs/internal.h b/fs/afs/internal.h
index 106a7fe06b56..f2898ce9c0e6 100644
--- a/fs/afs/internal.h
+++ b/fs/afs/internal.h
@@ -891,10 +891,7 @@ struct afs_operation {
const char *symlink;
} create;
struct {
- bool need_rehash;
- } unlink;
- struct {
- struct dentry *rehash;
+ struct dentry *unblock;
struct dentry *tmp;
unsigned int rename_flags;
bool new_negative;
--
2.50.0.107.gf914562f5916.dirty
^ permalink raw reply related
* [PATCH 21/53] afs: use d_splice_alias() in afs_vnode_new_inode()
From: NeilBrown @ 2026-03-12 21:12 UTC (permalink / raw)
To: Linus Torvalds, Alexander Viro, Christian Brauner, Jan Kara,
Jeff Layton, Trond Myklebust, Anna Schumaker, Carlos Maiolino,
Miklos Szeredi, Amir Goldstein, Jan Harkes, Hugh Dickins,
Baolin Wang, David Howells, Marc Dionne, Steve French,
Namjae Jeon, Sungjong Seo, Yuezhang Mo, Andreas Hindborg,
Breno Leitao, Theodore Ts'o, Andreas Dilger, Steven Rostedt,
Masami Hiramatsu, Ilya Dryomov, Alex Markuze, Viacheslav Dubeyko,
Tyler Hicks, Andreas Gruenbacher, Richard Weinberger,
Anton Ivanov, Johannes Berg, Jeremy Kerr, Ard Biesheuvel
Cc: linux-fsdevel, linux-nfs, linux-xfs, linux-unionfs, coda,
linux-mm, linux-afs, linux-cifs, linux-ext4, linux-kernel,
linux-trace-kernel, ceph-devel, ecryptfs, gfs2, linux-um,
linux-efi
In-Reply-To: <20260312214330.3885211-1-neilb@ownmail.net>
From: NeilBrown <neil@brown.name>
As afs supports the fhandle interfaces there is a theoretical possibility
that the inode created for mkdir could be found by open_by_handle_at()
and given a dentry before d_instantiate() is called. This would result
in two dentries for the one directory inode, which is not permitted.
So this patch changes afs_mkdir() to use d_splice_alias() and to
return the alternate dentry from ->mkdir() if appropriate.
Signed-off-by: NeilBrown <neil@brown.name>
---
fs/afs/dir.c | 14 ++++++++++----
fs/afs/internal.h | 1 +
2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/fs/afs/dir.c b/fs/afs/dir.c
index 9c57614feccf..1e472768e1f1 100644
--- a/fs/afs/dir.c
+++ b/fs/afs/dir.c
@@ -1248,7 +1248,7 @@ void afs_check_for_remote_deletion(struct afs_operation *op)
/*
* Create a new inode for create/mkdir/symlink
*/
-static void afs_vnode_new_inode(struct afs_operation *op)
+static struct dentry *afs_vnode_new_inode(struct afs_operation *op)
{
struct afs_vnode_param *dvp = &op->file[0];
struct afs_vnode_param *vp = &op->file[1];
@@ -1265,7 +1265,7 @@ static void afs_vnode_new_inode(struct afs_operation *op)
* the new directory on the server.
*/
afs_op_accumulate_error(op, PTR_ERR(inode), 0);
- return;
+ return NULL;
}
vnode = AFS_FS_I(inode);
@@ -1276,7 +1276,7 @@ static void afs_vnode_new_inode(struct afs_operation *op)
afs_init_new_symlink(vnode, op);
if (!afs_op_error(op))
afs_cache_permit(vnode, op->key, vnode->cb_break, &vp->scb);
- d_instantiate(op->dentry, inode);
+ return d_splice_alias(inode, op->dentry);
}
static void afs_create_success(struct afs_operation *op)
@@ -1285,7 +1285,7 @@ static void afs_create_success(struct afs_operation *op)
op->ctime = op->file[0].scb.status.mtime_client;
afs_vnode_commit_status(op, &op->file[0]);
afs_update_dentry_version(op, &op->file[0], op->dentry);
- afs_vnode_new_inode(op);
+ op->create.ret = afs_vnode_new_inode(op);
}
static void afs_create_edit_dir(struct afs_operation *op)
@@ -1356,6 +1356,12 @@ static struct dentry *afs_mkdir(struct mnt_idmap *idmap, struct inode *dir,
op->ops = &afs_mkdir_operation;
ret = afs_do_sync_operation(op);
afs_dir_unuse_cookie(dvnode, ret);
+ if (op->create.ret) {
+ /* Alternate dentry */
+ if (ret == 0)
+ return op->create.ret;
+ dput(op->create.ret);
+ }
return ERR_PTR(ret);
}
diff --git a/fs/afs/internal.h b/fs/afs/internal.h
index f2898ce9c0e6..ce94f10a14c0 100644
--- a/fs/afs/internal.h
+++ b/fs/afs/internal.h
@@ -889,6 +889,7 @@ struct afs_operation {
int reason; /* enum afs_edit_dir_reason */
mode_t mode;
const char *symlink;
+ struct dentry *ret;
} create;
struct {
struct dentry *unblock;
--
2.50.0.107.gf914562f5916.dirty
^ permalink raw reply related
* [PATCH 22/53] afs: use d_alloc_nonblock in afs_sillyrename()
From: NeilBrown @ 2026-03-12 21:12 UTC (permalink / raw)
To: Linus Torvalds, Alexander Viro, Christian Brauner, Jan Kara,
Jeff Layton, Trond Myklebust, Anna Schumaker, Carlos Maiolino,
Miklos Szeredi, Amir Goldstein, Jan Harkes, Hugh Dickins,
Baolin Wang, David Howells, Marc Dionne, Steve French,
Namjae Jeon, Sungjong Seo, Yuezhang Mo, Andreas Hindborg,
Breno Leitao, Theodore Ts'o, Andreas Dilger, Steven Rostedt,
Masami Hiramatsu, Ilya Dryomov, Alex Markuze, Viacheslav Dubeyko,
Tyler Hicks, Andreas Gruenbacher, Richard Weinberger,
Anton Ivanov, Johannes Berg, Jeremy Kerr, Ard Biesheuvel
Cc: linux-fsdevel, linux-nfs, linux-xfs, linux-unionfs, coda,
linux-mm, linux-afs, linux-cifs, linux-ext4, linux-kernel,
linux-trace-kernel, ceph-devel, ecryptfs, gfs2, linux-um,
linux-efi
In-Reply-To: <20260312214330.3885211-1-neilb@ownmail.net>
From: NeilBrown <neil@brown.name>
Rather than performing a normal lookup (which will be awkward with future
locking changes) use d_alloc_noblock() to find a dentry for an
unused name, and use an open-coded lookup_slow() to see if it is free on
the server.
Signed-off-by: NeilBrown <neil@brown.name>
---
fs/afs/dir_silly.c | 51 ++++++++++++++++++++++++++++++----------------
1 file changed, 34 insertions(+), 17 deletions(-)
diff --git a/fs/afs/dir_silly.c b/fs/afs/dir_silly.c
index 982bb6ec15f0..699143b21cdd 100644
--- a/fs/afs/dir_silly.c
+++ b/fs/afs/dir_silly.c
@@ -112,7 +112,9 @@ int afs_sillyrename(struct afs_vnode *dvnode, struct afs_vnode *vnode,
struct dentry *dentry, struct key *key)
{
static unsigned int sillycounter;
- struct dentry *sdentry = NULL;
+ struct dentry *sdentry = NULL, *old;
+ struct inode *dir = dentry->d_parent->d_inode;
+ struct qstr qsilly;
unsigned char silly[16];
int ret = -EBUSY;
@@ -122,23 +124,38 @@ int afs_sillyrename(struct afs_vnode *dvnode, struct afs_vnode *vnode,
if (dentry->d_flags & DCACHE_NFSFS_RENAMED)
return -EBUSY;
- sdentry = NULL;
- do {
- dput(sdentry);
- sillycounter++;
-
- /* Create a silly name. Note that the ".__afs" prefix is
- * understood by the salvager and must not be changed.
- */
- scnprintf(silly, sizeof(silly), ".__afs%04X", sillycounter);
- sdentry = lookup_noperm(&QSTR(silly), dentry->d_parent);
+newname:
+ sillycounter++;
- /* N.B. Better to return EBUSY here ... it could be dangerous
- * to delete the file while it's in use.
- */
- if (IS_ERR(sdentry))
- goto out;
- } while (!d_is_negative(sdentry));
+ /* Create a silly name. Note that the ".__afs" prefix is
+ * understood by the salvager and must not be changed.
+ */
+ scnprintf(silly, sizeof(silly), ".__afs%04X", sillycounter);
+ qsilly = QSTR(silly);
+ sdentry = try_lookup_noperm(&qsilly, dentry->d_parent);
+ if (!sdentry)
+ sdentry = d_alloc_noblock(dentry->d_parent, &qsilly);
+ if (sdentry == ERR_PTR(-EWOULDBLOCK))
+ /* try another name */
+ goto newname;
+ /* N.B. Better to return EBUSY here ... it could be dangerous
+ * to delete the file while it's in use.
+ */
+ if (IS_ERR(sdentry))
+ goto out;
+ if (d_is_positive(sdentry)) {
+ dput(sdentry);
+ goto newname;
+ }
+ /* This name isn't known locally - check on server */
+ old = dir->i_op->lookup(dir, sdentry, 0);
+ d_lookup_done(sdentry);
+ if (old || d_is_positive(sdentry)) {
+ if (!IS_ERR(old))
+ dput(old);
+ dput(sdentry);
+ goto newname;
+ }
ihold(&vnode->netfs.inode);
--
2.50.0.107.gf914562f5916.dirty
^ permalink raw reply related
* [PATCH 23/53] afs: lookup_atsys to drop and reclaim lock.
From: NeilBrown @ 2026-03-12 21:12 UTC (permalink / raw)
To: Linus Torvalds, Alexander Viro, Christian Brauner, Jan Kara,
Jeff Layton, Trond Myklebust, Anna Schumaker, Carlos Maiolino,
Miklos Szeredi, Amir Goldstein, Jan Harkes, Hugh Dickins,
Baolin Wang, David Howells, Marc Dionne, Steve French,
Namjae Jeon, Sungjong Seo, Yuezhang Mo, Andreas Hindborg,
Breno Leitao, Theodore Ts'o, Andreas Dilger, Steven Rostedt,
Masami Hiramatsu, Ilya Dryomov, Alex Markuze, Viacheslav Dubeyko,
Tyler Hicks, Andreas Gruenbacher, Richard Weinberger,
Anton Ivanov, Johannes Berg, Jeremy Kerr, Ard Biesheuvel
Cc: linux-fsdevel, linux-nfs, linux-xfs, linux-unionfs, coda,
linux-mm, linux-afs, linux-cifs, linux-ext4, linux-kernel,
linux-trace-kernel, ceph-devel, ecryptfs, gfs2, linux-um,
linux-efi
In-Reply-To: <20260312214330.3885211-1-neilb@ownmail.net>
From: NeilBrown <neil@brown.name>
If afs is asked to lookup a name ending with @sys, it needs to look up
a different name for which is allocates a dentry with
d_alloc_parallel().
This is done while the parent lock is held which will be a problem in
a future patch where the ordering of the parent lock and
d_alloc_parallel() locking is reversed.
There is no actual need to hold the lock while d_alloc_parallel() is
called, so with this patch we drop the lock and reclaim it.
Signed-off-by: NeilBrown <neil@brown.name>
---
fs/afs/dir.c | 22 +++++++++++++++++++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/fs/afs/dir.c b/fs/afs/dir.c
index 1e472768e1f1..c195ee851191 100644
--- a/fs/afs/dir.c
+++ b/fs/afs/dir.c
@@ -908,12 +908,14 @@ static struct inode *afs_do_lookup(struct inode *dir, struct dentry *dentry)
/*
* Look up an entry in a directory with @sys substitution.
*/
-static struct dentry *afs_lookup_atsys(struct inode *dir, struct dentry *dentry)
+static struct dentry *afs_lookup_atsys(struct inode *dir, struct dentry *dentry,
+ unsigned int flags)
{
struct afs_sysnames *subs;
struct afs_net *net = afs_i2net(dir);
struct dentry *ret;
char *buf, *p, *name;
+ struct qstr nm;
int len, i;
_enter("");
@@ -933,6 +935,13 @@ static struct dentry *afs_lookup_atsys(struct inode *dir, struct dentry *dentry)
refcount_inc(&subs->usage);
read_unlock(&net->sysnames_lock);
+ /* Calling d_alloc_parallel() while holding parent locked is undesirable.
+ * We don't really need the lock any more.
+ */
+ if (flags & LOOKUP_SHARED)
+ inode_unlock_shared(dir);
+ else
+ inode_unlock(dir);
for (i = 0; i < subs->nr; i++) {
name = subs->subs[i];
len = dentry->d_name.len - 4 + strlen(name);
@@ -942,7 +951,10 @@ static struct dentry *afs_lookup_atsys(struct inode *dir, struct dentry *dentry)
}
strcpy(p, name);
- ret = lookup_noperm(&QSTR(buf), dentry->d_parent);
+ nm = QSTR(buf);
+ ret = try_lookup_noperm(&nm, dentry->d_parent);
+ if (!ret)
+ ret = d_alloc_parallel(dentry->d_parent, &nm);
if (IS_ERR(ret) || d_is_positive(ret))
goto out_s;
dput(ret);
@@ -953,6 +965,10 @@ static struct dentry *afs_lookup_atsys(struct inode *dir, struct dentry *dentry)
*/
ret = NULL;
out_s:
+ if (flags & LOOKUP_SHARED)
+ inode_lock_shared(dir);
+ else
+ inode_lock_nested(dir, I_MUTEX_PARENT);
afs_put_sysnames(subs);
kfree(buf);
out_p:
@@ -998,7 +1014,7 @@ static struct dentry *afs_lookup(struct inode *dir, struct dentry *dentry,
dentry->d_name.name[dentry->d_name.len - 3] == 's' &&
dentry->d_name.name[dentry->d_name.len - 2] == 'y' &&
dentry->d_name.name[dentry->d_name.len - 1] == 's')
- return afs_lookup_atsys(dir, dentry);
+ return afs_lookup_atsys(dir, dentry, flags);
afs_stat_v(dvnode, n_lookup);
inode = afs_do_lookup(dir, dentry);
--
2.50.0.107.gf914562f5916.dirty
^ permalink raw reply related
* [PATCH 24/53] afs: use d_duplicate()
From: NeilBrown @ 2026-03-12 21:12 UTC (permalink / raw)
To: Linus Torvalds, Alexander Viro, Christian Brauner, Jan Kara,
Jeff Layton, Trond Myklebust, Anna Schumaker, Carlos Maiolino,
Miklos Szeredi, Amir Goldstein, Jan Harkes, Hugh Dickins,
Baolin Wang, David Howells, Marc Dionne, Steve French,
Namjae Jeon, Sungjong Seo, Yuezhang Mo, Andreas Hindborg,
Breno Leitao, Theodore Ts'o, Andreas Dilger, Steven Rostedt,
Masami Hiramatsu, Ilya Dryomov, Alex Markuze, Viacheslav Dubeyko,
Tyler Hicks, Andreas Gruenbacher, Richard Weinberger,
Anton Ivanov, Johannes Berg, Jeremy Kerr, Ard Biesheuvel
Cc: linux-fsdevel, linux-nfs, linux-xfs, linux-unionfs, coda,
linux-mm, linux-afs, linux-cifs, linux-ext4, linux-kernel,
linux-trace-kernel, ceph-devel, ecryptfs, gfs2, linux-um,
linux-efi
In-Reply-To: <20260312214330.3885211-1-neilb@ownmail.net>
From: NeilBrown <neil@brown.name>
To prepare for d_alloc_parallel() being permitted without a directory
lock, use d_duplicate() when duplicating a dentry in order to create a
whiteout.
Signed-off-by: NeilBrown <neil@brown.name>
---
fs/afs/dir.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/fs/afs/dir.c b/fs/afs/dir.c
index c195ee851191..b5c593f50079 100644
--- a/fs/afs/dir.c
+++ b/fs/afs/dir.c
@@ -2047,6 +2047,8 @@ static void afs_rename_put(struct afs_operation *op)
if (op->rename.unblock)
store_release_wake_up(&op->rename.unblock->d_fsdata, NULL);
store_release_wake_up(&op->dentry->d_fsdata, NULL);
+ if (op->rename.tmp)
+ d_lookup_done(op->rename.tmp);
dput(op->rename.tmp);
}
@@ -2175,8 +2177,7 @@ static int afs_rename(struct mnt_idmap *idmap, struct inode *old_dir,
if (d_count(new_dentry) > 2) {
spin_unlock(&new_dentry->d_lock);
/* copy the target dentry's name */
- op->rename.tmp = d_alloc(new_dentry->d_parent,
- &new_dentry->d_name);
+ op->rename.tmp = d_duplicate(new_dentry);
if (!op->rename.tmp) {
afs_op_nomem(op);
goto error;
--
2.50.0.107.gf914562f5916.dirty
^ permalink raw reply related
* [PATCH 25/53] smb/client: use d_time to store a timestamp in dentry, not d_fsdata
From: NeilBrown @ 2026-03-12 21:12 UTC (permalink / raw)
To: Linus Torvalds, Alexander Viro, Christian Brauner, Jan Kara,
Jeff Layton, Trond Myklebust, Anna Schumaker, Carlos Maiolino,
Miklos Szeredi, Amir Goldstein, Jan Harkes, Hugh Dickins,
Baolin Wang, David Howells, Marc Dionne, Steve French,
Namjae Jeon, Sungjong Seo, Yuezhang Mo, Andreas Hindborg,
Breno Leitao, Theodore Ts'o, Andreas Dilger, Steven Rostedt,
Masami Hiramatsu, Ilya Dryomov, Alex Markuze, Viacheslav Dubeyko,
Tyler Hicks, Andreas Gruenbacher, Richard Weinberger,
Anton Ivanov, Johannes Berg, Jeremy Kerr, Ard Biesheuvel
Cc: linux-fsdevel, linux-nfs, linux-xfs, linux-unionfs, coda,
linux-mm, linux-afs, linux-cifs, linux-ext4, linux-kernel,
linux-trace-kernel, ceph-devel, ecryptfs, gfs2, linux-um,
linux-efi
In-Reply-To: <20260312214330.3885211-1-neilb@ownmail.net>
From: NeilBrown <neil@brown.name>
smb/client uses d_fsdata is exactly the way that d_time is intended to
be used. It previous used d_time but this was changed in
Commit: a00be0e31f8d ("cifs: don't use ->d_time")
without any reason being given.
This patch effectively reverts that patch (though it doesn't remove the
helpers) so that d_fsdata can be used for something more generic.
Cc: Miklos Szeredi <miklos@szeredi.hu>
Signed-off-by: NeilBrown <neil@brown.name>
---
fs/smb/client/cifsfs.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/smb/client/cifsfs.h b/fs/smb/client/cifsfs.h
index e320d39b01f5..5153e811c50b 100644
--- a/fs/smb/client/cifsfs.h
+++ b/fs/smb/client/cifsfs.h
@@ -30,12 +30,12 @@ cifs_uniqueid_to_ino_t(u64 fileid)
static inline void cifs_set_time(struct dentry *dentry, unsigned long time)
{
- dentry->d_fsdata = (void *) time;
+ dentry->d_time = time;
}
static inline unsigned long cifs_get_time(struct dentry *dentry)
{
- return (unsigned long) dentry->d_fsdata;
+ return dentry->d_time;
}
extern struct file_system_type cifs_fs_type, smb3_fs_type;
--
2.50.0.107.gf914562f5916.dirty
^ permalink raw reply related
* [PATCH 26/53] smb/client: don't unhashed and rehash to prevent new opens.
From: NeilBrown @ 2026-03-12 21:12 UTC (permalink / raw)
To: Linus Torvalds, Alexander Viro, Christian Brauner, Jan Kara,
Jeff Layton, Trond Myklebust, Anna Schumaker, Carlos Maiolino,
Miklos Szeredi, Amir Goldstein, Jan Harkes, Hugh Dickins,
Baolin Wang, David Howells, Marc Dionne, Steve French,
Namjae Jeon, Sungjong Seo, Yuezhang Mo, Andreas Hindborg,
Breno Leitao, Theodore Ts'o, Andreas Dilger, Steven Rostedt,
Masami Hiramatsu, Ilya Dryomov, Alex Markuze, Viacheslav Dubeyko,
Tyler Hicks, Andreas Gruenbacher, Richard Weinberger,
Anton Ivanov, Johannes Berg, Jeremy Kerr, Ard Biesheuvel
Cc: linux-fsdevel, linux-nfs, linux-xfs, linux-unionfs, coda,
linux-mm, linux-afs, linux-cifs, linux-ext4, linux-kernel,
linux-trace-kernel, ceph-devel, ecryptfs, gfs2, linux-um,
linux-efi
In-Reply-To: <20260312214330.3885211-1-neilb@ownmail.net>
From: NeilBrown <neil@brown.name>
smb/client needs to block new opens of the target of unlink and rename
while the operation is progressing. This stablises d_count() and allows
a determination of whether a "silly-rename" is required.
It currently unhashes the dentry which will cause lookup to block on
the parent directory i_rwsem. Proposed changes to locking will cause
this approach to stop working and the exclusivity will be provided for
the dentry only, and only while it is hashed.
So we introduce a new machanism similar to that used by nfs and afs.
->d_fsdata (currently unused by smb/client) is set to a non-NULL
value when lookups need to be blocked. ->d_revalidate checks for this
and blocks. This might still allow d_count() to increment, but once it
has been tested as 1, there can be no new opens completed.
Signed-off-by: NeilBrown <neil@brown.name>
---
fs/smb/client/dir.c | 3 +++
fs/smb/client/inode.c | 48 +++++++++++++++++--------------------------
2 files changed, 22 insertions(+), 29 deletions(-)
diff --git a/fs/smb/client/dir.c b/fs/smb/client/dir.c
index cb10088197d2..cecbc0cce5c5 100644
--- a/fs/smb/client/dir.c
+++ b/fs/smb/client/dir.c
@@ -790,6 +790,9 @@ cifs_d_revalidate(struct inode *dir, const struct qstr *name,
if (flags & LOOKUP_RCU)
return -ECHILD;
+ /* Wait for pending rename/unlink */
+ wait_var_event(&direntry->d_fsdata, direntry->d_fsdata == NULL);
+
if (d_really_is_positive(direntry)) {
int rc;
struct inode *inode = d_inode(direntry);
diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index d4d3cfeb6c90..3549605fa9c2 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -28,6 +28,13 @@
#include "cached_dir.h"
#include "reparse.h"
+/* This is stored in ->d_fsdata to block d_revalidate on a
+ * file dentry that is being removed - unlink or rename target.
+ * This causes any open attempt to block. There may be existing opens
+ * but they can be detected by checking d_count() under ->d_lock.
+ */
+#define CIFS_FSDATA_BLOCKED ((void *)1)
+
/*
* Set parameters for the netfs library
*/
@@ -1946,27 +1953,21 @@ static int __cifs_unlink(struct inode *dir, struct dentry *dentry, bool sillyren
__u32 dosattr = 0, origattr = 0;
struct TCP_Server_Info *server;
struct iattr *attrs = NULL;
- bool rehash = false;
cifs_dbg(FYI, "cifs_unlink, dir=0x%p, dentry=0x%p\n", dir, dentry);
if (unlikely(cifs_forced_shutdown(cifs_sb)))
return smb_EIO(smb_eio_trace_forced_shutdown);
- /* Unhash dentry in advance to prevent any concurrent opens */
- spin_lock(&dentry->d_lock);
- if (!d_unhashed(dentry)) {
- __d_drop(dentry);
- rehash = true;
- }
- spin_unlock(&dentry->d_lock);
-
tlink = cifs_sb_tlink(cifs_sb);
if (IS_ERR(tlink))
return PTR_ERR(tlink);
tcon = tlink_tcon(tlink);
server = tcon->ses->server;
+ /* Set d_fsdata to prevent any concurrent opens */
+ dentry->d_fsdata = CIFS_FSDATA_BLOCKED;
+
xid = get_xid();
page = alloc_dentry_path();
@@ -2083,8 +2084,9 @@ static int __cifs_unlink(struct inode *dir, struct dentry *dentry, bool sillyren
kfree(attrs);
free_xid(xid);
cifs_put_tlink(tlink);
- if (rehash)
- d_rehash(dentry);
+
+ /* Allow lookups */
+ store_release_wake_up(&dentry->d_fsdata, NULL);
return rc;
}
@@ -2501,7 +2503,6 @@ cifs_rename2(struct mnt_idmap *idmap, struct inode *source_dir,
struct cifs_sb_info *cifs_sb;
struct tcon_link *tlink;
struct cifs_tcon *tcon;
- bool rehash = false;
unsigned int xid;
int rc, tmprc;
int retry_count = 0;
@@ -2517,23 +2518,15 @@ cifs_rename2(struct mnt_idmap *idmap, struct inode *source_dir,
if (unlikely(cifs_forced_shutdown(cifs_sb)))
return smb_EIO(smb_eio_trace_forced_shutdown);
- /*
- * Prevent any concurrent opens on the target by unhashing the dentry.
- * VFS already unhashes the target when renaming directories.
- */
- if (d_is_positive(target_dentry) && !d_is_dir(target_dentry)) {
- if (!d_unhashed(target_dentry)) {
- d_drop(target_dentry);
- rehash = true;
- }
- }
-
tlink = cifs_sb_tlink(cifs_sb);
if (IS_ERR(tlink))
return PTR_ERR(tlink);
tcon = tlink_tcon(tlink);
server = tcon->ses->server;
+ /* Set d_fsdata to prevent any concurrent opens */
+ target_dentry->d_fsdata = CIFS_FSDATA_BLOCKED;
+
page1 = alloc_dentry_path();
page2 = alloc_dentry_path();
xid = get_xid();
@@ -2570,8 +2563,6 @@ cifs_rename2(struct mnt_idmap *idmap, struct inode *source_dir,
}
}
- if (!rc)
- rehash = false;
/*
* No-replace is the natural behavior for CIFS, so skip unlink hacks.
*/
@@ -2662,8 +2653,6 @@ cifs_rename2(struct mnt_idmap *idmap, struct inode *source_dir,
}
rc = cifs_do_rename(xid, source_dentry, from_name,
target_dentry, to_name);
- if (!rc)
- rehash = false;
}
}
@@ -2671,8 +2660,9 @@ cifs_rename2(struct mnt_idmap *idmap, struct inode *source_dir,
CIFS_I(source_dir)->time = CIFS_I(target_dir)->time = 0;
cifs_rename_exit:
- if (rehash)
- d_rehash(target_dentry);
+ /* Allow lookups */
+ store_release_wake_up(&target_dentry->d_fsdata, NULL);
+
kfree(info_buf_source);
free_dentry_path(page2);
free_dentry_path(page1);
--
2.50.0.107.gf914562f5916.dirty
^ permalink raw reply related
* [PATCH 27/53] smb/client: use d_splice_alias() in atomic_open
From: NeilBrown @ 2026-03-12 21:12 UTC (permalink / raw)
To: Linus Torvalds, Alexander Viro, Christian Brauner, Jan Kara,
Jeff Layton, Trond Myklebust, Anna Schumaker, Carlos Maiolino,
Miklos Szeredi, Amir Goldstein, Jan Harkes, Hugh Dickins,
Baolin Wang, David Howells, Marc Dionne, Steve French,
Namjae Jeon, Sungjong Seo, Yuezhang Mo, Andreas Hindborg,
Breno Leitao, Theodore Ts'o, Andreas Dilger, Steven Rostedt,
Masami Hiramatsu, Ilya Dryomov, Alex Markuze, Viacheslav Dubeyko,
Tyler Hicks, Andreas Gruenbacher, Richard Weinberger,
Anton Ivanov, Johannes Berg, Jeremy Kerr, Ard Biesheuvel
Cc: linux-fsdevel, linux-nfs, linux-xfs, linux-unionfs, coda,
linux-mm, linux-afs, linux-cifs, linux-ext4, linux-kernel,
linux-trace-kernel, ceph-devel, ecryptfs, gfs2, linux-um,
linux-efi
In-Reply-To: <20260312214330.3885211-1-neilb@ownmail.net>
From: NeilBrown <neil@brown.name>
atomic_open can be called with a hashed-negative dentry or an in-lookup
dentry. Rather than d_drop() and d_add() we can use d_splice_alias()
which keeps the dentry hashed - important for proposed locking changes.
Signed-off-by: NeilBrown <neil@brown.name>
---
fs/smb/client/dir.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/fs/smb/client/dir.c b/fs/smb/client/dir.c
index cecbc0cce5c5..361a20987927 100644
--- a/fs/smb/client/dir.c
+++ b/fs/smb/client/dir.c
@@ -439,8 +439,7 @@ static int cifs_do_create(struct inode *inode, struct dentry *direntry, unsigned
goto out_err;
}
- d_drop(direntry);
- d_add(direntry, newinode);
+ d_splice_alias(newinode, direntry);
out:
free_dentry_path(page);
--
2.50.0.107.gf914562f5916.dirty
^ permalink raw reply related
* [PATCH 28/53] smb/client: Use d_alloc_noblock() in cifs_prime_dcache()
From: NeilBrown @ 2026-03-12 21:12 UTC (permalink / raw)
To: Linus Torvalds, Alexander Viro, Christian Brauner, Jan Kara,
Jeff Layton, Trond Myklebust, Anna Schumaker, Carlos Maiolino,
Miklos Szeredi, Amir Goldstein, Jan Harkes, Hugh Dickins,
Baolin Wang, David Howells, Marc Dionne, Steve French,
Namjae Jeon, Sungjong Seo, Yuezhang Mo, Andreas Hindborg,
Breno Leitao, Theodore Ts'o, Andreas Dilger, Steven Rostedt,
Masami Hiramatsu, Ilya Dryomov, Alex Markuze, Viacheslav Dubeyko,
Tyler Hicks, Andreas Gruenbacher, Richard Weinberger,
Anton Ivanov, Johannes Berg, Jeremy Kerr, Ard Biesheuvel
Cc: linux-fsdevel, linux-nfs, linux-xfs, linux-unionfs, coda,
linux-mm, linux-afs, linux-cifs, linux-ext4, linux-kernel,
linux-trace-kernel, ceph-devel, ecryptfs, gfs2, linux-um,
linux-efi
In-Reply-To: <20260312214330.3885211-1-neilb@ownmail.net>
From: NeilBrown <neil@brown.name>
cifs uses the results of readdir to prime the dcache. Using
d_alloc_parallel() can block if there is a concurrent lookup. Blocking
in that case is pointless as the lookup will add info to the dcache and
there is no value in the readdir waiting to see if it should add the
info too.
Also this call to d_alloc_parallel() is made while the parent
directory is locked. A proposed change to locking will lock the parent
later, after d_alloc_parallel(). This means it won't be safe to wait in
d_alloc_parallel() while holding the directory lock.
So change to use d_alloc_noblock().
Signed-off-by: NeilBrown <neil@brown.name>
---
fs/smb/client/readdir.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/smb/client/readdir.c b/fs/smb/client/readdir.c
index 47f5d620b750..dabf9507bc40 100644
--- a/fs/smb/client/readdir.c
+++ b/fs/smb/client/readdir.c
@@ -104,7 +104,7 @@ cifs_prime_dcache(struct dentry *parent, struct qstr *name,
(fattr->cf_flags & CIFS_FATTR_NEED_REVAL))
return;
- dentry = d_alloc_parallel(parent, name);
+ dentry = d_alloc_noblock(parent, name);
}
if (IS_ERR(dentry))
return;
--
2.50.0.107.gf914562f5916.dirty
^ permalink raw reply related
* [PATCH 29/53] exfat: simplify exfat_lookup()
From: NeilBrown @ 2026-03-12 21:12 UTC (permalink / raw)
To: Linus Torvalds, Alexander Viro, Christian Brauner, Jan Kara,
Jeff Layton, Trond Myklebust, Anna Schumaker, Carlos Maiolino,
Miklos Szeredi, Amir Goldstein, Jan Harkes, Hugh Dickins,
Baolin Wang, David Howells, Marc Dionne, Steve French,
Namjae Jeon, Sungjong Seo, Yuezhang Mo, Andreas Hindborg,
Breno Leitao, Theodore Ts'o, Andreas Dilger, Steven Rostedt,
Masami Hiramatsu, Ilya Dryomov, Alex Markuze, Viacheslav Dubeyko,
Tyler Hicks, Andreas Gruenbacher, Richard Weinberger,
Anton Ivanov, Johannes Berg, Jeremy Kerr, Ard Biesheuvel
Cc: linux-fsdevel, linux-nfs, linux-xfs, linux-unionfs, coda,
linux-mm, linux-afs, linux-cifs, linux-ext4, linux-kernel,
linux-trace-kernel, ceph-devel, ecryptfs, gfs2, linux-um,
linux-efi
In-Reply-To: <20260312214330.3885211-1-neilb@ownmail.net>
From: NeilBrown <neil@brown.name>
1/ exfat_d_anon_disconn() serves no purpose.
It is only called (on alias) when
alias->d_parent == dentry->d_parent
and in that case IS_ROOT(dentry) will return false, so the whole
function will return false.
So we can remove it.
2/ When an alias for the inode is found in the same parent
it is always sufficient to d_move() the alias to the new
name. This will keep just one dentry around when there are multiple
effective names, and it will always show the most recently used name,
which appears to be the intention.
Signed-off-by: NeilBrown <neil@brown.name>
---
fs/exfat/namei.c | 36 +++++++-----------------------------
1 file changed, 7 insertions(+), 29 deletions(-)
diff --git a/fs/exfat/namei.c b/fs/exfat/namei.c
index 670116ae9ec8..e04cda7425da 100644
--- a/fs/exfat/namei.c
+++ b/fs/exfat/namei.c
@@ -711,11 +711,6 @@ static int exfat_find(struct inode *dir, const struct qstr *qname,
return 0;
}
-static int exfat_d_anon_disconn(struct dentry *dentry)
-{
- return IS_ROOT(dentry) && (dentry->d_flags & DCACHE_DISCONNECTED);
-}
-
static struct dentry *exfat_lookup(struct inode *dir, struct dentry *dentry,
unsigned int flags)
{
@@ -750,32 +745,15 @@ static struct dentry *exfat_lookup(struct inode *dir, struct dentry *dentry,
* Checking "alias->d_parent == dentry->d_parent" to make sure
* FS is not corrupted (especially double linked dir).
*/
- if (alias && alias->d_parent == dentry->d_parent &&
- !exfat_d_anon_disconn(alias)) {
-
+ if (alias && alias->d_parent == dentry->d_parent) {
/*
- * Unhashed alias is able to exist because of revalidate()
- * called by lookup_fast. You can easily make this status
- * by calling create and lookup concurrently
- * In such case, we reuse an alias instead of new dentry
+ * As EXFAT does not support hard-links this must
+ * be an alternate name for the same file,
+ * possibly longname vs 8.3 alias.
+ * Rather than allocating a new dentry, use the old
+ * one but keep the most recently used name.
*/
- if (d_unhashed(alias)) {
- WARN_ON(alias->d_name.hash_len !=
- dentry->d_name.hash_len);
- exfat_info(sb, "rehashed a dentry(%p) in read lookup",
- alias);
- d_drop(dentry);
- d_rehash(alias);
- } else if (!S_ISDIR(i_mode)) {
- /*
- * This inode has non anonymous-DCACHE_DISCONNECTED
- * dentry. This means, the user did ->lookup() by an
- * another name (longname vs 8.3 alias of it) in past.
- *
- * Switch to new one for reason of locality if possible.
- */
- d_move(alias, dentry);
- }
+ d_move(alias, dentry);
iput(inode);
mutex_unlock(&EXFAT_SB(sb)->s_lock);
return alias;
--
2.50.0.107.gf914562f5916.dirty
^ 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