Hello everyone,
I just wanted to start a discussion about the lport locking.
I've got a set of patches that I'm working on to clean-up this
area in the code but I'm having a hard time settling on a direction
so I wanted to open it up to see if anyone has good ideas.
I think that the main thing that the lport's state_lock needs
to protect against is someone resetting the lport while it's
processing something. For example, we've just received a GPN_FT
response and the user does an issue_lip which puts the lport into
the RESET state. The RESET state will "zero-out" some of the
negotiated properties of the lport and so aside from not wanting
to send frames/packets out because we're in RESET we don't want
to send out frames/packets with incorrect information.
Here's my short-list of what gets "zero'd-out" in the lport
when it's RESET.
What is currently being reset?
- dns_rp is set to NULL
- ptp_rp is set to NULL
- EM is reset
- fid is set to 0
- state is set to RESET
- retry_count is set to 0 (in fc_lport_state_enter)
What else should be reset?
- rports should be blocked
- ns_disc_done should be set to 0
- state_timer should be canceled
- ns_disc_work should be canceled
There are other members of the lport that are reset in other
places. For example, the ns_disc_buf is reset when we enter
the GPN_FT code. Also, there are some members that aren't
reset, these are generally properties of the lport that
aren't negotiated. If there's something not in the lists above
that you think I should be worried about please comment.
My lport locking policy right now is the following.
1) always lock when changing the state
2) when we receive a response
- lock
- check the state
- if the state is not as expected
- don't process the response
- unlock and return
- else
- do minimum response processing with lock held
(for example parse the GPN_FT buffer)
- if request needs to be generated
- cache the fid
- change the state if necessary
- unlock
- process the rest of the response if necessary
- send the next request using the cached fid
I think this mostly makes sense, but as you can see I'm
caching the fid so that I can use it later. It makes the code
a bit ugly because then I have to pass this fid around if the
request is not generated from the same function that the response
is parsed (not sure how frequently that happens). It also opens
up the possibility that we could be reset and still send out
a request with the old fid, which I'm not terribly concerned
about, maybe I should be.
Anyway, the alternative that I see would be to hold the
lport lock from the response through the following send request.
I was really trying to avoid holding the lock for that long, but I'm
starting to not worry about it so much because it's not the fast-path
that we're dealing with, just the setup of the lport.
Holding the lock through the next request would prevent
a RESET when we're in between a response and the next request. We'd
send that request, and then be RESET. When the last response is
received its exchange would be invalid, so it wouldn't be passed
to the local port block from the EM.
What do you guys think?
//Rob
_______________________________________________
devel mailing list
[email protected]
http://www.open-fcoe.org/mailman/listinfo/devel
