Summary: | g_bio leak after zfs ABD commit | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | Base System | Reporter: | Dan Nelson <dnelson> | ||||||
Component: | kern | Assignee: | Andriy Gapon <avg> | ||||||
Status: | Closed FIXED | ||||||||
Severity: | Affects Many People | CC: | avg, borjam, devin, dnelson_1901, eugen, execve, fk, fs, pi | ||||||
Priority: | --- | ||||||||
Version: | 11.1-STABLE | ||||||||
Hardware: | Any | ||||||||
OS: | Any | ||||||||
Attachments: |
|
Description
Dan Nelson
2017-09-13 14:57:00 UTC
(In reply to Dan Nelson from comment #0) Thank you very much for the report! As soon as I read it I noticed that I have the same kind of issue. I dug through the biozone to examine the leaked bio-s and they all seem to have bio_cmd = 5 and bio_flags = 8. So, they seem to be BIO_FLUSH bio-s. Their zio-s must have been ZIO_TYPE_IOCTL. Now, those zio-s use ZIO_IOCTL_PIPELINE and it is defined as: #define ZIO_IOCTL_PIPELINE \ (ZIO_INTERLOCK_STAGES | \ ZIO_STAGE_VDEV_IO_START | \ ZIO_STAGE_VDEV_IO_ASSESS) So, ZIO_STAGE_VDEV_IO_START is in the pipeline, but ZIO_STAGE_VDEV_IO_DONE is not as you have correctly theorised. The normal I/O pipelines always include ZIO_VDEV_IO_STAGES #define ZIO_VDEV_IO_STAGES \ (ZIO_STAGE_VDEV_IO_START | \ ZIO_STAGE_VDEV_IO_DONE | \ ZIO_STAGE_VDEV_IO_ASSESS) so the problem does not affect them. I will double-check why the ioctl pipeline omitted ZIO_STAGE_VDEV_IO_DONE and will test adding that stage. Created attachment 186415 [details]
work in progress
This is a patch that I am testing right now.
So far so good.
Thanks a lot for the report Dan. I noticed that something was leaking but didn't have time to track it down yet. Thanks to the report I didn't have to. To work around the issue in ElectroBSD I've reverted r321610/a0dddc24c9050 after reverting the follow-up commits that would cause revert conflicts. Your patch looks good to me, Andriy. I've imported it and will test it in the next couple of days. Thanks. It occurred to me that this issue could be easily detected automatically if there was a way to specify a time limit between uma_zalloc() and uma_zfree() calls for a given zone (or item from the zone). Obviously this only works if an upper limit makes sense (and items are expected to be freed), but in case of g_bio I believe that this is the case and there are a bunch of other zones where enforcing allocation time limits should work. I wouldn't be surprised if there were a bunch of other zone item leaks that haven't been detected yet because they don't occur frequently enough to have a big impact. Created attachment 186510 [details]
alternative patch
Here is an alternative patch that does not require the modification of ZIO_IOCTL_PIPELINE.
I've applied the second patch. Seems to be working fine, no leaks. Only one thing has me slightly puzzled, a very high used UMA slabs count. ITEM SIZE LIMIT USED FREE REQ FAIL SLEEP UMA Kegs: 384, 0, 242, 8, 242, 0, 0 UMA Zones: 4736, 0, 259, 0, 259, 0, 0 UMA Slabs: 80, 0, 542261, 39, 553319, 0, 0 UMA Hash: 256, 0, 60, 60, 109, 0, 0 The other server running Elasticsearch, which is still on 11.1-RELEASE and has also 8 GB of memory, shows this. Note the high number of free UMA slabs. ITEM SIZE LIMIT USED FREE REQ FAIL SLEEP UMA Kegs: 384, 0, 240, 0, 240, 0, 0 UMA Zones: 2176, 0, 257, 0, 257, 0, 0 UMA Slabs: 80, 0, 207526, 243674, 2339176, 0, 0 I'll let it run for several days anyway. As I said it's not critical. Thanks! (In reply to Borja Marcos from comment #5) Ignore the UMA slab comment, it's irrelevant. Sorry. A commit references this bug: Author: avg Date: Wed Sep 20 08:27:21 UTC 2017 New revision: 323796 URL: https://svnweb.freebsd.org/changeset/base/323796 Log: fix memory leak in g_bio zone introduced in r320452, another ABD fallout I overlooked the fact that that ZIO_IOCTL_PIPELINE does not include ZIO_STAGE_VDEV_IO_DONE stage. We do allocate a struct bio for an ioctl zio (a disk cache flush), but we never freed it. This change splits bio handling into two groups, one for normal read/write i/o that passes data around and, thus, needs the abd data tranform; the other group is for "data-less" i/o such as trim and cache flush. PR: 222288 Reported by: Dan Nelson <dnelson@allantgroup.com> Tested by: Borja Marcos <borjam@sarenet.es> MFC after: 10 days Changes: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c For the record, I have applied the patch to 11-STABLE from today and so far it's running fine on my worst case server (running Elasticsearch). No leaks. I also applied the patch to 11-STABLE and have been running for a few days with it. No leaks here either; everything looks back to normal. A commit references this bug: Author: avg Date: Sun Oct 1 15:03:44 UTC 2017 New revision: 324161 URL: https://svnweb.freebsd.org/changeset/base/324161 Log: MFV r323796: fix memory leak in g_bio zone introduced in r320452 I overlooked the fact that that ZIO_IOCTL_PIPELINE does not include ZIO_STAGE_VDEV_IO_DONE stage. We do allocate a struct bio for an ioctl zio (a disk cache flush), but we never freed it. This change splits bio handling into two groups, one for normal read/write i/o that passes data around and, thus, needs the abd data tranform; the other group is for "data-less" i/o such as trim and cache flush. PR: 222288 Changes: _U stable/11/ stable/11/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c |