Messages in this thread | | | Date | Mon, 19 Sep 2011 21:54:27 +0100 | From | Chris Boot <> | Subject | Re: iommu_iova leak [inside 3w-9xxx] |
| |
On 19/09/2011 20:26, adam radford wrote: > On Sun, Sep 18, 2011 at 7:56 AM, James Bottomley > <James.Bottomley@hansenpartnership.com> wrote: >> On Sun, 2011-09-18 at 18:25 +0400, James Bottomley wrote: >>> On Sun, 2011-09-18 at 15:05 +0100, Chris Boot wrote: >>>>> Hardly ... all it's saying is that twa_exit doesn't wait for pending I/O >>>>> to complete, so when you remove the module it tears down in the middle >>>>> of an I/O. A bug, yes, but it's not indicative of any sort of leak in >>>>> the maps/unmaps. >>>> >>>> James, >>>> >>>> I don't think that's the case - I had unmounted all filesystems, deactivated all volume groups, and performed a sync before waiting a few seconds and running rmmod. Next time I'll also 'echo 1> /sys/block/sdX/device/delete' if that's helpful. >>> Actually, I take all that back: the driver has a bug in QUEUE_FULL >>> handling: twa_scsi_queue() calls twa_scsiop_execute_scsi(), which maps >>> the dma buffer, but if the card responds QUEUE_FULL it just returns >>> SCSI_MLQUEUE_HOST_BUSY without ever unmapping. That leg in the code >>> frees the request but also doesn't unmap it. In fact any error return >>> from twa_scsiop_execute_scsi() seems to have the same problem (but >>> QUEUE_FULL is the only silent one). >>> >>> I trust Adam will fix this. >> Actually, while Adam's mulling this, try the following. It should at >> least confirm we're on the right track. >> >> James >> >> --- >> >> diff --git a/drivers/scsi/3w-9xxx.c b/drivers/scsi/3w-9xxx.c >> index b7bd5b0..3868ab2 100644 >> --- a/drivers/scsi/3w-9xxx.c >> +++ b/drivers/scsi/3w-9xxx.c >> @@ -1800,10 +1800,12 @@ static int twa_scsi_queue_lck(struct scsi_cmnd *SCpnt, void (*done)(struct scsi_ >> switch (retval) { >> case SCSI_MLQUEUE_HOST_BUSY: >> twa_free_request_id(tw_dev, request_id); >> + twa_unmap_scsi_data(tw_dev, request_id); >> break; >> case 1: >> tw_dev->state[request_id] = TW_S_COMPLETED; >> twa_free_request_id(tw_dev, request_id); >> + twa_unmap_scsi_data(tw_dev, request_id); >> SCpnt->result = (DID_ERROR<< 16); >> done(SCpnt); >> retval = 0; >> >> >> > James, > > Your patch looks correct. > > Acked-by: Adam Radford<aradford@gmail.com> > > -Adam
Reported-by: Chris Boot <bootc@bootc.net> Tested-by: Chris Boot <bootc@bootc.net>
-- Chris Boot bootc@bootc.net
| |