On Wed, Dec 13, 2023 at 09:31:13AM +1100, NeilBrown wrote: > On Wed, 13 Dec 2023, Dave Chinner wrote: > > > > What you are suggesting is that we now duplicate filehandle encoding > > into every filesystem's statx() implementation. That's a bad > > trade-off from a maintenance, testing and consistency POV because > > now we end up with lots of individual, filehandle encoding > > implementations in addition to the generic filehandle > > infrastructure that we all have to test and validate. > > Not correct. We are suggesting an interface, not an implementation. > Here you are proposing a suboptimal implementation, pointing out its > weakness, and suggesting the has consequences for the interface > proposal. Is that the strawman fallacy? No, you simply haven't followed deep enough into the rabbit hole to understand Kent was suggesting potential implementation details to address hot path performance concerns with filehandle encoding. > vfs_getattr_nosec could, after calling i_op->getattr, check if > STATX_HANDLE is set in request_mask but not in ->result_mask. > If so it could call exportfs_encode_fh() and handle the result. > > No filesystem need to be changed. Well, yes, it's pretty damn obvious that is exactly what I've been advocating for here - if we are going to put filehandles in statx(), then it must use the same infrastructure as name_to_handle_at(). i.e. calling exportfs_encode_fh(EXPORT_FH_FID) to generate the filehandle. The important discussion detail you've missed about exportfs_encode_fh() is that it *requires* adding a new indirect call (via export_ops->encode_fh) in the statx path to encode the filehandle, and that's exactly what Kent was suggesting we can code the implementation to avoid. Avoiding an indirect function call is an implementation detail, not an interface design requirement. And the only way to avoid adding new indirect calls to encoding filesystem specific filehandles is to implement the encoding in the existing individual filesystem i_op->getattr methods. i.e. duplicate the filehandle encoding in the statx path rather than use exportfs_encode_fh()..... -Dave. -- Dave Chinner david@xxxxxxxxxxxxx
