On 31/10/2020 04:40, Jens Axboe wrote: > On 10/30/20 7:51 AM, Naohiro Aota wrote: >> From: Johannes Thumshirn <johannes.thumshirn@xxxxxxx> >> >> Add bio_add_zone_append_page(), a wrapper around bio_add_hw_page() which >> is intended to be used by file systems that directly add pages to a bio >> instead of using bio_iov_iter_get_pages(). > > Not sure what this is for, since I'm only on one patch in the series... Sorry, we'll Cc you on the whole series. > >> +/** >> + * bio_add_zone_append_page - attempt to add page to zone-append bio >> + * @bio: destination bio >> + * @page: page to add >> + * @len: vec entry length >> + * @offset: vec entry offset >> + * >> + * Attempt to add a page to the bio_vec maplist of a bio that will be submitted >> + * for a zone-append request. This can fail for a number of reasons, such as the >> + * bio being full or the target block device is not a zoned block device or >> + * other limitations of the target block device. The target block device must >> + * allow bio's up to PAGE_SIZE, so it is always possible to add a single page >> + * to an empty bio. >> + */ > > This should include a > > Return value: > > section, explaining how it returns number of bytes added (and why 0 is thus > a failure case). I'll update the comment to include the return value. It was just a copy and paste of bio_add_page() and it didn't have the return value documented, so this is why I missed it here. I'll probably should document the existing functions as well. >> +int bio_add_zone_append_page(struct bio *bio, struct page *page, >> + unsigned int len, unsigned int offset) > > Should this return unsigned int? If not, how would it work if someone > asked for INT_MAX + 4k. > I don't think this is needed as we can't go over 2G anyways, can't we? bio_add_page() also returns an int and I wanted to have a consistent interface here.