On Thu, Mar 25, 2021 at 7:10 PM John Fastabend <john.fastabend@xxxxxxxxx> wrote: > Hi Cong, > > I'm trying to understand if the workqueue logic will somehow prevent the > following, > > CPU0 CPU1 > > work dequeue > sk_psock_backlog() > ... do backlog > ... also maybe sleep > > schedule_work() > work_dequeue > sk_psock_backlog() > > <----- multiple runners --------> > > work_complete > > It seems we could get multiple instances of sk_psock_backlog(), unless > the max_active is set to 1 in __queue_work() which would push us through > the WORK_STRUCT_DELAYED state. At least thats my initial read. Before > it didn't matter because we had the sock_lock to ensure we have only a > single runner here. > > I need to study the workqueue code here to be sure, but I'm thinking > this might a problem unless we set up the workqueue correctly. > > Do you have any extra details on why above can't happen thanks. Very good question! I thought a same work callback is never executed concurrently, but after reading the workqueue code, actually I agree with you on this, that is, a same work callback can be executed concurrently on different CPU's. Limiting max_active to 1 is not a solution here, as we still want to keep different items running concurrently. Therefore, we still need a mutex here, just to protect this scenario. I will add a psock->work_mutex inside sk_psock_backlog(). Thanks!
