Re: [PATCH v9 21/25] block/rnbd: server: functionality for IO submission to file or block dev

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2020-02-21 02:47, Jack Wang wrote:
> +static struct bio *rnbd_bio_map_kern(struct request_queue *q, void *data,
> +				      struct bio_set *bs,
> +				      unsigned int len, gfp_t gfp_mask)
> +{
> +	unsigned long kaddr = (unsigned long)data;
> +	unsigned long end = (kaddr + len + PAGE_SIZE - 1) >> PAGE_SHIFT;
> +	unsigned long start = kaddr >> PAGE_SHIFT;
> +	const int nr_pages = end - start;
> +	int offset, i;
> +	struct bio *bio;
> +
> +	bio = bio_alloc_bioset(gfp_mask, nr_pages, bs);
> +	if (!bio)
> +		return ERR_PTR(-ENOMEM);
> +
> +	offset = offset_in_page(kaddr);
> +	for (i = 0; i < nr_pages; i++) {
> +		unsigned int bytes = PAGE_SIZE - offset;
> +
> +		if (len <= 0)
> +			break;
> +
> +		if (bytes > len)
> +			bytes = len;
> +
> +		if (bio_add_pc_page(q, bio, virt_to_page(data), bytes,
> +				    offset) < bytes) {
> +			/* we don't support partial mappings */
> +			bio_put(bio);
> +			return ERR_PTR(-EINVAL);
> +		}
> +
> +		data += bytes;
> +		len -= bytes;
> +		offset = 0;
> +	}
> +
> +	bio->bi_end_io = bio_put;
> +	return bio;
> +}

The above function is almost identical to bio_map_kern(). Please find a
way to prevent such code duplication.

> +static inline int rnbd_dev_get_logical_bsize(const struct rnbd_dev *dev)
> +{
> +	return bdev_logical_block_size(dev->bdev);
> +}
> +
> +static inline int rnbd_dev_get_phys_bsize(const struct rnbd_dev *dev)
> +{
> +	return bdev_physical_block_size(dev->bdev);
> +}
> +
> +static inline int
> +rnbd_dev_get_max_write_same_sects(const struct rnbd_dev *dev)
> +{
> +	return bdev_write_same(dev->bdev);
> +}

Are you sure that the above functions are useful? Please do not
introduce inline functions for well known functions, especially if their
function name is longer than their implementation.

Thanks,

Bart.



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux