[PATCH] loop: properly observe rotational flag of underlying device

[PATCH] loop: properly observe rotational flag of underlying device

[Benjamin Gordon]

From: Holger Hoffstätte <holger.hoffstaette@googlemail.com>

The loop driver always declares the rotational flag of its device as
rotational, even when the device of the mapped file is nonrotational,
as is the case with SSDs or on tmpfs. This can confuse filesystem tools
which are SSD-aware; in my case I frequently forget to tell mkfs.btrfs
that my loop device on tmpfs is nonrotational, and that I really don't
need any automatic metadata redundancy.

The attached patch fixes this by introspecting the rotational flag of the
mapped file's underlying block device, if it exists. If the mapped file's
filesystem has no associated block device - as is the case on e.g. tmpfs -
we assume nonrotational storage. If there is a better way to identify such
non-devices I'd love to hear them.

Cc: Jens Axboe <axboe@kernel.dk>
Cc: linux-block@vger.kernel.org
Cc: holger@applied-asynchrony.com
Signed-off-by: Holger Hoffstätte <holger.hoffstaette@googlemail.com>
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
Signed-off-by: Benjamin Gordon <bmgordon@chromium.org>
Reviewed-by: Guenter Roeck <groeck@chromium.org>
---
 This is a resend of Holger's original patch from
 https://lkml.org/lkml/2015/11/11/288 with the _unlocked functions
 updated.  We keep running into the same problem on Chrome OS that this
 originally solved; any chance it can go in?

 drivers/block/loop.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index cf5538942834..6c0fc0d49dc0 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -900,6 +900,24 @@ static int loop_prepare_queue(struct loop_device *lo)
 	return 0;
 }
 
+static void loop_update_rotational(struct loop_device *lo)
+{
+	struct file *file = lo->lo_backing_file;
+	struct inode *file_inode = file->f_mapping->host;
+	struct block_device *file_bdev = file_inode->i_sb->s_bdev;
+	struct request_queue *q = lo->lo_queue;
+	bool nonrot = true;
+
+	/* not all filesystems (e.g. tmpfs) have a sb->s_bdev */
+	if (file_bdev)
+		nonrot = blk_queue_nonrot(bdev_get_queue(file_bdev));
+
+	if (nonrot)
+		blk_queue_flag_set(QUEUE_FLAG_NONROT, q);
+	else
+		blk_queue_flag_clear(QUEUE_FLAG_NONROT, q);
+}
+
 static int loop_set_fd(struct loop_device *lo, fmode_t mode,
 		       struct block_device *bdev, unsigned int arg)
 {
@@ -963,6 +981,7 @@ static int loop_set_fd(struct loop_device *lo, fmode_t mode,
 	if (!(lo_flags & LO_FLAGS_READ_ONLY) && file->f_op->fsync)
 		blk_queue_write_cache(lo->lo_queue, true, false);
 
+	loop_update_rotational(lo);
 	loop_update_dio(lo);
 	set_capacity(lo->lo_disk, size);
 	bd_set_size(bdev, size << 9);
-- 
2.20.1.791.gb4d0f1c61a-goog

Re: [PATCH] loop: properly observe rotational flag of underlying device

[Holger Hoffstätte]

Ping! Jens, can we please let this finally land in 5.2?

thanks,
Holger

On 2/12/19 11:54 PM, Benjamin Gordon wrote:
> From: Holger Hoffstätte <holger.hoffstaette@googlemail.com>
> 
> The loop driver always declares the rotational flag of its device as
> rotational, even when the device of the mapped file is nonrotational,
> as is the case with SSDs or on tmpfs. This can confuse filesystem tools
> which are SSD-aware; in my case I frequently forget to tell mkfs.btrfs
> that my loop device on tmpfs is nonrotational, and that I really don't
> need any automatic metadata redundancy.
> 
> The attached patch fixes this by introspecting the rotational flag of the
> mapped file's underlying block device, if it exists. If the mapped file's
> filesystem has no associated block device - as is the case on e.g. tmpfs -
> we assume nonrotational storage. If there is a better way to identify such
> non-devices I'd love to hear them.
> 
> Cc: Jens Axboe <axboe@kernel.dk>
> Cc: linux-block@vger.kernel.org
> Cc: holger@applied-asynchrony.com
> Signed-off-by: Holger Hoffstätte <holger.hoffstaette@googlemail.com>
> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
> Signed-off-by: Benjamin Gordon <bmgordon@chromium.org>
> Reviewed-by: Guenter Roeck <groeck@chromium.org>
> ---
>   This is a resend of Holger's original patch from
>   https://lkml.org/lkml/2015/11/11/288 with the _unlocked functions
>   updated.  We keep running into the same problem on Chrome OS that this
>   originally solved; any chance it can go in?
> 
>   drivers/block/loop.c | 19 +++++++++++++++++++
>   1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
> index cf5538942834..6c0fc0d49dc0 100644
> --- a/drivers/block/loop.c
> +++ b/drivers/block/loop.c
> @@ -900,6 +900,24 @@ static int loop_prepare_queue(struct loop_device *lo)
>   	return 0;
>   }
>   
> +static void loop_update_rotational(struct loop_device *lo)
> +{
> +	struct file *file = lo->lo_backing_file;
> +	struct inode *file_inode = file->f_mapping->host;
> +	struct block_device *file_bdev = file_inode->i_sb->s_bdev;
> +	struct request_queue *q = lo->lo_queue;
> +	bool nonrot = true;
> +
> +	/* not all filesystems (e.g. tmpfs) have a sb->s_bdev */
> +	if (file_bdev)
> +		nonrot = blk_queue_nonrot(bdev_get_queue(file_bdev));
> +
> +	if (nonrot)
> +		blk_queue_flag_set(QUEUE_FLAG_NONROT, q);
> +	else
> +		blk_queue_flag_clear(QUEUE_FLAG_NONROT, q);
> +}
> +
>   static int loop_set_fd(struct loop_device *lo, fmode_t mode,
>   		       struct block_device *bdev, unsigned int arg)
>   {
> @@ -963,6 +981,7 @@ static int loop_set_fd(struct loop_device *lo, fmode_t mode,
>   	if (!(lo_flags & LO_FLAGS_READ_ONLY) && file->f_op->fsync)
>   		blk_queue_write_cache(lo->lo_queue, true, false);
>   
> +	loop_update_rotational(lo);
>   	loop_update_dio(lo);
>   	set_capacity(lo->lo_disk, size);
>   	bd_set_size(bdev, size << 9);
> 

Re: [PATCH] loop: properly observe rotational flag of underlying device

[Jens Axboe]

On 3/26/19 11:04 AM, Benjamin Gordon wrote:
> On Tue, Mar 26, 2019 at 10:55 AM Holger Hoffstätte <holger@applied-asynchrony.com <mailto:holger@applied-asynchrony.com>> wrote:
> 
> 
>     Ping! Jens, can we please let this finally land in 5.2?
> 
> 
> Yes, please!  I've just double-checked and this patch still applies cleanly.

I've added it for 5.2, thanks everyone.

-- 
Jens Axboe

Re: [PATCH] loop: properly observe rotational flag of underlying device

[Holger Hoffstätte]

[cc: linux-block]

On 05/12/16 22:28, gwendal grignou wrote:
> Holger Hoffstätte <holger.hoffstaette <at> googlemail.com> writes:
> 
>>
>> On 11/11/15 23:08, Holger Hoffstätte wrote:
>>> On 11/11/15 22:29, Jens Axboe wrote:
>>>> On 11/11/2015 08:21 AM, Holger Hoffstätte wrote:
>>>>>
>>>>> The loop driver always declares the rotational flag of its device as
>>>>> rotational, even when the device of the mapped file is nonrotational,
>>>>> as is the case with SSDs or on tmpfs. This can confuse filesystem 
> tools
>>>>> which are SSD-aware; in my case I frequently forget to tell 
> mkfs.btrfs
>>>>> that my loop device on tmpfs is nonrotational, and that I really 
> don't
>>>>> need any automatic metadata redundancy.
>>>>>
>>>>> The attached patch fixes this by introspecting the rotational flag of 
> the
>>>>> mapped file's underlying block device, if it exists. If the mapped 
> file's
>>>>> filesystem has no associated block device - as is the case on e.g. 
> tmpfs -
>>>>> we assume nonrotational storage. If there is a better way to identify 
> such
>>>>> non-devices I'd love to hear them.
>>>>>
>>>>> Signed-off-by: Holger Hoffstätte <holger.hoffstaette <at> 
> googlemail.com>
> 
>>
>> Jens,
>>
>> I haven't seen this merged in any trees yet and was wondering if there's
>> any chance to get this into 4.5? If there's something left to fix up 
> please
>> let me know.
>>
>> Thanks,
>> Holger
>>
>>
> This patch proved useful for ureadahead: when we use it on a loop device, 
> it would use the HDD method to place the data in cache using the pack 
> information instead of the SSD method.
> 
> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>

I had completely forgotten about this, and apparently so had Jens. ;)
Thanks for the feedback, glad to hear it is useful.

Jens, any objections to merge this for 4.7? It should still apply
cleanly. The original patch was at: https://lkml.org/lkml/2015/11/11/288

Thanks,
Holger