On Monday 05 May 2008, FUJITA Tomonori wrote: > On Sat, 3 May 2008 15:10:02 +0200 > Bartlomiej Zolnierkiewicz <bzolnier@xxxxxxxxx> wrote: > > > > > Hi, > > > > On Thursday 01 May 2008, FUJITA Tomonori wrote: > > > This is an updated version of the patchset to clean up the asymmetry > > > of blk_get/put_request usage: > > > > > > http://marc.info/?l=linux-ide&m=120882410712466&w=2 > > > > > > This patchset removes the code calling blk_put_request against the > > > requests that are not allocated via blk_get_request. They can use > > > __GFP_WAIT allocation so we can easily convert them to > > > use blk_get/put_request properly. > > > > > > This patchset enables us to remove the following hack in > > > blk_put_request: > > > > > > /* > > > * Gee, IDE calls in w/ NULL q. Fix IDE and remove the > > > * following if (q) test. > > > */ > > > if (q) { > > > spin_lock_irqsave(q->queue_lock, flags); > > > __blk_put_request(q, req); > > > spin_unlock_irqrestore(q->queue_lock, flags); > > > } > > > > > > The major changes are: > > > > > > - I remove the ide_wait/head_wait path in ide_do_drive_cmd(). It does > > > the same thing that blk_execute_rq() does. So let's simply use > > > blk_execute_rq(). The nice side effect is that I unexport > > > blk_end_sync_rq() since I converted all the users. > > > > > > - I drop the unnecessary patch to make ide_do_drive_cmd return > > > rq->errors: > > > > > > http://marc.info/?l=linux-ide&m=120882410612456&w=2 > > > > > > #1-10 are for the ide subsystem and #11-13 for the block layer. This > > > is against the lastest Linus tree. > > > > Thanks. > > > > I applied patches #1-10 after making ide_do_drive_cmd() more similar to > > blk_execute_rq() (to ease the conversion - it is really better to handle > > subtle changes first). [ I'll send these pre-patches in separate mails. ] > > Thanks for applying the patches. > > > > However I'm still not quite sure about following change in patch #1: > > > > @@ -456,24 +452,21 @@ int ide_cdrom_packet(struct cdrom_device_info *cdi, > > ... > > - cgc->stat = ide_cd_queue_pc(drive, &req); > > + cgc->stat = ide_cd_queue_pc(drive, cgc->cmd, > > + cgc->data_direction == CGC_DATA_WRITE, > > + cgc->buffer, cgc->buflen, > > + cgc->sense, cgc->timeout, flags); > > if (!cgc->stat) > > - cgc->buflen -= req.data_len; > > + cgc->buflen = 0; > > ... > > > > IIRC rq->data_len may be modified while the request is processed? > > I think that rq->data_len is modified only when a request is > successful but I might overlook or misunderstand something. How about > the attached patch (against the latest your quilt patchset)? It's a > bit hacky but it should work as before. > > > > [ The only other (minor) complaint is that patches #9-10 should be at the > > beginning of the patch series but since no patches contained problems it > > wouldn't be wise to ask respin just for that (and I'm too lazy to review > > it all again ;-). ] > > Thanks. > > If you are ok with the attached patch, I'm happy to incorporate it > into the patchset and send a new version as you like. > > > == > From: FUJITA Tomonori <fujita.tomonori@xxxxxxxxxxxxx> > Subject: ide: let ide_cd_queue_pc return rq->data_len > > ide_cdrom_packet needs rq->data_len after the completion so this patch > makes ide_cd_queue_pc return rq->data_len. > > Signed-off-by: FUJITA Tomonori <fujita.tomonori@xxxxxxxxxxxxx> Thanks! I just integrated this change with patch #1 so there is no need for you to repost the whole patchset. WRT patches #11-13: since they are quite straightforward I wonder whether it might make sense to push them through IDE tree (they depend on #1-10)? Jens, if you are fine with it I'll add your SOB and add them to the queue. Thanks, Bart -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
