Re: [PATCH v5 bpf-next 0/5] bpf path iterator

[Song Liu <songliubraving@meta.com>]

> On Jun 26, 2025, at 3:22 AM, NeilBrown <neil@brown.name> wrote:

[...]

>> I guess I misunderstood the proposal of vfs_walk_ancestors() 
>> initially, so some clarification:
>> 
>> I think vfs_walk_ancestors() is good for the rcu-walk, and some 
>> rcu-then-ref-walk. However, I don’t think it fits all use cases. 
>> A reliable step-by-step ref-walk, like this set, works well with 
>> BPF, and we want to keep it.
> 
> The distinction between rcu-walk and ref-walk is an internal
> implementation detail.  You as a caller shouldn't need to think about
> the difference.  You just want to walk.  Note that LOOKUP_RCU is
> documented in namei.h as "semi-internal".  The only uses outside of
> core-VFS code is in individual filesystem's d_revalidate handler - they
> are checking if they are allowed to sleep or not.  You should never
> expect to pass LOOKUP_RCU to an VFS API - no other code does.
> 
> It might be reasonable for you as a caller to have some control over
> whether the call can sleep or not.  LOOKUP_CACHED is a bit like that.
> But for dotdot lookup the code will never sleep - so that is not
> relevant.

Unfortunately, the BPF use case is more complicated. In some cases, 
the callback function cannot be call in rcu critical sections. For 
example, the callback may need to read xatter. For these cases, we
we cannot use RCU walk at all. 

> I strongly suggest you stop thinking about rcu-walk vs ref-walk.  Think
> about the needs of your code.  If you need a high-performance API, then
> ask for a high-performance API, don't assume what form it will take or
> what the internal implementation details will be.

At the moment, we need a ref-walk API on the BPF side. The RCU walk
is a totally separate topic. 

> I think you already have a clear answer that a step-by-step API will not
> be read-only on the dcache (i.e.  it will adjust refcounts) and so will
> not be high performance.  If you want high performance, you need to
> accept a different style of API.

Agreed. 

Thanks,
Song