On 5/22/2024 7:31 PM, Namjae Jeon wrote:
2024년 5월 23일 (목) 오전 4:47, Tom Talpey <tom@xxxxxxxxxx>님이 작성:
On 5/22/2024 1:13 AM, Namjae Jeon wrote:
2024년 5월 22일 (수) 오전 12:10, Tom Talpey <tom@xxxxxxxxxx>님이 작성:
On 5/21/2024 9:57 AM, Namjae Jeon wrote:
The expired durable opens should not be reclaimed by client.
This patch add ->durable_scavenger_timeout to fp and check it in
ksmbd_lookup_durable_fd().
Signed-off-by: Namjae Jeon <linkinjeon@xxxxxxxxxx>
---
fs/smb/server/vfs_cache.c | 9 ++++++++-
fs/smb/server/vfs_cache.h | 1 +
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/fs/smb/server/vfs_cache.c b/fs/smb/server/vfs_cache.c
index 6cb599cd287e..a6804545db28 100644
--- a/fs/smb/server/vfs_cache.c
+++ b/fs/smb/server/vfs_cache.c
@@ -476,7 +476,10 @@ struct ksmbd_file *ksmbd_lookup_durable_fd(unsigned long long id)
struct ksmbd_file *fp;
fp = __ksmbd_lookup_fd(&global_ft, id);
- if (fp && fp->conn) {
+ if (fp && (fp->conn ||
+ (fp->durable_scavenger_timeout &&
+ (fp->durable_scavenger_timeout <
+ jiffies_to_msecs(jiffies))))) {
Do I understand correctly that this case means the fd is valid,
and only the durable timeout has been exceeded?
Yes.
If so, I believe it is overly strict behavior. MS-SMB2 specifically
states that the timer is a lower bound:
3.3.2.2 Durable Open Scavenger Timer This timer controls the amount
of time the server keeps a durable handle active after the
underlying transport connection to the client is lost.<210> The
server MUST keep the durable handle active for at least this amount
of time, except in the cases of an oplock break indicated by the
object store as specified in section 3.3.4.6, administrative actions,
or resource constraints.
What defect does this patch fix?
Durable open scavenger timer has not been added yet.
I will be adding this timer with this next patch. Nonetheless, this
patch is needed.
i.e. we need both ones.
So this code has no effect until then? And presumably, the scavenger
will be closing the fd, so it won't have any effect later, either.
Not at all. We can first take steps to prevent the timeout of durable v2
open from being used. There is absolutely no harm in this.
I disagree with "no harm".
As I said, the new behavior is more strict than MS-SMB2, and therefore
also stricter than Windows behavior.
Additionally, in the absence of a yet-to-be-written scavenger, this
means that fd's will remain cached and unclosed by ksmbd. The client,
in turn, will reopen the file, which seems like a source of sharing
violations, which become unrecallable in fact.
Finally, from a code standpoint, I still don't see why it's being
added before the scavenger functionality is even ready to review.
Tom.
Thanks.
The patch should not be applied at this time, and the full change
should be reviewed when it's ready.
Tom.
Thanks!
Tom.
ksmbd_put_durable_fd(fp);
fp = NULL;
}
@@ -717,6 +720,10 @@ static bool session_fd_check(struct ksmbd_tree_connect *tcon,
fp->tcon = NULL;
fp->volatile_id = KSMBD_NO_FID;
+ if (fp->durable_timeout)
+ fp->durable_scavenger_timeout =
+ jiffies_to_msecs(jiffies) + fp->durable_timeout;
+
return true;
}
diff --git a/fs/smb/server/vfs_cache.h b/fs/smb/server/vfs_cache.h
index 5a225e7055f1..f2ab1514e81a 100644
--- a/fs/smb/server/vfs_cache.h
+++ b/fs/smb/server/vfs_cache.h
@@ -101,6 +101,7 @@ struct ksmbd_file {
struct list_head lock_list;
int durable_timeout;
+ int durable_scavenger_timeout;
/* if ls is happening on directory, below is valid*/
struct ksmbd_readdir_data readdir_data;
