On Tue, Dec 04 2007, Kiyoshi Ueda wrote:
> Hi Boaz and Jens,
>
> On Tue, 04 Dec 2007 15:56:32 +0200, Boaz Harrosh <[email protected]> wrote:
> > > +/**
> > > + * blk_end_request - Helper function for drivers to complete the request.
> > > + * @rq: the request being processed
> > > + * @uptodate: 1 for success, 0 for I/O error, < 0 for specific error
> > > + * @nr_bytes: number of bytes to complete
> > > + *
> > > + * Description:
> > > + * Ends I/O on a number of bytes attached to @rq.
> > > + * If @rq has leftover, sets it up for the next range of segments.
> > > + *
> > > + * Return:
> > > + * 0 - we are done with this request
> > > + * 1 - still buffers pending for this request
> > > + **/
> > > +int blk_end_request(struct request *rq, int uptodate, int nr_bytes)
> >
> > I always hated that uptodate boolean with possible negative error value.
> > I guess it was done for backward compatibility of then users of
> > end_that_request_first(). But since you are introducing a new API then
> > this is not the case. Just have regular status int where 0 means ALL_OK
> > and negative value means error code.
> > Just my $0.02.
>
> Thank you for the comment.
> I think it's quite reasonable.
> By doing that, we don't need end_io_error() anymore.
>
>
> Jens,
> What do you think?
I agree, the uptodate usage right now is horrible.
> If you agree with the interface change above, I would prefer to
> separate the patch-set from blk_end_request patch-set like below:
> o blk_end_request: remove end_that_request_*
> o change interface of 'uptodate' in blk_end_request to 'error'
> It makes the purpose of blk_end_request patch-set clear
> (and also, each patch of device drivers could be smaller).
> But, it doubles your merging work. So if you would like to get
> the changes at once, I'll merge them into blk_end_request patch-set.
Twice the merging is not an issue for me.
> As for the patch inclusion, do you push the driver changes to Linus
> all at once? Or should I ask each maintainer to take the patch?
Lets just try to get as many maintainer acks as possible, since the
patches need to go in together.
--
Jens Axboe
--
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]
