Date: Thu, 6 Mar 2025 19:32:02 +0800 From: Zhenlei Huang <zlei@FreeBSD.org> To: Mateusz Guzik <mjg@FreeBSD.org> Cc: "src-committers@freebsd.org" <src-committers@FreeBSD.org>, "dev-commits-src-all@freebsd.org" <dev-commits-src-all@FreeBSD.org>, Warner Losh <imp@freebsd.org>, "dev-commits-src-main@freebsd.org" <dev-commits-src-main@FreeBSD.org> Subject: Re: git: 234683726708 - main - devclass: make devclass_alloc_unit use M_NOWAIT Message-ID: <F1B3652E-0D0C-402A-8509-D510992DAC15@FreeBSD.org> In-Reply-To: <202503061103.526B32Id022652@gitrepo.freebsd.org> References: <202503061103.526B32Id022652@gitrepo.freebsd.org>
index | next in thread | previous in thread | raw e-mail
[-- Attachment #1 --] > On Mar 6, 2025, at 7:03 PM, Mateusz Guzik <mjg@FreeBSD.org> wrote: > > The branch main has been updated by mjg: > > URL: https://cgit.FreeBSD.org/src/commit/?id=234683726708cf5212d672d676d30056d4133859 > > commit 234683726708cf5212d672d676d30056d4133859 > Author: Mateusz Guzik <mjg@FreeBSD.org> > AuthorDate: 2025-03-06 11:01:49 +0000 > Commit: Mateusz Guzik <mjg@FreeBSD.org> > CommitDate: 2025-03-06 11:01:49 +0000 > > devclass: make devclass_alloc_unit use M_NOWAIT > > The only caller already does this. > > The routine can be called with a mutex held making M_WAITOK illegal. > > Sponsored by: Rubicon Communications, LLC ("Netgate") > --- > sys/kern/subr_bus.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/sys/kern/subr_bus.c b/sys/kern/subr_bus.c > index 9506e471705c..0422352bba51 100644 > --- a/sys/kern/subr_bus.c > +++ b/sys/kern/subr_bus.c > @@ -1208,6 +1208,7 @@ devclass_get_sysctl_tree(devclass_t dc) > static int > devclass_alloc_unit(devclass_t dc, device_t dev, int *unitp) > { > + device_t *devices; > const char *s; > int unit = *unitp; > > @@ -1264,8 +1265,11 @@ devclass_alloc_unit(devclass_t dc, device_t dev, int *unitp) > int newsize; > > newsize = unit + 1; > - dc->devices = reallocf(dc->devices, > - newsize * sizeof(*dc->devices), M_BUS, M_WAITOK); > + devices = reallocf(dc->devices, > + newsize * sizeof(*dc->devices), M_BUS, M_NOWAIT); I'd recommend against this. From the commit message of f3d3c63442ff, Warner said, > In addition, transition to M_WAITOK since this is a sleepable context So, the M_WAITOK is intentional. Rather than reverting this, the caller devclass_add_device() should use M_WAITOK. ``` - dev->nameunit = malloc(buflen, M_BUS, M_NOWAIT|M_ZERO); - if (!dev->nameunit) - return (ENOMEM); + dev->nameunit = malloc(buflen, M_BUS, M_WAITOK | M_ZERO); ``` Best regards, Zhenlei > + if (devices == NULL) > + return (ENOMEM); > + dc->devices = devices; > memset(dc->devices + dc->maxunit, 0, > sizeof(device_t) * (newsize - dc->maxunit)); > dc->maxunit = newsize; [-- Attachment #2 --] <html><head><meta http-equiv="Content-Type" content="text/html; charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><br class=""><div><br class=""><blockquote type="cite" class=""><div class="">On Mar 6, 2025, at 7:03 PM, Mateusz Guzik <<a href="mailto:mjg@FreeBSD.org" class="">mjg@FreeBSD.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div class="">The branch main has been updated by mjg:<br class=""><br class="">URL: <a href="https://cgit.FreeBSD.org/src/commit/?id=234683726708cf5212d672d676d30056d4133859" class="">https://cgit.FreeBSD.org/src/commit/?id=234683726708cf5212d672d676d30056d4133859</a><br class=""><br class="">commit 234683726708cf5212d672d676d30056d4133859<br class="">Author: Mateusz Guzik <<a href="mailto:mjg@FreeBSD.org" class="">mjg@FreeBSD.org</a>><br class="">AuthorDate: 2025-03-06 11:01:49 +0000<br class="">Commit: Mateusz Guzik <<a href="mailto:mjg@FreeBSD.org" class="">mjg@FreeBSD.org</a>><br class="">CommitDate: 2025-03-06 11:01:49 +0000<br class=""><br class=""> devclass: make devclass_alloc_unit use M_NOWAIT<br class=""><br class=""> The only caller already does this.<br class=""><br class=""> The routine can be called with a mutex held making M_WAITOK illegal.<br class=""><br class=""> Sponsored by: Rubicon Communications, LLC ("Netgate")<br class="">---<br class=""> sys/kern/subr_bus.c | 8 ++++++--<br class=""> 1 file changed, 6 insertions(+), 2 deletions(-)<br class=""><br class="">diff --git a/sys/kern/subr_bus.c b/sys/kern/subr_bus.c<br class="">index 9506e471705c..0422352bba51 100644<br class="">--- a/sys/kern/subr_bus.c<br class="">+++ b/sys/kern/subr_bus.c<br class="">@@ -1208,6 +1208,7 @@ devclass_get_sysctl_tree(devclass_t dc)<br class=""> static int<br class=""> devclass_alloc_unit(devclass_t dc, device_t dev, int *unitp)<br class=""> {<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>device_t *devices;<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>const char *s;<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>int unit = *unitp;<br class=""><br class="">@@ -1264,8 +1265,11 @@ devclass_alloc_unit(devclass_t dc, device_t dev, int *unitp)<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>int newsize;<br class=""><br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>newsize = unit + 1;<br class="">-<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>dc->devices = reallocf(dc->devices,<br class="">-<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span> newsize * sizeof(*dc->devices), M_BUS, M_WAITOK);<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>devices = reallocf(dc->devices,<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span> newsize * sizeof(*dc->devices), M_BUS, M_NOWAIT);<br class=""></div></div></blockquote><div><br class=""></div>I'd recommend against this. From the commit message of f3d3c63442ff, Warner said,</div><div>> In addition, transition to M_WAITOK since this is a sleepable context</div><div>So, the M_WAITOK is intentional.</div><div><br class=""></div><div>Rather than reverting this, the caller devclass_add_device() should use <span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);" class="">M_WAITOK.</span></div><div><span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);" class=""><br class=""></span></div><div><span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);" class="">```</span></div><div><div><font color="#000000" class=""><span style="caret-color: rgb(0, 0, 0);" class="">- dev->nameunit = malloc(buflen, M_BUS, M_NOWAIT|M_ZERO);</span></font></div><div><font color="#000000" class=""><span style="caret-color: rgb(0, 0, 0);" class="">- if (!dev->nameunit)</span></font></div><div><font color="#000000" class=""><span style="caret-color: rgb(0, 0, 0);" class="">- return (ENOMEM);</span></font></div><div><font color="#000000" class=""><span style="caret-color: rgb(0, 0, 0);" class="">+ dev->nameunit = malloc(buflen, M_BUS, M_WAITOK | M_ZERO);</span></font></div></div><div><span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);" class="">```</span></div><div><span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);" class=""><br class=""></span></div><div><div style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);">Best regards,</div><div style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);">Zhenlei</div></div><div><br class=""><blockquote type="cite" class=""><div class=""><div class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>if (devices == NULL)<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>return (ENOMEM);<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>dc->devices = devices;<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>memset(dc->devices + dc->maxunit, 0,<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span> sizeof(device_t) * (newsize - dc->maxunit));<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>dc->maxunit = newsize;<br class=""></div></div></blockquote></div><br class=""><div class=""> <div><br class=""></div> </div> <br class=""></body></html>help
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?F1B3652E-0D0C-402A-8509-D510992DAC15>