On 09/07/2024 0:41, Dai Ngo wrote:
On 7/8/24 11:56 AM, Sagi Grimberg wrote:
On 08/07/2024 20:39, Dai Ngo wrote:
On 7/8/24 7:18 AM, Chuck Lever III wrote:
On Jul 7, 2024, at 7:06 AM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
On Sun, 2024-07-07 at 01:42 +0300, Sagi Grimberg wrote:
Many applications open files with O_WRONLY, fully intending to write
into the opened file. There is no reason why these applications
should
not enjoy a write delegation handed to them.
Cc: Dai Ngo <dai.ngo@xxxxxxxxxx>
Signed-off-by: Sagi Grimberg <sagi@xxxxxxxxxxx>
---
Note: I couldn't find any reason to why the initial
implementation chose
to offer write delegations only to NFS4_SHARE_ACCESS_BOTH, but it
seemed
like an oversight to me. So I figured why not just send it out
and see who
objects...
fs/nfsd/nfs4state.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index a20c2c9d7d45..69d576b19eb6 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -5784,15 +5784,15 @@ nfs4_set_delegation(struct nfsd4_open
*open, struct nfs4_ol_stateid *stp,
* "An OPEN_DELEGATE_WRITE delegation allows the client to handle,
* on its own, all opens."
*
- * Furthermore the client can use a write delegation for most READ
- * operations as well, so we require a O_RDWR file here.
- *
- * Offer a write delegation in the case of a BOTH open, and ensure
- * we get the O_RDWR descriptor.
+ * Offer a write delegation in the case of a BOTH open (ensure
+ * a O_RDWR descriptor) Or WRONLY open (with a O_WRONLY
descriptor).
*/
if ((open->op_share_access & NFS4_SHARE_ACCESS_BOTH) ==
NFS4_SHARE_ACCESS_BOTH) {
nf = find_rw_file(fp);
dl_type = NFS4_OPEN_DELEGATE_WRITE;
+ } else if (open->op_share_access & NFS4_SHARE_ACCESS_WRITE) {
+ nf = find_writeable_file(fp);
+ dl_type = NFS4_OPEN_DELEGATE_WRITE;
}
/*
Thanks Sagi, I'm glad to see this posting!
I *think* the main reason we limited this before is because a write
delegation is really a read/write delegation. There is no such
thing as
a write-only delegation.
I recall (quite dimly) that Dai found some bad behavior
in a subtle corner case, so we decided to leave this on
the table as a possible future enhancement. Adding Dai.
If I remember correctly, granting write delegation on OPEN with
NFS4_SHARE_ACCESS_WRITE without additional changes causes the git
test to fail.
That's good to know.
Have you reported this over the list before?
I submitted a patch to allow the client to use write delegation, granted
on OPEN with NFS4_SHARE_ACCESS_WRITE, to read:
https://lore.kernel.org/linux-nfs/1688089960-24568-3-git-send-email-dai.ngo@xxxxxxxxxx/
This patch fixed the problem with the git test. However Jeff reported
another
problem might be related to this patch:
https://lore.kernel.org/linux-nfs/6756f84c4ff92845298ce4d7e3c4f0fb20671d3d.camel@xxxxxxxxxx
I did not investigate this pynfs problem since we dropped this support.
I see, thanks for the info. I initially thought that the client has an
issue. But now
I see that you referred to nfsd that is unable to process a READ with a
writedeleg stid
(which is allowed).
Is there any appetite to address this? Or are we fine with not handing
out delegations
in these case? The patch I sent served its goal, which was to understand
if there is
some reasoning behind the lack of it.
