All of lore.kernel.org
 help / color / mirror / Atom feed
From: "hch@lst.de" <hch@lst.de>
To: Chaitanya Kulkarni <Chaitanya.Kulkarni@wdc.com>
Cc: "hch@infradead.org" <hch@infradead.org>,
	"linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
	"linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org>,
	"sagi@grimberg.me" <sagi@grimberg.me>, "hch@lst.de" <hch@lst.de>,
	Damien Le Moal <Damien.LeMoal@wdc.com>,
	Johannes Thumshirn <Johannes.Thumshirn@wdc.com>
Subject: Re: [PATCH V4 1/9] block: allow bvec for zone append get pages
Date: Fri, 4 Dec 2020 09:46:34 +0100	[thread overview]
Message-ID: <20201204084634.GA5845@lst.de> (raw)
In-Reply-To: <BYAPR04MB49652F70D16357D420E616AE86F10@BYAPR04MB4965.namprd04.prod.outlook.com>

On Fri, Dec 04, 2020 at 02:43:10AM +0000, Chaitanya Kulkarni wrote:
> >> Remove the bvec check in the bio_iov_iter_get_pages() for
> >> REQ_OP_ZONE_APPEND so that we can reuse the code and build iter from
> >> bvec.
> > We should do the same optimization for bvec pages that the normal path
> > does.  That being said using bio_iov_iter_get_pages in nvmet does not
> > 	make any sense to me whatsover.
> >
> Are you referring to the inline bvec ? then yes, I'll add it in next
> version.
> 
> I did not understand bio_iov_iter_get_pages() comment though.
> 
> Reimplementing the bio loop over sg with the use of bio_add_hw_page() seems
> 
> repetition of the code which we already have in bio_iov_iter_get_pages().
> 
> 
> Can you please explain why bio_iov_iter_get_pages() not the right way ?

bio_iov_iter_get_pages is highly inefficient for this use case, as we'll
need to allocate two sets of bio_vecs.  It also is rather redundant as
Zone Append should be able to just largely reuse the write path.

WARNING: multiple messages have this Message-ID (diff)
From: "hch@lst.de" <hch@lst.de>
To: Chaitanya Kulkarni <Chaitanya.Kulkarni@wdc.com>
Cc: "linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
	Damien Le Moal <Damien.LeMoal@wdc.com>,
	"sagi@grimberg.me" <sagi@grimberg.me>,
	Johannes Thumshirn <Johannes.Thumshirn@wdc.com>,
	"linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org>,
	"hch@infradead.org" <hch@infradead.org>,
	"hch@lst.de" <hch@lst.de>
Subject: Re: [PATCH V4 1/9] block: allow bvec for zone append get pages
Date: Fri, 4 Dec 2020 09:46:34 +0100	[thread overview]
Message-ID: <20201204084634.GA5845@lst.de> (raw)
In-Reply-To: <BYAPR04MB49652F70D16357D420E616AE86F10@BYAPR04MB4965.namprd04.prod.outlook.com>

On Fri, Dec 04, 2020 at 02:43:10AM +0000, Chaitanya Kulkarni wrote:
> >> Remove the bvec check in the bio_iov_iter_get_pages() for
> >> REQ_OP_ZONE_APPEND so that we can reuse the code and build iter from
> >> bvec.
> > We should do the same optimization for bvec pages that the normal path
> > does.  That being said using bio_iov_iter_get_pages in nvmet does not
> > 	make any sense to me whatsover.
> >
> Are you referring to the inline bvec ? then yes, I'll add it in next
> version.
> 
> I did not understand bio_iov_iter_get_pages() comment though.
> 
> Reimplementing the bio loop over sg with the use of bio_add_hw_page() seems
> 
> repetition of the code which we already have in bio_iov_iter_get_pages().
> 
> 
> Can you please explain why bio_iov_iter_get_pages() not the right way ?

bio_iov_iter_get_pages is highly inefficient for this use case, as we'll
need to allocate two sets of bio_vecs.  It also is rather redundant as
Zone Append should be able to just largely reuse the write path.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

  reply	other threads:[~2020-12-04  8:47 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-02  6:22 [PATCH V4 0/9] nvmet: add ZBD backend support Chaitanya Kulkarni
2020-12-02  6:22 ` Chaitanya Kulkarni
2020-12-02  6:22 ` [PATCH V4 1/9] block: allow bvec for zone append get pages Chaitanya Kulkarni
2020-12-02  6:22   ` Chaitanya Kulkarni
2020-12-02  8:34   ` Johannes Thumshirn
2020-12-02  8:34     ` Johannes Thumshirn
2020-12-02  8:55   ` Christoph Hellwig
2020-12-02  8:55     ` Christoph Hellwig
2020-12-04  2:43     ` Chaitanya Kulkarni
2020-12-04  2:43       ` Chaitanya Kulkarni
2020-12-04  8:46       ` hch [this message]
2020-12-04  8:46         ` hch
2020-12-02  6:22 ` [PATCH V4 2/9] nvmet: add ZNS support for bdev-ns Chaitanya Kulkarni
2020-12-02  6:22   ` Chaitanya Kulkarni
2020-12-02  9:07   ` Christoph Hellwig
2020-12-02  9:07     ` Christoph Hellwig
2020-12-04  3:13     ` Chaitanya Kulkarni
2020-12-04  3:13       ` Chaitanya Kulkarni
2020-12-04  9:27       ` Christoph Hellwig
2020-12-04  9:27         ` Christoph Hellwig
2020-12-02  6:22 ` [PATCH V4 3/9] nvmet: trim down id-desclist to use req->ns Chaitanya Kulkarni
2020-12-02  6:22   ` Chaitanya Kulkarni
2020-12-02  9:08   ` Christoph Hellwig
2020-12-02  9:08     ` Christoph Hellwig
2020-12-04  3:14     ` Chaitanya Kulkarni
2020-12-04  3:14       ` Chaitanya Kulkarni
2020-12-02  6:22 ` [PATCH V4 4/9] nvmet: add NVME_CSI_ZNS in ns-desc for zbdev Chaitanya Kulkarni
2020-12-02  6:22   ` Chaitanya Kulkarni
2020-12-02  9:09   ` Christoph Hellwig
2020-12-02  9:09     ` Christoph Hellwig
2020-12-04  3:14     ` Chaitanya Kulkarni
2020-12-04  3:14       ` Chaitanya Kulkarni
2020-12-02  6:22 ` [PATCH V4 5/9] nvmet: add cns-cs-ctrl in id-ctrl for ZNS bdev Chaitanya Kulkarni
2020-12-02  6:22   ` Chaitanya Kulkarni
2020-12-02  9:10   ` Christoph Hellwig
2020-12-02  9:10     ` Christoph Hellwig
2020-12-04  3:20     ` Chaitanya Kulkarni
2020-12-04  3:20       ` Chaitanya Kulkarni
2020-12-04  9:28       ` Christoph Hellwig
2020-12-04  9:28         ` Christoph Hellwig
2020-12-02  6:22 ` [PATCH V4 6/9] nvmet: add cns-cs-ns " Chaitanya Kulkarni
2020-12-02  6:22   ` Chaitanya Kulkarni
2020-12-02  6:22 ` [PATCH V4 7/9] nvmet: add zns cmd effects to support zbdev Chaitanya Kulkarni
2020-12-02  6:22   ` Chaitanya Kulkarni
2020-12-02  6:22 ` [PATCH V4 8/9] nvmet: add zns bdev config support Chaitanya Kulkarni
2020-12-02  6:22   ` Chaitanya Kulkarni
2020-12-02  6:22 ` [PATCH V4 9/9] nvmet: add ZNS based I/O cmds handlers Chaitanya Kulkarni
2020-12-02  6:22   ` Chaitanya Kulkarni
2020-12-02  9:20 ` [PATCH V4 0/9] nvmet: add ZBD backend support Christoph Hellwig
2020-12-02  9:20   ` Christoph Hellwig
2020-12-10  3:07   ` Chaitanya Kulkarni
2020-12-10  3:07     ` Chaitanya Kulkarni
2020-12-10 15:15     ` Keith Busch
2020-12-10 15:15       ` Keith Busch

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=20201204084634.GA5845@lst.de \
    --to=hch@lst.de \
    --cc=Chaitanya.Kulkarni@wdc.com \
    --cc=Damien.LeMoal@wdc.com \
    --cc=Johannes.Thumshirn@wdc.com \
    --cc=hch@infradead.org \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-nvme@lists.infradead.org \
    --cc=sagi@grimberg.me \
    /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.