Re: CVS commit: src/sys/arch/xen/xen
On 18 April 2011 04:21, Mindaugas Rasiukevicius rm...@netbsd.org wrote: Masao Uebayashi uebay...@tombi.co.jp 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
Re: CVS commit: src/sys/arch/xen/xen
On 18 April 2011 08:00, Cherry G. Mathew cherry.g.mat...@gmail.com wrote: On 18 April 2011 04:21, Mindaugas Rasiukevicius rm...@netbsd.org wrote: Masao Uebayashi uebay...@tombi.co.jp 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.04.2011 09:23, Cherry G. Mathew wrote: On 18 April 2011 08:00, Cherry G. Mathew cherry.g.mat...@gmail.com wrote: On 18 April 2011 04:21, Mindaugas Rasiukevicius rm...@netbsd.org wrote: Masao Uebayashi uebay...@tombi.co.jp 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 09:04, Jean-Yves Migeon jeanyves.mig...@free.fr wrote: On 18.04.2011 09:23, Cherry G. Mathew wrote: On 18 April 2011 08:00, Cherry G. Mathew cherry.g.mat...@gmail.com wrote: On 18 April 2011 04:21, Mindaugas Rasiukevicius rm...@netbsd.org wrote: Masao Uebayashi uebay...@tombi.co.jp 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 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
Cherry G. Mathew cherry.g.mat...@gmail.com 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 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
On Apr,Monday 18 2011, at 10:26 AM, Mindaugas Rasiukevicius wrote: Cherry G. Mathew cherry.g.mat...@gmail.com 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
Jean-Yves Migeon jeanyves.mig...@free.fr 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
