diff mbox series

[v4,02/11] block: Call blkdev_dio_unaligned() from blkdev_direct_IO()

Message ID 20240219130109.341523-3-john.g.garry@oracle.com
State New
Headers show
Series block atomic writes | expand

Commit Message

John Garry Feb. 19, 2024, 1:01 p.m. UTC 
blkdev_dio_unaligned() is called from __blkdev_direct_IO(),
__blkdev_direct_IO_simple(), and __blkdev_direct_IO_async(), and all these
are only called from blkdev_direct_IO().

Move the blkdev_dio_unaligned() call to the common callsite,
blkdev_direct_IO().

Signed-off-by: John Garry <john.g.garry@oracle.com>
---
 block/fops.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

Comments

Keith Busch Feb. 19, 2024, 6:57 p.m. UTC | #1 
On Mon, Feb 19, 2024 at 01:01:00PM +0000, John Garry wrote:
> @@ -53,9 +53,6 @@ static ssize_t __blkdev_direct_IO_simple(struct kiocb *iocb,
>  	struct bio bio;
>  	ssize_t ret;
>  
> -	if (blkdev_dio_unaligned(bdev, pos, iter))
> -		return -EINVAL;
> -
>  	if (nr_pages <= DIO_INLINE_BIO_VECS)
>  		vecs = inline_vecs;
>  	else {
> @@ -171,9 +168,6 @@ static ssize_t __blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter,
>  	loff_t pos = iocb->ki_pos;
>  	int ret = 0;
>  
> -	if (blkdev_dio_unaligned(bdev, pos, iter))
> -		return -EINVAL;
> -
>  	if (iocb->ki_flags & IOCB_ALLOC_CACHE)
>  		opf |= REQ_ALLOC_CACHE;
>  	bio = bio_alloc_bioset(bdev, nr_pages, opf, GFP_KERNEL,
> @@ -310,9 +304,6 @@ static ssize_t __blkdev_direct_IO_async(struct kiocb *iocb,
>  	loff_t pos = iocb->ki_pos;
>  	int ret = 0;
>  
> -	if (blkdev_dio_unaligned(bdev, pos, iter))
> -		return -EINVAL;
> -
>  	if (iocb->ki_flags & IOCB_ALLOC_CACHE)
>  		opf |= REQ_ALLOC_CACHE;
>  	bio = bio_alloc_bioset(bdev, nr_pages, opf, GFP_KERNEL,
> @@ -365,11 +356,16 @@ static ssize_t __blkdev_direct_IO_async(struct kiocb *iocb,
>  
>  static ssize_t blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
>  {
> +	struct block_device *bdev = I_BDEV(iocb->ki_filp->f_mapping->host);
> +	loff_t pos = iocb->ki_pos;
>  	unsigned int nr_pages;

All three of the changed functions also want 'bdev' and 'pos', so maybe
pass on the savings to them? Unless you think the extended argument list
would harm readibilty, or perhaps the compiler optimizes the 2nd access
out anyway. Either way, this looks good to me.

Reviewed-by: Keith Busch <kbusch@kernel.org>
Christoph Hellwig Feb. 20, 2024, 6:54 a.m. UTC | #2 
Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>
John Garry Feb. 20, 2024, 8:31 a.m. UTC | #3 
On 19/02/2024 18:57, Keith Busch wrote:
> On Mon, Feb 19, 2024 at 01:01:00PM +0000, John Garry wrote:
>> @@ -53,9 +53,6 @@ static ssize_t __blkdev_direct_IO_simple(struct kiocb *iocb,
>>   	struct bio bio;
>>   	ssize_t ret;
>>   
>> -	if (blkdev_dio_unaligned(bdev, pos, iter))
>> -		return -EINVAL;
>> -
>>   	if (nr_pages <= DIO_INLINE_BIO_VECS)
>>   		vecs = inline_vecs;
>>   	else {
>> @@ -171,9 +168,6 @@ static ssize_t __blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter,
>>   	loff_t pos = iocb->ki_pos;
>>   	int ret = 0;
>>   
>> -	if (blkdev_dio_unaligned(bdev, pos, iter))
>> -		return -EINVAL;
>> -
>>   	if (iocb->ki_flags & IOCB_ALLOC_CACHE)
>>   		opf |= REQ_ALLOC_CACHE;
>>   	bio = bio_alloc_bioset(bdev, nr_pages, opf, GFP_KERNEL,
>> @@ -310,9 +304,6 @@ static ssize_t __blkdev_direct_IO_async(struct kiocb *iocb,
>>   	loff_t pos = iocb->ki_pos;
>>   	int ret = 0;
>>   
>> -	if (blkdev_dio_unaligned(bdev, pos, iter))
>> -		return -EINVAL;
>> -
>>   	if (iocb->ki_flags & IOCB_ALLOC_CACHE)
>>   		opf |= REQ_ALLOC_CACHE;
>>   	bio = bio_alloc_bioset(bdev, nr_pages, opf, GFP_KERNEL,
>> @@ -365,11 +356,16 @@ static ssize_t __blkdev_direct_IO_async(struct kiocb *iocb,
>>   
>>   static ssize_t blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
>>   {
>> +	struct block_device *bdev = I_BDEV(iocb->ki_filp->f_mapping->host);
>> +	loff_t pos = iocb->ki_pos;
>>   	unsigned int nr_pages;
> 
> All three of the changed functions also want 'bdev' and 'pos', so maybe
> pass on the savings to them? Unless you think the extended argument list
> would harm readibilty, or perhaps the compiler optimizes the 2nd access
> out anyway. Either way, this looks good to me.

Yeah, I was thinking about changing the arg lists. Specifically adding 
bdev, as that lookup takes many loads, so maybe I will make that change.

> 
> Reviewed-by: Keith Busch <kbusch@kernel.org>

cheers
diff mbox series

Patch

diff --git a/block/fops.c b/block/fops.c
index 0cf8cf72cdfa..28382b4d097a 100644
--- a/block/fops.c
+++ b/block/fops.c
@@ -53,9 +53,6 @@  static ssize_t __blkdev_direct_IO_simple(struct kiocb *iocb,
 	struct bio bio;
 	ssize_t ret;
 
-	if (blkdev_dio_unaligned(bdev, pos, iter))
-		return -EINVAL;
-
 	if (nr_pages <= DIO_INLINE_BIO_VECS)
 		vecs = inline_vecs;
 	else {
@@ -171,9 +168,6 @@  static ssize_t __blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter,
 	loff_t pos = iocb->ki_pos;
 	int ret = 0;
 
-	if (blkdev_dio_unaligned(bdev, pos, iter))
-		return -EINVAL;
-
 	if (iocb->ki_flags & IOCB_ALLOC_CACHE)
 		opf |= REQ_ALLOC_CACHE;
 	bio = bio_alloc_bioset(bdev, nr_pages, opf, GFP_KERNEL,
@@ -310,9 +304,6 @@  static ssize_t __blkdev_direct_IO_async(struct kiocb *iocb,
 	loff_t pos = iocb->ki_pos;
 	int ret = 0;
 
-	if (blkdev_dio_unaligned(bdev, pos, iter))
-		return -EINVAL;
-
 	if (iocb->ki_flags & IOCB_ALLOC_CACHE)
 		opf |= REQ_ALLOC_CACHE;
 	bio = bio_alloc_bioset(bdev, nr_pages, opf, GFP_KERNEL,
@@ -365,11 +356,16 @@  static ssize_t __blkdev_direct_IO_async(struct kiocb *iocb,
 
 static ssize_t blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
 {
+	struct block_device *bdev = I_BDEV(iocb->ki_filp->f_mapping->host);
+	loff_t pos = iocb->ki_pos;
 	unsigned int nr_pages;
 
 	if (!iov_iter_count(iter))
 		return 0;
 
+	if (blkdev_dio_unaligned(bdev, pos, iter))
+		return -EINVAL;
+
 	nr_pages = bio_iov_vecs_to_alloc(iter, BIO_MAX_VECS + 1);
 	if (likely(nr_pages <= BIO_MAX_VECS)) {
 		if (is_sync_kiocb(iocb))