On Tue, 31 May 2022, Davidlohr Bueso wrote:
On Tue, 31 May 2022, ira.weiny@xxxxxxxxx wrote:+static void doe_statemachine_work(struct work_struct *work) +{ + struct delayed_work *w = to_delayed_work(work); + struct pci_doe_mb *doe_mb = container_of(w, struct pci_doe_mb, + statemachine); + struct pci_dev *pdev = doe_mb->pdev; + int offset = doe_mb->cap_offset; + struct pci_doe_task *task; + u32 val; + int rc; + + mutex_lock(&doe_mb->task_lock); + task = doe_mb->cur_task; + mutex_unlock(&doe_mb->task_lock);Instead of a mutex, would it be better to use a rwsem here to protect the state machine and allow for concurrent reads for the work callback? It is a general interface and a trivial change, but not sure how much performance is cared about.
Actually why is this a sleeping lock at all? Afaict all critical regions are short and just deal with loads and stores of oe_mb->task_lock (and pci_doe_submit_task also checks the doe_mb->flags with the lock held). This could be a spinlock or similarly a rwlock. Thanks, Davidlohr
