On Wed, Aug 07, 2024 at 06:58:59AM -0700, Manoj Vishwanathan wrote: > Thanks Przemek & Olek for your quick feedback and responses. > Hi Olek, > I can add more details about the issue we faced in the commit message. > The bug we had here was a virtchnl delay leading to the xn->salt > mismatch. This could be due to several factors including default CPU > bounded kworker workqueue for virtchnl message processing being > starved by aggressive userspace load causing the virtchnl to be > delayed. While debugging this issue, this locking order appeared like > a potential issue, hence the change was made. > But, this change is more a clean up we felt based on concurrent access > to the virtchnl transaction struct and does not fix the issue. This is > more of the patch to do the right thing before we access the "xn". > I wanted to start with a first patch to the community for acceptance > followed by a series of other patches that are general clean up or > improvements to IDPF in general. Will follow with with [PATCH v3]
Still, I am a little confused about the protection offered to xn->salt. My analysis is as follows, where guarded is used loosely to mean the lock is held. * In idpf_vc_xn_pop_free() it is guarded by vcxn_mngr->xn_bm_lock. * In idpf_vc_xn_exec() it is guarded by: 1. vcxn_mngr->xn_bm_lock when idpf_vc_xn_pop_free is called 2. idpf_vc_xn_lock, otherwise * And with this patch, in idpf_vc_xn_forward_reply it is guarded by idpf_vc_xn_lock(). This doesn't seem entirely consistent. Also, please don't top-post on Kernel mailing lists. ...
