* Re: [PATCH v2 17/21] block, blksnap: snapshot image block device [not found] ` <20221209142331.26395-18-sergei.shtepa@veeam.com> @ 2023-01-01 7:18 ` Hillf Danton 2023-01-02 9:44 ` Sergei Shtepa 0 siblings, 1 reply; 4+ messages in thread From: Hillf Danton @ 2023-01-01 7:18 UTC (permalink / raw) To: Sergei Shtepa; +Cc: axboe, corbet, linux-mm, linux-kernel On 9 Dec 2022 15:23:27 +0100 Sergei Shtepa <sergei.shtepa@veeam.com> > Provides the operation of block devices of snapshot images. Read and > write operations are redirected to the regions of difference blocks for > block device (struct diff_area). > > Signed-off-by: Sergei Shtepa <sergei.shtepa@veeam.com> > --- Thanks for your patchset. > +static int snapimage_kthread_worker_fn(void *param) > +{ > + struct snapimage *snapimage = param; > + struct bio *bio; > + int ret = 0; > + > + while (!kthread_should_stop()) { > + bio = get_bio_from_queue(snapimage); > + if (!bio) { > + schedule_timeout_interruptible(HZ / 100); > + continue; > + } Given the wake_up_process() below, s$HZ / 100$HZ * 1000$ to avoid unnecessary wakeups [1]. And because of no signal handling added, use schedule_timeout_idle() instead. [1] https://lore.kernel.org/lkml/20210419085027.761150-2-elver@google.com/ > + > + snapimage_process_bio(snapimage, bio); > + } > + > + while ((bio = get_bio_from_queue(snapimage))) > + snapimage_process_bio(snapimage, bio); > + > + return ret; > +} > + > +static void snapimage_submit_bio(struct bio *bio) > +{ > + struct snapimage *snapimage = bio->bi_bdev->bd_disk->private_data; > + gfp_t gfp = GFP_NOIO; > + > + if (bio->bi_opf & REQ_NOWAIT) > + gfp |= GFP_NOWAIT; > + if (snapimage->is_ready) { > + spin_lock(&snapimage->queue_lock); > + bio_list_add(&snapimage->queue, bio); > + spin_unlock(&snapimage->queue_lock); > + > + wake_up_process(snapimage->worker); > + } else > + bio_io_error(bio); > +} ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2 17/21] block, blksnap: snapshot image block device 2023-01-01 7:18 ` [PATCH v2 17/21] block, blksnap: snapshot image block device Hillf Danton @ 2023-01-02 9:44 ` Sergei Shtepa 0 siblings, 0 replies; 4+ messages in thread From: Sergei Shtepa @ 2023-01-02 9:44 UTC (permalink / raw) To: Hillf Danton; +Cc: axboe, corbet, linux-mm, linux-kernel On 1/1/23 08:18, Hillf Danton wrote: > Subject: > Re: [PATCH v2 17/21] block, blksnap: snapshot image block device > From: > Hillf Danton <hdanton@sina.com> > Date: > 1/1/23, 08:18 > > To: > Sergei Shtepa <sergei.shtepa@veeam.com> > CC: > "axboe@kernel.dk" <axboe@kernel.dk>, "corbet@lwn.net" <corbet@lwn.net>, "linux-mm@kvack.org" <linux-mm@kvack.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org> > > > This is the first time you've received an email from this sender > hdanton@sina.com, please exercise caution when clicking on links or opening > attachments. > > > On 9 Dec 2022 15:23:27 +0100 Sergei Shtepa > > Provides the operation of block devices of snapshot images. Read and > > write operations are redirected to the regions of difference blocks for > > block device (struct diff_area). > > > > Signed-off-by: Sergei Shtepa > > --- > > Thanks for your patchset. Thanks for the review. > > > +static int snapimage_kthread_worker_fn(void *param) > > +{ > > + struct snapimage *snapimage = param; > > + struct bio *bio; > > + int ret = 0; > > + > > + while (!kthread_should_stop()) { > > + bio = get_bio_from_queue(snapimage); > > + if (!bio) { > > + schedule_timeout_interruptible(HZ / 100); > > + continue; > > + } > > Given the wake_up_process() below, s$HZ / 100$HZ * 1000$ to avoid > unnecessary wakeups [1]. > > And because of no signal handling added, use schedule_timeout_idle() instead. Yes, this function will be rewritten to eliminate unnecessary wake-ups. The code that Christoph proposed in the letter at Dec 15 should work well. > > [1] > https://nam10.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2F20210419085027.761150-2-elver%40google.com%2F&data=05%7C01%7Csergei.shtepa%40veeam.com%7C765d591d47ad4d93857208daebc92508%7Cba07baab431b49edadd7cbc3542f5140%7C1%7C1%7C638081546440884849%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=rbINOq7m31DFhjHLnQKFHQuoWB64ZhmnWbAVLuNwv7w%3D&reserved=0 > > > + > > + snapimage_process_bio(snapimage, bio); > > + } > > + > > + while ((bio = get_bio_from_queue(snapimage))) > > + snapimage_process_bio(snapimage, bio); > > + > > + return ret; > > +} > > + > > +static void snapimage_submit_bio(struct bio *bio) > > +{ > > + struct snapimage *snapimage = bio->bi_bdev->bd_disk->private_data; > > + gfp_t gfp = GFP_NOIO; > > + > > + if (bio->bi_opf & REQ_NOWAIT) > > + gfp |= GFP_NOWAIT; > > + if (snapimage->is_ready) { > > + spin_lock(&snapimage->queue_lock); > > + bio_list_add(&snapimage->queue, bio); > > + spin_unlock(&snapimage->queue_lock); > > + > > + wake_up_process(snapimage->worker); > > + } else > > + bio_io_error(bio); > > +} > ^ permalink raw reply [flat|nested] 4+ messages in thread
[parent not found: <20221209142331.26395-19-sergei.shtepa@veeam.com>]
* Re: [PATCH v2 18/21] block, blksnap: snapshot [not found] ` <20221209142331.26395-19-sergei.shtepa@veeam.com> @ 2023-01-01 11:05 ` Hillf Danton 2023-01-02 9:58 ` Sergei Shtepa 0 siblings, 1 reply; 4+ messages in thread From: Hillf Danton @ 2023-01-01 11:05 UTC (permalink / raw) To: Sergei Shtepa; +Cc: axboe, corbet, linux-mm, linux-kernel On 9 Dec 2022 15:23:28 +0100 Sergei Shtepa <sergei.shtepa@veeam.com> > +int snapshot_create(struct blk_snap_dev *dev_id_array, unsigned int count, > + uuid_t *id) > +{ > + struct snapshot *snapshot = NULL; > + int ret; > + unsigned int inx; > + > + pr_info("Create snapshot for devices:\n"); > + for (inx = 0; inx < count; ++inx) > + pr_info("\t%u:%u\n", dev_id_array[inx].mj, > + dev_id_array[inx].mn); > + > + ret = check_same_devices(dev_id_array, count); > + if (ret) > + return ret; > + > + snapshot = snapshot_new(count); > + if (IS_ERR(snapshot)) { > + pr_err("Unable to create snapshot: failed to allocate snapshot structure\n"); > + return PTR_ERR(snapshot); > + } > + > + ret = -ENODEV; > + for (inx = 0; inx < count; ++inx) { > + dev_t dev_id = > + MKDEV(dev_id_array[inx].mj, dev_id_array[inx].mn); > + struct tracker *tracker; > + > + tracker = tracker_create_or_get(dev_id); > + if (IS_ERR(tracker)) { > + pr_err("Unable to create snapshot\n"); > + pr_err("Failed to add device [%u:%u] to snapshot tracking\n", > + MAJOR(dev_id), MINOR(dev_id)); > + ret = PTR_ERR(tracker); > + goto fail; > + } > + > + snapshot->tracker_array[inx] = tracker; > + snapshot->count++; > + } > + > + down_write(&snapshots_lock); > + list_add_tail(&snapshots, &snapshot->link); > + up_write(&snapshots_lock); Given list_for_each_entry below, typo wrt &snapshots found in the fresh 2023? > + > + uuid_copy(id, &snapshot->id); > + pr_info("Snapshot %pUb was created\n", &snapshot->id); > + return 0; > +fail: > + pr_err("Snapshot cannot be created\n"); > + > + snapshot_put(snapshot); > + return ret; > +} > + > +static struct snapshot *snapshot_get_by_id(uuid_t *id) > +{ > + struct snapshot *snapshot = NULL; > + struct snapshot *s; > + > + down_read(&snapshots_lock); > + if (list_empty(&snapshots)) > + goto out; > + > + list_for_each_entry(s, &snapshots, link) { > + if (uuid_equal(&s->id, id)) { > + snapshot = s; > + snapshot_get(snapshot); > + break; > + } > + } > +out: > + up_read(&snapshots_lock); > + return snapshot; > +} ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2 18/21] block, blksnap: snapshot 2023-01-01 11:05 ` [PATCH v2 18/21] block, blksnap: snapshot Hillf Danton @ 2023-01-02 9:58 ` Sergei Shtepa 0 siblings, 0 replies; 4+ messages in thread From: Sergei Shtepa @ 2023-01-02 9:58 UTC (permalink / raw) To: Hillf Danton; +Cc: axboe, corbet, linux-mm, linux-kernel On 1/1/23 12:05, Hillf Danton wrote: > Subject: > Re: [PATCH v2 18/21] block, blksnap: snapshot > From: > Hillf Danton <hdanton@sina.com> > Date: > 1/1/23, 12:05 > > To: > Sergei Shtepa <sergei.shtepa@veeam.com> > CC: > axboe@kernel.dk, corbet@lwn.net, linux-mm@kvack.org, linux-kernel@vger.kernel.org > > > On 9 Dec 2022 15:23:28 +0100 Sergei Shtepa <sergei.shtepa@veeam.com> >> +int snapshot_create(struct blk_snap_dev *dev_id_array, unsigned int count, >> + uuid_t *id) >> +{ >> + struct snapshot *snapshot = NULL; >> + int ret; >> + unsigned int inx; >> + >> + pr_info("Create snapshot for devices:\n"); >> + for (inx = 0; inx < count; ++inx) >> + pr_info("\t%u:%u\n", dev_id_array[inx].mj, >> + dev_id_array[inx].mn); >> + >> + ret = check_same_devices(dev_id_array, count); >> + if (ret) >> + return ret; >> + >> + snapshot = snapshot_new(count); >> + if (IS_ERR(snapshot)) { >> + pr_err("Unable to create snapshot: failed to allocate snapshot structure\n"); >> + return PTR_ERR(snapshot); >> + } >> + >> + ret = -ENODEV; >> + for (inx = 0; inx < count; ++inx) { >> + dev_t dev_id = >> + MKDEV(dev_id_array[inx].mj, dev_id_array[inx].mn); >> + struct tracker *tracker; >> + >> + tracker = tracker_create_or_get(dev_id); >> + if (IS_ERR(tracker)) { >> + pr_err("Unable to create snapshot\n"); >> + pr_err("Failed to add device [%u:%u] to snapshot tracking\n", >> + MAJOR(dev_id), MINOR(dev_id)); >> + ret = PTR_ERR(tracker); >> + goto fail; >> + } >> + >> + snapshot->tracker_array[inx] = tracker; >> + snapshot->count++; >> + } >> + >> + down_write(&snapshots_lock); >> + list_add_tail(&snapshots, &snapshot->link); >> + up_write(&snapshots_lock); > Given list_for_each_entry below, typo wrt &snapshots found in the fresh 2023? > Thanks. It seems I just swapped the parameters by mistake. It should be better: ``` down_write(&snapshots_lock); list_add_tail(&snapshot->link, &snapshots); up_write(&snapshots_lock); ``` >> + >> + uuid_copy(id, &snapshot->id); >> + pr_info("Snapshot %pUb was created\n", &snapshot->id); >> + return 0; >> +fail: >> + pr_err("Snapshot cannot be created\n"); >> + >> + snapshot_put(snapshot); >> + return ret; >> +} >> + >> +static struct snapshot *snapshot_get_by_id(uuid_t *id) >> +{ >> + struct snapshot *snapshot = NULL; >> + struct snapshot *s; >> + >> + down_read(&snapshots_lock); >> + if (list_empty(&snapshots)) >> + goto out; >> + >> + list_for_each_entry(s, &snapshots, link) { >> + if (uuid_equal(&s->id, id)) { >> + snapshot = s; >> + snapshot_get(snapshot); >> + break; >> + } >> + } >> +out: >> + up_read(&snapshots_lock); >> + return snapshot; >> +} ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-01-02 9:58 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20221209142331.26395-1-sergei.shtepa@veeam.com>
[not found] ` <20221209142331.26395-18-sergei.shtepa@veeam.com>
2023-01-01 7:18 ` [PATCH v2 17/21] block, blksnap: snapshot image block device Hillf Danton
2023-01-02 9:44 ` Sergei Shtepa
[not found] ` <20221209142331.26395-19-sergei.shtepa@veeam.com>
2023-01-01 11:05 ` [PATCH v2 18/21] block, blksnap: snapshot Hillf Danton
2023-01-02 9:58 ` Sergei Shtepa
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox