| Message ID | d9a0a445560db3a9eb240c6535f8dd1bbd0abd96.1604065694.git.naohiro.aota@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: zoned block device support | expand |
On 10/30/20 7:51 AM, Naohiro Aota wrote: > From: Johannes Thumshirn <johannes.thumshirn@wdc.com> > > 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... > +/** > + * 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). > +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.
On Fri, Oct 30, 2020 at 09:40:08PM -0600, Jens Axboe wrote: >On 10/30/20 7:51 AM, Naohiro Aota wrote: >> From: Johannes Thumshirn <johannes.thumshirn@wdc.com> >> >> 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... I'm sorry for the missing context. This patch is for this series. https://lore.kernel.org/linux-btrfs/cover.1604065156.git.naohiro.aota@wdc.com/T/ This patch uses bio_add_zone_append_page in place of bio_add_page for zoned device. https://lore.kernel.org/linux-btrfs/cover.1604065156.git.naohiro.aota@wdc.com/T/#m88184d5dd11ac30c0878582898cd9d6f7cbc21fc > >> +/** >> + * 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). > >> +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. > >-- >Jens Axboe >
On 31/10/2020 04:40, Jens Axboe wrote: > On 10/30/20 7:51 AM, Naohiro Aota wrote: >> From: Johannes Thumshirn <johannes.thumshirn@wdc.com> >> >> 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.
diff --git a/block/bio.c b/block/bio.c index 58d765400226..2dfe40be4d6b 100644 --- a/block/bio.c +++ b/block/bio.c @@ -853,6 +853,42 @@ int bio_add_pc_page(struct request_queue *q, struct bio *bio, } EXPORT_SYMBOL(bio_add_pc_page); +/** + * 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. + */ +int bio_add_zone_append_page(struct bio *bio, struct page *page, + unsigned int len, unsigned int offset) +{ + struct request_queue *q; + bool same_page = false; + + if (WARN_ON_ONCE(bio_op(bio) != REQ_OP_ZONE_APPEND)) + return 0; + + if (WARN_ON_ONCE(!bio->bi_disk)) + return 0; + + q = bio->bi_disk->queue; + + if (WARN_ON_ONCE(!blk_queue_is_zoned(q))) + return 0; + + return bio_add_hw_page(q, bio, page, len, offset, + queue_max_zone_append_sectors(q), &same_page); +} +EXPORT_SYMBOL_GPL(bio_add_zone_append_page); + /** * __bio_try_merge_page - try appending data to an existing bvec. * @bio: destination bio diff --git a/include/linux/bio.h b/include/linux/bio.h index c6d765382926..7ef300cb4e9a 100644 --- a/include/linux/bio.h +++ b/include/linux/bio.h @@ -442,6 +442,8 @@ void bio_chain(struct bio *, struct bio *); extern int bio_add_page(struct bio *, struct page *, unsigned int,unsigned int); extern int bio_add_pc_page(struct request_queue *, struct bio *, struct page *, unsigned int, unsigned int); +int bio_add_zone_append_page(struct bio *bio, struct page *page, + unsigned int len, unsigned int offset); bool __bio_try_merge_page(struct bio *bio, struct page *page, unsigned int len, unsigned int off, bool *same_page); void __bio_add_page(struct bio *bio, struct page *page,