On Thu, May 30, 2019 at 09:33:34AM +0800, Wang, Haiyue wrote: > > 在 2019-05-30 07:11, Eduardo Valentin 写道: > >>>>>>+ > >>>>>>+ case I2C_SLAVE_WRITE_RECEIVED: > >>>>>>+ if (msg->len < MQ_MSGBUF_SIZE) { > >>>>>>+ msg->buf[msg->len++] = *val; > >>>>>Do we need to lock the accesses to msg->buf? how about to msg->len? > >>>this code goes access and modify data here, e.g. msg->len and msg->buf. > >>> > >>>On this case (I2C_SLAVE_WRITE_RECEIVED), this code wont protect access. > >>> > >>>This can cause concurrence issues if you receive an IRQ when the user > >>>is on your bin_read(). > >>User will not touch 'msg = mq->curr;', just touch 'msg = > >>&mq->queue[mq->out];' > >What happens if mq->curr == mq->queue[mq->out]? > > > 1. The Read will check. > > + spin_lock_irqsave(&mq->lock, flags); > + if (mq->out != mq->in) { > + msg = &mq->queue[mq->out]; > > 2. Flush the oldeast message. ^_^ > > + case I2C_SLAVE_STOP: > + if (unlikely(mq->truncated || msg->len < 2)) > + break; > + > + spin_lock(&mq->lock); > + mq->in = MQ_QUEUE_NEXT(mq->in); > + mq->curr = &mq->queue[mq->in]; > + mq->curr->len = 0; > + > + /* Flush the oldest message */ > + if (mq->out == mq->in) > + mq->out = MQ_QUEUE_NEXT(mq->out); Yeah, I see. We keep on dropping messages (old ones) when the queue is full... > + spin_unlock(&mq->lock); > -- All the best, Eduardo Valentin