Bug 165740 - [cam] SCSI code must drain callbacks before free
Summary: [cam] SCSI code must drain callbacks before free
Status: Open
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: Unspecified
Hardware: Any Any
: Normal Affects Only Me
Assignee: freebsd-bugs (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-03-05 18:50 UTC by Hans Petter Selasky
Modified: 2017-12-31 22:27 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Hans Petter Selasky 2012-03-05 18:50:13 UTC
In /sys/cam/cam_xpt.c:

4569                    void
4570                    xpt_release_device(struct cam_ed *device)
4571                    {
4572                    
4573    mjacob  224806  if (device->refcount == 1) {
4574    gibbs   39212   struct cam_devq *devq;
4575                    
4576    gibbs   44500   if (device->alloc_ccb_entry.pinfo.index != CAM_UNQUEUED_INDEX
4577                    || device->send_ccb_entry.pinfo.index != CAM_UNQUEUED_INDEX)
4578                    panic("Removing device while still queued for ccbs");
4579    gibbs   49927   
4580                    if ((device->flags & CAM_DEV_REL_TIMEOUT_PENDING) != 0)

Here callout_drain() should be used unconditionally. callout_drain()
requires that the caller is not locked. I don't have enough information
if it is possible to drop/pickup locks at this point in the code.

4581    mjacob  224806  callout_stop(&device->callout);


4583    mav     198748  TAILQ_REMOVE(&device->target->ed_entries, device,links);
4584                    device->target->generation++;
4585                    device->target->bus->sim->max_ccbs -= device->ccbq.devq_openings;
4586    trasz   186184  /* Release our slot in the devq */
4587    mav     198748  devq = device->target->bus->sim->devq;
4588    trasz   186184  cam_devq_resize(devq, devq->alloc_queue.array_size - 1);
4589    avatar  147571  camq_fini(&device->drvq);
4590    mav     198377  cam_ccbq_fini(&device->ccbq);
4591    ken     230590  /*
4592                    * Free allocated memory. free(9) does nothing if the
4593                    * supplied pointer is NULL, so it is safe to call without
4594                    * checking.
4595                    */
4596                    free(device->supported_vpds, M_CAMXPT);
4597                    free(device->device_id, M_CAMXPT);
4598                    free(device->physpath, M_CAMXPT);
4599                    free(device->rcap_buf, M_CAMXPT);
4600                    free(device->serial_num, M_CAMXPT);
4601                    
4602    mav     198748  xpt_release_target(device->target);
4603    avatar  147723  free(device, M_CAMXPT);

--HPS
Comment 1 Mark Linimon freebsd_committer freebsd_triage 2012-03-10 05:49:55 UTC
Responsible Changed
From-To: freebsd-bugs->freebsd-scsi

Over to maintainer(s).
Comment 2 seanwbruno 2013-04-14 20:25:11 UTC
Hans:

Can you regenerate this patch?  It looks like it got garbled by gnats.
Or this is a copy/paste from an annotated version of a web page?

Sean
Comment 3 Hans Petter Selasky 2013-04-15 06:58:40 UTC
On 04/14/13 21:25, Sean Bruno wrote:
> Hans:
>
> Can you regenerate this patch?  It looks like it got garbled by gnats.
> Or this is a copy/paste from an annotated version of a web page?
>
> Sean
>


Hi,

Can you check the commit logs? I wonder if Alexander has fixed this issue.

--HPS
Comment 4 Alexander Motin freebsd_committer freebsd_triage 2013-04-15 08:58:32 UTC
On 15.04.2013 08:58, Hans Petter Selasky wrote:
> On 04/14/13 21:25, Sean Bruno wrote:
>> Can you regenerate this patch?  It looks like it got garbled by gnats.
>> Or this is a copy/paste from an annotated version of a web page?
>
> Can you check the commit logs? I wonder if Alexander has fixed this issue.

No, I haven't, but I've noticed it also myself. I think that the code 
around these callouts is historically not exactly correct and needs some 
more attention then just dropping the lock and draining.

BTW one way to avoid dropping lock there could be in taking extra 
reference to device before arming it. That would keep device from 
destruction until callout actually fire.

-- 
Alexander Motin
Comment 5 Eitan Adler freebsd_committer freebsd_triage 2017-12-31 08:00:54 UTC
For bugs matching the following criteria:

Status: In Progress Changed: (is less than) 2014-06-01

Reset to default assignee and clear in-progress tags.

Mail being skipped