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

[John Garry <john.g.garry@oracle.com>]

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