> -----Original Message----- > From: Jan Beulich <jbeulich@xxxxxxxx> > Sent: 29 November 2019 11:56 > To: Durrant, Paul <pdurrant@xxxxxxxxxx> > Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; linux-block@xxxxxxxxxxxxxxx; linux- > kernel@xxxxxxxxxxxxxxx; Roger Pau Monné <roger.pau@xxxxxxxxxx>; Jens Axboe > <axboe@xxxxxxxxx>; Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx> > Subject: Re: [PATCH] xen-blkback: allow module to be cleanly unloaded > > On 29.11.2019 12:31, Paul Durrant wrote: > > --- a/drivers/block/xen-blkback/xenbus.c > > +++ b/drivers/block/xen-blkback/xenbus.c > > @@ -173,6 +173,8 @@ static struct xen_blkif *xen_blkif_alloc(domid_t > domid) > > init_completion(&blkif->drain_complete); > > INIT_WORK(&blkif->free_work, xen_blkif_deferred_free); > > > > + __module_get(THIS_MODULE); > > + > > return blkif; > > } > > > > @@ -320,6 +322,8 @@ static void xen_blkif_free(struct xen_blkif *blkif) > > > > /* Make sure everything is drained before shutting down */ > > kmem_cache_free(xen_blkif_cachep, blkif); > > + > > + module_put(THIS_MODULE); > > } > > I realize there are various example of this in the tree, but > isn't this a flawed approach? __module_get() (nor even > try_module_get()) will prevent an unload attempt ahead of it > getting invoked, while execution is already in this module's > .text section. Good point. That does appear to be a race. > I think the xenbus driver should do this > before calling ->probe(), in case of its failure, and after > a successful call to ->remove(). > That does sound better. I'll see if I can pick up other occurrences (certainly netback) and fix. Paul