diff mbox

[v6,5/8] fs: xfs: replace BIO_MAX_SECTORS with BIO_MAX_PAGES

Message ID 1464615294-9946-6-git-send-email-ming.lei@canonical.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ming Lei May 30, 2016, 1:34 p.m. UTC 
BIO_MAX_PAGES is used as maximum count of bvecs, so
replace BIO_MAX_SECTORS with BIO_MAX_PAGES since
BIO_MAX_SECTORS is to be removed.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 fs/xfs/xfs_buf.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

Comments

Christoph Hellwig June 1, 2016, 1:48 p.m. UTC | #1 
On Mon, May 30, 2016 at 09:34:33PM +0800, Ming Lei wrote:
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index e71cfbd..e5d713b 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -1157,9 +1157,7 @@ xfs_buf_ioapply_map(
>  
>  next_chunk:
>  	atomic_inc(&bp->b_io_remaining);
> -	nr_pages = BIO_MAX_SECTORS >> (PAGE_SHIFT - BBSHIFT);
> -	if (nr_pages > total_nr_pages)
> -		nr_pages = total_nr_pages;
> +	nr_pages = min(total_nr_pages, BIO_MAX_PAGES);
>  
>  	bio = bio_alloc(GFP_NOIO, nr_pages);

While I think this is a useful cleanup on it's own I think
you'd make everyones life easier if bio_alloc simply clamped down
the passed nr_pages value to the maximum allowed.
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ming Lei June 2, 2016, 3:32 a.m. UTC | #2 
On Wed, Jun 1, 2016 at 9:48 PM, Christoph Hellwig <hch@infradead.org> wrote:
> On Mon, May 30, 2016 at 09:34:33PM +0800, Ming Lei wrote:
>> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
>> index e71cfbd..e5d713b 100644
>> --- a/fs/xfs/xfs_buf.c
>> +++ b/fs/xfs/xfs_buf.c
>> @@ -1157,9 +1157,7 @@ xfs_buf_ioapply_map(
>>
>>  next_chunk:
>>       atomic_inc(&bp->b_io_remaining);
>> -     nr_pages = BIO_MAX_SECTORS >> (PAGE_SHIFT - BBSHIFT);
>> -     if (nr_pages > total_nr_pages)
>> -             nr_pages = total_nr_pages;
>> +     nr_pages = min(total_nr_pages, BIO_MAX_PAGES);
>>
>>       bio = bio_alloc(GFP_NOIO, nr_pages);
>
> While I think this is a useful cleanup on it's own I think
> you'd make everyones life easier if bio_alloc simply clamped down
> the passed nr_pages value to the maximum allowed.

Yes, that looks a good cleanup, but need be careful because the
passed 'nr_pages' can be used after returning from bio_alloc() in
the current function, and it is easy to see this usage.

So we can do that in another patchset instead of this one.

Thanks,

> --
> To unsubscribe from this list: send the line "unsubscribe linux-block" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index e71cfbd..e5d713b 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -1157,9 +1157,7 @@  xfs_buf_ioapply_map(
 
 next_chunk:
 	atomic_inc(&bp->b_io_remaining);
-	nr_pages = BIO_MAX_SECTORS >> (PAGE_SHIFT - BBSHIFT);
-	if (nr_pages > total_nr_pages)
-		nr_pages = total_nr_pages;
+	nr_pages = min(total_nr_pages, BIO_MAX_PAGES);
 
 	bio = bio_alloc(GFP_NOIO, nr_pages);
 	bio->bi_bdev = bp->b_target->bt_bdev;