Joe Eykholt wrote:
This seems pretty basic, but I'm having trouble with it.
The current libfc code has a private structure called fc_rport_libfc_priv
that gets allocated along with the fc_rport structure.
I'm working on patches that restructure this to be separately allocated,
since we need a the private structure to be around before we can do
fc_remote_port_add(). It's a long story, but my question applies to
any FC driver that wants to allocate its rport private data separately.
Most of the other fc drivers are in this position already, and always
have been. They've been the reverse - they desire a position where they
could move to rport data, but you seem to always need a context before
you know what it is enough to talk to the transport.
How should rport->dd_data be set to NULL before/after calling
fc_rport_delete().
You never do set it to NULL. This is the role of the transport, and it
will do so after calling the devloss_tmo_callbk(), which is the
"end-of-life" indicator for the rport.
If you are changing it - it can cause problems as the transport still
has the rport active, may make other calls, etc until
devloss_tmo_callbk() would occur.
I used to set it to NULL before, but figure it's safer to clear it after
terminate_io has been called. I did a get_device(&rport->dev) first
to be sure the rport doesn't get freed in the meantime.
terminate_io is one of those calls the transport tries to make before
the rport truly dies.
However, other threads may be in queuecommand already and already
beyond their call to fc_remote_port_chkready() and will reference dd_data.
Maybe they just aren't allowed do that?
Or maybe dd_data isn't allowed to go NULL until the rport times
out and is getting deleted?
Yes. This last point.
Maybe we need a small private rport data with just enough info for the I/O
that's already in progress, or I can recode the I/O path to not use
dd_data (it's mostly stuff that can be gotten through scsi_host instead).
have it be whatever you need it to be. You can always maintain your own
state flag in your own structure.
Or maybe we need to do more in our fc_rport_terminate_io callback
from fc_remote_port_delete() to be sure that all I/O really ceases.
We currently do an exch_mgr_reset(), but perhaps some I/O thread hasn't
allocated an exchange yet.
Sounds like race conditions within all the library routines. The
transport has some simple expectations, and really doesn't care how many
internal states the LLDD has. fc_rport_terminated_io() should terminate
(or at least initiate termination) of anything within the driver (in a
path post queuecommand()). Fc_remote_port_delete() is really just a
notifier to the transport that connection to the rport has gone away and
to start the devloss timers and block/reject new io requests - so it's
too early in the process. I/O may or may not be killed as of
rport_delete, as that's somewhat a LLDD policy (depends on FCP-2
Recovery desires and how discovery engines work).
The devloss_tmo_callbk(), which is the "end-of-life" indicator for the
rport, should never return from the LLDD until anything referencing the
rport (a if you're using it's dd_data, that too), has been killed - as
the transport may immediately "free" the rport after it's return. Do
you have a devloss_tmo_callbk() ?
-- james
Ideas?
Thanks,
Joe
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxx
http://www.open-fcoe.org/mailman/listinfo/devel
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
