2024년 5월 23일 (목) 오후 10:36, Tom Talpey <tom@xxxxxxxxxx>님이 작성:
>
> 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.
Okay, I understood your point. I will apply this patch with durable
scavenger timer.
>
> Finally, from a code standpoint, I still don't see why it's being
> added before the scavenger functionality is even ready to review.
Okay. Thanks for pointing this out:)
>
> 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;
> >>>
> >
