On 18 Oct 2017, at 20:48, Andrew W Elble wrote:
Benjamin Coddington <bcodding@xxxxxxxxxx> writes:
On 17 Oct 2017, at 14:19, Jeff Layton wrote:
Also we now have to take the st_mutex in nfsd4_find_existing_open,
just to
check sc_type. Neither of those are probably unreasonable, it's
just
messier than I'd like.
It is indeed messy.. no argument. I'll spin up your suggestion to
unhash
the stateid before updating and take it for a ride and let you know
the
results. Thanks for looking at this.
I threw this against a quick lockdep run and didn't see anything that
surprised me. I think we developed a harmless warning in
nfsd4_process_open2() a ways back?
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 94ef63a10146..87535f2688be 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -64,6 +64,9 @@
static const stateid_t currentstateid = {
.si_generation = 1,
};
+static const stateid_t invalidstateid = {
+ .si_generation = U32_MAX,
+};
static u64 current_sessionid = 1;
@@ -5362,11 +5365,11 @@ static void nfsd4_close_open_stateid(struct
nfs4_ol_stateid *s)
nfsd4_bump_seqid(cstate, status);
if (status)
goto out;
- nfs4_inc_and_copy_stateid(&close->cl_stateid, &stp->st_stid);
- mutex_unlock(&stp->st_mutex);
+ memcpy(&close->cl_stateid, &invalidstateid, sizeof(stateid_t));
nfsd4_close_open_stateid(stp);
+ mutex_unlock(&stp->st_mutex);
/* put reference from nfs4_preprocess_seqid_op */
nfs4_put_stid(&stp->st_stid);
out:
I don't think this resolves the race. We'll still race in and find the
stateid in
nfsd4_process_open2()
nfsd4_find_existing_open()
So the client will see the response to the second OPEN after the CLOSE
as
having valid state with seqid of 2, and then the server will dispose of
that
state. The next operation will return with BAD_STATEID.
Ben
--
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
