Ceph » CephFS

Dhairya Parmar wrote:

Kernel client code when getting caps would increment cap ref count of both `need` and `want` caps but in userspace client, it would only increment the cap ref count of `need` caps while `want` caps are left to be handled manually all over the code. While this approach allows use to have more control over `want` caps

Sorry, how do we get more control over want caps?

and we've never faced any issues/crashes as such but this makes it a bit complicated to manage the caps, and with code base growing, this is ought to get cumbersome to deal with.
Therefore, align the caps code with that of kclient's which is much better in terms of simplicity and also caps management (e.g. [0] could've been avoided if the cap ref count of `Fc` had been incremented while calling `Client::get_caps` itself and then decrement the cap ref count in respective places.).

This refactoring will require a lot of changes, especially cases like `Fl` and `Fb` will need to handled carefully since the client code binds `Fl` to `Fb` but it is not the case in kclient. This is one of the special cases,

Could you explain why the special case exists?

I'm not sure whether there are anymore or any other caveats that needs to be considered, therefore a thorough reading of relevant code in kclient is must.

While having similar implementation in the kclient and user-space is desired, I don't think the effort required here is going to result in any performance improvement and/or fixing any bugs - Its a mere code reorganization. I think it's done this way ever since - unless the change is going to result in streamlines caps management.

Dhairya, do you have any comments in the same?

Venky Shankar wrote:

Dhairya, do you have any comments in the same?

The same point we discussed in standup - better (and simpler) the caps management, the less fuel for bugs. This is the only motive that i can see, no performance benefits as such

While having similar implementation in the kclient and user-space is desired, I don't think the effort required here is going to result in any performance improvement and/or fixing any bugs

I agree with the performance thing but there are code paths that somewhere perform the same thing i.e. let us consider async and sync I/O APIs, which both need to accquire Fc to wait for the I/O to finish if ObjectCacher::file_read returns zero bytes but then it becomes near impossible to figure out whether this case was hit and should we decrement the ref count right before we call do_readahead() in the later phases. For now this is the only example I know of but to make sure we don't run into problems like this, although the risk:reward might not be justified right now but I think in long run this should be beneficial.