Re: [PATCH v2 1/2] bus: mhi: host: Add spinlock to protect WP access when queueing TREs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 11/6/2023 12:41 PM, Manivannan Sadhasivam wrote:
On Wed, Sep 13, 2023 at 04:47:40PM +0800, Qiang Yu wrote:
From: Bhaumik Bhatt <bbhatt@xxxxxxxxxxxxxx>

Protect WP accesses such that multiple threads queueing buffers for
incoming data do not race and access the same WP twice. Ensure read and
write locks for the channel are not taken in succession by dropping the
read lock from parse_xfer_event() such that a callback given to client
can potentially queue buffers and acquire the write lock in that process.
Any queueing of buffers should be done without channel read lock acquired
as it can result in multiple locks and a soft lockup.

This change is doing two things:

1. Unlocking xfer_cb to prevent potential lockup
2. Protecting mhi_gen_tre() against concurrent access

So you should split this into two patches and also add Fixes tag if appropriate.

- Mani
Hi Mani, thanks for review. I split this into two patch and add Fixes tag.

Signed-off-by: Bhaumik Bhatt <bbhatt@xxxxxxxxxxxxxx>
Signed-off-by: Qiang Yu <quic_qianyu@xxxxxxxxxxx>
---
  drivers/bus/mhi/host/main.c | 11 ++++++++++-
  1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/mhi/host/main.c b/drivers/bus/mhi/host/main.c
index dcf627b..13c4b89 100644
--- a/drivers/bus/mhi/host/main.c
+++ b/drivers/bus/mhi/host/main.c
@@ -642,6 +642,7 @@ static int parse_xfer_event(struct mhi_controller *mhi_cntrl,
  			mhi_del_ring_element(mhi_cntrl, tre_ring);
  			local_rp = tre_ring->rp;
+ read_unlock_bh(&mhi_chan->lock);
  			/* notify client */
  			mhi_chan->xfer_cb(mhi_chan->mhi_dev, &result);
@@ -667,6 +668,7 @@ static int parse_xfer_event(struct mhi_controller *mhi_cntrl,
  					kfree(buf_info->cb_buf);
  				}
  			}
+			read_lock_bh(&mhi_chan->lock);
  		}
  		break;
  	} /* CC_EOT */
@@ -1204,6 +1206,9 @@ int mhi_gen_tre(struct mhi_controller *mhi_cntrl, struct mhi_chan *mhi_chan,
  	int eot, eob, chain, bei;
  	int ret;
+ /* Protect accesses for reading and incrementing WP */
+	write_lock_bh(&mhi_chan->lock);
+
  	buf_ring = &mhi_chan->buf_ring;
  	tre_ring = &mhi_chan->tre_ring;
@@ -1221,8 +1226,10 @@ int mhi_gen_tre(struct mhi_controller *mhi_cntrl, struct mhi_chan *mhi_chan, if (!info->pre_mapped) {
  		ret = mhi_cntrl->map_single(mhi_cntrl, buf_info);
-		if (ret)
+		if (ret) {
+			write_unlock_bh(&mhi_chan->lock);
  			return ret;
+		}
  	}
eob = !!(flags & MHI_EOB);
@@ -1239,6 +1246,8 @@ int mhi_gen_tre(struct mhi_controller *mhi_cntrl, struct mhi_chan *mhi_chan,
  	mhi_add_ring_element(mhi_cntrl, tre_ring);
  	mhi_add_ring_element(mhi_cntrl, buf_ring);
+ write_unlock_bh(&mhi_chan->lock);
+
  	return 0;
  }
--
2.7.4






[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux