Re: [PATCH v8 2/2] loop: Better discard support for block devices

From: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: evgreen@chromium.org, asavery@chromium.org, axboe@kernel.dk,
	bvanassche@acm.org, darrick.wong@oracle.com,
	dianders@chromium.org, gwendal@chromium.org,
	linux-block@vger.kernel.org, martin.petersen@oracle.com,
	ming.lei@redhat.com, kernel@collabora.com
Subject: Re: [PATCH v8 2/2] loop: Better discard support for block devices
Date: Fri, 3 Apr 2020 12:27:26 +0200	[thread overview]
Message-ID: <49a20f42-f082-be3a-52f8-a4179ab886c0@collabora.com> (raw)
In-Reply-To: <20200403063955.GB28875@infradead.org>

Hi Christoph,

W dniu 03.04.2020 o 08:39, Christoph Hellwig pisze:
> On Thu, Apr 02, 2020 at 07:06:03PM +0200, Andrzej Pietrasiewicz wrote:
>> From: Evan Green <evgreen@chromium.org>
>>

<snip>

>> +	struct request_queue *backingq;
>> +
>> +	/*
>> +	 * If the backing device is a block device, mirror its zeroing
>> +	 * capability. Set the discard sectors to the block device's zeroing
>> +	 * capabilities because loop discards result in blkdev_issue_zeroout(),
>> +	 * not blkdev_issue_discard(). This maintains consistent behavior with
>> +	 * file-backed loop devices: discarded regions read back as zero.
>> +	 */
>> +	if (S_ISBLK(inode->i_mode) && !lo->lo_encrypt_key_size) {
>> +		backingq = bdev_get_queue(inode->i_bdev);
> 
> The backingq could move into this local scope.
> 
>> +	} else if ((!file->f_op->fallocate) || lo->lo_encrypt_key_size) {
> 
> No need for the inner braces.
> 
> But the actual functionality looks good to me.
> 

Would you A-b or R-b if I corrected the two small issues which you found?

Andrzej