Re: [virtio] Re: CVS commit: src/sys/dev/pci
Le 01/06/2020 à 03:23, Shoichi Yamaguchi a écrit : Hi, On Wed, May 27, 2020 at 8:47 PM Shoichi Yamaguchi wrote: I modified virtio(4) not to allocate unused memory. I guess it fixes the issue. Could you check this? I confirmed your closing the report on syzbot. https://syzkaller.appspot.com/bug?id=73f690e8a3d8e304407808555f1dacfa004685d1 Thank you for your response. Sorry for my lack of response -- I was waiting for the kMSan instance to sync, but it is currently down, and has been down for four days and a half now. The kMSan instance got the time to run 24h before it broke for unrelated reasons. 24h before your patch, it triggered the bug 6 times. 24h after your patch, it triggered the bug 0 times. So indeed, we can call it fixed, thanks for the fix
Re: [virtio] Re: CVS commit: src/sys/dev/pci
Hi, On Wed, May 27, 2020 at 8:47 PM Shoichi Yamaguchi wrote: > > I modified virtio(4) not to allocate unused memory. > I guess it fixes the issue. > > Could you check this? I confirmed your closing the report on syzbot. https://syzkaller.appspot.com/bug?id=73f690e8a3d8e304407808555f1dacfa004685d1 Thank you for your response. Regards, yamaguchi
Re: CVS commit: src/sys/ddb
On 2020/06/01 9:36, Kamil Rytarowski wrote: If you can find a use-case today in DDB for an exclusive allocator. I have it ready and tested. I intended to use it in ddb for one project, but that project is still unfinished. Good to know! I will ask you when I want it! Thanks, rin
Re: CVS commit: src/sys/ddb
On 01.06.2020 02:31, Rin Okuyama wrote: > On 2020/06/01 9:23, Kamil Rytarowski wrote: >> I wrote a tiny malloc (libc-style) implementation over a small static >> storage (could be over stack or preallocated on the heap) without any >> external dependencies. >> >> Would it be useful for you? > > At the moment, we need buffers only in db_show_callout(), and static > buffers are enough there, IMO. > > However, in future, if we want to add new features that require dynamic > buffer allocation, we need to implement exclusive allocator for DDB. > > Thanks, > rin If you can find a use-case today in DDB for an exclusive allocator. I have it ready and tested. I intended to use it in ddb for one project, but that project is still unfinished. signature.asc Description: OpenPGP digital signature
Re: CVS commit: src/sys/ddb
On 2020/06/01 9:23, Kamil Rytarowski wrote: I wrote a tiny malloc (libc-style) implementation over a small static storage (could be over stack or preallocated on the heap) without any external dependencies. Would it be useful for you? At the moment, we need buffers only in db_show_callout(), and static buffers are enough there, IMO. However, in future, if we want to add new features that require dynamic buffer allocation, we need to implement exclusive allocator for DDB. Thanks, rin
Re: CVS commit: src/sys/ddb
I wrote a tiny malloc (libc-style) implementation over a small static
storage (could be over stack or preallocated on the heap) without any
external dependencies.
Would it be useful for you?
On 01.06.2020 01:34, Rin Okuyama wrote:
> Module Name: src
> Committed By: rin
> Date: Sun May 31 23:34:34 UTC 2020
>
> Modified Files:
> src/sys/ddb: db_kernel.c
>
> Log Message:
> Switch from kmem_intr_alloc(sz, KM_NOSLEEP) to kmem_alloc(sz, KM_SLEEP).
>
> Clearly document these functions are *not* for DDB session, but for
> permanent data storage when initializing DDB.
>
>
> To generate a diff of this commit:
> cvs rdiff -u -r1.3 -r1.4 src/sys/ddb/db_kernel.c
>
> Please note that diffs are not public domain; they are subject to the
> copyright notices on the relevant files.
>
>
> Modified files:
>
> Index: src/sys/ddb/db_kernel.c
> diff -u src/sys/ddb/db_kernel.c:1.3 src/sys/ddb/db_kernel.c:1.4
> --- src/sys/ddb/db_kernel.c:1.3 Sun May 31 09:42:46 2020
> +++ src/sys/ddb/db_kernel.c Sun May 31 23:34:34 2020
> @@ -1,4 +1,4 @@
> -/* $NetBSD: db_kernel.c,v 1.3 2020/05/31 09:42:46 rin Exp $*/
> +/* $NetBSD: db_kernel.c,v 1.4 2020/05/31 23:34:34 rin Exp $*/
>
> /*-
> * Copyright (c) 2009 The NetBSD Foundation, Inc.
> @@ -30,7 +30,7 @@
> */
>
> #include
> -__KERNEL_RCSID(0, "$NetBSD: db_kernel.c,v 1.3 2020/05/31 09:42:46 rin Exp
> $");
> +__KERNEL_RCSID(0, "$NetBSD: db_kernel.c,v 1.4 2020/05/31 23:34:34 rin Exp
> $");
>
> #include
> #include
> @@ -39,28 +39,33 @@ __KERNEL_RCSID(0, "$NetBSD: db_kernel.c,
>
> /*
> * XXX
> - * DDB can be running in the interrupt context, e.g., when activated from
> - * console. Therefore, we use kmem_intr_alloc(9) and friends here to avoid
> - * assertion failure.
> + * These routines are *not* intended for a DDB session:
> + *
> + * - It disturbes on-going debugged kernel datastructures.
> + *
> + * - DDB can be running in the interrupt context, e.g., when activated from
> + * console. This results in assertion failures in the allocator.
> + *
> + * Use these only for permanent data storage when initializing DDB.
> */
>
> void *
> db_alloc(size_t sz)
> {
>
> - return kmem_intr_alloc(sz, KM_NOSLEEP);
> + return kmem_alloc(sz, KM_SLEEP);
> }
>
> void *
> db_zalloc(size_t sz)
> {
>
> - return kmem_intr_zalloc(sz, KM_NOSLEEP);
> + return kmem_zalloc(sz, KM_SLEEP);
> }
>
> void
> db_free(void *p, size_t sz)
> {
>
> - kmem_intr_free(p, sz);
> + kmem_free(p, sz);
> }
>
signature.asc
Description: OpenPGP digital signature
re: CVS commit: src/sys/kern
"Rin Okuyama" writes: > Module Name: src > Committed By: rin > Date: Sun May 31 08:33:48 UTC 2020 > > Modified Files: > src/sys/kern: kern_timeout.c > > Log Message: > db_show_callout(): struct callout_cpu and cpu_info are too much for stack. > > XXX > DDB can be running in the interrupt context, e.g., when activated from > console. Therefore, use kmem_intr_alloc(9) instead of kmem_alloc(9). > > Frame size, e.g. for m68k, becomes: > 9212 (oops!) --> 0 please don't use allocators from ddb. inspection from ddb shouldn't go change internal datastructures and rely upon things that can break working where possible allocate a single static in the bss to use? .mrg.
Re: CVS commit: src/sys/kern
> On May 31, 2020, at 1:33 AM, Rin Okuyama wrote: > > db_show_callout(): struct callout_cpu and cpu_info are too much for stack. > > XXX > DDB can be running in the interrupt context, e.g., when activated from > console. Therefore, use kmem_intr_alloc(9) instead of kmem_alloc(9). > > Frame size, e.g. for m68k, becomes: >9212 (oops!) --> 0 > I'm not sure we want to be calling memory allocators from within ddb. I think it would be better to simply allocate that space in the .bss segment -- ddb only runs on 1 CPU at a time. -- thorpej
