From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752801AbcELJTt (ORCPT ); Thu, 12 May 2016 05:19:49 -0400 Received: from metis.ext.4.pengutronix.de ([92.198.50.35]:53039 "EHLO metis.ext.4.pengutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752098AbcELJTq (ORCPT ); Thu, 12 May 2016 05:19:46 -0400 From: Markus Pargmann To: "Pranay Kr. Srivastava" Cc: nbd-general@lists.sourceforge.net, linux-kernel@vger.kernel.org, gregkh@linuxfoundation.org Subject: Re: [PATCH v4 18/18] make nbd device wait for its users in case of timeout Date: Thu, 12 May 2016 11:19:42 +0200 Message-ID: <28709789.5mkdsd5mKW@adelgunde> User-Agent: KMail/4.14.1 (Linux/4.5.0-0.bpo.1-amd64; KDE/4.14.2; x86_64; ; ) In-Reply-To: <1462954726-11825-19-git-send-email-pranjas@gmail.com> References: <1462954726-11825-1-git-send-email-pranjas@gmail.com> <1462954726-11825-19-git-send-email-pranjas@gmail.com> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="nextPart5502205.JBJVQAi9kA"; micalg="pgp-sha256"; protocol="application/pgp-signature" X-SA-Exim-Connect-IP: 2001:67c:670:100:a61f:72ff:fe68:75ba X-SA-Exim-Mail-From: mpa@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linux-kernel@vger.kernel.org Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --nextPart5502205.JBJVQAi9kA Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="us-ascii" Hi, On Wednesday 11 May 2016 11:18:46 Pranay Kr. Srivastava wrote: > When a timeout occurs or a recv fails, then > instead of abruplty killing nbd block device > wait for it's users to finish. >=20 > This is more required when filesystem(s) like > ext2 or ext3 don't expect their buffer heads to > disappear while the filesystem is mounted. >=20 > The change is described below: > a) Add a users count to nbd_device structure. > b) Add a bit flag to nbd_device structure of unsigned long. >=20 > If the current user count is not 1 then make nbd-client wait > for the in_use bit to be cleared. Thanks, I like this approach much more. >=20 > Signed-off-by: Pranay Kr. Srivastava > --- > drivers/block/nbd.c | 40 ++++++++++++++++++++++++++++++++++++++= ++ > include/uapi/linux/nbd.h | 1 + > 2 files changed, 41 insertions(+) >=20 > diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c > index 482a3c0..9b024d8 100644 > --- a/drivers/block/nbd.c > +++ b/drivers/block/nbd.c > @@ -59,6 +59,7 @@ struct nbd_device { > =09int xmit_timeout; > =09atomic_t timedout; > =09bool disconnect; /* a disconnect has been requested by user */ > +=09u32 users; Perhaps it is better to use kref for this? > =20 > =09struct timer_list timeout_timer; > =09/* protects initialization and shutdown of the socket */ > @@ -69,6 +70,7 @@ struct nbd_device { > #if IS_ENABLED(CONFIG_DEBUG_FS) > =09struct dentry *dbg_dir; > #endif > +=09unsigned long bflags;=09/* word size bit flags for use. */ Maybe it is better to use a completion instead of a bitfield. > }; > =20 > #if IS_ENABLED(CONFIG_DEBUG_FS) > @@ -822,6 +824,15 @@ static int __nbd_ioctl(struct block_device *bdev= , struct nbd_device *nbd, > =09=09sock_shutdown(nbd); > =09=09mutex_lock(&nbd->tx_lock); > =09=09nbd_clear_que(nbd); > +=09=09/* > +=09=09 * Wait for any users currently using > +=09=09 * this block device. > +=09=09 */ > +=09=09mutex_unlock(&nbd->tx_lock); > +=09=09pr_info("Waiting for users to release device %s ...\n", > +=09=09=09=09=09=09bdev->bd_disk->disk_name); > +=09=09wait_on_bit(&nbd->bflags, NBD_BFLAG_INUSE_BIT, TASK_INTERRUPTI= BLE); > +=09=09mutex_lock(&nbd->tx_lock); > =09=09kill_bdev(bdev); > =09=09nbd_bdev_reset(bdev); > =20 > @@ -870,10 +881,39 @@ static int nbd_ioctl(struct block_device *bdev,= fmode_t mode, > =09return error; > } > =20 > +static int nbd_open(struct block_device *bdev, fmode_t mode) > +{ > +=09struct nbd_device *nbd_dev =3D bdev->bd_disk->private_data; Here is a new line missing otherwise checkpatch will probably warn abou= t this? Should we check here if we are connected here? And check whether the connection is about to be closed? Best Regards, Markus > +=09nbd_dev->users++; > +=09pr_debug("Opening nbd_dev %s. Active users =3D %u\n", > +=09=09=09bdev->bd_disk->disk_name, nbd_dev->users); > +=09if (nbd_dev->users > 1) > +=09{ > +=09=09set_bit(NBD_BFLAG_INUSE_BIT, &nbd_dev->bflags); > +=09} > +=09return 0; > +} > + > +static void nbd_release(struct gendisk *disk, fmode_t mode) > +{ > +=09struct nbd_device *nbd_dev =3D disk->private_data; > +=09nbd_dev->users--; > +=09pr_debug("Closing nbd_dev %s. Active users =3D %u\n", > +=09=09=09disk->disk_name, nbd_dev->users); > +=09if (nbd_dev->users =3D=3D 1) > +=09{ > +=09=09clear_bit(NBD_BFLAG_INUSE_BIT, &nbd_dev->bflags); > +=09=09smp_mb(); > +=09=09wake_up_bit(&nbd_dev->bflags, NBD_BFLAG_INUSE_BIT); > +=09} > +} > + > static const struct block_device_operations nbd_fops =3D { > =09.owner =3D=09THIS_MODULE, > =09.ioctl =3D=09nbd_ioctl, > =09.compat_ioctl =3D=09nbd_ioctl, > +=09.open =3D =09nbd_open, > +=09.release =3D =09nbd_release > }; > =20 > #if IS_ENABLED(CONFIG_DEBUG_FS) > diff --git a/include/uapi/linux/nbd.h b/include/uapi/linux/nbd.h > index e08e413..8f3d3f0 100644 > --- a/include/uapi/linux/nbd.h > +++ b/include/uapi/linux/nbd.h > @@ -44,6 +44,7 @@ enum { > /* there is a gap here to match userspace */ > #define NBD_FLAG_SEND_TRIM (1 << 5) /* send trim/discard */ > =20 > +#define NBD_BFLAG_INUSE_BIT=09(1) /* bit number for bflags */ > /* userspace doesn't need the nbd_device structure */ > =20 > /* These are sent over the network in the request/reply magic fields= */ >=20 =2D-=20 Pengutronix e.K. | = | Industrial Linux Solutions | http://www.pengutronix.de/= | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 = | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-555= 5 | --nextPart5502205.JBJVQAi9kA Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part. Content-Transfer-Encoding: 7Bit -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAABCAAGBQJXNEquAAoJEEpcgKtcEGQQiuEP/2N6PuWsr76Yl+O4XVQaHBvr 8uUPWjNqYoYxsmbIU8ebRVjMOHT67FtmRpN7VP+hAwoUomOrcVpgorPfh5bdAqZk L4aSBrv7HoBct1+aTShIwLmJnNA3P+mo+FIVO4h7csnZXZxHhx/1fMbBSgpbmiWi ccJoi0er9uN/Q3FpIcysOg2Gwhh5x61TdmHYEcch55Hk0v4GQUi4IZ+hT6Kd4oNh 6JxysTabN85vSrswxjsGL2pAhg7cjYKcYlUdODvTm6+e7wQjvpZo4nXLS29vVuUT iOma+77cXFEOzjbIZ0ujy22xHnB6xPiH/YBe/cl4V152h6KS4Do01woj8KV+PwG+ 8LjF41L7folxsHFYBsTz2Q8GWyu5MiGs8ciFpYxx0HKNaXJ135CRQ0f8KWgSUXR+ CPLHsvxjjdu25GwLiEN2z7oXoQFbkI+4Jt+Co5JphNhR2lfadBLjVyp41IUxRnxK ctqEevGNXe8RIqpmrPGCGUAfE2sKM8BPLmj3iQMdl1aqJzGyyUWX+zAOIvm737Fl rbNche7bu3gVGfTFCMXmgSB6konmRuiX97epQjvQZ04lX7lFKyDWSTRxM9JHyqR8 RA4u2s268q8eytD2koD9jAeASMKl4SsYOoVCZ497dYJ7w+ARhdVYi0YkyOc8uTk3 mP+h2bRAY4NZAHB8txKj =sY0R -----END PGP SIGNATURE----- --nextPart5502205.JBJVQAi9kA--