On 17.05.2021 16:23, Juergen Gross wrote:
> On 17.05.21 16:11, Jan Beulich wrote:
>> On 13.05.2021 12:02, Juergen Gross wrote:
>>> @@ -1574,10 +1580,16 @@ static irqreturn_t blkif_interrupt(int irq, void *dev_id)
>>> spin_lock_irqsave(&rinfo->ring_lock, flags);
>>> again:
>>> rp = rinfo->ring.sring->rsp_prod;
>>> + if (RING_RESPONSE_PROD_OVERFLOW(&rinfo->ring, rp)) {
>>> + pr_alert("%s: illegal number of responses %u\n",
>>> + info->gd->disk_name, rp - rinfo->ring.rsp_cons);
>>> + goto err;
>>> + }
>>> rmb(); /* Ensure we see queued responses up to 'rp'. */
>>
>> I think you want to insert after the barrier.
>
> Why? The relevant variable which is checked is "rp". The result of the
> check is in no way depending on the responses themselves. And any change
> of rsp_cons is protected by ring_lock, so there is no possibility of
> reading an old value here.
But this is a standard double read situation: You might check a value
and then (via a separate read) use a different one past the barrier.
Jan
