lkml.org 
[lkml]   [2009]   [Aug]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH]: Implementation of blk_rq_map_kern_sg() (aka New implementation of scsi_execute_async() v3)
    On Wed, Aug 12 2009, Vladislav Bolkhovitin wrote:
    > This patch implements function blk_rq_map_kern_sg(), which allows to map
    > a kernel-originated SG vector to a block request. It is necessary to execute
    > SCSI commands with from kernel going SG buffer. At the moment SCST is the only
    > user of this functionality. It needs it, because its target drivers, which
    > are, basically, SCSI drivers, can deal only with SGs, not with BIOs. But,
    > according to the latest discussions, there can be other potential users for of
    > this functionality, so I'm sending this patch in a hope that it will be
    > also useful for them and eventually will be merged in the mainline kernel.
    >
    > In the previous submissions this patch was called "New implementation of
    > scsi_execute_async()", but since in this version scsi_execute_async() was
    > removed from it by request of Boaz Harrosh the name was changed accordingly.

    Generally this patch looks great, I just have one little thing I'd like
    to point out:

    > + while (hbio != NULL) {
    > + bio = hbio;
    > + hbio = hbio->bi_next;
    > + bio->bi_next = NULL;
    > +
    > + blk_queue_bounce(q, &bio);
    > +
    > + res = blk_rq_append_bio(q, rq, bio);
    > + if (unlikely(res != 0)) {
    > + bio->bi_next = hbio;
    > + hbio = bio;
    > + /* We can have one or more bios bounced */
    > + goto out_unmap_bios;
    > + }
    > + }

    Constructs like this are always dangerous, because of how mempools work.
    __blk_queue_bounce() will internally do:

    bio = bio_alloc(GFP_NOIO, cnt);

    so you could potentially enter a deadlock if a) you are the only one
    allocating a bio currently, and b) the alloc fails and we wait for a bio
    to be returned to the pool. This is highly unlikely and requires other
    conditions to be dire, but it is a problem. This is not restricted to
    the swap out path, the problem is purely lack of progress. So the golden
    rule is always that you either allocate these units from a private pool
    (which is hard for bouncing, since it does both page and bio allocations
    from a mempool), or that you always ensure that a previously allocated
    bio is in flight before attempting a new alloc.

    --
    Jens Axboe



    \
     
     \ /
      Last update: 2009-08-15 10:25    [W:0.025 / U:1.224 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site