From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752830AbbKKWId (ORCPT ); Wed, 11 Nov 2015 17:08:33 -0500 Received: from static.68.134.40.188.clients.your-server.de ([188.40.134.68]:43628 "EHLO mail02.iobjects.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752404AbbKKWIc (ORCPT ); Wed, 11 Nov 2015 17:08:32 -0500 Subject: Re: [PATCH] loop: properly observe rotational flag of underlying device To: Jens Axboe , LKML References: <56435D0F.80006@googlemail.com> <5643B341.9010600@fb.com> From: =?UTF-8?Q?Holger_Hoffst=c3=a4tte?= X-Enigmail-Draft-Status: N1110 Organization: Applied Asynchrony, Inc. Message-ID: <5643BC5E.8060701@googlemail.com> Date: Wed, 11 Nov 2015 23:08:30 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: <5643B341.9010600@fb.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 >> --- >> drivers/block/loop.c | 19 +++++++++++++++++++ >> 1 file changed, 19 insertions(+) >> >> diff --git a/drivers/block/loop.c b/drivers/block/loop.c >> index 423f4ca..2984aca 100644 >> --- a/drivers/block/loop.c >> +++ b/drivers/block/loop.c >> @@ -843,6 +843,24 @@ static void loop_config_discard(struct loop_device *lo) >> queue_flag_set_unlocked(QUEUE_FLAG_DISCARD, q); >> } >> >> +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) >> + queue_flag_set_unlocked(QUEUE_FLAG_NONROT, q); >> + else >> + queue_flag_clear_unlocked(QUEUE_FLAG_NONROT, q); >> +} > > Are we sure we want to change the default from rot to nonrot? Well, that's why I asked for a better way to identify tmpfs. It took me several hours to figure out that tmpfs doesn't have an s_bdev, and could not find a better way than to assume that a superblock without backing device is probably something virtual/nonrotational/nvm etc. Alternatively I could look at sb->s_type and set nonrot for known fs types, but that seemed too ugly - not to mention conceptually weird. > Apart from that, looks good. phew :) thanks, Holger