| Message ID | 20190121025509.32084-2-lsahlber@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | add credits we receive from oplock/break PDUs | expand |
вс, 20 янв. 2019 г. в 23:43, Ronnie Sahlberg <lsahlber@redhat.com>: > > Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com> > --- > fs/cifs/smb2misc.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c > index 6a9c47541c53..ec1372d2f3df 100644 > --- a/fs/cifs/smb2misc.c > +++ b/fs/cifs/smb2misc.c > @@ -642,12 +642,19 @@ smb2_is_valid_oplock_break(char *buffer, struct TCP_Server_Info *server) > struct cifs_tcon *tcon; > struct cifsInodeInfo *cinode; > struct cifsFileInfo *cfile; > + struct cifs_credits credits; > > cifs_dbg(FYI, "Checking for oplock break\n"); > > if (rsp->sync_hdr.Command != SMB2_OPLOCK_BREAK) > return false; > > + if (rsp->sync_hdr.CreditRequest) { > + credits.value = le16_to_cpu(rsp->sync_hdr.CreditRequest); > + credits.instance = server->reconnect_instance; > + add_credits(server, &credits, 0); add_credits() modifies server->in_flight field. Let's do something like the following instead: diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c index 6a9c475..7b8b58f 100644 --- a/fs/cifs/smb2misc.c +++ b/fs/cifs/smb2misc.c @@ -648,6 +648,13 @@ smb2_is_valid_oplock_break(char *buffer, struct TCP_Server_Info *server) if (rsp->sync_hdr.Command != SMB2_OPLOCK_BREAK) return false; + if (rsp->sync_hdr.CreditRequest) { + spin_lock(&server->req_lock); + server->credits += le16_to_cpu(rsp->sync_hdr.CreditRequest); + spin_unlock(&server->req_lock); + wake_up(&server->request_q); + } + It look like it should apply to stable kernels without problems. -- Best regards, Pavel Shilovsky
On Wed, Jan 23, 2019 at 11:31 AM Pavel Shilovsky <piastryyy@gmail.com> wrote: > > вс, 20 янв. 2019 г. в 23:43, Ronnie Sahlberg <lsahlber@redhat.com>: > > > > Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com> > > --- > > fs/cifs/smb2misc.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c > > index 6a9c47541c53..ec1372d2f3df 100644 > > --- a/fs/cifs/smb2misc.c > > +++ b/fs/cifs/smb2misc.c > > @@ -642,12 +642,19 @@ smb2_is_valid_oplock_break(char *buffer, struct TCP_Server_Info *server) > > struct cifs_tcon *tcon; > > struct cifsInodeInfo *cinode; > > struct cifsFileInfo *cfile; > > + struct cifs_credits credits; > > > > cifs_dbg(FYI, "Checking for oplock break\n"); > > > > if (rsp->sync_hdr.Command != SMB2_OPLOCK_BREAK) > > return false; > > > > + if (rsp->sync_hdr.CreditRequest) { > > + credits.value = le16_to_cpu(rsp->sync_hdr.CreditRequest); > > + credits.instance = server->reconnect_instance; > > + add_credits(server, &credits, 0); > > add_credits() modifies server->in_flight field. Let's do something > like the following instead: > > diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c > index 6a9c475..7b8b58f 100644 > --- a/fs/cifs/smb2misc.c > +++ b/fs/cifs/smb2misc.c > @@ -648,6 +648,13 @@ smb2_is_valid_oplock_break(char *buffer, struct > TCP_Server_Info *server) > if (rsp->sync_hdr.Command != SMB2_OPLOCK_BREAK) > return false; > > + if (rsp->sync_hdr.CreditRequest) { > + spin_lock(&server->req_lock); > + server->credits += le16_to_cpu(rsp->sync_hdr.CreditRequest); > + spin_unlock(&server->req_lock); > + wake_up(&server->request_q); > + } > + > > It look like it should apply to stable kernels without problems. Sounds good, but why the wake_up() I will resend without wake_up() > > -- > Best regards, > Pavel Shilovsky
вт, 22 янв. 2019 г. в 19:20, ronnie sahlberg <ronniesahlberg@gmail.com>: > > On Wed, Jan 23, 2019 at 11:31 AM Pavel Shilovsky <piastryyy@gmail.com> wrote: > > > > вс, 20 янв. 2019 г. в 23:43, Ronnie Sahlberg <lsahlber@redhat.com>: > > > > > > Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com> > > > --- > > > fs/cifs/smb2misc.c | 7 +++++++ > > > 1 file changed, 7 insertions(+) > > > > > > diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c > > > index 6a9c47541c53..ec1372d2f3df 100644 > > > --- a/fs/cifs/smb2misc.c > > > +++ b/fs/cifs/smb2misc.c > > > @@ -642,12 +642,19 @@ smb2_is_valid_oplock_break(char *buffer, struct TCP_Server_Info *server) > > > struct cifs_tcon *tcon; > > > struct cifsInodeInfo *cinode; > > > struct cifsFileInfo *cfile; > > > + struct cifs_credits credits; > > > > > > cifs_dbg(FYI, "Checking for oplock break\n"); > > > > > > if (rsp->sync_hdr.Command != SMB2_OPLOCK_BREAK) > > > return false; > > > > > > + if (rsp->sync_hdr.CreditRequest) { > > > + credits.value = le16_to_cpu(rsp->sync_hdr.CreditRequest); > > > + credits.instance = server->reconnect_instance; > > > + add_credits(server, &credits, 0); > > > > add_credits() modifies server->in_flight field. Let's do something > > like the following instead: > > > > diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c > > index 6a9c475..7b8b58f 100644 > > --- a/fs/cifs/smb2misc.c > > +++ b/fs/cifs/smb2misc.c > > @@ -648,6 +648,13 @@ smb2_is_valid_oplock_break(char *buffer, struct > > TCP_Server_Info *server) > > if (rsp->sync_hdr.Command != SMB2_OPLOCK_BREAK) > > return false; > > > > + if (rsp->sync_hdr.CreditRequest) { > > + spin_lock(&server->req_lock); > > + server->credits += le16_to_cpu(rsp->sync_hdr.CreditRequest); > > + spin_unlock(&server->req_lock); > > + wake_up(&server->request_q); > > + } > > + > > > > It look like it should apply to stable kernels without problems. > > Sounds good, but why the wake_up() > I will resend without wake_up() > wake_up() is needed to wake up threads which are waiting for credits (e.g. in wait_for_free_credits()). Please resend with wake_up. -- Best regards, Pavel Shilovsky
ronnie sahlberg <ronniesahlberg@gmail.com> writes: > Sounds good, but why the wake_up() > I will resend without wake_up() to add to Pavel explanations (I also had to look it up) wait_for_free_credits() calls wait_event_killable(server->request_q, has_credits(server, credits)); And the func doc is: * wait_event_killable - sleep until a condition gets true * @wq_head: the waitqueue to wait on * @condition: a C expression for the event to wait for * * The process is put to sleep (TASK_KILLABLE) until the * @condition evaluates to true or a signal is received. * The @condition is checked each time the waitqueue @wq_head is woken up. * * wake_up() has to be called after changing any variable that could * change the result of the wait condition. Cheers,
diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c index 6a9c47541c53..ec1372d2f3df 100644 --- a/fs/cifs/smb2misc.c +++ b/fs/cifs/smb2misc.c @@ -642,12 +642,19 @@ smb2_is_valid_oplock_break(char *buffer, struct TCP_Server_Info *server) struct cifs_tcon *tcon; struct cifsInodeInfo *cinode; struct cifsFileInfo *cfile; + struct cifs_credits credits; cifs_dbg(FYI, "Checking for oplock break\n"); if (rsp->sync_hdr.Command != SMB2_OPLOCK_BREAK) return false; + if (rsp->sync_hdr.CreditRequest) { + credits.value = le16_to_cpu(rsp->sync_hdr.CreditRequest); + credits.instance = server->reconnect_instance; + add_credits(server, &credits, 0); + } + if (rsp->StructureSize != smb2_rsp_struct_sizes[SMB2_OPLOCK_BREAK_HE]) { if (le16_to_cpu(rsp->StructureSize) == 44)
Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com> --- fs/cifs/smb2misc.c | 7 +++++++ 1 file changed, 7 insertions(+)