Re: CVS commit: src/sys/arch/xen/xen
Jean-Yves Migeon wrote: > On 18.04.2011 10:35, Mindaugas Rasiukevicius wrote: > > We used to check the return of big size allocations, when kmem(9) could > > fail even with KM_SLEEP due to KVA starvation. However, pretty much all > > kernel does not perform checks for smaller allocations and since the bug > > was fixed - we are no longer checking for big ones as well. > > > > This applies to all allocators. > > So, any thread sleeping for an allocation cannot be interrupted? > Nope. However, for nearly all cases there is no need for that. For the previous reserve_pages() case, pointed out by Cherry, code should rather be restructured to pre-allocate memory before acquiring the locks. That would have avoided the problem. -- Mindaugas
Re: CVS commit: src/sys/arch/xen/xen
On Apr,Monday 18 2011, at 10:26 AM, Mindaugas Rasiukevicius wrote: > "Cherry G. Mathew" wrote: > On Mon, Apr 18, 2011 at 03:04:32AM +, Mindaugas Rasiukevicius > wrote: >> Module Name:src >> Committed By: rmind >> Date: Mon Apr 18 03:04:31 UTC 2011 >> >> Modified Files: >> src/sys/arch/xen/xen: balloon.c >> >> Log Message: >> balloon_xenbus_attach: use KM_SLEEP for allocation. >> >> Note: please do not use KM_NOSLEEP. > > And, according to yamt@, KM_SLEEP can fail in the current design... IIRC yamt@ fixed it a year or few ago. >>> >>> And in the more specific immediate context: >>> http://mail-index.netbsd.org/port-xen/2011/04/07/msg006613.html >>> >>> >> >> Hi, >> >> PS: Can you please revert ? Unless it's breaking anything else, or you >> have a fix for the problem mentioned in the thread above, ie; > > Change is for balloon_xenbus_attach(), which is irrelevant to the issue > described in that email. > > Are you aware that KM_NOSLEEP might fail not only because there is no > memory (or KVA) available, though? I had several problems with memory hungry zfs, where allocation failed just because some lock in uvm was hold when KM_NOSLEEP allocation was issued. We should avoid of using KM_NOSLEEP whenever is it possible. Regards Adam.
Re: CVS commit: src/sys/arch/xen/xen
On 18.04.2011 10:35, Mindaugas Rasiukevicius wrote: > We used to check the return of big size allocations, when kmem(9) could > fail even with KM_SLEEP due to KVA starvation. However, pretty much all > kernel does not perform checks for smaller allocations and since the bug > was fixed - we are no longer checking for big ones as well. > > This applies to all allocators. So, any thread sleeping for an allocation cannot be interrupted? -- Jean-Yves Migeon jeanyves.mig...@free.fr
Re: CVS commit: src/sys/arch/xen/xen
Jean-Yves Migeon wrote: > On 18.04.2011 05:04, Mindaugas Rasiukevicius wrote: > > Module Name:src > > Committed By: rmind > > Date: Mon Apr 18 03:04:31 UTC 2011 > > > > Modified Files: > > src/sys/arch/xen/xen: balloon.c > > > > Log Message: > > balloon_xenbus_attach: use KM_SLEEP for allocation. > > > > Note: please do not use KM_NOSLEEP. > > Ah yes, forgot about this one, thanks. > > Although I am still unsure about the check, in-kernel NULL deref is... > problematic. > > I am not so sure whether it is safe to assume non-NULL return if caller > can sleep. It's something that ought to be specified for all available > memoryallocators(9) especially as the code behind can evolve (hey, Lars > :) ). > We used to check the return of big size allocations, when kmem(9) could fail even with KM_SLEEP due to KVA starvation. However, pretty much all kernel does not perform checks for smaller allocations and since the bug was fixed - we are no longer checking for big ones as well. This applies to all allocators. -- Mindaugas
Re: CVS commit: src/sys/arch/xen/xen
"Cherry G. Mathew" wrote: > >>> On Mon, Apr 18, 2011 at 03:04:32AM +, Mindaugas Rasiukevicius > >>> wrote: > >>> > Module Name: src > >>> > Committed By: rmind > >>> > Date: Mon Apr 18 03:04:31 UTC 2011 > >>> > > >>> > Modified Files: > >>> > src/sys/arch/xen/xen: balloon.c > >>> > > >>> > Log Message: > >>> > balloon_xenbus_attach: use KM_SLEEP for allocation. > >>> > > >>> > Note: please do not use KM_NOSLEEP. > >>> > >>> And, according to yamt@, KM_SLEEP can fail in the current design... > >> > >> IIRC yamt@ fixed it a year or few ago. > >> > > > > And in the more specific immediate context: > > http://mail-index.netbsd.org/port-xen/2011/04/07/msg006613.html > > > > > > Hi, > > PS: Can you please revert ? Unless it's breaking anything else, or you > have a fix for the problem mentioned in the thread above, ie; Change is for balloon_xenbus_attach(), which is irrelevant to the issue described in that email. Are you aware that KM_NOSLEEP might fail not only because there is no memory (or KVA) available, though? -- Mindaugas
Re: CVS commit: src/sys/arch/xen/xen
On 18.04.2011 05:04, Mindaugas Rasiukevicius wrote: > Module Name: src > Committed By: rmind > Date: Mon Apr 18 03:04:31 UTC 2011 > > Modified Files: > src/sys/arch/xen/xen: balloon.c > > Log Message: > balloon_xenbus_attach: use KM_SLEEP for allocation. > > Note: please do not use KM_NOSLEEP. Ah yes, forgot about this one, thanks. Although I am still unsure about the check, in-kernel NULL deref is... problematic. I am not so sure whether it is safe to assume non-NULL return if caller can sleep. It's something that ought to be specified for all available memoryallocators(9) especially as the code behind can evolve (hey, Lars :) ). -- Jean-Yves Migeon jeanyves.mig...@free.fr
Re: CVS commit: src/sys/arch/xen/xen
On 18 April 2011 09:04, Jean-Yves Migeon wrote: > On 18.04.2011 09:23, Cherry G. Mathew wrote: >> On 18 April 2011 08:00, Cherry G. Mathew wrote: >>> On 18 April 2011 04:21, Mindaugas Rasiukevicius wrote: Masao Uebayashi wrote: > On Mon, Apr 18, 2011 at 03:04:32AM +, Mindaugas Rasiukevicius wrote: >> Module Name: src >> Committed By: rmind >> Date: Mon Apr 18 03:04:31 UTC 2011 >> >> Modified Files: >> src/sys/arch/xen/xen: balloon.c >> >> Log Message: >> balloon_xenbus_attach: use KM_SLEEP for allocation. >> >> Note: please do not use KM_NOSLEEP. > > And, according to yamt@, KM_SLEEP can fail in the current design... IIRC yamt@ fixed it a year or few ago. >>> >>> And in the more specific immediate context: >>> http://mail-index.netbsd.org/port-xen/2011/04/07/msg006613.html >>> >> PS: Can you please revert ? Unless it's breaking anything else, or you >> have a fix for the problem mentioned in the thread above, ie; > > Uho, I forgot to mention in my commit log that I fixed it. I am > allocating bpg_entries via pool_cache(9), and the constructor > bpge_ctor() will return an error if uvm(9) fails to find a free page. In > that case, the thread will just bail out and start waiting again. Ah right, I missed your commit, sorry. Thanks, -- ~Cherry
Re: CVS commit: src/sys/arch/xen/xen
On 18.04.2011 09:23, Cherry G. Mathew wrote: > On 18 April 2011 08:00, Cherry G. Mathew wrote: >> On 18 April 2011 04:21, Mindaugas Rasiukevicius wrote: >>> Masao Uebayashi wrote: On Mon, Apr 18, 2011 at 03:04:32AM +, Mindaugas Rasiukevicius wrote: > Module Name:src > Committed By: rmind > Date: Mon Apr 18 03:04:31 UTC 2011 > > Modified Files: > src/sys/arch/xen/xen: balloon.c > > Log Message: > balloon_xenbus_attach: use KM_SLEEP for allocation. > > Note: please do not use KM_NOSLEEP. And, according to yamt@, KM_SLEEP can fail in the current design... >>> >>> IIRC yamt@ fixed it a year or few ago. >> >> And in the more specific immediate context: >> http://mail-index.netbsd.org/port-xen/2011/04/07/msg006613.html >> > PS: Can you please revert ? Unless it's breaking anything else, or you > have a fix for the problem mentioned in the thread above, ie; Uho, I forgot to mention in my commit log that I fixed it. I am allocating bpg_entries via pool_cache(9), and the constructor bpge_ctor() will return an error if uvm(9) fails to find a free page. In that case, the thread will just bail out and start waiting again. -- Jean-Yves Migeon jeanyves.mig...@free.fr
Re: CVS commit: src/sys/arch/xen/xen
On 18 April 2011 08:00, Cherry G. Mathew wrote: > On 18 April 2011 04:21, Mindaugas Rasiukevicius wrote: >> Masao Uebayashi wrote: >>> On Mon, Apr 18, 2011 at 03:04:32AM +, Mindaugas Rasiukevicius wrote: >>> > Module Name: src >>> > Committed By: rmind >>> > Date: Mon Apr 18 03:04:31 UTC 2011 >>> > >>> > Modified Files: >>> > src/sys/arch/xen/xen: balloon.c >>> > >>> > Log Message: >>> > balloon_xenbus_attach: use KM_SLEEP for allocation. >>> > >>> > Note: please do not use KM_NOSLEEP. >>> >>> And, according to yamt@, KM_SLEEP can fail in the current design... >> >> IIRC yamt@ fixed it a year or few ago. >> > > And in the more specific immediate context: > http://mail-index.netbsd.org/port-xen/2011/04/07/msg006613.html > > Hi, PS: Can you please revert ? Unless it's breaking anything else, or you have a fix for the problem mentioned in the thread above, ie; Cheers, -- ~Cherry
Re: CVS commit: src/sys/arch/xen/xen
On 18 April 2011 04:21, Mindaugas Rasiukevicius wrote: > Masao Uebayashi wrote: >> On Mon, Apr 18, 2011 at 03:04:32AM +, Mindaugas Rasiukevicius wrote: >> > Module Name: src >> > Committed By: rmind >> > Date: Mon Apr 18 03:04:31 UTC 2011 >> > >> > Modified Files: >> > src/sys/arch/xen/xen: balloon.c >> > >> > Log Message: >> > balloon_xenbus_attach: use KM_SLEEP for allocation. >> > >> > Note: please do not use KM_NOSLEEP. >> >> And, according to yamt@, KM_SLEEP can fail in the current design... > > IIRC yamt@ fixed it a year or few ago. > And in the more specific immediate context: http://mail-index.netbsd.org/port-xen/2011/04/07/msg006613.html -- ~Cherry
