Project

General

Profile

Actions
Copy link

Bug #12300

closed

Memory leak in rfs3_readdirplus()

Added by Marcel Telka almost 4 years ago. Updated almost 4 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Marcel Telka
Category:
nfs - NFS server and client
Start date:
Due date:
% Done:

100%

Estimated time:
Difficulty:
Bite-size
Tags:
Gerrit CR:
External Bug:

Description

There is this memory leak in rfs3_readdirplus():

kmem_alloc_8192 leak: 1 buffer, 8192 bytes
            ADDR          BUFADDR        TIMESTAMP           THREAD
                            CACHE          LASTLOG         CONTENTS
ffffff02197d6c60 ffffff021d999000       3c8a305e1c ffffff0208623840
                 ffffff01e9031b08 ffffff01e9db8640                0
                 kmem_slab_alloc_impl+0x198
                 kmem_slab_alloc+0x62
                 kmem_cache_alloc+0x15b
                 kmem_alloc+0x4b
                 rfs3_readdirplus+0x322
                 common_dispatch+0x9ba
                 rfs_dispatch+0x23
                 svc_getreq+0x24e
                 svc_run+0x170
                 svc_do_run+0x8b
                 nfssys+0xf9

Related issues

Related to illumos gate - Feature #11083: support NFS server in zoneClosedDan McDonald

Actions
Actions
Copy link
 #1

Updated by Marcel Telka almost 4 years ago

Actions
Copy link
 #2

Updated by Marcel Telka almost 4 years ago

Root cause

#11083 removed the required kmem_free() call.

Actions
Copy link
 #3

Updated by Marcel Telka almost 4 years ago

Steps to reproduce

At server:

# share -o "iso8859-1=*" /tmp

At client:

# mount -o vers=3 SERVER:/tmp /mnt
# ls /mnt
Actions
Copy link
 #5

Updated by Vitaliy Gusev almost 4 years ago

Interesting, Dan you said that you did ::findleaks with no complaints?

Actions
Copy link
 #6

Updated by Marcel Telka almost 4 years ago

  • Status changed from In Progress to Pending RTI
Actions
Copy link
 #7

Updated by Dan McDonald almost 4 years ago

(ick, pardon for the earlier accidental edits, @Vitaliy)

I did indeed try my best to squash leaks during #11083 testing... I never saw this leak. I notice the "iso8859-1" share option, though, and did NOT use that during my tests. I'm glad Marcel included this in the steps to reproduce.

A quick glance at the code, however, makes me wonder if this leak existed pre-nfs-zone. I'm going to attempt a reproduction on an older OmniOS for the record.

Actions
Copy link
 #8

Updated by Marcel Telka almost 4 years ago

Dan McDonald wrote:

A quick glance at the code, however, makes me wonder if this leak existed pre-nfs-zone. I'm going to attempt a reproduction on an older OmniOS for the record.

Obviously, it didn't.

Actions
Copy link
 #9

Updated by Dan McDonald almost 4 years ago

BTW, I'm keeping the `nfs-zone` branch of https://github.com/danmcd/illumos-gate/ around to see where problems like this happened.

One of the cherry-picks from illumos-nexenta was botched in the nfs-zone branch:

commit e98b6dbd8c259d0414f5519823d70023fd22d854
Author: Joyce McIntosh <joyce.mcintosh@nexenta.com>
Date:   Wed Apr 4 14:00:12 2018 -0600

   NEX-16712 NFS dtrace providers do not support per-share filtering

   Reviewed by: Evan Layton <evan.layton@nexenta.com>
   Reviewed by: Gordon Ross <gordon.ross@nexenta.com>
   Reviewed by: Yuri Pankon <yuri.pankov@nexenta.com>

There's merge problem where the merger also deleted a kmem_free() call. If you compare this with the original version of NEX-16712 in illumos-nexenta (commit 7e25247248cc565839a6ba3bab590094947437d5 in that repo) no kmem_free() calls were deleted in Joyce M's original. This problem was specific to the nfs-zone branch, and made it into #11083.

Actions
Copy link
 #10

Updated by Electric Monk almost 4 years ago

  • Status changed from Pending RTI to Closed
  • % Done changed from 0 to 100

git commit dfdcac05b5cbcf4d3441fd96be492cec26eb3d24

commit  dfdcac05b5cbcf4d3441fd96be492cec26eb3d24
Author: Marcel Telka <marcel@telka.sk>
Date:   2020-02-12T15:53:04.000Z

    12300 Memory leak in rfs3_readdirplus()
    Reviewed by: Matthias Scheler <mscheler@tintri.com>
    Reviewed by: Vitaliy Gusev <gusev.vitaliy@icloud.com>
    Approved by: Dan McDonald <danmcd@joyent.com>

Actions
Copy link

Also available in: Atom PDF