Richard Kojedzinszky
2014-02-13 02:56:07 UTC
Dear Matt,
I am not an Illumos developer, even I haven't compiled it at all. I was fixing FreeBSD's ZFS port when noticed this leak. And under FreeBSD the leak could be reproduced. I've also sent a mail to advocates@ a month ago and was told the same you wrote to me, get two reviewers, and a test suite. Unfortunately I even cannot compile an Illumos kernel now.
Regards,
KÃŒldve az én HTC-mrÅl
----- Reply message -----
Feladó: "Matthew Ahrens" <***@delphix.com>
CÃmzett: "illumos-zfs" <***@lists.illumos.org>
Tárgy: [zfs] review needed
Dátum: Cs, febr. 13, 2014 00:30
Anyone else care to review this one-liner?The fix seems obviously correct by inspection, but what testing have you done? Ideally you would reproduce the problem (the vnode hold leak) and then test again with the fix in place and see that the problem does not reproduce.Assuming the testing is good, your next step is to submit an RTI by emailing the illumos advocates. See http://wiki.illumos.org/display/illumos/How+To+Contribute#HowToContribute-5SubmittingAPatch--mattOn Wed, Feb 12, 2014 at 5:29 AM, Richard Kojedzinszky <***@cflinux.hu> wrote:Is there someone also to review/approve?
2014-02-10 19:31 idÅpontban Matthew Ahrens ezt írta:
Looks good to me.
--matt
On Mon, Feb 10, 2014 at 6:53 AM, <***@cflinux.hu> wrote:
Dear ZFS team,
I found that in zfs/zfs_ctldir.c the snapshot_inactive() function
obtains a hold on its parent directory with gfs_dir_lookup(), but
does not drop that hold when in the meantime other thread uses the
actual snapshot vnode.
Please review the attached patch to eliminate this leak.
Thanks in advance,
Richard Kojedzinszky
-------------------------------------------
illumos-zfs
Archives: https://www.listbox.com/member/archive/182191/=now [1]
RSS Feed:https://www.listbox.com/member/archive/rss/182191/21635000-ebd1d460
[2]
Modify Your Subscription:https://www.listbox.com/member/?& [3]
Powered by Listbox: http://www.listbox.com [4]
ILLUMOS-ZFS | Archives [1] [5] | Modify [6] Your Subscription [4]
Links:
------
[1] https://www.listbox.com/member/archive/182191/=now
[2] https://www.listbox.com/member/archive/rss/182191/21635000-ebd1d460
[3] https://www.listbox.com/member/?&
[4] http://www.listbox.com
[5] https://www.listbox.com/member/archive/rss/182191/25402478-0858cafa
[6]https://www.listbox.com/member/?&
--
Richard Kojedzinszky
-------------------------------------------
illumos-zfs
Archives: https://www.listbox.com/member/archive/182191/=now
RSS Feed: https://www.listbox.com/member/archive/rss/182191/21635000-ebd1d460
Modify Your Subscription: https://www.listbox.com/member/?&
Powered by Listbox: http://www.listbox.com
illumos-zfs | Archives
| Modify
Your Subscription
-------------------------------------------
illumos-zfs
Archives: https://www.listbox.com/member/archive/182191/=now
RSS Feed: https://www.listbox.com/member/archive/rss/182191/23047029-187a0c8d
Modify Your Subscription: https://www.listbox.com/member/?member_id=23047029&id_secret=23047029-2e85923f
Powered by Listbox: http://www.listbox.com
I am not an Illumos developer, even I haven't compiled it at all. I was fixing FreeBSD's ZFS port when noticed this leak. And under FreeBSD the leak could be reproduced. I've also sent a mail to advocates@ a month ago and was told the same you wrote to me, get two reviewers, and a test suite. Unfortunately I even cannot compile an Illumos kernel now.
Regards,
KÃŒldve az én HTC-mrÅl
----- Reply message -----
Feladó: "Matthew Ahrens" <***@delphix.com>
CÃmzett: "illumos-zfs" <***@lists.illumos.org>
Tárgy: [zfs] review needed
Dátum: Cs, febr. 13, 2014 00:30
Anyone else care to review this one-liner?The fix seems obviously correct by inspection, but what testing have you done? Ideally you would reproduce the problem (the vnode hold leak) and then test again with the fix in place and see that the problem does not reproduce.Assuming the testing is good, your next step is to submit an RTI by emailing the illumos advocates. See http://wiki.illumos.org/display/illumos/How+To+Contribute#HowToContribute-5SubmittingAPatch--mattOn Wed, Feb 12, 2014 at 5:29 AM, Richard Kojedzinszky <***@cflinux.hu> wrote:Is there someone also to review/approve?
2014-02-10 19:31 idÅpontban Matthew Ahrens ezt írta:
Looks good to me.
--matt
On Mon, Feb 10, 2014 at 6:53 AM, <***@cflinux.hu> wrote:
Dear ZFS team,
I found that in zfs/zfs_ctldir.c the snapshot_inactive() function
obtains a hold on its parent directory with gfs_dir_lookup(), but
does not drop that hold when in the meantime other thread uses the
actual snapshot vnode.
Please review the attached patch to eliminate this leak.
Thanks in advance,
Richard Kojedzinszky
-------------------------------------------
illumos-zfs
Archives: https://www.listbox.com/member/archive/182191/=now [1]
RSS Feed:https://www.listbox.com/member/archive/rss/182191/21635000-ebd1d460
[2]
Modify Your Subscription:https://www.listbox.com/member/?& [3]
Powered by Listbox: http://www.listbox.com [4]
ILLUMOS-ZFS | Archives [1] [5] | Modify [6] Your Subscription [4]
Links:
------
[1] https://www.listbox.com/member/archive/182191/=now
[2] https://www.listbox.com/member/archive/rss/182191/21635000-ebd1d460
[3] https://www.listbox.com/member/?&
[4] http://www.listbox.com
[5] https://www.listbox.com/member/archive/rss/182191/25402478-0858cafa
[6]https://www.listbox.com/member/?&
--
Richard Kojedzinszky
-------------------------------------------
illumos-zfs
Archives: https://www.listbox.com/member/archive/182191/=now
RSS Feed: https://www.listbox.com/member/archive/rss/182191/21635000-ebd1d460
Modify Your Subscription: https://www.listbox.com/member/?&
Powered by Listbox: http://www.listbox.com
illumos-zfs | Archives
| Modify
Your Subscription
-------------------------------------------
illumos-zfs
Archives: https://www.listbox.com/member/archive/182191/=now
RSS Feed: https://www.listbox.com/member/archive/rss/182191/23047029-187a0c8d
Modify Your Subscription: https://www.listbox.com/member/?member_id=23047029&id_secret=23047029-2e85923f
Powered by Listbox: http://www.listbox.com