On Wed, 13 Dec 2023, Dave Chinner wrote: > 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. Yes, I missed that. Sorry. NeilBrown > > > 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 >
