Hi Steve, Shyam, I'd like to make the attached change to add_credits_and_wake_if(). It's called in various places along the error handling paths, but it's not obvious that it's consistent and that we don't get double accounting. The obvious change would be to clear credits->value if we return the credits back to the pool. Assuming that's how this works. That makes it easier to (a) clean up the netfs_io_subrequest struct or (b) renegotiate and retry with it because I can just call it multiple times. Also, add_credits_and_wake_if() wakes up server->request_q... but so do cifs_add_credits() and smb2_add_credits(), so is this superfluous? Additionally, what's the scope of a 'xid'? I think I should add one to each subrequest I generate and deallocate it when the subrequest is freed. If that's the case, can I add an explicit "rc" argument to free_xid()? And finally, I have my cifs conversion to netfslib down to the same xfstest failures as upstream[*]. Those patches can be found here, with an additional one on top to address the credits and part of the xid thing: https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=cifs-netfs David [*] With Samba; I still need to sort out ksmbd. --- @@ -878,11 +878,12 @@ add_credits(struct TCP_Server_Info *server, const struct cifs_credits *credits, static inline void add_credits_and_wake_if(struct TCP_Server_Info *server, - const struct cifs_credits *credits, const int optype) + struct cifs_credits *credits, const int optype) { if (credits->value) { server->ops->add_credits(server, credits, optype); wake_up(&server->request_q); + credits->value = 0; } }
