Date: Tue, 18 Aug 2020 09:49:00 -0400 From: Mark Johnston <markj@freebsd.org> To: Mateusz Guzik <mjguzik@gmail.com> Cc: current@freebsd.org Subject: Re: "panic: malloc(M_WAITOK) in non-sleepable context" after r364296 -> r364341 Message-ID: <20200818134900.GA28906@raichu> In-Reply-To: <CAGudoHHJLoQhU%2Bb4%2Btt_dmChYj=qXA%2BiVaC5mFW8sFxoqq54pw@mail.gmail.com> References: <20200818124419.GO1394@albert.catwhisker.org> <CAGudoHEEnZVaBwY1L-wjmDcrsHyP4pE3-0qyurh-kWsCO9Edhg@mail.gmail.com> <20200818125825.GP1394@albert.catwhisker.org> <CAGudoHHJLoQhU%2Bb4%2Btt_dmChYj=qXA%2BiVaC5mFW8sFxoqq54pw@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Aug 18, 2020 at 03:24:52PM +0200, Mateusz Guzik wrote:
> Try this:
>
> diff --git a/sys/kern/kern_malloc.c b/sys/kern/kern_malloc.c
> index 46eb1c4347c..7b94ca7b880 100644
> --- a/sys/kern/kern_malloc.c
> +++ b/sys/kern/kern_malloc.c
> @@ -618,8 +618,8 @@ void *
> unsigned long osize = size;
> #endif
>
> - KASSERT((flags & M_WAITOK) == 0 || THREAD_CAN_SLEEP(),
> - ("malloc(M_WAITOK) in non-sleepable context"));
> + if ((flags & M_WAITOK) != 0)
> + WITNESS_WARN(WARN_GIANTOK | WARN_SLEEPOK, NULL, __func__);
>
> #ifdef MALLOC_DEBUG
> va = NULL;
> diff --git a/sys/vm/uma_core.c b/sys/vm/uma_core.c
> index 37d78354200..2e1267ec02f 100644
> --- a/sys/vm/uma_core.c
> +++ b/sys/vm/uma_core.c
> @@ -3355,8 +3355,8 @@ uma_zalloc_arg(uma_zone_t zone, void *udata, int flags)
> uma_cache_bucket_t bucket;
> uma_cache_t cache;
>
> - KASSERT((flags & M_WAITOK) == 0 || THREAD_CAN_SLEEP(),
> - ("uma_zalloc(M_WAITOK) in non-sleepable context"));
> + if ((flags & M_WAITOK) != 0)
> + WITNESS_WARN(WARN_GIANTOK | WARN_SLEEPOK, NULL, __func__);
>
> /* Enable entropy collection for RANDOM_ENABLE_UMA kernel option */
> random_harvest_fast_uma(&zone, sizeof(zone), RANDOM_UMA);
Doesn't it only print a warning if non-sleepable locks are held?
THREAD_CAN_SLEEP() catches other cases, like epoch sections and worker
threads that are not allowed to sleep (CAM doneq threads in this case).
Otherwise uma_zalloc_debug() already checks with WITNESS.
I'm inclined to simply revert the commit for now. Alternately,
disk_alloc() could be modified to handle M_NOWAIT/M_WAITOK flags, and
that could be used in daregister() and other CAM periph drivers.
daregister() already uses M_NOWAIT to allocate the driver softc itself.
diff --git a/sys/cam/scsi/scsi_da.c b/sys/cam/scsi/scsi_da.c
index d648d511b1a2..d940e7db24e0 100644
--- a/sys/cam/scsi/scsi_da.c
+++ b/sys/cam/scsi/scsi_da.c
@@ -2767,18 +2767,23 @@ daregister(struct cam_periph *periph, void *arg)
return(CAM_REQ_CMP_ERR);
}
- softc = (struct da_softc *)malloc(sizeof(*softc), M_DEVBUF,
- M_NOWAIT|M_ZERO);
-
+ softc = malloc(sizeof(*softc), M_DEVBUF, M_NOWAIT | M_ZERO);
if (softc == NULL) {
printf("daregister: Unable to probe new device. "
"Unable to allocate softc\n");
return(CAM_REQ_CMP_ERR);
}
+ softc->disk = disk_alloc_flags(M_NOWAIT);
+ if (softc->disk == NULL) {
+ printf("daregister: Unable to probe new device. "
+ "Unable to allocate disk structure\n");
+ return (CAM_REQ_CMP_ERR);
+ }
if (cam_iosched_init(&softc->cam_iosched, periph) != 0) {
printf("daregister: Unable to probe new device. "
"Unable to allocate iosched memory\n");
+ disk_destroy(softc->disk);
free(softc, M_DEVBUF);
return(CAM_REQ_CMP_ERR);
}
@@ -2898,7 +2903,6 @@ daregister(struct cam_periph *periph, void *arg)
/*
* Register this media as a disk.
*/
- softc->disk = disk_alloc();
softc->disk->d_devstat = devstat_new_entry(periph->periph_name,
periph->unit_number, 0,
DEVSTAT_BS_UNAVAILABLE,
diff --git a/sys/geom/geom_disk.c b/sys/geom/geom_disk.c
index eaba770828d0..c07494ab4ec0 100644
--- a/sys/geom/geom_disk.c
+++ b/sys/geom/geom_disk.c
@@ -850,15 +850,22 @@ g_disk_ident_adjust(char *ident, size_t size)
}
struct disk *
-disk_alloc(void)
+disk_alloc_flags(int mflag)
{
struct disk *dp;
- dp = g_malloc(sizeof(struct disk), M_WAITOK | M_ZERO);
- LIST_INIT(&dp->d_aliases);
+ dp = g_malloc(sizeof(struct disk), mflag | M_ZERO);
+ if (dp != NULL)
+ LIST_INIT(&dp->d_aliases);
return (dp);
}
+struct disk *
+disk_alloc(void)
+{
+ return (disk_alloc_flags(M_WAITOK));
+}
+
void
disk_create(struct disk *dp, int version)
{
diff --git a/sys/geom/geom_disk.h b/sys/geom/geom_disk.h
index 8e2282a09a3a..794ce2cc38bd 100644
--- a/sys/geom/geom_disk.h
+++ b/sys/geom/geom_disk.h
@@ -137,6 +137,7 @@ struct disk {
#define DISKFLAG_WRITE_PROTECT 0x0100
struct disk *disk_alloc(void);
+struct disk *disk_alloc_flags(int mflag);
void disk_create(struct disk *disk, int version);
void disk_destroy(struct disk *disk);
void disk_gone(struct disk *disk);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20200818134900.GA28906>