Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753328Ab0BAUuq (ORCPT ); Mon, 1 Feb 2010 15:50:46 -0500 Received: from liberdade.minaslivre.org ([72.232.254.139]:52447 "EHLO liberdade.minaslivre.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752937Ab0BAUuo (ORCPT ); Mon, 1 Feb 2010 15:50:44 -0500 Date: Mon, 1 Feb 2010 18:46:32 -0200 From: Thadeu Lima de Souza Cascardo To: John Kacur Cc: Dmitry Torokhov , Arnd Bergmann , linux-input@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] input: remove BKL from uinput open function Message-ID: <20100201204632.GK1414@holoscopio.com> References: <1264800197-29523-1-git-send-email-cascardo@holoscopio.com> <201001302257.09354.arnd@arndb.de> <520f0cf11001301507k20e3cf8dqa73026e12f3a1767@mail.gmail.com> <201001310520.55813.arnd@arndb.de> <20100131052942.GA12320@core.coreip.homeip.net> <520f0cf11002011222h134dbf06rf1db612da9a9728@mail.gmail.com> <520f0cf11002011227s74e57673j3922941f7ee87989@mail.gmail.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="54ZiyWcDhi/7bWb8" Content-Disposition: inline In-Reply-To: <520f0cf11002011227s74e57673j3922941f7ee87989@mail.gmail.com> User-Agent: Mutt/1.5.20 (2009-06-14) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5198 Lines: 140 --54ZiyWcDhi/7bWb8 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Feb 01, 2010 at 09:27:22PM +0100, John Kacur wrote: > On Mon, Feb 1, 2010 at 9:22 PM, John Kacur wrote: > > On Sun, Jan 31, 2010 at 6:29 AM, Dmitry Torokhov > > wrote: > >> On Sun, Jan 31, 2010 at 05:20:55AM +0100, Arnd Bergmann wrote: > >>> On Sunday 31 January 2010, John Kacur wrote: > >>> > > Sorry, I should have been clearer, but not implementing llseek > >>> > > is the problem I was referring to: When a driver has no explicit > >>> > > .llseek operation in its file operations and does not call > >>> > > nonseekable_open from its open operation, the VFS layer will > >>> > > implicitly use default_llseek, which takes the BKL. We're > >>> > > in the process of changing drivers not to do this, one by one > >>> > > so we can kill the BKL in the end. > >>> > > > >>> > > >>> > I know we've discussed this before, but why wouldn't the following > >>> > make more sense? > >>> > =C2=A0.llseek =C2=A0 =C2=A0 =C2=A0 =C2=A0 =3D no_llseek, > >>> > >>> That's one of the possible solutions. Assigning it to generic_file_ll= seek > >>> also gets rid of the BKL but keeps the current behaviour (calling seek > >>> returns success without having an effect, no_llseek returns -ESPIPE), > >>> while calling nonseekable_open has the other side-effect of making > >>> pread/pwrite fail with -ESPIPE, which is more consistent than > >>> only failing seek. > >>> > >> > >> OK, so how about the patch below (on top of Thadeu's patch)? > >> > >> -- > >> Dmitry > >> > >> Input: uinput - use nonseekable_open > >> > >> Seeking does not make sense for uinput so let's use nonseekable_open > >> to mark the device non-seekable. > >> > >> Signed-off-by: Dmitry Torokhov > >> --- > >> > >> =C2=A0drivers/input/misc/uinput.c | =C2=A0 =C2=A07 +++++++ > >> =C2=A01 files changed, 7 insertions(+), 0 deletions(-) > >> > >> > >> diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c > >> index 18206e1..7089151 100644 > >> --- a/drivers/input/misc/uinput.c > >> +++ b/drivers/input/misc/uinput.c > >> @@ -278,6 +278,7 @@ static int uinput_create_device(struct uinput_devi= ce *udev) > >> =C2=A0static int uinput_open(struct inode *inode, struct file *file) > >> =C2=A0{ > >> =C2=A0 =C2=A0 =C2=A0 =C2=A0struct uinput_device *newdev; > >> + =C2=A0 =C2=A0 =C2=A0 int error; > >> > >> =C2=A0 =C2=A0 =C2=A0 =C2=A0newdev =3D kzalloc(sizeof(struct uinput_dev= ice), GFP_KERNEL); > >> =C2=A0 =C2=A0 =C2=A0 =C2=A0if (!newdev) > >> @@ -291,6 +292,12 @@ static int uinput_open(struct inode *inode, struc= t file *file) > >> > >> =C2=A0 =C2=A0 =C2=A0 =C2=A0file->private_data =3D newdev; > >> > >> + =C2=A0 =C2=A0 =C2=A0 error =3D nonseekable_open(inode, file); > >> + =C2=A0 =C2=A0 =C2=A0 if (error) { > >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 kfree(newdev); > >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return error; > >> + =C2=A0 =C2=A0 =C2=A0 } > >> + > >> =C2=A0 =C2=A0 =C2=A0 =C2=A0return 0; > >> =C2=A0} > >> > >> > > > > Hmnn, if you look at nonseekable_open() it will always return 0. I > > think you can just do the following. > > > > diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c > > index 18206e1..697c0a6 100644 > > --- a/drivers/input/misc/uinput.c > > +++ b/drivers/input/misc/uinput.c > > @@ -291,7 +291,7 @@ static int uinput_open(struct inode *inode, struct = file *fil > > > > =C2=A0 =C2=A0 =C2=A0 =C2=A0file->private_data =3D newdev; > > > > - =C2=A0 =C2=A0 =C2=A0 return 0; > > + =C2=A0 =C2=A0 =C2=A0 return nonseekable_open(inode, file); > > =C2=A0} > > > > Signed-off-by: John Kacur > > >=20 > Btw, Thadeu Lima de Souza Cascardo should just combine that all into > one patch, no point really in making two patches out of that. That's fine to me. But since Dmitry has already applied it, I see no problem at all that this is two commits. Or would there be any problem removing the lock in open and not doing nonseekable_open? As far as I get, nonseekable_open only resets the flags that will make it do the right thing for lseek, pread and pwrite. This will get rid of the BKL for these calls, but this is independent of getting rid of it for the open call. I don't disagree that doing both at the same time is OK. But I don't agree that doing them separately is not OK. This way, you may get the credits for what you (and not I) have done. :-) But either way is fine for me. Regards, Cascardo. --54ZiyWcDhi/7bWb8 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.10 (GNU/Linux) iEYEARECAAYFAktnPacACgkQyTpryRcqtS2QrQCeI1N2i2kfYG8l3ATSL2MIdFIX /m4AnA3me6jis5755RjWTvjlkB65zmvf =XQrZ -----END PGP SIGNATURE----- --54ZiyWcDhi/7bWb8-- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/