On Wed, Aug 5, 2015 at 4:08 PM, Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote: > On Wed, 5 Aug 2015 09:46:41 -0400 Dan Streetman <ddstreet@xxxxxxxx> wrote: > >> Add zpool_has_pool() function, indicating if the specified type of zpool >> is available (i.e. zsmalloc or zbud). This allows checking if a pool is >> available, without actually trying to allocate it, similar to >> crypto_has_alg(). >> >> This is used by a following patch to zswap that enables the dynamic >> runtime creation of zswap zpools. >> >> ... >> >> /** >> + * zpool_has_pool() - Check if the pool driver is available >> + * @type The type of the zpool to check (e.g. zbud, zsmalloc) >> + * >> + * This checks if the @type pool driver is available. >> + * >> + * Returns: true if @type pool is available, false if not >> + */ >> +bool zpool_has_pool(char *type) >> +{ >> + struct zpool_driver *driver = zpool_get_driver(type); >> + >> + if (!driver) { >> + request_module("zpool-%s", type); >> + driver = zpool_get_driver(type); >> + } >> + >> + if (!driver) >> + return false; >> + >> + zpool_put_driver(driver); >> + return true; >> +} > > This looks racy: after that zpool_put_driver() has completed, an rmmod > will invalidate zpool_has_pool()'s return value. the true/false return value is only a snapshot of that moment in time; zswap's use of this is only to validate that the user-provided zpool name is valid; if this fails, zswap will just return failure to the user (or if this happens at init-time, falls back to LZO). If this succeeds, zswap still must use zpool_create_pool() which will fail if the requested module can't be loaded. essentially zswap does: if (!zpool_has_pool(zpool_type) || !crypto_has_comp(compressor_type)) return -EINVAL; that allows it to check that the requested zpool and compressor types are valid, before actually creating anything. The creation of the zpool and compressor do have error handling if either of them fail. > > If there's some reason why this can't happen, can we please have a code > comment which reveals that reason? zpool_create_pool() should work if this returns true, unless as you say the module is rmmod'ed *and* removed from the system - since zpool_create_pool() will call request_module() just as this function does. I can add a comment explaining that. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx"> email@xxxxxxxxx </a>