On Mon, Oct 03, 2011 at 05:13:24PM +0200, Benny Halevy wrote: > On 2011-10-03 16:57, J. Bruce Fields wrote: > > On Mon, Oct 03, 2011 at 04:43:40PM +0200, Benny Halevy wrote: > >> Bruce, it seems with this patch, doing si_opaque.so_id = current_stateid > >> makes all stateid's unique, regardless of their type. > >> Is find_stateid_by_type still needed? > >> > >> And if the opaque part of the stateid isn't unique, > >> shouldn't find_stateid_by_type go over the hash bucket by itself > >> to look for other stateid's sharing the same opaque but having > >> the type it is looking for? > > > > Stateid's have actually always been unique--they'd have to be, since > > e.g. READ doesn't come with any indication of which type of stateid > > you're being given. All find_stateid_by_type() does is fail when it > > doesn't find something of the right type. > > > > So I'm still confused. That's what I thought. > So in what cases find_stateid that find_stateid_by_type calls > will find a stateid structure with a non-matching sc_type? Suppose, for example, the server gets a DELEGRETURN, looks up the provided stateid, and finds an open stateid. At this point either the client is buggy, or else something very unlikely has happened. (E.g. a stateid was retired and then reused). So find_stateid_by_type() returns NULL which the caller will likely translate to something like BAD_STATEID. Other cases: open_confirm and close require an open stateid, ulock or lock with the !new_lock_owner case requires a lock stateid, etc. --b. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html