Greetings, James. :-)
On Fri, Apr 01, 2005 at 12:23:37PM -0600, James Bottomley wrote:
> On Thu, 2005-03-31 at 18:08 +0900, Tejun Heo wrote:
> > When scsi_init_io() returns BLKPREP_DEFER or BLKPREP_KILL,
> > it's supposed to free resources itself. This patch
> > consolidates defer and kill handling into scsi_prep_fn().
> > This fixes a queue stall bug which occurred when sgtable
> > allocation failed and device_busy == 0.
>
> This one I like, but I think it doesn't go far enough. There are
> situations where a re-queue can also re-prepare, which means we leak sg
> lists and commands on BLKPREP_KILL because of SCSI state.
With later request_fn() rewrite patch, all state checking are moved
to request_fn() and gets terminated with __scsi_done(). prep_fn()
only kills invalid (so, unprepped) requests. It's in the fixed bugs
list of the request_fn() rewrite patch. BTW, there's one more path
which leaks cmnd/sgtable, the end_that_request_*() on offline path of
request_fn() which is also fixed by request_fn() rewrite patch.
I'm sorry about not mentioning that the bug is fixed in the later
patch. Please review request_fn() rewrite patch.
> How about the attached.
>
> Note, I've also altered the prepare function so req->special never gets
> altered if it points to a scsi_request. Previously it would be altered
> to point to the command, but now we couldn't put the command since the
> scsi_request we can't get to also has a copy of if. So, if you can make
> your later REQ_SPECIAL removal work, we can dispense with sr_magic and
> simply use REQ_SPECIAL as the discriminator for the contents of req-
> >special.
>
> James
Yeah, I like not overwriting ->special, and removing magic is always
good. :-)
Hmmm, I have another proposal. What do you think about replacing
scsi_request with scsi_cmnd. I've been looking into it and it doesn't
seem to be too much work and it won't cause problems. The only
bothering thing is that scsi_cmnd is limited resource and should be
returned to the pool ASAP. AFAIK, all current users of
scsi_allocate_request() releses the scsi_request as soon as they're
finished, and we can make the requirement clear in the comment
(i.e. do not cache or hold on to scsi_cmnd). This will remove the
scsi_request/cmnd dance and make scsi midlayer slimmer in other
places. What do you think?
Thanks.
--
tejun
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
