All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@lst.de>
To: Johannes Thumshirn <Johannes.Thumshirn@wdc.com>
Cc: Christoph Hellwig <hch@lst.de>, Jens Axboe <axboe@kernel.dk>,
	"linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
	Damien Le Moal <Damien.LeMoal@wdc.com>,
	Naohiro Aota <Naohiro.Aota@wdc.com>
Subject: Re: [PATCH] block: support zone append bvecs
Date: Wed, 24 Mar 2021 08:11:19 +0100	[thread overview]
Message-ID: <20210324071119.GA647@lst.de> (raw)
In-Reply-To: <PH0PR04MB7416CEB0FE5E8E56370A0A1D9B639@PH0PR04MB7416.namprd04.prod.outlook.com>

On Wed, Mar 24, 2021 at 07:07:27AM +0000, Johannes Thumshirn wrote:
> >>  	if (iov_iter_is_bvec(iter)) {
> >> -		if (WARN_ON_ONCE(bio_op(bio) == REQ_OP_ZONE_APPEND))
> >> -			return -EINVAL;
> >> +		if (bio_op(bio) == REQ_OP_ZONE_APPEND) {
> >> +			struct request_queue *q = bio->bi_bdev->bd_disk->queue;
> >> +			unsigned int max_append =
> >> +				queue_max_zone_append_sectors(q) << 9;
> >> +
> >> +			if (WARN_ON_ONCE(iter->count > max_append))
> >> +				return -EINVAL;
> >> +		}
> > 
> > That is not correct.  bio_iov_iter_get_pages just fills the bio as far
> > as we can, and then returns 0 for the next call to continue.  Basically
> > what you want here is a partial version of bio_iov_bvec_set.
> > 
> 
> Isn't that what I did? The above is checking if we have REQ_OP_ZONE_APPEND and
> then returns EINVAL if iter->count is bigger than queue_max_zone_append_sectors().
> If the check doesn't fail, its going to call bio_iov_bvec_set().

And that is the problem.  It should not fail, the payload is decoupled
from the max_append size.

Doing the proper thing is not too hard as described above - make sure
the bi_iter points to only the chunk of iter passed in that fits, and
only advance the passed in iter by that amount.

  reply	other threads:[~2021-03-24  7:12 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-23 11:06 [PATCH] block: support zone append bvecs Johannes Thumshirn
2021-03-23 12:30 ` Christoph Hellwig
2021-03-24  7:07   ` Johannes Thumshirn
2021-03-24  7:11     ` Christoph Hellwig [this message]
2021-03-24  7:12       ` Johannes Thumshirn
2021-03-24 10:09         ` Johannes Thumshirn
2021-03-24 10:22           ` Damien Le Moal
2021-03-24 11:05           ` Christoph Hellwig
2021-03-24 12:12             ` Johannes Thumshirn

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=20210324071119.GA647@lst.de \
    --to=hch@lst.de \
    --cc=Damien.LeMoal@wdc.com \
    --cc=Johannes.Thumshirn@wdc.com \
    --cc=Naohiro.Aota@wdc.com \
    --cc=axboe@kernel.dk \
    --cc=linux-block@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.