Re: [PATCH 4/4] brd: Request from fdisk 4k alignment

[Ross Zwisler]

On Wed, 2014-08-06 at 14:35 +0300, Boaz Harrosh wrote:
> Because of the direct_access() API which returns a PFN. partitions
> better start on 4K boundary, else offset ZERO of a partition will
> not be aligned and blk_direct_access() will fail the call.
> 
> By setting blk_queue_physical_block_size(PAGE_SIZE) we can communicate
> this to fdisk and friends.
> Note that blk_queue_physical_block_size() also trashes io_min, but
> we can leave this one to be 512.
> 
> Signed-off-by: Boaz Harrosh <boaz@xxxxxxxxxxxxx>
> ---
>  drivers/block/brd.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/block/brd.c b/drivers/block/brd.c
> index 9673704..514cfe1 100644
> --- a/drivers/block/brd.c
> +++ b/drivers/block/brd.c
> @@ -495,10 +495,17 @@ static struct brd_device *brd_alloc(int i)
>  	brd->brd_queue = blk_alloc_queue(GFP_KERNEL);
>  	if (!brd->brd_queue)
>  		goto out_free_dev;
> +
>  	blk_queue_make_request(brd->brd_queue, brd_make_request);
>  	blk_queue_max_hw_sectors(brd->brd_queue, 1024);
>  	blk_queue_bounce_limit(brd->brd_queue, BLK_BOUNCE_ANY);
>  
> +	/* This is so fdisk will align partitions on 4k, because of
> +	 * direct_access API needing 4k alignment, returning a PFN
> +	 */
> +	blk_queue_physical_block_size(brd->brd_queue, PAGE_SIZE);
> +	brd->brd_queue->limits.io_min = 512; /* Don't use the accessor */
> +
>  	brd->brd_queue->limits.discard_granularity = PAGE_SIZE;
>  	brd->brd_queue->limits.max_discard_sectors = UINT_MAX;
>  	brd->brd_queue->limits.discard_zeroes_data = 1;

Is there an error case that this patch fixes?  I've had page alignment checks
in my PRD direct_access code forever, and I don't know if they've ever
tripped.  

Also, blk_queue_physical_block_size() seems wrong - here's the comment for
that function:

	/**
	 * blk_queue_physical_block_size - set physical block size for the queue
	 * @q:  the request queue for the device
	 * @size:  the physical block size, in bytes
	 *
	 * Description:
	 *   This should be set to the lowest possible sector size that the
	 *   hardware can operate on without reverting to read-modify-write
	 *   operations.
	 */

It doesn't sound like this is what you're after?  It sounds like instead you
want to control the alignment, not minimum natural I/O size?  It seems like if
you did want to do this, blk_queue_alignment_offset() would be more what you
were after?

- Ross


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/