ср, 29 авг. 2018 г. в 15:34, Pavel Shilovsky <piastryyy@xxxxxxxxx>:
>
> вт, 28 авг. 2018 г. в 19:19, Ronnie Sahlberg <lsahlber@xxxxxxxxxx>:
> >
> > For compounded PDUs we whould only wake the waiting thread for the
> > very last PDU of the compound.
> > We do this so that we are guaranteed that the demultiplex_thread will
> > not process or access any of those MIDs any more once the send/recv
> > thread starts processing.
> >
> > Else there is a race where at the end of the send/recv processing we
> > will try to delete all the mids of the compound. If the multiplex
> > thread still has other mids to process at this point for this compound
> > this can lead to an oops.
> >
> > Signed-off-by: Ronnie Sahlberg <lsahlber@xxxxxxxxxx>
> > ---
> > fs/cifs/transport.c | 19 ++++++++++++++++++-
> > 1 file changed, 18 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
> > index 9e3a1e87ae1c..49a800ce75d6 100644
> > --- a/fs/cifs/transport.c
> > +++ b/fs/cifs/transport.c
> > @@ -773,6 +773,11 @@ cifs_setup_request(struct cifs_ses *ses, struct smb_rqst *rqst)
> > return mid;
> > }
> >
> > +static void
> > +cifs_noop_callback(struct mid_q_entry *mid)
> > +{
> > +}
> > +
> > int
> > compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
> > const int flags, const int num_rqst, struct smb_rqst *rqst,
> > @@ -826,9 +831,15 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
> > return PTR_ERR(midQ[i]);
> > }
> >
> > + /*
> > + * We don't invoke the callback compounds unless it is the last
> > + * request.
> > + */
> > midQ[i]->mid_state = MID_REQUEST_SUBMITTED;
> > + if (ses->server->ops->next_header &&
> > + ses->server->ops->next_header(rqst[i].rq_iov[0].iov_base))
> > + midQ[i]->callback = cifs_noop_callback;
>
> Wouldn't it be easier to just check for i < num_rqst - 1 ? Having
> ses->server->ops->next_header(rqst[i].rq_iov[0].iov_base) return false
> should be a bug here. SMB1 code doesn't support compounding at all, so
> num_rqst is always 1.
Also I thought that we may use a similar change you did in WIP patch -
just set mids[i]->multiRsp and mids[i]->multiEnd properly for
compounded requests. I mean the following:
if (num_req > 1) {
mids[i]->multiRsp = 1;
if (i == num_req - 1) {
mids[i]->multiEnd = 1;
}
}
This will work for both compound and non-compound requests and utilize
the existing multiRsp/multiEnd fields. Thoughts?
--
Best regards,
Pavel Shilovsky
