lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <20061106204550.GI19471@kernel.dk> Date: Mon, 6 Nov 2006 21:45:51 +0100 From: Jens Axboe <jens.axboe@...cle.com> To: "Mike Miller (OS Dev)" <mikem@...rdog.cca.cpqcorp.net> Cc: akpm@...l.org, linux-kernel@...r.kernel.org, linux-scsi@...r.kernel.org Subject: Re: [PATCH 12/12] cciss: fix for iostat On Mon, Nov 06 2006, Mike Miller (OS Dev) wrote: > Patch 12 of 12 > > This patch replaces complete_buffers with end_that_request_first to fix > programs like iostat. This has been broken for the last few kernel releases. > Please consider this for inclusion. > > Thanks, > mikem > > Signed-off-by: Mike Miller <mike.miller@...com> > > > --- > > > --- > > drivers/block/cciss.c | 20 ++++---------------- > 1 files changed, 4 insertions(+), 16 deletions(-) > > diff -puN drivers/block/cciss.c~cciss_update_diskstats_fix drivers/block/cciss.c > --- linux-2.6/drivers/block/cciss.c~cciss_update_diskstats_fix 2006-11-06 13:28:53.000000000 -0600 > +++ linux-2.6-root/drivers/block/cciss.c 2006-11-06 13:28:53.000000000 -0600 > @@ -1156,18 +1156,6 @@ static int cciss_ioctl(struct inode *ino > } > } > > -static inline void complete_buffers(struct bio *bio, int status) > -{ > - while (bio) { > - struct bio *xbh = bio->bi_next; > - int nr_sectors = bio_sectors(bio); > - > - bio->bi_next = NULL; > - bio_endio(bio, nr_sectors << 9, status ? 0 : -EIO); > - bio = xbh; > - } > -} > - > static void cciss_check_queues(ctlr_info_t *h) > { > int start_queue = h->next_to_run; > @@ -1236,15 +1224,15 @@ static void cciss_softirq_done(struct re > pci_unmap_page(h->pdev, temp64.val, cmd->SG[i].Len, ddir); > } > > - complete_buffers(rq->bio, rq->errors); > - > #ifdef CCISS_DEBUG > printk("Done with %p\n", rq); > #endif /* CCISS_DEBUG */ > > - add_disk_randomness(rq->rq_disk); > spin_lock_irqsave(&h->lock, flags); > - end_that_request_last(rq, rq->errors); > + if (!end_that_request_first(rq, rq->errors, rq->nr_sectors)) { > + add_disk_randomness(rq->rq_disk); > + end_that_request_last(rq, rq->errors); > + } > cmd_free(h, cmd, 1); > cciss_check_queues(h); > spin_unlock_irqrestore(&h->lock, flags); Ah, so there's where that went. Your code isn't clear, though - end_that_request_first() _must_ return 0, so the above looks confusing. It would look cleaner and more informative like: if (end_that_request_first(rq, rq->errors, rq->nr_sectors)) BUG(); add_disk_randomness(rq->rq_disk); end_that_request_last(rq, rq->errors); ... and so on. Additionally you don't need the lock for end_that_request_first(), so it's a lot more optimal to rearrange it again. add_disk_randomness(rq->rq_disk); if (end_that_request_first(rq, rq->errors, rq->nr_sectors)) BUG(); spin_lock_irqsave(&h->lock, flags); end_that_request_last(rq, rq->errors); cmd_free(h, cmd, 1); ... Not only cleaner to read since it's obvious what will happen, also moves the heavy path (end_that_request_first()) outside of the controller lock. -- Jens Axboe - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists