Am Dienstag, 24. Mai 2011, 10:48:09 schrieb Prarit Bhargava:
> On 05/20/2011 04:17 PM, Rolf Eike Beer wrote:
> > This could be simpler if you would use devres and devm_kzalloc() and
> > devm_request_irq(). You simply need to return with error then and the
> > driver core would free everything you already allocated.
> >
> > Eike
>
> Thanks for the suggestion Eike.
>
> I've never used devres before. This seems to work -- please review as [v3].
No, sorry, this wont work. You need to change your call of pci_enable_device()
to pcim_enable_device(). Afterwards you should check what else in your probe
routine can be converted to devres. This is optional, but why duplicate work?
What you need to take care of: resources that you do not allocate by devres
(e.g. the scsi_host) and which are explicitely freed by you must not be needed
e.g. in the IRQ handler if that would be freed by devres, i.e.:
int ret;
pcim_enable_device()
beiscsi_hba_alloc()
devm_request_irq()
ret = something();
if (ret != 0) {
beiscsi_free_hba();
return ret;
}
// devres would now free IRQs etc.
If an IRQ happens right before it is freed by devres (which could e.g. happen
if you enable IRQ debugging) you could hit a NULL or stale host pointer. If
your IRQ handler requires some resources to always be present (e.g. the
scsi_host) then you must explicitely deregister it before releasing those
resources. At the end this makes the init savings void. So you better add a
single check
if (unlikely(my_hba == NULL))
return IRQ_NONE;
to your IRQ handler to be safe.
EikeAttachment:
signature.asc
Description: This is a digitally signed message part.
