From: Matias Bjorling <Matias.Bjorling@wdc.com> To: Bart Van Assche <bvanassche@acm.org>, "axboe@kernel.dk" <axboe@kernel.dk>, "kbusch@kernel.org" <kbusch@kernel.org>, "hch@lst.de" <hch@lst.de>, "sagi@grimberg.me" <sagi@grimberg.me>, "martin.petersen@oracle.com" <martin.petersen@oracle.com>, Damien Le Moal <Damien.LeMoal@wdc.com>, Niklas Cassel <Niklas.Cassel@wdc.com>, Hans Holmberg <Hans.Holmberg@wdc.com> Cc: "linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "linux-block@vger.kernel.org" <linux-block@vger.kernel.org>, "linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org> Subject: RE: [PATCH 2/2] block: add BLKSETDESCZONE ioctl for Zoned Block Devices Date: Tue, 30 Jun 2020 13:28:07 +0000 [thread overview] Message-ID: <MN2PR04MB6223940C9DEDCEB7209FB7D2F16F0@MN2PR04MB6223.namprd04.prod.outlook.com> (raw) In-Reply-To: <cbb91e7d-5cc8-eeb7-5219-b712545cb5c4@acm.org> > -----Original Message----- > From: Bart Van Assche <bvanassche@acm.org> > Sent: Monday, 29 June 2020 03.36 > To: Matias Bjorling <Matias.Bjorling@wdc.com>; axboe@kernel.dk; > kbusch@kernel.org; hch@lst.de; sagi@grimberg.me; > martin.petersen@oracle.com; Damien Le Moal <Damien.LeMoal@wdc.com>; > Niklas Cassel <Niklas.Cassel@wdc.com>; Hans Holmberg > <Hans.Holmberg@wdc.com> > Cc: linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org; linux- > block@vger.kernel.org; linux-nvme@lists.infradead.org > Subject: Re: [PATCH 2/2] block: add BLKSETDESCZONE ioctl for Zoned Block > Devices > > On 2020-06-28 16:01, Matias Bjørling wrote: > > + /* This may take a while, so be nice to others */ > > + cond_resched(); > > + > > + return submit_bio_wait(&bio); > > A cond_resched() call before a submit_bio_wait() call? I think it's the first time > that I see this. Is that call really necessary? Isn't the > wait_for_completion() call inside submit_bio_wait() sufficient? > One can't be too careful these days, heh. I'll fix it up. Thanks! > > + /* no flags is currently supported */ > ^^ > are? > > > + /* allocate the size of the zone descriptor extension and fill > > + * with the data in the user data buffer. If the data size is less > > + * than the zone descriptor extension size, then the rest of the > > + * zone description extension data buffer is zero-filled. > > + */ > > + zsd_data = (void *) get_zeroed_page(GFP_KERNEL); > > + if (!zsd_data) > > + return -ENOMEM; > > + > > + if (copy_from_user(zsd_data, argp + sizeof(struct blk_zone_set_desc), > > + zsd.len)) { > > + ret = -EFAULT; > > + goto free; > > + } > > Has it been considered to use kmalloc() instead of get_zeroed_page()? > > > diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h > > index ccb895f911b1..53b7b05b0004 100644 > > --- a/include/linux/blk_types.h > > +++ b/include/linux/blk_types.h > > @@ -316,6 +316,8 @@ enum req_opf { > > REQ_OP_ZONE_FINISH = 12, > > /* write data at the current zone write pointer */ > > REQ_OP_ZONE_APPEND = 13, > > + /* associate zone desc extension data to a zone */ > > + REQ_OP_ZONE_SET_DESC = 14, > > > > /* SCSI passthrough using struct scsi_request */ > > REQ_OP_SCSI_IN = 32, > > Does REQ_OP_ZONE_SET_DESC count as a read or as a write operation? See > also: > > static inline bool op_is_write(unsigned int op) { > return (op & 1); > } > > > +/** > > + * struct blk_zone_set_desc - BLKSETDESCZONE ioctl requests > > + * @sector: Starting sector of the zone to operate on. > > + * @flags: Feature flags. > > + * @len: size, in bytes, of the data to be associated to the zone. > > + * @data: data to be associated. > > + */ > > +struct blk_zone_set_desc { > > + __u64 sector; > > + __u32 flags; > > + __u32 len; > > + __u8 data[0]; > > +}; > > Isn't the recommended style to use a flexible array ([] instead of [0])? > See also > https://lore.kernel.org/lkml/20200608213711.GA22271@embeddedor/. > > Thanks, > > Bart.
WARNING: multiple messages have this Message-ID (diff)
From: Matias Bjorling <Matias.Bjorling@wdc.com> To: Bart Van Assche <bvanassche@acm.org>, "axboe@kernel.dk" <axboe@kernel.dk>, "kbusch@kernel.org" <kbusch@kernel.org>, "hch@lst.de" <hch@lst.de>, "sagi@grimberg.me" <sagi@grimberg.me>, "martin.petersen@oracle.com" <martin.petersen@oracle.com>, Damien Le Moal <Damien.LeMoal@wdc.com>, Niklas Cassel <Niklas.Cassel@wdc.com>, Hans Holmberg <Hans.Holmberg@wdc.com> Cc: "linux-block@vger.kernel.org" <linux-block@vger.kernel.org>, "linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org> Subject: RE: [PATCH 2/2] block: add BLKSETDESCZONE ioctl for Zoned Block Devices Date: Tue, 30 Jun 2020 13:28:07 +0000 [thread overview] Message-ID: <MN2PR04MB6223940C9DEDCEB7209FB7D2F16F0@MN2PR04MB6223.namprd04.prod.outlook.com> (raw) In-Reply-To: <cbb91e7d-5cc8-eeb7-5219-b712545cb5c4@acm.org> > -----Original Message----- > From: Bart Van Assche <bvanassche@acm.org> > Sent: Monday, 29 June 2020 03.36 > To: Matias Bjorling <Matias.Bjorling@wdc.com>; axboe@kernel.dk; > kbusch@kernel.org; hch@lst.de; sagi@grimberg.me; > martin.petersen@oracle.com; Damien Le Moal <Damien.LeMoal@wdc.com>; > Niklas Cassel <Niklas.Cassel@wdc.com>; Hans Holmberg > <Hans.Holmberg@wdc.com> > Cc: linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org; linux- > block@vger.kernel.org; linux-nvme@lists.infradead.org > Subject: Re: [PATCH 2/2] block: add BLKSETDESCZONE ioctl for Zoned Block > Devices > > On 2020-06-28 16:01, Matias Bjørling wrote: > > + /* This may take a while, so be nice to others */ > > + cond_resched(); > > + > > + return submit_bio_wait(&bio); > > A cond_resched() call before a submit_bio_wait() call? I think it's the first time > that I see this. Is that call really necessary? Isn't the > wait_for_completion() call inside submit_bio_wait() sufficient? > One can't be too careful these days, heh. I'll fix it up. Thanks! > > + /* no flags is currently supported */ > ^^ > are? > > > + /* allocate the size of the zone descriptor extension and fill > > + * with the data in the user data buffer. If the data size is less > > + * than the zone descriptor extension size, then the rest of the > > + * zone description extension data buffer is zero-filled. > > + */ > > + zsd_data = (void *) get_zeroed_page(GFP_KERNEL); > > + if (!zsd_data) > > + return -ENOMEM; > > + > > + if (copy_from_user(zsd_data, argp + sizeof(struct blk_zone_set_desc), > > + zsd.len)) { > > + ret = -EFAULT; > > + goto free; > > + } > > Has it been considered to use kmalloc() instead of get_zeroed_page()? > > > diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h > > index ccb895f911b1..53b7b05b0004 100644 > > --- a/include/linux/blk_types.h > > +++ b/include/linux/blk_types.h > > @@ -316,6 +316,8 @@ enum req_opf { > > REQ_OP_ZONE_FINISH = 12, > > /* write data at the current zone write pointer */ > > REQ_OP_ZONE_APPEND = 13, > > + /* associate zone desc extension data to a zone */ > > + REQ_OP_ZONE_SET_DESC = 14, > > > > /* SCSI passthrough using struct scsi_request */ > > REQ_OP_SCSI_IN = 32, > > Does REQ_OP_ZONE_SET_DESC count as a read or as a write operation? See > also: > > static inline bool op_is_write(unsigned int op) { > return (op & 1); > } > > > +/** > > + * struct blk_zone_set_desc - BLKSETDESCZONE ioctl requests > > + * @sector: Starting sector of the zone to operate on. > > + * @flags: Feature flags. > > + * @len: size, in bytes, of the data to be associated to the zone. > > + * @data: data to be associated. > > + */ > > +struct blk_zone_set_desc { > > + __u64 sector; > > + __u32 flags; > > + __u32 len; > > + __u8 data[0]; > > +}; > > Isn't the recommended style to use a flexible array ([] instead of [0])? > See also > https://lore.kernel.org/lkml/20200608213711.GA22271@embeddedor/. > > Thanks, > > Bart. _______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme
next prev parent reply other threads:[~2020-06-30 13:28 UTC|newest] Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-06-28 23:01 [PATCH 0/2] Zone Descriptor Extension for Zoned Block Devices Matias Bjørling 2020-06-28 23:01 ` Matias Bjørling 2020-06-28 23:01 ` [PATCH 1/2] block: add zone_desc_ext_bytes to sysfs Matias Bjørling 2020-06-28 23:01 ` Matias Bjørling 2020-06-29 0:52 ` Damien Le Moal 2020-06-29 0:52 ` Damien Le Moal 2020-06-29 9:03 ` Niklas Cassel 2020-06-29 9:03 ` Niklas Cassel 2020-06-29 9:07 ` Matias Bjorling 2020-06-29 9:07 ` Matias Bjorling 2020-06-28 23:01 ` [PATCH 2/2] block: add BLKSETDESCZONE ioctl for Zoned Block Devices Matias Bjørling 2020-06-28 23:01 ` Matias Bjørling 2020-06-29 1:00 ` Damien Le Moal 2020-06-29 1:00 ` Damien Le Moal 2020-06-29 19:39 ` Javier González 2020-06-29 19:39 ` Javier González 2020-07-03 9:44 ` Matias Bjorling 2020-07-03 9:44 ` Matias Bjorling 2020-07-07 8:43 ` Javier González 2020-07-07 8:43 ` Javier González 2020-07-07 16:03 ` Matias Bjorling 2020-07-07 16:03 ` Matias Bjorling 2020-06-29 1:36 ` Bart Van Assche 2020-06-29 1:36 ` Bart Van Assche 2020-06-30 13:28 ` Matias Bjorling [this message] 2020-06-30 13:28 ` Matias Bjorling
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=MN2PR04MB6223940C9DEDCEB7209FB7D2F16F0@MN2PR04MB6223.namprd04.prod.outlook.com \ --to=matias.bjorling@wdc.com \ --cc=Damien.LeMoal@wdc.com \ --cc=Hans.Holmberg@wdc.com \ --cc=Niklas.Cassel@wdc.com \ --cc=axboe@kernel.dk \ --cc=bvanassche@acm.org \ --cc=hch@lst.de \ --cc=kbusch@kernel.org \ --cc=linux-block@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-nvme@lists.infradead.org \ --cc=linux-scsi@vger.kernel.org \ --cc=martin.petersen@oracle.com \ --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: linkBe 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.