On Tue, 2011-03-22 at 09:17 +1100, NeilBrown wrote: > On Mon, 21 Mar 2011 17:02:00 -0400 Trond Myklebust > <Trond.Myklebust@xxxxxxxxxx> wrote: > I must admit that I found the terminology a bit confusing for > FLUSH_COND_STABLE. > What is means is "if this turns out to be part of a larger request, then clear > FLUSH_STABLE" - which sounds a bit more like "FLUSH_COND_UNSTABLE" - i.e. if > a condition is met, then make it unstable. > But I don't think that change would help at all. > > How about changing the test in nfs_write_rpcsetup to > if (how & (FLUSH_STABLE|FLUSH_CONDSTABLE)) { > data->args.stable = NFS_DATA_SYNC; > if (!nfs_need_commit(NFS_I(inode))) > data->args.stable = NFS_FILE_SYNC; > } > > and then just set either FLUSH_STABLE or FLUSH_COND_STABLE - never both - > and when you test FLUSH_COND_STABLE and then some other condition, just > clear FLUSH_COND_STABLE. > > I would find that quite a bit more readable. The reason why I opted not to go for that approach was because nfs_write_rpcsetup() doesn't really know about whether or not there are any more RPCs pending (and so having a 'conditional' flag being interpreted there didn't appear to make sense), but if you feel that is easier on the eyes then I'm happy to change my opinion. -- Trond Myklebust Linux NFS client maintainer NetApp Trond.Myklebust@xxxxxxxxxx www.netapp.com -- 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