[RFC PATCH 0/8] block: fix bio_add_XXX_page() return type

[RFC PATCH 0/8] block: fix bio_add_XXX_page() return type

[Chaitanya Kulkarni]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 1335 bytes --]

Hi,                                                                                 

The helper functions bio_add_XXX_page() returns the length which is
unsigned int but the return type of those functions is defined
as int instead of unsigned int.

This is an attempt to fix the return type of those functions
and few callers. There are many places where this fix is needed
in the callers, if this series makes it to the upstream I'll convert
those callers gradually.

Any feedback is welcome.

-ck

Chaitanya Kulkarni (8):
  block: fix return type of bio_add_hw_page()
  block: fix return type of bio_add_pc_page()
  block: fix return type of bio_add_zone_append_page
  block: fix return type of bio_add_page()
  lightnvm: fix variable type pblk-core
  pscsi: fix variable type pscsi_map_sg
  btrfs: fix variable type in btrfs_bio_add_page
  block: fix variable type for zero pages

 block/bio.c                        | 20 +++++++++++---------
 block/blk-lib.c                    |  2 +-
 block/blk.h                        |  7 ++++---
 drivers/lightnvm/pblk-core.c       |  3 ++-
 drivers/target/target_core_pscsi.c |  6 ++++--
 fs/btrfs/extent_io.c               |  2 +-
 include/linux/bio.h                | 11 ++++++-----
 7 files changed, 29 insertions(+), 22 deletions(-)

-- 
2.24.0

[RFC PATCH 1/8] block: fix return type of bio_add_hw_page()

[Chaitanya Kulkarni]

Fix bio_add_hw_page() return type to unsigned int as it returns the
length which is of type unsigned int and not int.

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
 block/bio.c | 7 ++++---
 block/blk.h | 7 ++++---
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/block/bio.c b/block/bio.c
index 44205dfb6b60..531dbf15c0d9 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -743,9 +743,10 @@ static bool bio_try_merge_hw_seg(struct request_queue *q, struct bio *bio,
  * Add a page to a bio while respecting the hardware max_sectors, max_segment
  * and gap limitations.
  */
-int bio_add_hw_page(struct request_queue *q, struct bio *bio,
-		struct page *page, unsigned int len, unsigned int offset,
-		unsigned int max_sectors, bool *same_page)
+unsigned int bio_add_hw_page(struct request_queue *q, struct bio *bio,
+			     struct page *page, unsigned int len,
+			     unsigned int offset, unsigned int max_sectors,
+			     bool *same_page)
 {
 	struct bio_vec *bvec;
 
diff --git a/block/blk.h b/block/blk.h
index 8b3591aee0a5..a8930ecd4a5e 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -355,8 +355,9 @@ int bdev_del_partition(struct block_device *bdev, int partno);
 int bdev_resize_partition(struct block_device *bdev, int partno,
 		sector_t start, sector_t length);
 
-int bio_add_hw_page(struct request_queue *q, struct bio *bio,
-		struct page *page, unsigned int len, unsigned int offset,
-		unsigned int max_sectors, bool *same_page);
+unsigned int bio_add_hw_page(struct request_queue *q, struct bio *bio,
+			     struct page *page, unsigned int len,
+			     unsigned int offset, unsigned int max_sectors,
+			     bool *same_page);
 
 #endif /* BLK_INTERNAL_H */
-- 
2.24.0

[RFC PATCH 2/8] block: fix return type of bio_add_pc_page()

[Chaitanya Kulkarni]

Fix bio_add_pc_page() return type to unsigned int as it returns the
length which is of type unsigned int and not int.

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
 block/bio.c         | 5 +++--
 include/linux/bio.h | 4 ++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/block/bio.c b/block/bio.c
index 531dbf15c0d9..803a0b5894bc 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -799,8 +799,9 @@ unsigned int bio_add_hw_page(struct request_queue *q, struct bio *bio,
  *
  * This should only be used by passthrough bios.
  */
-int bio_add_pc_page(struct request_queue *q, struct bio *bio,
-		struct page *page, unsigned int len, unsigned int offset)
+unsigned int bio_add_pc_page(struct request_queue *q, struct bio *bio,
+			     struct page *page, unsigned int len,
+			     unsigned int offset)
 {
 	bool same_page = false;
 	return bio_add_hw_page(q, bio, page, len, offset,
diff --git a/include/linux/bio.h b/include/linux/bio.h
index a0b4cfdf62a4..72b28d06064c 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -467,8 +467,8 @@ extern void bio_reset(struct bio *);
 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);
+unsigned 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,
-- 
2.24.0

[RFC PATCH 3/8] block: fix return type of bio_add_zone_append_page

[Chaitanya Kulkarni]

Fix bio_add_zone_append_page() return type to unsigned int as it
returns the length which is of type unsigned int and not int.

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
 block/bio.c         | 4 ++--
 include/linux/bio.h | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/block/bio.c b/block/bio.c
index 803a0b5894bc..bc0e6e93ed58 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -825,8 +825,8 @@ EXPORT_SYMBOL(bio_add_pc_page);
  *
  * Returns: number of bytes added to the bio, or 0 in case of a failure.
  */
-int bio_add_zone_append_page(struct bio *bio, struct page *page,
-			     unsigned int len, unsigned int offset)
+unsigned int bio_add_zone_append_page(struct bio *bio, struct page *page,
+				      unsigned int len, unsigned int offset)
 {
 	struct request_queue *q = bio->bi_bdev->bd_disk->queue;
 	bool same_page = false;
diff --git a/include/linux/bio.h b/include/linux/bio.h
index 72b28d06064c..548d6a3342c3 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -469,8 +469,8 @@ void bio_chain(struct bio *, struct bio *);
 extern int bio_add_page(struct bio *, struct page *, unsigned int,unsigned int);
 unsigned 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);
+unsigned 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,
-- 
2.24.0

[RFC PATCH 4/8] block: fix return type of bio_add_page()

[Chaitanya Kulkarni]

Fix bio_add_page() return type to unsigned int as it returns the
length which is of type unsigned int and not int.

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
 block/bio.c         | 4 ++--
 include/linux/bio.h | 3 ++-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/block/bio.c b/block/bio.c
index bc0e6e93ed58..346909d35bd1 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -921,8 +921,8 @@ EXPORT_SYMBOL_GPL(__bio_add_page);
  *	Attempt to add page(s) to the bio_vec maplist. This will only fail
  *	if either bio->bi_vcnt == bio->bi_max_vecs or it's a cloned bio.
  */
-int bio_add_page(struct bio *bio, struct page *page,
-		 unsigned int len, unsigned int offset)
+unsigned int bio_add_page(struct bio *bio, struct page *page,
+			  unsigned int len, unsigned int offset)
 {
 	bool same_page = false;
 
diff --git a/include/linux/bio.h b/include/linux/bio.h
index 548d6a3342c3..f4f0b19b2ef3 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -466,7 +466,8 @@ extern void bio_uninit(struct bio *);
 extern void bio_reset(struct bio *);
 void bio_chain(struct bio *, struct bio *);
 
-extern int bio_add_page(struct bio *, struct page *, unsigned int,unsigned int);
+unsigned int bio_add_page(struct bio *, struct page *, unsigned int,
+			   unsigned int);
 unsigned int bio_add_pc_page(struct request_queue *, struct bio *,
 			     struct page *, unsigned int, unsigned int);
 unsigned int bio_add_zone_append_page(struct bio *bio, struct page *page,
-- 
2.24.0

[RFC PATCH 5/8] lightnvm: fix variable type pblk-core

[Chaitanya Kulkarni]

Make variable ret unsigned int since bio_add_pc_page() now returns
unsigned int.

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
 drivers/lightnvm/pblk-core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c
index 33d39d3dd343..cf148cee99a1 100644
--- a/drivers/lightnvm/pblk-core.c
+++ b/drivers/lightnvm/pblk-core.c
@@ -341,7 +341,8 @@ int pblk_bio_add_pages(struct pblk *pblk, struct bio *bio, gfp_t flags,
 {
 	struct request_queue *q = pblk->dev->q;
 	struct page *page;
-	int i, ret;
+	unsigned int ret;
+	int i;
 
 	for (i = 0; i < nr_pages; i++) {
 		page = mempool_alloc(&pblk->page_bio_pool, flags);
-- 
2.24.0

[RFC PATCH 6/8] pscsi: fix variable type pscsi_map_sg

[Chaitanya Kulkarni]

Use ret variable of type unsigned int since bio_add_pc_page() now
returns unsigned int.

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
 drivers/target/target_core_pscsi.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/target/target_core_pscsi.c b/drivers/target/target_core_pscsi.c
index f2a11414366d..6087661a482a 100644
--- a/drivers/target/target_core_pscsi.c
+++ b/drivers/target/target_core_pscsi.c
@@ -875,6 +875,8 @@ pscsi_map_sg(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents,
 			goto fail;
 
 		if (len > 0 && data_len > 0) {
+			unsigned int ret;
+
 			bytes = min_t(unsigned int, len, PAGE_SIZE - off);
 			bytes = min(bytes, data_len);
 
@@ -900,11 +902,11 @@ pscsi_map_sg(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents,
 				" bio: %p page: %p len: %d off: %d\n", i, bio,
 				page, len, off);
 
-			rc = bio_add_pc_page(pdv->pdv_sd->request_queue,
+			ret = bio_add_pc_page(pdv->pdv_sd->request_queue,
 					bio, page, bytes, off);
 			pr_debug("PSCSI: bio->bi_vcnt: %d nr_vecs: %d\n",
 				bio_segments(bio), nr_vecs);
-			if (rc != bytes) {
+			if (ret != bytes) {
 				pr_debug("PSCSI: Reached bio->bi_vcnt max:"
 					" %d i: %d bio: %p, allocating another"
 					" bio\n", bio->bi_vcnt, i, bio);
-- 
2.24.0

[RFC PATCH 7/8] btrfs: fix variable type in btrfs_bio_add_page

[Chaitanya Kulkarni]

Make variable ret unsigned int since bio_add_zone_append_page() now
returns unsigned int.

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
 fs/btrfs/extent_io.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 074a78a202b8..6ef276a95237 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -3158,8 +3158,8 @@ static bool btrfs_bio_add_page(struct bio *bio, struct page *page,
 			       unsigned long bio_flags)
 {
 	const sector_t sector = disk_bytenr >> SECTOR_SHIFT;
+	unsigned int ret;
 	bool contig;
-	int ret;
 
 	if (prev_bio_flags != bio_flags)
 		return false;
-- 
2.24.0

[RFC PATCH 8/8] block: fix variable type for zero pages

[Chaitanya Kulkarni]

Make variable bi_size unsigned int since bio_add_page() now returns
unsigned int.

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
 block/blk-lib.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/blk-lib.c b/block/blk-lib.c
index 7b256131b20b..67062066c293 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -305,7 +305,7 @@ static int __blkdev_issue_zero_pages(struct block_device *bdev,
 {
 	struct request_queue *q = bdev_get_queue(bdev);
 	struct bio *bio = *biop;
-	int bi_size = 0;
+	unsigned int bi_size = 0;
 	unsigned int sz;
 
 	if (!q)
-- 
2.24.0

Re: [RFC PATCH 0/8] block: fix bio_add_XXX_page() return type

[Johannes Thumshirn]

On 20/05/2021 08:23, Chaitanya Kulkarni wrote:
> Hi,                                                                                 
> 
> The helper functions bio_add_XXX_page() returns the length which is
> unsigned int but the return type of those functions is defined
> as int instead of unsigned int.
> 
> This is an attempt to fix the return type of those functions
> and few callers. There are many places where this fix is needed
> in the callers, if this series makes it to the upstream I'll convert
> those callers gradually.
> 
> Any feedback is welcome.
> 
> -ck
> 
> Chaitanya Kulkarni (8):
>   block: fix return type of bio_add_hw_page()
>   block: fix return type of bio_add_pc_page()
>   block: fix return type of bio_add_zone_append_page
>   block: fix return type of bio_add_page()
>   lightnvm: fix variable type pblk-core
>   pscsi: fix variable type pscsi_map_sg
>   btrfs: fix variable type in btrfs_bio_add_page
>   block: fix variable type for zero pages
> 
>  block/bio.c                        | 20 +++++++++++---------
>  block/blk-lib.c                    |  2 +-
>  block/blk.h                        |  7 ++++---
>  drivers/lightnvm/pblk-core.c       |  3 ++-
>  drivers/target/target_core_pscsi.c |  6 ++++--
>  fs/btrfs/extent_io.c               |  2 +-
>  include/linux/bio.h                | 11 ++++++-----
>  7 files changed, 29 insertions(+), 22 deletions(-)
> 

I couldn't spot any errors, but I'm not sure it's worth the effort.

If Jens decides to take it:
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Re: [RFC PATCH 0/8] block: fix bio_add_XXX_page() return type

[Chaitanya Kulkarni]

On 5/21/21 03:25, Johannes Thumshirn wrote:
> I couldn't spot any errors, but I'm not sure it's worth the effort.
>
> If Jens decides to take it:
> Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
>

It does create confusion on the code level which can result in
invalid error checks.

Re: [RFC PATCH 0/8] block: fix bio_add_XXX_page() return type

[Omar Sandoval]

On Fri, May 21, 2021 at 09:37:43PM +0000, Chaitanya Kulkarni wrote:
> On 5/21/21 03:25, Johannes Thumshirn wrote:
> > I couldn't spot any errors, but I'm not sure it's worth the effort.
> >
> > If Jens decides to take it:
> > Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> >
> 
> It does create confusion on the code level which can result in
> invalid error checks.

Do you have any examples of bugs caused by this confusion (whether they
were fixed in the past, currently exist, or were caught in code review)?
That would be good justification for doing this.

Re: [RFC PATCH 0/8] block: fix bio_add_XXX_page() return type

[Chaitanya Kulkarni]

On 5/21/21 15:37, Omar Sandoval wrote:
> On Fri, May 21, 2021 at 09:37:43PM +0000, Chaitanya Kulkarni wrote:
>> On 5/21/21 03:25, Johannes Thumshirn wrote:
>>> I couldn't spot any errors, but I'm not sure it's worth the effort.
>>>
>>> If Jens decides to take it:
>>> Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
>>>
>> It does create confusion on the code level which can result in
>> invalid error checks.
> Do you have any examples of bugs caused by this confusion (whether they
> were fixed in the past, currently exist, or were caught in code review)?
> That would be good justification for doing this.
>

Yes, while implementing the ZBD over NVMeOF I accidentally ended up checking
the error value < 0 based on the function following declaration :-

  1 F   f    bio_add_zone_append_page  block/bio.c
  int bio_add_zone_append_page(struct bio *bio, struct page *page,

I did fix it later when doing the next code review myself, maybe that is
just
me :P.

Also, new functions inherit the same style such as
bio_add_zone_append_page() from bio_add_page() and bio_add_pc_page() we
can prevent that.

What you do you think about the approach suggested by Matthew  size_t ?


If it is too much trouble we can drop it.

Re: [RFC PATCH 0/8] block: fix bio_add_XXX_page() return type

[Matthew Wilcox]

On Wed, May 19, 2021 at 11:22:47PM -0700, Chaitanya Kulkarni wrote:
> The helper functions bio_add_XXX_page() returns the length which is
> unsigned int but the return type of those functions is defined
> as int instead of unsigned int.

I've been thinking about this for a few weeks as part of the folio
patches:

https://lore.kernel.org/linux-fsdevel/20210505150628.111735-72-willy@infradead.org/

 - len and off are measured in bytes
 - neither are permitted to be negative
 - for efficiency we only permit them to be up to 4GB

I therefore believe the correct type for these parameters to be size_t,
and we should range-check them if they're too large.  they should
actually always fit within the page that they're associated with, but
people do allocate non-compound pages and i'm not trying to break that
today.

using size_t makes it clear that these are byte counts, not (eg) sector
counts.  i do think it's good to make the return value unsigned so we
don't have people expecting a negative errno on failure.

Re: [RFC PATCH 0/8] block: fix bio_add_XXX_page() return type

[Chaitanya Kulkarni]

On 5/21/21 04:32, Matthew Wilcox wrote:
> On Wed, May 19, 2021 at 11:22:47PM -0700, Chaitanya Kulkarni wrote:
>> The helper functions bio_add_XXX_page() returns the length which is
>> unsigned int but the return type of those functions is defined
>> as int instead of unsigned int.
> I've been thinking about this for a few weeks as part of the folio
> patches:
>
> https://lore.kernel.org/linux-fsdevel/20210505150628.111735-72-willy@infradead.org/
>
>  - len and off are measured in bytes
>  - neither are permitted to be negative
>  - for efficiency we only permit them to be up to 4GB
>
> I therefore believe the correct type for these parameters to be size_t,
> and we should range-check them if they're too large.  they should
> actually always fit within the page that they're associated with, but
> people do allocate non-compound pages and i'm not trying to break that
> today.
>
> using size_t makes it clear that these are byte counts, not (eg) sector
> counts.  i do think it's good to make the return value unsigned so we
> don't have people expecting a negative errno on failure.
>

Sounds good, I'll wait for few days for the feedback from others and then
send out the V1 with your suggestions for using size_t and bounds check.

Re: [RFC PATCH 0/8] block: fix bio_add_XXX_page() return type

[Christoph Hellwig]

On Fri, May 21, 2021 at 12:30:45PM +0100, Matthew Wilcox wrote:
> On Wed, May 19, 2021 at 11:22:47PM -0700, Chaitanya Kulkarni wrote:
> > The helper functions bio_add_XXX_page() returns the length which is
> > unsigned int but the return type of those functions is defined
> > as int instead of unsigned int.
> 
> I've been thinking about this for a few weeks as part of the folio
> patches:
> 
> https://lore.kernel.org/linux-fsdevel/20210505150628.111735-72-willy@infradead.org/
> 
>  - len and off are measured in bytes
>  - neither are permitted to be negative
>  - for efficiency we only permit them to be up to 4GB
> 
> I therefore believe the correct type for these parameters to be size_t,
> and we should range-check them if they're too large.  they should
> actually always fit within the page that they're associated with, but
> people do allocate non-compound pages and i'm not trying to break that
> today.
> 
> using size_t makes it clear that these are byte counts, not (eg) sector
> counts.  i do think it's good to make the return value unsigned so we
> don't have people expecting a negative errno on failure.

I think the right type is bool.  We always return either 0 or the full
length we tried to add.  Instead of optimizing for a partial add (which
only makes sense for bio_add_hw_page anyway), I'd rather make the
interface as simple as possible.

Re: [RFC PATCH 0/8] block: fix bio_add_XXX_page() return type

[Matthew Wilcox]

On Mon, May 24, 2021 at 09:35:27AM +0200, Christoph Hellwig wrote:
> > using size_t makes it clear that these are byte counts, not (eg) sector
> > counts.  i do think it's good to make the return value unsigned so we
> > don't have people expecting a negative errno on failure.
> 
> I think the right type is bool.  We always return either 0 or the full
> length we tried to add.  Instead of optimizing for a partial add (which
> only makes sense for bio_add_hw_page anyway), I'd rather make the
> interface as simple as possible.

Sounds good to me.

Re: [RFC PATCH 0/8] block: fix bio_add_XXX_page() return type

[Chaitanya Kulkarni]

Christoph,

On 5/24/21 00:35, Christoph Hellwig wrote:
> On Fri, May 21, 2021 at 12:30:45PM +0100, Matthew Wilcox wrote:
>> On Wed, May 19, 2021 at 11:22:47PM -0700, Chaitanya Kulkarni wrote:
>>> The helper functions bio_add_XXX_page() returns the length which is
>>> unsigned int but the return type of those functions is defined
>>> as int instead of unsigned int.
>> I've been thinking about this for a few weeks as part of the folio
>> patches:
>>
>> https://lore.kernel.org/linux-fsdevel/20210505150628.111735-72-willy@infradead.org/
>>
>>  - len and off are measured in bytes
>>  - neither are permitted to be negative
>>  - for efficiency we only permit them to be up to 4GB
>>
>> I therefore believe the correct type for these parameters to be size_t,
>> and we should range-check them if they're too large.  they should
>> actually always fit within the page that they're associated with, but
>> people do allocate non-compound pages and i'm not trying to break that
>> today.
>>
>> using size_t makes it clear that these are byte counts, not (eg) sector
>> counts.  i do think it's good to make the return value unsigned so we
>> don't have people expecting a negative errno on failure.
> I think the right type is bool.  We always return either 0 or the full
> length we tried to add.  Instead of optimizing for a partial add (which
> only makes sense for bio_add_hw_page anyway), I'd rather make the
> interface as simple as possible.
>

Is above comment is on this series or on the API present in the folio
patches [1] ?

Since if we change the return type to bool for the functions in
question [2] in this series we also need to modify the callers, I'm not sure
that is worth it though.

Please confirm.

[1]
https://lore.kernel.org/linux-fsdevel/20210505150628.111735-72-willy@infradead.org/


[2]
bio_add_hw_page()
bio_add_pc_page()
bio_add_zone_append_page()
bio_add page()

Re: [RFC PATCH 0/8] block: fix bio_add_XXX_page() return type

[Christoph Hellwig]

On Wed, May 26, 2021 at 02:55:57AM +0000, Chaitanya Kulkarni wrote:
> Is above comment is on this series or on the API present in the folio
> patches [1] ?

All of the above.

> Since if we change the return type to bool for the functions in
> question [2] in this series we also need to modify the callers, I'm not sure
> that is worth it though.

Yes, all the caller would need to change as well.

Re: [RFC PATCH 0/8] block: fix bio_add_XXX_page() return type

[Chaitanya Kulkarni]

On 5/27/21 04:43, Christoph Hellwig wrote:
> On Wed, May 26, 2021 at 02:55:57AM +0000, Chaitanya Kulkarni wrote:
>> Is above comment is on this series or on the API present in the folio
>> patches [1] ?
> All of the above.
>
>> Since if we change the return type to bool for the functions in
>> question [2] in this series we also need to modify the callers, I'm not sure
>> that is worth it though.
> Yes, all the caller would need to change as well.
>

I see, thanks, I'll work on V1 with this approach.