From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758256AbbA3HyU (ORCPT ); Fri, 30 Jan 2015 02:54:20 -0500 Received: from metis.ext.pengutronix.de ([92.198.50.35]:55301 "EHLO metis.ext.pengutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753418AbbA3HyT (ORCPT ); Fri, 30 Jan 2015 02:54:19 -0500 Date: Fri, 30 Jan 2015 08:54:15 +0100 From: Markus Pargmann To: Paul Clements Cc: "nbd-general@lists.sourceforge.net" , kernel list , kernel@pengutronix.de Subject: Re: [RFC 3/4] nbd: Create helper functions for ioctls Message-ID: <20150130075415.GC16879@pengutronix.de> References: <1421156665-27318-1-git-send-email-mpa@pengutronix.de> <1421156665-27318-4-git-send-email-mpa@pengutronix.de> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="raC6veAxrt5nqIoY" Content-Disposition: inline In-Reply-To: X-Sent-From: Pengutronix Hildesheim X-URL: http://www.pengutronix.de/ X-IRC: #ptxdist @freenode X-Accept-Language: de,en X-Accept-Content-Type: text/plain X-Uptime: 08:37:57 up 8 days, 21:45, 33 users, load average: 0.30, 0.34, 0.42 User-Agent: Mutt/1.5.23 (2014-03-12) X-SA-Exim-Connect-IP: 2001:67c:670:100:1d::7 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 --raC6veAxrt5nqIoY Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hi Paul, On Wed, Jan 28, 2015 at 04:04:28PM -0500, Paul Clements wrote: > Markus, >=20 > Comments below... >=20 > On Tue, Jan 13, 2015 at 8:44 AM, Markus Pargmann wro= te: > > This patch prepares the nbd code for the nbd-root device patch. It > > moves the ioctl code into separate functions so they can be called > > directly. The patch creates nbd_set_total_size(), nbd_set_blksize(), > > nbd_set_sock() and nbd_set_timeout(). > > > > Signed-off-by: Markus Pargmann > > --- > > drivers/block/nbd.c | 79 ++++++++++++++++++++++++++++++++++++++-------= -------- > > 1 file changed, 57 insertions(+), 22 deletions(-) > > > > diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c > > index 0ff6ebbec252..11f7644be111 100644 > > --- a/drivers/block/nbd.c > > +++ b/drivers/block/nbd.c > > @@ -596,6 +596,55 @@ static void do_nbd_request(struct request_queue *q) > > } > > } > > > > +/* > > + * Call with tx_lock held > > + */ > > +static void nbd_set_total_size(struct block_device *bdev, > > + struct nbd_device *nbd, size_t size) > > +{ > > + nbd->bytesize =3D size; > > + bdev->bd_inode->i_size =3D size; > > + set_blocksize(bdev, nbd->blksize); > > + set_capacity(nbd->disk, size >> 9); > > +} > > + > > +/* > > + * Call with tx_lock held > > + */ > > +static void nbd_set_blksize(struct block_device *bdev, struct nbd_devi= ce *nbd, > > + size_t blksize) > > +{ > > + u64 new_bytesize; > > + > > + nbd->blksize =3D blksize; > > + bdev->bd_inode->i_size =3D nbd->bytesize; > > + set_blocksize(bdev, nbd->blksize); > > + > > + new_bytesize =3D nbd->bytesize & ~(nbd->blksize-1); > > + if (new_bytesize !=3D nbd->bytesize) > > + nbd_set_total_size(bdev, nbd, new_bytesize); >=20 > could just do set_capacity here to save a few cycles, but not a big > deal either way... Yes, however I will rework this again because set_blocksize returns error values. So these functions should make error handling. >=20 >=20 > > +} > > + > > +/* > > + * Call with tx_lock held > > + */ > > +static void nbd_set_sock(struct block_device *bdev, struct nbd_device = *nbd, > > + struct socket *sock) > > +{ > > + nbd->sock =3D sock; > > + if (max_part > 0) > > + bdev->bd_invalidated =3D 1; > > + nbd->disconnect =3D 0; /* we're connected now */ > > +} > > + > > +/* > > + * Call with tx_lock held > > + */ > > +static void nbd_set_timeout(struct nbd_device *nbd, int timeout) > > +{ > > + nbd->xmit_timeout =3D timeout * HZ; > > +} > > + > > static int nbd_connection_start(struct block_device *bdev, > > struct nbd_device *nbd) > > { > > @@ -733,33 +782,22 @@ static int __nbd_ioctl(struct block_device *bdev,= struct nbd_device *nbd, > > if (nbd->sock) > > return -EBUSY; > > sock =3D sockfd_lookup(arg, &err); > > - if (sock) { > > - nbd->sock =3D sock; > > - if (max_part > 0) > > - bdev->bd_invalidated =3D 1; > > - nbd->disconnect =3D 0; /* we're connected now */ > > - return 0; > > - } > > - return -EINVAL; > > + if (!sock) > > + return -EINVAL; > > + > > + nbd_set_sock(bdev, nbd, sock); >=20 > need a return 0 here Yes, fixed. >=20 >=20 > > } > > > > case NBD_SET_BLKSIZE: > > - nbd->blksize =3D arg; > > - nbd->bytesize &=3D ~(nbd->blksize-1); > > - bdev->bd_inode->i_size =3D nbd->bytesize; > > - set_blocksize(bdev, nbd->blksize); > > - set_capacity(nbd->disk, nbd->bytesize >> 9); > > + nbd_set_blksize(bdev, nbd, arg); > > return 0; > > > > case NBD_SET_SIZE: > > - nbd->bytesize =3D arg & ~(nbd->blksize-1); > > - bdev->bd_inode->i_size =3D nbd->bytesize; > > - set_blocksize(bdev, nbd->blksize); > > - set_capacity(nbd->disk, nbd->bytesize >> 9); > > + nbd_set_total_size(bdev, nbd, arg & ~(nbd->blksize-1)); > > return 0; >=20 > Might consider deprecating this one...it's pretty useless these days, > as it has the same function as NBD_SET_SIZE_BLOCKS, but has a smaller > device size limit (at least on some platforms)... nbd-client does not > use it anymore... Yes I will consider this. I am not sure yet as it isn't much overhead to have this additional ioctl. Thanks, Markus --=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-5555 | --raC6veAxrt5nqIoY Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJUyzinAAoJEEpcgKtcEGQQeXoP/1NUfcvY/HgYTEpKjfSKEBDa MAKa/dpWfztg004ftpQzpaBRQ9nQaaaQEvrBXAnJMUT/6vQemVzan135AY/qLEu7 3SyxjQ+EU2gctaLUgzViGowBfrzXFzjUVEGM657VTPeHIeHFFZsZx68wOa2Hcq49 zzfBn/aNX/NVSQUcFy7evnmvdkpW1FNR0VbYgAJWh0VdAL3h0Uyjv/J9En7nXjgd PLsr+B9MYr+XBmpaXNBA7E73kg3FSJYlaE+S5uFgSvD6NzhXkMl0dxU5A49YKIL9 TA1Bc8Cw6U2kGhsTkPzLt0x5DBsp1k7ePpspCOnpMYXgGqytpfLkC6v6X0Hjf/Nj VcbR0c13ktIv/oSHutBGlXOigskUfJ8DHWlyPpqUDYoz0xs7enTXKvmvGbJ5Wges hWbi5aIa0mcCg5q7JD7tGw0NrAYUyhYbdL0+qv/p5h4xVTYbUG8L5n5KHPuwcV1y voQf+68jgkPVij1PmFRGAx6b2SuvvsNiLSEA4zy6N5wmp5lzGNSDxxL2lXF3f395 WDDeLZRW/iSOe8AgiDJigmkzIdX9n/AtCTkz7+po0k2oBB3mmdr0maa6UPBL8z+f VJHZrdKUreArbx74/79trSQT7DtTd507U5AKwakX4BKT6gYpeRpjd2GqZE5R1H0j DKGiA9H3iQXhJgskvdP6 =lc6r -----END PGP SIGNATURE----- --raC6veAxrt5nqIoY--