Page MenuHomeFreeBSD

virstor: write large maps in chunks during label
ClosedPublic

Authored by rlibby on Jun 6 2024, 5:45 PM.
Tags
None
Referenced Files
F98622567: D45517.diff
Fri, Oct 4, 3:01 AM
Unknown Object (File)
Sun, Sep 22, 12:33 AM
Unknown Object (File)
Sep 3 2024, 9:52 AM
Unknown Object (File)
Sep 2 2024, 6:47 AM
Unknown Object (File)
Aug 22 2024, 11:19 PM
Unknown Object (File)
Aug 15 2024, 8:53 AM
Unknown Object (File)
Aug 6 2024, 12:47 PM
Unknown Object (File)
Aug 6 2024, 12:47 PM
Subscribers

Details

Summary

During the initial label of a virstor device, write out the allocation
map in chunks if it is large (> 1 MB) in order to avoid large mallocs.

Even though the kernel virstor geom may still do a large malloc to
represent the allocation map, this may still be useful to avoid a
ulimit.

Sponsored by: Dell EMC Isilon


I am unsure if geom virstor is generally in use these days. My web
searches have not turned up much. I am interested in using it to
simulate very large drives for test purposes. In my scenario, I may
need a very large map (gigabytes). So I am looking at making virstor
changes to support this.

Test Plan

New test in D45535

% sudo kyua debug -k /usr/tests/sys/Kyuafile geom/class/virstor/virstor_test:basic
Executing command [ gvirstor label -v -s 64M -m 512 virstor.bjyhzA /dev/md0 ]
Executing command [ gvirstor destroy virstor.bjyhzA ]

Destroying test virstor device: virstor.bjyhzA
Removing test memory disk: md0
geom/class/virstor/virstor_test:basic -> passed

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 58089
Build 54977: arc lint + arc unit

Event Timeline

rlibby requested review of this revision.Jun 6 2024, 5:45 PM
rlibby edited the test plan for this revision. (Show Details)
markj added inline comments.
lib/geom/virstor/geom_virstor.c
342

Not your bug, but, shouldn't we return here?

362

Do you need to free the chunk in this error path?

This revision is now accepted and ready to land.Jun 7 2024, 1:29 PM
rlibby added inline comments.
lib/geom/virstor/geom_virstor.c
342

Yes... I guess we EFAULT on the write and exit then. I'll fix it.

Looking around, there are other instances of that too. The fd check just above I can also sneak into this. For the "Device %s is too small" case, I'm not sure if that was intended to be a hard error or a warning, so I'll leave that alone for the moment.

362

I guess I should. We're about to exit, but it could avoid tripping static analysis or a memory checker.

lib/geom/virstor/geom_virstor.c
362

Since this is library code, I'd tend to be more careful about cleaning up in cases like this.

rlibby marked 3 inline comments as done.

markj feedback: clean up error paths

This revision now requires review to proceed.Jun 8 2024, 12:35 AM
This revision is now accepted and ready to land.Jun 10 2024, 1:54 PM