linux-kernel - Re: [RFC PATCH 2/2] nvme: add simple copy support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Wed, 2 Dec 2020 13:59:57 +0530
From:   Selva Jove <selvajove@...il.com>
To:     Keith Busch <kbusch@...nel.org>
Cc:     SelvaKumar S <selvakuma.s1@...sung.com>,
        linux-nvme@...ts.infradead.org, axboe@...nel.dk,
        Damien Le Moal <damien.lemoal@....com>, hch@....de,
        sagi@...mberg.me, linux-block@...r.kernel.org,
        linux-kernel@...r.kernel.org, nj.shetty@...sung.com,
        joshi.k@...sung.com, javier.gonz@...sung.com
Subject: Re: [RFC PATCH 2/2] nvme: add simple copy support

On Tue, Dec 1, 2020 at 8:46 PM Keith Busch <kbusch@...nel.org> wrote:
>
> On Tue, Dec 01, 2020 at 11:09:49AM +0530, SelvaKumar S wrote:
> > +static void nvme_config_copy(struct gendisk *disk, struct nvme_ns *ns,
> > +                                    struct nvme_id_ns *id)
> > +{
> > +     struct nvme_ctrl *ctrl = ns->ctrl;
> > +     struct request_queue *queue = disk->queue;
> > +
> > +     if (!(ctrl->oncs & NVME_CTRL_ONCS_COPY)) {
> > +             queue->limits.max_copy_sectors = 0;
> > +             blk_queue_flag_clear(QUEUE_FLAG_COPY, queue);
> > +             return;
> > +     }
> > +
> > +     /* setting copy limits */
> > +     ns->mcl = le64_to_cpu(id->mcl);
> > +     ns->mssrl = le32_to_cpu(id->mssrl);
> > +     ns->msrc = id->msrc;
>
> These are not used anywhere outside this function, so there's no need to
> add members to the struct.

Sure. Will remove these entries from nvme_ns.

>
> > +     if (blk_queue_flag_test_and_set(QUEUE_FLAG_COPY, queue))
> > +             return;
>
> The queue limits are not necessarily the same each time we're called to
> update the disk info, so this return shouldn't be here.
>

Makes sense.

> > +
> > +     queue->limits.max_copy_sectors = ns->mcl * (1 << (ns->lba_shift - 9));
> > +     queue->limits.max_copy_range_sectors = ns->mssrl *
> > +             (1 << (ns->lba_shift - 9));
> > +     queue->limits.max_copy_nr_ranges = ns->msrc + 1;
> > +}
>
> <>
>
> > @@ -2045,6 +2133,7 @@ static void nvme_update_disk_info(struct gendisk *disk,
> >       set_capacity_and_notify(disk, capacity);
> >
> >       nvme_config_discard(disk, ns);
> > +     nvme_config_copy(disk, ns, id);
> >       nvme_config_write_zeroes(disk, ns);
> >
> >       if (id->nsattr & NVME_NS_ATTR_RO)
> > @@ -3014,6 +3103,7 @@ int nvme_init_identify(struct nvme_ctrl *ctrl)
> >       ctrl->oaes = le32_to_cpu(id->oaes);
> >       ctrl->wctemp = le16_to_cpu(id->wctemp);
> >       ctrl->cctemp = le16_to_cpu(id->cctemp);
> > +     ctrl->ocfs = le32_to_cpu(id->ocfs);
>
> ocfs is not used anywhere.