Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH 1/4] loop: Refactor size calculation.
To:     Martijn Coenen <maco@android.com>, axboe@kernel.dk, hch@lst.de,
        ming.lei@redhat.com
Cc:     narayan@google.com, zezeozue@google.com, kernel-team@android.com,
        linux-block@vger.kernel.org, linux-kernel@vger.kernel.org,
        maco@google.com, Chaitanya.Kulkarni@wdc.com,
        Jaegeuk Kim <jaegeuk@kernel.org>
References: <20200420080409.111693-1-maco@android.com>
 <20200420080409.111693-2-maco@android.com>
From:   Bart Van Assche <bvanassche@acm.org>
Message-ID: <b546eff4-616a-8488-fc11-9b7e23d44bcf@acm.org>
Date:   Mon, 20 Apr 2020 06:22:12 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.6.0
MIME-Version: 1.0
In-Reply-To: <20200420080409.111693-2-maco@android.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 4/20/20 1:04 AM, Martijn Coenen wrote:
> +/**
> + * loop_set_size - sets device size and notifies userspace
> + * @lo: struct loop_device to set the size for
> + * @size: new size of the loop device
> + *
> + * Callers must validate that the size passed into this function fits into
> + * a sector_t.
> + */
> +static void loop_set_size(struct loop_device *lo, loff_t size)
> +{
> +	struct block_device *bdev = lo->lo_device;
> +
> +	set_capacity(lo->lo_disk, size);
> +	bd_set_size(bdev, size << 9);
> +	/* let user-space know about the new size */
> +	kobject_uevent(&disk_to_dev(bdev->bd_disk)->kobj, KOBJ_CHANGE);
> +}

How about using the SECTOR_SHIFT constant instead of "9"? I think the 
following has been added recently in include/linux/blkdev.h:

#define SECTOR_SHIFT 9

> @@ -1295,6 +1321,15 @@ loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
>   	/* I/O need to be drained during transfer transition */
>   	blk_mq_freeze_queue(lo->lo_queue);
>   
> +	if (size_changed && lo->lo_device->bd_inode->i_mapping->nrpages) {
> +		/* kill_bdev should have truncated all the pages */
> +		err = -EAGAIN;
> +		pr_warn("%s: loop%d (%s) has still dirty pages (nrpages=%lu)\n",
> +			__func__, lo->lo_number, lo->lo_file_name,
> +			lo->lo_device->bd_inode->i_mapping->nrpages);
> +		goto out_unfreeze;
> +	}
> +
>   	err = loop_release_xfer(lo);
>   	if (err)
>   		goto out_unfreeze;

Please Cc Jaegeuk for any changes in this code (see also 
https://lore.kernel.org/linux-block/20190518005304.GA19446@jaegeuk-macbookpro.roam.corp.google.com/).

Please also change the "kill_bdev should have truncated all the pages" 
comment into something like "return -EAGAIN if any pages have been 
dirtied after kill_bdev() returned".

Thanks,

Bart.