ср, 29 авг. 2018 г. в 16:01, Pavel Shilovsky <piastryyy@xxxxxxxxx>:
>
> ср, 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?
>
Just noticed that you proposed the same idea in another email thread.
