On Sun, 27 Apr 2008 11:26:20 +0300 Boaz Harrosh <bharrosh@xxxxxxxxxxx> wrote: > On Fri, Apr 25 2008 at 13:03 +0300, FUJITA Tomonori <fujita.tomonori@xxxxxxxxxxxxx> wrote: > > On Fri, 25 Apr 2008 11:31:41 +0200 > > Jens Axboe <jens.axboe@xxxxxxxxxx> wrote: > > > >> On Fri, Apr 25 2008, FUJITA Tomonori wrote: > >>> On Fri, 25 Apr 2008 11:22:04 +0200 > >>> Jens Axboe <jens.axboe@xxxxxxxxxx> wrote: > >>> > >>>> On Fri, Apr 25 2008, FUJITA Tomonori wrote: > >>>>> On Thu, 24 Apr 2008 12:49:30 +0200 > >>>>> Jens Axboe <jens.axboe@xxxxxxxxxx> wrote: > >>>>> > >>>>>> On Thu, Apr 24 2008, FUJITA Tomonori wrote: > >>>>>>> On Thu, 24 Apr 2008 13:31:21 +0900 > >>>>>>> FUJITA Tomonori <fujita.tomonori@xxxxxxxxxxxxx> wrote: > >>>>>>> > >>>>>>>> On Wed, 23 Apr 2008 17:50:42 +0300 > >>>>>>>> Boaz Harrosh <bharrosh@xxxxxxxxxxx> wrote: > >>>>>>>> > >>>>>>>>> The support for large commands was dropped from the for-2.6.26 branch > >>>>>>>>> and will probably not get accepted into next kernel. > >>>>>>>>> > >>>>>>>>> I have tried to take all comments from Jens and Bart. and incorporate > >>>>>>>>> it into a new patchset. This is basically Tomo's patchset but with > >>>>>>>>> proposed changes. > >>>>>>>> Have you seen the patchset to remove request on the stack? > >>>>>>>> > >>>>>>>> http://marc.info/?l=linux-ide&m=120882410712466&w=2 > >>>>>>>> > >>>>>>>> > >>>>>>>>> They are based on current linux-block/master. They will probably conflict with > >>>>>>>>> latest patch sent by Tomo for the blk_get_request(). Once those patches > >>>>>>>>> get accepted at some git tree, (Where will that be?), I will rebase these > >>>>>>>>> on top of them. Please CC me of any progress. > >>>>>>>>> > >>>>>>>>> [PATCH 1/5] block: no need to initialize rq->cmd > >>>>>>>>> This is 2 of Tomo's patches squashed together as they are > >>>>>>>>> small and do the same. Tomo is this OK? > >>>>>>>>> > >>>>>>>>> [PATCH 2/5] block: replace sizeof(rq->cmd) with BLK_MAX_CDB > >>>>>>>>> Tomos patch rebased to here > >>>>>>>>> > >>>>>>>>> [PATCH 3/5] block: Export rq_init, rename to blk_init_rq > >>>>>>>>> [PATCH 4/5] block: Use new blk_init_rq > >>>>>>>>> These patches are basically what Jens and Bart has suggested, that with > >>>>>>>>> a small code change to blk-core.c we can memset at rq_init() and only set > >>>>>>>>> none zero members. We can also export that initializer and use it all over > >>>>>>>>> the ide tree where ever requests don't come from a request queue. (OK also > >>>>>>>>> at scsi_error.c) > >>>>>>>> +void blk_init_rq(struct request_queue *q, struct request *rq, int cmd_flags) > >>>>>>>> > >>>>>>>> Hmm, would it be better to modify the block layer to let rq_init just > >>>>>>>> memset() the request structure? > >>>>>>> I think, if we move rq_init to blk_alloc_request from get_request, > >>>>>>> rq_init can just memset() the structure. > >>>>>>> > >>>>>>> Then we can export rq_init and rq_init works for everyone. > >>>>>> Wont work, see the io scheduler set_request() functions. > >>>>> Sorry, can you be more specific? > >>>>> > >>>>> Only cfq uses set_request for now. cfq_set_request uses rq->cmd_flags > >>>>> and stores information at rq->elevator_private and > >>>>> rq->elevator_private2. > >>>>> > >>>>> The patch does memset() the request structure and sets up > >>>>> rq->cmd_flags, and then elv_set_request. Doesn't it work? > >>>> Sorry, with the moved rq_init() it should work, didn't look closely > >>>> enough. > >>> No problem. > >>> > >>> So are you ok with the patch? If so, I'll re-send it with a proper > >>> description and the signed-off. > >> Please do - I actually already merged it, but do resend with a full > >> description and signed-off etc. > > > > I just stole your description and added my signed-off. > > > > Will you merge the large command support for 2.6.26? Or only this > > clean-up patch? > > > > = > > From: FUJITA Tomonori <fujita.tomonori@xxxxxxxxxxxxx> > > Subject: [PATCH] block: make rq_init() do a full memset() > > > > This requires moving rq_init() from get_request() to blk_alloc_request(). > > The upside is that we can now require an rq_init() from any path that > > wishes to hand the request to the block layer. > > > > rq_init() will be exported for the code that uses struct request > > without blk_get_request. > > > > This is a preparation for large command support, which needs to > > initialize struct request in a proper way (that is, just doing a > > memset() will not work). > > > > Signed-off-by: FUJITA Tomonori <fujita.tomonori@xxxxxxxxxxxxx> > > Signed-off-by: Jens Axboe <jens.axboe@xxxxxxxxxx> > > --- > <snip> > > Sorry for the late response. Those hebrew holidays on they way of Linux > coding ;-). > > I don't mind as long as these things get accepted. But how is that any > different then the patch I sent? > [PATCH 3/5] block: Export rq_init, rename to blk_init_rq > It does exactly 100% the same move of rq_init to blk_alloc_request and the memset and > all, Have you looked at the patches at all? I feel like a mute person ;-( No, it's not same at all. Please look at the patchset again. Your interface is: +void blk_init_rq(struct request_queue *q, struct request *rq, int cmd_flags) I don't like that. It's hacky. You needed that hacky inferface since you didn't fix the root problem, rq_init was not able to do a full memset(). I fixed the root problem with this patch: http://marc.info/?l=linux-scsi&m=120901807514386&w=2 Then, we have the same interface as before: +void blk_rq_init(struct request_queue *q, struct request *rq) > If you are going to export the rq_init function then I think the name is very > wrong. And you have not exported it? It was renamed blk_rq_init. -- 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
