From: Bart Van Assche <bvanassche@acm.org>
To: Jaegeuk Kim <jaegeuk@kernel.org>, Jens Axboe <axboe@kernel.dk>
Cc: linux-kernel@vger.kernel.org, linux-block@vger.kernel.org
Subject: Re: [PATCH v3] loop: drop caches if offset or block_size are changed
Date: Wed, 09 Jan 2019 12:51:28 -0800 [thread overview]
Message-ID: <1547067088.83374.41.camel@acm.org> (raw)
In-Reply-To: <20181218224139.GB31062@jaegeuk-macbookpro.roam.corp.google.com>
On Tue, 2018-12-18 at 14:41 -0800, Jaegeuk Kim wrote:
> [ ... ]
Please post new versions of a patch as a new e-mail thread instead of
as a reply to a previous e-mail.
> [ ... ]
>
> if (lo->lo_offset != info->lo_offset ||
> lo->lo_sizelimit != info->lo_sizelimit) {
> + /* kill_bdev should have truncated all the pages */
> + if (lo->lo_device->bd_inode->i_mapping->nrpages) {
> + err = -EAGAIN;
> + goto exit;
> + }
Please add a pr_info() or pr_warn() statement here such that it becomes
easy for the user to figure out why EAGAIN has been returned.
> blk_mq_freeze_queue(lo->lo_queue);
>
> + /* kill_bdev should have truncated all the pages */
> + if (lo->lo_queue->limits.logical_block_size != arg &&
> + lo->lo_device->bd_inode->i_mapping->nrpages) {
> + err = -EAGAIN;
> + goto out;
> + }
Same comment here. Additionally, please consider renaming the "out" label
into "unfreeze" or so. I think that will make the use of label names more
consistent with the rest of the block layer. Once these two comments are
addressed, feel free to add:
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
prev parent reply other threads:[~2019-01-09 20:51 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-12-14 20:32 [PATCH] loop: drop caches if offset is changed Jaegeuk Kim
2018-12-17 19:42 ` [PATCH v2] loop: drop caches if offset or block_size are changed Jaegeuk Kim
2018-12-18 17:48 ` Jens Axboe
2018-12-18 22:41 ` [PATCH v3] " Jaegeuk Kim
2019-01-09 5:22 ` Jaegeuk Kim
2019-01-09 20:51 ` Bart Van Assche [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1547067088.83374.41.camel@acm.org \
--to=bvanassche@acm.org \
--cc=axboe@kernel.dk \
--cc=jaegeuk@kernel.org \
--cc=linux-block@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).