Re: [PATCH] ksmbd: refactor smb2_notify() to a blocking wait, matching smb2_lock()

Thanks for your patch.

On 7/24/26 23:05, Gaël Blivet-Bailly wrote:
>   /**
>    * smb2_notify() - handler for smb2 notify request
>    * @work:   smb work containing notify command buffer
>    *
> + * KSMBD does not implement a real change-notification backend yet.
...
> + * wait to be interrupted matters more here.

Are these comments a bit too detailed for the source code? Perhaps it 
would be better to put them in the commit message.

> @@ -10969,54 +10939,26 @@ int smb2_notify(struct ksmbd_work *work)
>   	if (req->VolatileFileId == SMB2_NO_FID &&
>   	    req->PersistentFileId == SMB2_NO_FID) {
...
>   		smb2_send_interim_resp(work, STATUS_PENDING);
> +		rsp->hdr.Status = STATUS_NOTIFY_CLEANUP;
> +		smb2_send_interim_resp(work, STATUS_NOTIFY_CLEANUP);

There is no need to send interim responses STATUS_PENDING and 
STATUS_NOTIFY_CLEANUP, just call smb2_set_err_rsp() with 
STATUS_INVALID_PARAMETER.

> @@ -11024,96 +10966,52 @@ int smb2_notify(struct ksmbd_work *work)
...
> -	in_work->owns_conn_ref = true;

Please delete owns_conn_ref in struct ksmbd_work.

> @@ -650,50 +649,15 @@ static void __ksmbd_close_fd(struct ksmbd_file_table *ft, struct ksmbd_file *fp)
> +	 * Any CHANGE_NOTIFY blocked on this handle is woken by
> +	 * set_close_state_blocked_works() (via ksmbd_close_fd()/
> +	 * ksmbd_mark_fp_closed(), both called before __ksmbd_close_fd() is
> +	 * ever reached), the same generic fp->blocked_works mechanism
> +	 * smb2_lock() uses for a pending byte-range lock -- nothing further
> +	 * to do here. The waiter holds its own reference on fp (from
> +	 * ksmbd_lookup_fd_slow()) until it finishes handling the wake and
> +	 * calls ksmbd_fd_put(), so fp can't be freed out from under it.
>   	 */

These comments can be removed, since a CHANGE_NOTIFY request blocked on 
this handle can also be woken by an event.


-- 
ChenXiaoSong <[email protected]>
Chinese Homepage: https://chenxiaosong.com
English Homepage: https://chenxiaosong.com/en