On Fri, Aug 11, 2017 at 04:36:49PM +0200, Christoph Hellwig wrote:
> On Fri, Aug 11, 2017 at 03:49:29PM +0200, Benjamin Block wrote:
> > On Fri, Aug 11, 2017 at 11:14:15AM +0200, Christoph Hellwig wrote:
> > > But patch 1 still creates an additional copy of the sense data for
> > > all bsg users.
> > >
> >
> > Huh? What additional copy? There is one reply-buffer and that is copied
> > into the user-buffer should it contain valid data. Just like in your
> > patch, neither you, nor me touches any of the copy-code. There is also
> > no changes to how the driver get their data into that buffer, it will
> > still be copied in both cases.
>
> You're right - I misread your patch. But that does make it worse as
> this means that with your patch we re-assign the scsi_request.sense
> pointer when using bsg. That will lead to crashes if using the bsg
> code against e.g. a normal scsi device using bsg when that request
> later gets reused for something that is not bsg.
>
So when the bsg interface is used with something different than the
bsg-lib request queue? I haven't actually thought about that (presuming
the bsg-lib queue was the only one being used). Fair enough, I haven't
completely read that code now, but that seems bad then, to reassign a
space allocated in someone else's request queue.
That still leaves open that we now over-allocate space in bsg-lib, or?
>
> >
> > >
> > > Can you test the patch below which implements my suggestion? Your
> > > other patches should still apply fine on top modulo minor context
> > > changes.
> >
> > Only your patch on top of 4.13-rc4. din_xferp (D) is also empty, which is
> > not taken from the sense-buffer.
>
> Can't parse this.
>
> > =============================================================================
> > BUG kmalloc-1024 (Not tainted): Invalid object pointer 0x000000004ad9e0f0
> > -----------------------------------------------------------------------------
>
> Oops - if we don't allocate the job separately we should not free it either.
> Updated patch for that below:
>
My diff tells that this was the same patch as before.
Beste Grüße / Best regards,
- Benjamin Block
--
Linux on z Systems Development / IBM Systems & Technology Group
IBM Deutschland Research & Development GmbH
Vorsitz. AufsR.: Martina Koederitz / Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen / Registergericht: AmtsG Stuttgart, HRB 243294
